From 56708808fe17e9707680a04d5084a70dcbdfccbc Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 3 May 2018 17:45:51 +0200 Subject: [PATCH 1/9] wallet: Reset BerkeleyDB handle after connection fails According to the BerkeleyDB docs, the DbEnv handle may not be accessed after close() has been called. This change ensures that we create a new handle after close() is called. This avoids a segfault when the first connection attempt fails and then a second connection attempt tries to call open() on the already closed DbEnv handle. --- src/wallet/db.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 89b1399faa031..b57391f8b3897 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -173,6 +173,7 @@ bool BerkeleyEnvironment::Open(bool retry) S_IRUSR | S_IWUSR); if (ret != 0) { dbenv->close(0); + Reset(); LogPrintf("BerkeleyEnvironment::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); if (retry) { // try moving the database env out of the way From cba20b947639a51e2fead14fa31372e7c04cc622 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 9 May 2018 00:20:12 +0200 Subject: [PATCH 2/9] wallet: Improve logging when BerkeleyDB environment fails to close --- src/wallet/db.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index b57391f8b3897..11b262c41b042 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -106,7 +106,7 @@ void BerkeleyEnvironment::Close() int ret = dbenv->close(0); if (ret != 0) - LogPrintf("Error %d shutting down database environment: %s\n", ret, DbEnv::strerror(ret)); + LogPrintf("%s: Error %d closing database environment: %s\n", __func__, ret, DbEnv::strerror(ret)); if (!fMockDb) DbEnv((u_int32_t)0).remove(strPath.c_str(), 0); } @@ -172,9 +172,12 @@ bool BerkeleyEnvironment::Open(bool retry) nEnvFlags, S_IRUSR | S_IWUSR); if (ret != 0) { - dbenv->close(0); + LogPrintf("%s: Error %d opening database environment: %s\n", __func__, ret, DbEnv::strerror(ret)); + int ret2 = dbenv->close(0); + if (ret2 != 0) { + LogPrintf("%s: Error %d closing failed database environment: %s\n", __func__, ret2, DbEnv::strerror(ret2)); + } Reset(); - LogPrintf("BerkeleyEnvironment::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); if (retry) { // try moving the database env out of the way fs::path pathDatabaseBak = pathIn / strprintf("database.%d.bak", GetTime()); From 8e57466e82c38a0a60b8bac3db50af566431b512 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 13 Jul 2018 19:15:30 -0700 Subject: [PATCH 3/9] Move BerkeleyEnvironment deletion from internal method to callsite Instead of having the object destroy itself, having the caller destroy it. --- src/wallet/db.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 11b262c41b042..105a527161e2a 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -697,7 +697,6 @@ void BerkeleyEnvironment::Flush(bool fShutdown) if (!fMockDb) { fs::remove_all(fs::path(strPath) / "database"); } - g_dbenvs.erase(strPath); } } } @@ -797,7 +796,11 @@ void BerkeleyDatabase::Flush(bool shutdown) { if (!IsDummy()) { env->Flush(shutdown); - if (shutdown) env = nullptr; + if (shutdown) { + LOCK(cs_db); + g_dbenvs.erase(env->Directory().string()); + env = nullptr; + } } } From f4200eb7fd09acd9fb2a8ae2ceedf95c216b38f0 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 24 Nov 2021 15:56:14 -0300 Subject: [PATCH 4/9] walletdb: remove unused CloseAndReset() function --- src/wallet/db.cpp | 6 ------ src/wallet/db.h | 4 ---- 2 files changed, 10 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 105a527161e2a..1b5fb859a77c6 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -803,9 +803,3 @@ void BerkeleyDatabase::Flush(bool shutdown) } } } - -void BerkeleyDatabase::CloseAndReset() -{ - env->Close(); - env->Reset(); -} diff --git a/src/wallet/db.h b/src/wallet/db.h index b34ec06732c41..aafdf8ec66e18 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -143,10 +143,6 @@ class BerkeleyDatabase */ void Flush(bool shutdown); - /** Close and reset. - */ - void CloseAndReset(); - void IncrementUpdateCounter(); std::atomic nUpdateCounter; unsigned int nLastSeen; From 17ac47eb846cbbcb433623d74bc6ef1b4cc27573 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 20 Feb 2018 15:28:42 -0500 Subject: [PATCH 5/9] Add function to close all Db's and reload the databae environment Adds a ReloadDbEnv function to BerkeleyEnvironment in order to close all Db instances, closes the environment, resets it, and then reopens the BerkeleyEnvironment. Also adds a ReloadDbEnv function to BerkeleyDatabase that calls BerkeleyEnvironment's ReloadDbEnv. --- src/wallet/db.cpp | 34 ++++++++++++++++++++++++++++++++++ src/wallet/db.h | 5 +++++ 2 files changed, 39 insertions(+) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 1b5fb859a77c6..104338b5d705c 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -556,6 +556,7 @@ void BerkeleyBatch::Close() LOCK(cs_db); --env->mapFileUseCount[strFile]; } + env->m_db_in_use.notify_all(); } void BerkeleyEnvironment::CloseDb(const std::string& strFile) @@ -572,6 +573,32 @@ void BerkeleyEnvironment::CloseDb(const std::string& strFile) } } +void BerkeleyEnvironment::ReloadDbEnv() +{ + // Make sure that no Db's are in use + AssertLockNotHeld(cs_db); + std::unique_lock lock(cs_db); + m_db_in_use.wait(lock, [this](){ + for (auto& count : mapFileUseCount) { + if (count.second > 0) return false; + } + return true; + }); + + std::vector filenames; + for (auto it : mapDb) { + filenames.push_back(it.first); + } + // Close the individual Db's + for (const std::string& filename : filenames) { + CloseDb(filename); + } + // Reset the environment + Flush(true); // This will flush and close the environment + Reset(); + Open(true); +} + bool BerkeleyBatch::Rewrite(BerkeleyDatabase& database, const char* pszSkip) { if (database.IsDummy()) { @@ -803,3 +830,10 @@ void BerkeleyDatabase::Flush(bool shutdown) } } } + +void BerkeleyDatabase::ReloadDbEnv() +{ + if (!IsDummy()) { + env->ReloadDbEnv(); + } +} diff --git a/src/wallet/db.h b/src/wallet/db.h index aafdf8ec66e18..34fae12fc6579 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -37,6 +37,7 @@ class BerkeleyEnvironment std::unique_ptr dbenv; std::map mapFileUseCount; std::map mapDb; + std::condition_variable_any m_db_in_use; BerkeleyEnvironment(const fs::path& env_directory); ~BerkeleyEnvironment(); @@ -75,6 +76,7 @@ class BerkeleyEnvironment void CheckpointLSN(const std::string& strFile); void CloseDb(const std::string& strFile); + void ReloadDbEnv(); DbTxn* TxnBegin(int flags = DB_TXN_WRITE_NOSYNC) { @@ -144,6 +146,9 @@ class BerkeleyDatabase void Flush(bool shutdown); void IncrementUpdateCounter(); + + void ReloadDbEnv(); + std::atomic nUpdateCounter; unsigned int nLastSeen; unsigned int nLastFlushed; From 64a8a73f5ef1e0008325cc0838c88b079b467ef4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 20 Feb 2018 16:08:36 -0500 Subject: [PATCH 6/9] After encrypting the wallet, reload the database environment Calls ReloadDbEnv after encrypting the wallet so that the database environment is flushed, closed, and reopened to prevent unencrypted keys from being saved on disk. --- src/wallet/wallet.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9409db15fe26f..3dec71b370986 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -879,6 +879,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Need to completely rewrite the wallet file; if we don't, bdb might keep // bits of the unencrypted private key in slack space in the database file. database->Rewrite(); + + // BDB seems to have a bad habit of writing old data into + // slack space in .dat files; that is bad if the old data is + // unencrypted private keys. So: + database->ReloadDbEnv(); + } NotifyStatusChanged(this); From 6e03bf20857cd63e676fd19d3960df787a0727ad Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 24 Nov 2021 16:06:24 -0300 Subject: [PATCH 7/9] No longer shutdown after encrypting the wallet Since the database environment is flushed, closed, and reopened during EncryptWallet, there is no need to shut down the software anymore. Adaptation of btc@c1dde3a949b36ce9c2155777b3fa1372e7ed97d8 --- src/qt/askpassphrasedialog.cpp | 5 ++--- src/wallet/rpcwallet.cpp | 7 +------ test/functional/rpc_fundrawtransaction.py | 7 +------ test/functional/sapling_wallet_nullifiers.py | 15 ++------------- test/functional/test_framework/test_node.py | 8 -------- test/functional/wallet_bumpfee.py | 5 +---- test/functional/wallet_dump.py | 3 +-- test/functional/wallet_encryption.py | 3 +-- test/functional/wallet_keypool.py | 4 +--- 9 files changed, 10 insertions(+), 47 deletions(-) diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index e3073693c0229..9fe8566d71b4e 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -351,9 +351,9 @@ void AskPassphraseDialog::warningMessage() openStandardDialog( tr("Wallet encrypted"), "" + - tr("%1 will close now to finish the encryption process. " + tr("Your wallet is now encrypted. " "Remember that encrypting your wallet cannot fully protect " - "your PIVs from being stolen by malware infecting your computer.").arg(PACKAGE_NAME) + + "your PIVs from being stolen by malware infecting your computer.") + "

" + tr("IMPORTANT: Any previous backups you have made of your wallet file " "should be replaced with the newly generated, encrypted wallet file. " @@ -362,7 +362,6 @@ void AskPassphraseDialog::warningMessage() "", tr("OK") ); - QApplication::quit(); } void AskPassphraseDialog::errorEncryptingWallet() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d4491475941dc..f070d7d04e9e8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3630,7 +3630,6 @@ UniValue encryptwallet(const JSONRPCRequest& request) "will require the passphrase to be set prior the making these calls.\n" "Use the walletpassphrase call for this, and then walletlock call.\n" "If the wallet is already encrypted, use the walletpassphrasechange call.\n" - "Note that this will shutdown the server.\n" "\nArguments:\n" "1. \"passphrase\" (string) The pass phrase to encrypt the wallet with. It must be at least 1 character, but should be long.\n" @@ -3669,11 +3668,7 @@ UniValue encryptwallet(const JSONRPCRequest& request) if (!pwallet->EncryptWallet(strWalletPass)) throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: Failed to encrypt the wallet."); - // BDB seems to have a bad habit of writing old data into - // slack space in .dat files; that is bad if the old data is - // unencrypted private keys. So: - StartShutdown(); - return "wallet encrypted; pivx server stopping, restart to run with encrypted wallet. The keypool has been flushed, you need to make a new backup."; + return "wallet encrypted; The keypool has been flushed, you need to make a new backup."; } UniValue listunspent(const JSONRPCRequest& request) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 6913386dbf0fe..578bc59d7c645 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -10,7 +10,6 @@ assert_raises_rpc_error, assert_greater_than, assert_greater_than_or_equal, - connect_nodes, count_bytes, find_vout_for_address, Decimal, @@ -371,11 +370,7 @@ def test_spend_2of2(self): def test_locked_wallet(self): self.log.info("test locked wallet") - self.nodes[1].node_encrypt_wallet("test") - self.start_node(1) - connect_nodes(self.nodes[0], 1) - connect_nodes(self.nodes[1], 2) - self.sync_all() + self.nodes[1].encryptwallet("test") # Drain the keypool. self.nodes[1].getnewaddress() diff --git a/test/functional/sapling_wallet_nullifiers.py b/test/functional/sapling_wallet_nullifiers.py index e5a2edeea3038..7d6309e41edb7 100755 --- a/test/functional/sapling_wallet_nullifiers.py +++ b/test/functional/sapling_wallet_nullifiers.py @@ -7,11 +7,7 @@ from decimal import Decimal from test_framework.test_framework import PivxTestFramework -from test_framework.util import assert_equal, assert_true, connect_nodes, get_coinstake_address - -def connect_nodes_bi(nodes, a, b): - connect_nodes(nodes[a], b) - connect_nodes(nodes[b], a) +from test_framework.util import assert_equal, assert_true, get_coinstake_address class WalletNullifiersTest(PivxTestFramework): @@ -45,14 +41,7 @@ def run_test (self): self.nodes[1].importsaplingkey(myzkey) # encrypt node 1 wallet and wait to terminate - self.nodes[1].node_encrypt_wallet("test") - - # restart node 1 - self.start_node(1, self.extra_args[1]) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 2, 1) - connect_nodes_bi(self.nodes, 3, 1) - self.sync_all() + self.nodes[1].encryptwallet("test") # send node 0 shield addr to note 2 zaddr recipients = [] diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index bf5d9c8e1842a..ebccb5adc1ae8 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -277,14 +277,6 @@ def assert_debug_log(self, expected_msgs): if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None: raise AssertionError('Expected message "{}" does not partially match log:\n\n{}\n\n'.format(expected_msg, print_log)) - def node_encrypt_wallet(self, passphrase): - """"Encrypts the wallet. - - This causes pivxd to shutdown, so this method takes - care of cleaning up resources.""" - self.encryptwallet(passphrase) - self.wait_until_stopped() - def add_p2p_connection(self, p2p_conn, *args, wait_for_verack=True, **kwargs): """Add a p2p connection to the node. diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 188af82d1cee4..b0f767a87f182 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -26,7 +26,6 @@ assert_greater_than, assert_raises_rpc_error, bytes_to_hex_str, - connect_nodes, hex_str_to_bytes ) @@ -47,11 +46,9 @@ def set_test_params(self): def run_test(self): # Encrypt wallet for test_locked_wallet_fails test - self.nodes[1].node_encrypt_wallet(WALLET_PASSPHRASE) - self.start_node(1) + self.nodes[1].encryptwallet(WALLET_PASSPHRASE) self.nodes[1].walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) - connect_nodes(self.nodes[0], 1) self.sync_all() peer_node, rbf_node = self.nodes diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index ab830a5fa5f60..4a61ce1dea301 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -107,8 +107,7 @@ def run_test (self): assert_equal(found_addr_rsv, 90 * 3) # 90 keys external plus 100% internal keys plus 100% staking keys #encrypt wallet, restart, unlock and dump - self.nodes[0].node_encrypt_wallet('test') - self.start_node(0) + self.nodes[0].encryptwallet('test') self.nodes[0].walletpassphrase('test', 10) # Should be a no-op: self.nodes[0].keypoolrefill() diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py index 4b82a5689d58f..a8c4d6b54b718 100755 --- a/test/functional/wallet_encryption.py +++ b/test/functional/wallet_encryption.py @@ -31,8 +31,7 @@ def run_test(self): assert_equal(len(privkey), 52) # Encrypt the wallet - self.nodes[0].node_encrypt_wallet(passphrase) - self.start_node(0) + self.nodes[0].encryptwallet(passphrase) # Test that the wallet is encrypted assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].dumpprivkey, address) diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index f3effc97cb977..427c2ed5188c4 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -21,9 +21,7 @@ def run_test(self): nodes[0].validateaddress(addr_before_encrypting) # Encrypt wallet and wait to terminate - nodes[0].node_encrypt_wallet('test') - # Restart node 0 - self.start_node(0, self.extra_args[0]) + nodes[0].encryptwallet('test') # Keep creating keys addr = nodes[0].getnewaddress() nodes[0].validateaddress(addr) From 28d17a1223833a6d7a6c65e20f412e135b56d4b9 Mon Sep 17 00:00:00 2001 From: Chun Kuan Lee Date: Tue, 25 Sep 2018 21:56:16 +0800 Subject: [PATCH 8/9] wallet: Fix duplicate fileid --- src/wallet/db.cpp | 33 ++++++++++++++++++++------------- src/wallet/db.h | 7 +++++++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 104338b5d705c..1f69de6331858 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -24,6 +24,7 @@ namespace { + //! Make sure database has a unique fileid within the environment. If it //! doesn't, throw an error. BDB caches do not work properly when more than one //! open database has the same fileid (values written to one database may show @@ -33,25 +34,19 @@ namespace { //! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html), //! so bitcoin should never create different databases with the same fileid, but //! this error can be triggered if users manually copy database files. -void CheckUniqueFileid(const BerkeleyEnvironment& env, const std::string& filename, Db& db) +void CheckUniqueFileid(const BerkeleyEnvironment& env, const std::string& filename, Db& db, WalletDatabaseFileId& fileid) { if (env.IsMock()) return; - u_int8_t fileid[DB_FILE_ID_LEN]; - int ret = db.get_mpf()->get_fileid(fileid); + int ret = db.get_mpf()->get_fileid(fileid.value); if (ret != 0) { throw std::runtime_error(strprintf("BerkeleyBatch: Can't open database %s (get_fileid failed with %d)", filename, ret)); } - for (const auto& item : env.mapDb) { - u_int8_t item_fileid[DB_FILE_ID_LEN]; - if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 && - memcmp(fileid, item_fileid, sizeof(fileid)) == 0) { - const char* item_filename = nullptr; - item.second->get_dbname(&item_filename, nullptr); + for (const auto& item : env.m_fileids) { + if (fileid == item.second && &fileid != &item.second) { throw std::runtime_error(strprintf("BerkeleyBatch: Can't open database %s (duplicates fileid %s from %s)", filename, - HexStr(item_fileid), - item_filename ? item_filename : "(unknown database)")); + HexStr(item.second.value), item.first)); } } } @@ -60,6 +55,11 @@ RecursiveMutex cs_db; std::map g_dbenvs; //!< Map from directory name to open db environment. } // namespace +bool WalletDatabaseFileId::operator==(const WalletDatabaseFileId& rhs) const +{ + return memcmp(value, &rhs.value, sizeof(value)) == 0; +} + BerkeleyEnvironment* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) { fs::path env_directory; @@ -503,8 +503,8 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo // be implemented, so no equality checks are needed at all. (Newer // versions of BDB have an set_lk_exclusive method for this // purpose, but the older version we use does not.) - for (auto& env : g_dbenvs) { - CheckUniqueFileid(env.second, strFilename, *pdb_temp); + for (const auto& env : g_dbenvs) { + CheckUniqueFileid(env.second, strFilename, *pdb_temp, this->env->m_fileids[strFilename]); } pdb = pdb_temp.release(); @@ -827,6 +827,13 @@ void BerkeleyDatabase::Flush(bool shutdown) LOCK(cs_db); g_dbenvs.erase(env->Directory().string()); env = nullptr; + } else { + // TODO: To avoid g_dbenvs.erase erasing the environment prematurely after the + // first database shutdown when multiple databases are open in the same + // environment, should replace raw database `env` pointers with shared or weak + // pointers, or else separate the database and environment shutdowns so + // environments can be shut down after databases. + env->m_fileids.erase(strFile); } } } diff --git a/src/wallet/db.h b/src/wallet/db.h index 34fae12fc6579..447d51af90a3f 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -24,6 +25,11 @@ static const unsigned int DEFAULT_WALLET_DBLOGSIZE = 100; static const bool DEFAULT_WALLET_PRIVDB = true; +struct WalletDatabaseFileId { + u_int8_t value[DB_FILE_ID_LEN]; + bool operator==(const WalletDatabaseFileId& rhs) const; +}; + class BerkeleyEnvironment { private: @@ -37,6 +43,7 @@ class BerkeleyEnvironment std::unique_ptr dbenv; std::map mapFileUseCount; std::map mapDb; + std::unordered_map m_fileids; std::condition_variable_any m_db_in_use; BerkeleyEnvironment(const fs::path& env_directory); From c5340ec4eb7044483f270fcd034ef3d5278c1ab0 Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 28 Nov 2021 13:45:27 -0300 Subject: [PATCH 9/9] test: add temp wallet testing setup. There are tests, like the ones that uses the wallet encryption, that need access to a real wallet database and not to a mock. --- src/test/librust/sapling_rpc_wallet_tests.cpp | 12 +++++++++++- src/wallet/test/wallet_test_fixture.cpp | 11 ++++++++--- src/wallet/test/wallet_test_fixture.h | 14 ++++++++++---- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index d6ee944b7f200..c5c98c66aa046 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -14,6 +14,7 @@ #include "key_io.h" #include "consensus/merkle.h" #include "wallet/wallet.h" +#include "wallet/walletutil.h" #include "sapling/key_io_sapling.h" #include "sapling/address.h" @@ -497,7 +498,14 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) vpwallets.erase(vpwallets.begin()); } -BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) +struct RealWalletTestingSetup : public WalletTestingSetupBase +{ + RealWalletTestingSetup() : WalletTestingSetupBase(CBaseChainParams::MAIN, + "test_wallet", + WalletDatabase::Create(fs::absolute("test_wallet", GetWalletDir()))) {}; +}; + +BOOST_FIXTURE_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys, RealWalletTestingSetup) { UniValue retValue; int n = 100; @@ -535,6 +543,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) PushCurrentDirectory push_dir(gArgs.GetArg("-datadir","/tmp/thisshouldnothappen")); BOOST_CHECK(m_wallet.EncryptWallet(strWalletPass)); + BOOST_CHECK(m_wallet.IsCrypted()); // Verify we can still list the keys imported BOOST_CHECK_NO_THROW(retValue = CallRPC("listshieldaddresses")); @@ -547,6 +556,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) // We can't call RPC walletpassphrase as that invokes RPCRunLater which breaks tests. // So we manually unlock. BOOST_CHECK(m_wallet.Unlock(strWalletPass)); + BOOST_CHECK(m_wallet.IsCrypted()); // Now add a key BOOST_CHECK_NO_THROW(CallRPC("getnewshieldaddress")); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index aec006823ead0..240c4394ada8b 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -10,8 +10,10 @@ #include "wallet/rpcwallet.h" #include "wallet/wallet.h" -WalletTestingSetup::WalletTestingSetup(const std::string& chainName): - SaplingTestingSetup(chainName), m_wallet("mock", WalletDatabase::CreateMock()) +WalletTestingSetupBase::WalletTestingSetupBase(const std::string& chainName, + const std::string& wallet_name, + std::unique_ptr db) : + SaplingTestingSetup(chainName), m_wallet(wallet_name, std::move(db)) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); @@ -20,7 +22,10 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): RegisterWalletRPCCommands(tableRPC); } -WalletTestingSetup::~WalletTestingSetup() +WalletTestingSetupBase::~WalletTestingSetupBase() { UnregisterValidationInterface(&m_wallet); } + +WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : + WalletTestingSetupBase(chainName, "mock", WalletDatabase::CreateMock()) {} diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 6c76d5abddc65..a30068382e213 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -12,14 +12,20 @@ /** Testing setup and teardown for wallet. */ -struct WalletTestingSetup : public SaplingTestingSetup +struct WalletTestingSetupBase : public SaplingTestingSetup { - WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); - ~WalletTestingSetup(); - + WalletTestingSetupBase(const std::string& chainName, + const std::string& wallet_name, + std::unique_ptr db); + ~WalletTestingSetupBase(); CWallet m_wallet; }; +struct WalletTestingSetup : public WalletTestingSetupBase +{ + WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); +}; + struct WalletRegTestingSetup : public WalletTestingSetup { WalletRegTestingSetup() : WalletTestingSetup(CBaseChainParams::REGTEST) {}