diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -35,7 +35,7 @@ // (https://github.com/bitcoin/bitcoin/issues/7883#issuecomment-224807484) static void CoinSelection(benchmark::State &state) { SelectParams(CBaseChainParams::REGTEST); - const CWallet wallet(Params()); + const CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); std::vector vCoins; LOCK(wallet.cs_wallet); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -120,10 +120,7 @@ test.CreateAndProcessBlock( {}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - bitdb.MakeMock(); - std::unique_ptr dbw( - new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - CWallet wallet(Params(), std::move(dbw)); + CWallet wallet(Params(), "mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet.LoadWallet(firstRun); { @@ -241,9 +238,6 @@ receiveCoinsDialog.findChild("removeRequestButton"); removeRequestButton->click(); QCOMPARE(requestTableModel->rowCount({}), currentRowCount - 1); - - bitdb.Flush(true); - bitdb.Reset(); } } diff --git a/src/wallet/db.h b/src/wallet/db.h --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -32,20 +33,19 @@ // pointer. std::string strPath; - void EnvShutdown(); - public: - mutable CCriticalSection cs_db; std::unique_ptr dbenv; std::map mapFileUseCount; std::map mapDb; - CDBEnv(); + CDBEnv(const fs::path &env_directory); ~CDBEnv(); void Reset(); void MakeMock(); bool IsMock() const { return fMockDb; } + bool IsInitialized() const { return fDbEnvInit; } + fs::path Directory() const { return strPath; } /** * Verify that database file strFile is OK. If it is not, call the callback @@ -54,7 +54,7 @@ * Returns true if strFile is OK. */ enum class VerifyResult { VERIFY_OK, RECOVER_OK, RECOVER_FAIL }; - typedef bool (*recoverFunc_type)(const std::string &strFile, + typedef bool (*recoverFunc_type)(const fs::path &file_path, std::string &out_backup_filename); VerifyResult Verify(const std::string &strFile, recoverFunc_type recoverFunc, @@ -72,7 +72,7 @@ bool Salvage(const std::string &strFile, bool fAggressive, std::vector &vResult); - bool Open(const fs::path &path, bool retry = 0); + bool Open(bool retry); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string &strFile); @@ -87,7 +87,9 @@ } }; -extern CDBEnv bitdb; +/** Get CDBEnv and database filename given a wallet path. */ +CDBEnv *GetWalletEnv(const fs::path &wallet_path, + std::string &database_filename); /** * An instance of this class represents one database. @@ -103,9 +105,32 @@ nLastWalletUpdate(0), env(nullptr) {} /** Create DB handle to real database */ - CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in) + CWalletDBWrapper(const fs::path &wallet_path, bool mock = false) : nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), - nLastWalletUpdate(0), env(env_in), strFile(strFile_in) {} + nLastWalletUpdate(0) { + env = GetWalletEnv(wallet_path, strFile); + if (mock) { + env->Close(); + env->Reset(); + env->MakeMock(); + } + } + + /** Return object for accessing database at specified path. */ + static std::unique_ptr Create(const fs::path &path) { + return std::make_unique(path); + } + + /** Return object for accessing dummy database with no read/write + * capabilities. */ + static std::unique_ptr CreateDummy() { + return std::make_unique(); + } + + /** Return object for accessing temporary in-memory database. */ + static std::unique_ptr CreateMock() { + return std::make_unique("", true /* mock */); + } /** Rewrite the entire database on disk, with the exception of key pszSkip * if non-zero @@ -116,10 +141,6 @@ */ bool Backup(const std::string &strDest); - /** Get a name for this database, for debugging etc. - */ - std::string GetName() const { return strFile; } - /** Make sure all changes are flushed to disk. */ void Flush(bool shutdown); @@ -164,7 +185,7 @@ void Flush(); void Close(); - static bool Recover(const std::string &filename, void *callbackDataIn, + static bool Recover(const fs::path &file_path, void *callbackDataIn, bool (*recoverKVcallback)(void *callbackData, CDataStream ssKey, CDataStream ssValue), @@ -174,12 +195,10 @@ ideal to be called periodically */ static bool PeriodicFlush(CWalletDBWrapper &dbw); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string &walletFile, - const fs::path &walletDir, + static bool VerifyEnvironment(const fs::path &file_path, std::string &errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string &walletFile, - const fs::path &walletDir, + static bool VerifyDatabaseFile(const fs::path &file_path, std::string &warningStr, std::string &errorStr, CDBEnv::recoverFunc_type recoverFunc); @@ -348,7 +367,7 @@ if (!pdb || activeTxn) { return false; } - DbTxn *ptxn = bitdb.TxnBegin(); + DbTxn *ptxn = env->TxnBegin(); if (!ptxn) { return false; } diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -59,20 +59,49 @@ } } } + +CCriticalSection cs_db; +//!< Map from directory name to open db environment. +std::map g_dbenvs; } // namespace +CDBEnv *GetWalletEnv(const fs::path &wallet_path, + std::string &database_filename) { + fs::path env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + LOCK(cs_db); + // Note: An ununsed temporary CDBEnv object may be created inside the + // emplace function if the key already exists. This is a little inefficient, + // but not a big concern since the map will be changed in the future to hold + // pointers instead of objects, anyway. + return &g_dbenvs + .emplace(std::piecewise_construct, + std::forward_as_tuple(env_directory.string()), + std::forward_as_tuple(env_directory)) + .first->second; +} + // // CDB // -CDBEnv bitdb; - -void CDBEnv::EnvShutdown() { +void CDBEnv::Close() { if (!fDbEnvInit) { return; } fDbEnvInit = false; + + for (auto &db : mapDb) { + auto count = mapFileUseCount.find(db.first); + assert(count == mapFileUseCount.end() || count->second == 0); + if (db.second) { + db.second->close(0); + delete db.second; + db.second = nullptr; + } + } + int ret = dbenv->close(0); if (ret != 0) { LogPrintf("CDBEnv::EnvShutdown: Error %d shutting down database " @@ -91,26 +120,22 @@ fMockDb = false; } -CDBEnv::CDBEnv() { +CDBEnv::CDBEnv(const fs::path &dir_path) : strPath(dir_path.string()) { Reset(); } CDBEnv::~CDBEnv() { - EnvShutdown(); + Close(); } -void CDBEnv::Close() { - EnvShutdown(); -} - -bool CDBEnv::Open(const fs::path &pathIn, bool retry) { +bool CDBEnv::Open(bool retry) { if (fDbEnvInit) { return true; } boost::this_thread::interruption_point(); - strPath = pathIn.string(); + fs::path pathIn = strPath; if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another " "instance of bitcoin may be using it.\n", @@ -163,7 +188,7 @@ // we started with) } // try opening it again one more time - if (!Open(pathIn, false)) { + if (!Open(false /* retry */)) { // if it still fails, it probably means we can't even create the // database env return false; @@ -223,15 +248,19 @@ } // Try to recover: - bool fRecovered = (*recoverFunc)(strFile, out_backup_filename); + bool fRecovered = + (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename); return (fRecovered ? VerifyResult::RECOVER_OK : VerifyResult::RECOVER_FAIL); } -bool CDB::Recover(const std::string &filename, void *callbackDataIn, +bool CDB::Recover(const fs::path &file_path, void *callbackDataIn, bool (*recoverKVcallback)(void *callbackData, CDataStream ssKey, CDataStream ssValue), std::string &newFilename) { + std::string filename; + CDBEnv *env = GetWalletEnv(file_path, filename); + // Recovery procedure: // Move wallet file to walletfilename.timestamp.bak // Call Salvage with fAggressive=true to get as much data as possible. @@ -240,8 +269,8 @@ int64_t now = GetTime(); newFilename = strprintf("%s.%d.bak", filename, now); - int result = bitdb.dbenv->dbrename(nullptr, filename.c_str(), nullptr, - newFilename.c_str(), DB_AUTO_COMMIT); + int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, + newFilename.c_str(), DB_AUTO_COMMIT); if (result == 0) { LogPrintf("Renamed %s to %s\n", filename, newFilename); } else { @@ -250,14 +279,14 @@ } std::vector salvagedData; - bool fSuccess = bitdb.Salvage(newFilename, true, salvagedData); + bool fSuccess = env->Salvage(newFilename, true, salvagedData); if (salvagedData.empty()) { LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); return false; } LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - std::unique_ptr pdbCopy = std::make_unique(bitdb.dbenv.get(), 0); + std::unique_ptr pdbCopy = std::make_unique(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -270,7 +299,7 @@ return false; } - DbTxn *ptxn = bitdb.TxnBegin(); + DbTxn *ptxn = env->TxnBegin(); for (CDBEnv::KeyValPair &row : salvagedData) { if (recoverKVcallback) { CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); @@ -293,8 +322,11 @@ return fSuccess; } -bool CDB::VerifyEnvironment(const std::string &walletFile, - const fs::path &walletDir, std::string &errorStr) { +bool CDB::VerifyEnvironment(const fs::path &file_path, std::string &errorStr) { + std::string walletFile; + CDBEnv *env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0)); LogPrintf("Using wallet %s\n", walletFile); @@ -305,7 +337,7 @@ return false; } - if (!bitdb.Open(walletDir, true)) { + if (!env->Open(true /* retry */)) { errorStr = strprintf( _("Error initializing wallet database environment %s!"), walletDir); return false; @@ -314,14 +346,17 @@ return true; } -bool CDB::VerifyDatabaseFile(const std::string &walletFile, - const fs::path &walletDir, std::string &warningStr, +bool CDB::VerifyDatabaseFile(const fs::path &file_path, std::string &warningStr, std::string &errorStr, CDBEnv::recoverFunc_type recoverFunc) { + std::string walletFile; + CDBEnv *env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + if (fs::exists(walletDir / walletFile)) { std::string backup_filename; CDBEnv::VerifyResult r = - bitdb.Verify(walletFile, recoverFunc, backup_filename); + env->Verify(walletFile, recoverFunc, backup_filename); if (r == CDBEnv::VerifyResult::RECOVER_OK) { warningStr = strprintf( _("Warning: Wallet file corrupt, data salvaged!" @@ -440,8 +475,8 @@ } { - LOCK(env->cs_db); - if (!env->Open(GetWalletDir())) { + LOCK(cs_db); + if (!env->Open(false /* retry */)) { throw std::runtime_error( "CDB: Failed to open database environment."); } @@ -475,7 +510,25 @@ throw std::runtime_error(strprintf( "CDB: Error %d, can't open database %s", ret, strFilename)); } - CheckUniqueFileid(*env, strFilename, *pdb_temp); + + // Call CheckUniqueFileid on the containing BDB environment to + // avoid BDB data consistency bugs that happen when different data + // files in the same environment have the same fileid. + // + // Also call CheckUniqueFileid on all the other g_dbenvs to prevent + // bitcoin from opening the same data file through another + // environment when the file is referenced through equivalent but + // not obviously identical symlinked or hard linked or bind mounted + // paths. In the future a more relaxed check for equal inode and + // device ids could be done instead, which would allow opening + // different backup copies of a wallet at the same time. Maybe even + // more ideally, an exclusive lock for accessing the database could + // 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 &dbenv : g_dbenvs) { + CheckUniqueFileid(dbenv.second, strFilename, *pdb_temp); + } pdb = pdb_temp.release(); env->mapDb[strFilename] = pdb; @@ -527,7 +580,7 @@ Flush(); } - LOCK(env->cs_db); + LOCK(cs_db); --env->mapFileUseCount[strFile]; } @@ -550,7 +603,7 @@ const std::string &strFile = dbw.strFile; while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file @@ -705,7 +758,7 @@ bool ret = false; CDBEnv *env = dbw.env; const std::string &strFile = dbw.strFile; - TRY_LOCK(bitdb.cs_db, lockDb); + TRY_LOCK(cs_db, lockDb); if (lockDb) { // Don't do this if any databases are in use int nRefCount = 0; @@ -748,7 +801,7 @@ } while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -355,16 +355,16 @@ } std::string strError; - if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), - strError)) { + if (!CWalletDB::VerifyEnvironment(wallet_path, strError)) { return InitError(strError); } if (gArgs.GetBoolArg("-salvagewallet", false)) { // Recover readable keypairs: - CWallet dummyWallet(chainParams); + CWallet dummyWallet(chainParams, "dummy", + CWalletDBWrapper::CreateDummy()); std::string backup_filename; - if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, + if (!CWalletDB::Recover(wallet_path, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { return false; @@ -372,8 +372,8 @@ } std::string strWarning; - bool dbV = CWalletDB::VerifyDatabaseFile( - walletFile, GetWalletDir().string(), strWarning, strError); + bool dbV = + CWalletDB::VerifyDatabaseFile(wallet_path, strWarning, strError); if (!strWarning.empty()) { InitWarning(strWarning); } @@ -393,8 +393,8 @@ } for (const std::string &walletFile : gArgs.GetArgs("-wallet")) { - CWallet *const pwallet = - CWallet::CreateWalletFromFile(chainParams, walletFile); + CWallet *const pwallet = CWallet::CreateWalletFromFile( + chainParams, walletFile, fs::absolute(walletFile, GetWalletDir())); if (!pwallet) { return false; } diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -12,13 +12,13 @@ BOOST_FIXTURE_TEST_SUITE(accounting_tests, WalletTestingSetup) -static void GetResults(CWallet *wallet, +static void GetResults(CWallet &wallet, std::map &results) { std::list aes; results.clear(); - BOOST_CHECK(wallet->ReorderTransactions() == DBErrors::LOAD_OK); - wallet->ListAccountCreditDebit("", aes); + BOOST_CHECK(wallet.ReorderTransactions() == DBErrors::LOAD_OK); + wallet.ListAccountCreditDebit("", aes); for (CAccountingEntry &ae : aes) { results[ae.nOrderPos * SATOSHI] = ae; } @@ -30,28 +30,28 @@ CAccountingEntry ae; std::map results; - LOCK(pwalletMain->cs_wallet); + LOCK(m_wallet.cs_wallet); ae.strAccount = ""; ae.nCreditDebit = SATOSHI; ae.nTime = 1333333333; ae.strOtherAccount = "b"; ae.strComment = ""; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); wtx.mapValue["comment"] = "z"; - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetId())); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetId())); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; ae.nTime = 1333333336; ae.strOtherAccount = "c"; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); - BOOST_CHECK(pwalletMain->nOrderPosNext == 3); + BOOST_CHECK(m_wallet.nOrderPosNext == 3); BOOST_CHECK(2 == results.size()); BOOST_CHECK(results[Amount::zero()].nTime == 1333333333); BOOST_CHECK(results[Amount::zero()].strComment.empty()); @@ -61,13 +61,13 @@ ae.nTime = 1333333330; ae.strOtherAccount = "d"; - ae.nOrderPos = pwalletMain->IncOrderPosNext(); - pwalletMain->AddAccountingEntry(ae); + ae.nOrderPos = m_wallet.IncOrderPosNext(); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 3); - BOOST_CHECK(pwalletMain->nOrderPosNext == 4); + BOOST_CHECK(m_wallet.nOrderPosNext == 4); BOOST_CHECK(results[Amount::zero()].nTime == 1333333333); BOOST_CHECK(1 == vpwtx[0]->nOrderPos); BOOST_CHECK(results[2 * SATOSHI].nTime == 1333333336); @@ -81,8 +81,8 @@ --tx.nLockTime; wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetId())); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetId())); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -92,15 +92,15 @@ --tx.nLockTime; wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetId())); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetId())); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 3); - BOOST_CHECK(pwalletMain->nOrderPosNext == 6); + BOOST_CHECK(m_wallet.nOrderPosNext == 6); BOOST_CHECK(0 == vpwtx[2]->nOrderPos); BOOST_CHECK(results[SATOSHI].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); @@ -112,12 +112,12 @@ ae.nTime = 1333333334; ae.strOtherAccount = "e"; ae.nOrderPos = -1; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 4); - BOOST_CHECK(pwalletMain->nOrderPosNext == 7); + BOOST_CHECK(m_wallet.nOrderPosNext == 7); BOOST_CHECK(0 == vpwtx[2]->nOrderPos); BOOST_CHECK(results[SATOSHI].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -17,7 +17,7 @@ const std::string &chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr pwalletMain; + CWallet m_wallet; }; #endif diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,25 +8,19 @@ #include #include #include +#include WalletTestingSetup::WalletTestingSetup(const std::string &chainName) - : TestingSetup(chainName) { - bitdb.MakeMock(); - + : TestingSetup(chainName), + m_wallet(Params(), "mock", CWalletDBWrapper::CreateMock()) { bool fFirstRun; - std::unique_ptr dbw( - new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = std::make_unique(Params(), std::move(dbw)); - pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain.get()); + m_wallet.LoadWallet(fFirstRun); + RegisterValidationInterface(&m_wallet); RegisterWalletRPCCommands(tableRPC); RegisterDumpRPCCommands(tableRPC); } WalletTestingSetup::~WalletTestingSetup() { - UnregisterValidationInterface(pwalletMain.get()); - - bitdb.Flush(true); - bitdb.Reset(); + UnregisterValidationInterface(&m_wallet); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -86,7 +86,7 @@ CoinSet setCoinsRet, setCoinsRet2; Amount nValueRet; - const CWallet wallet(Params()); + const CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); LOCK(walletCriticalSection); // test multiple times to allow for differences in the shuffle order @@ -435,7 +435,7 @@ CoinSet setCoinsRet; Amount nValueRet; - const CWallet wallet(Params()); + const CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); LOCK(walletCriticalSection); empty_wallet(); @@ -472,7 +472,7 @@ // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -488,7 +488,7 @@ // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -501,7 +501,7 @@ // before the missing block, and success for a key whose creation time is // after. { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); vpwallets.insert(vpwallets.begin(), &wallet); UniValue keys; keys.setArray(); @@ -576,7 +576,7 @@ // Import key into wallet and call dumpwallet to create backup file. { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); LOCK(wallet.cs_wallet); wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; @@ -592,7 +592,7 @@ // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); JSONRPCRequest request; request.params.setArray(); @@ -621,7 +621,7 @@ // function. Similar tests probably should be written for the other credit and // debit functions. BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { - CWallet wallet(Params()); + CWallet wallet(Params(), "dummy", CWalletDBWrapper::CreateDummy()); CWalletTx wtx(&wallet, MakeTransactionRef(coinbaseTxns.back())); LOCK2(cs_main, wallet.cs_wallet); wtx.hashBlock = chainActive.Tip()->GetBlockHash(); @@ -669,27 +669,25 @@ // Simple test to verify assignment of CWalletTx::nSmartTime value. Could be // expanded to cover more corner cases of smart time logic. BOOST_AUTO_TEST_CASE(ComputeTimeSmart) { - CWallet wallet(Params()); - // New transaction should use clock time if lower than block time. - BOOST_CHECK_EQUAL(AddTx(wallet, 1, 100, 120), 100); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100); // Test that updating existing transaction does not change smart time. - BOOST_CHECK_EQUAL(AddTx(wallet, 1, 200, 220), 100); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100); // New transaction should use clock time if there's no block time. - BOOST_CHECK_EQUAL(AddTx(wallet, 2, 300, 0), 300); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 2, 300, 0), 300); // New transaction should use block time if lower than clock time. - BOOST_CHECK_EQUAL(AddTx(wallet, 3, 420, 400), 400); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 3, 420, 400), 400); // New transaction should use latest entry time if higher than // min(block time, clock time). - BOOST_CHECK_EQUAL(AddTx(wallet, 4, 500, 390), 400); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, 500, 390), 400); // If there are future entries, new transaction should use time of the // newest entry that is no more than 300 seconds ahead of the clock time. - BOOST_CHECK_EQUAL(AddTx(wallet, 5, 50, 600), 300); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 5, 50, 600), 300); // Reset mock time for other tests. SetMockTime(0); @@ -697,12 +695,12 @@ BOOST_AUTO_TEST_CASE(LoadReceiveRequests) { CTxDestination dest = CKeyID(); - LOCK(pwalletMain->cs_wallet); - pwalletMain->AddDestData(dest, "misc", "val_misc"); - pwalletMain->AddDestData(dest, "rr0", "val_rr0"); - pwalletMain->AddDestData(dest, "rr1", "val_rr1"); + LOCK(m_wallet.cs_wallet); + m_wallet.AddDestData(dest, "misc", "val_misc"); + m_wallet.AddDestData(dest, "rr0", "val_rr0"); + m_wallet.AddDestData(dest, "rr1", "val_rr1"); - auto values = pwalletMain->GetDestValues("rr"); + auto values = m_wallet.GetDestValues("rr"); BOOST_CHECK_EQUAL(values.size(), 2); BOOST_CHECK_EQUAL(values[0], "val_rr0"); BOOST_CHECK_EQUAL(values[1], "val_rr1"); @@ -713,10 +711,8 @@ ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - ::bitdb.MakeMock(); - wallet.reset(new CWallet( - Params(), std::unique_ptr( - new CWalletDBWrapper(&bitdb, "wallet_test.dat")))); + wallet = std::make_unique(Params(), "mock", + CWalletDBWrapper::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); @@ -726,11 +722,7 @@ reserver); } - ~ListCoinsTestingSetup() { - wallet.reset(); - ::bitdb.Flush(true); - ::bitdb.Reset(); - } + ~ListCoinsTestingSetup() { wallet.reset(); } CWalletTx &AddTx(CRecipient recipient) { CTransactionRef tx; diff --git a/src/wallet/test/walletdb_tests.cpp b/src/wallet/test/walletdb_tests.cpp --- a/src/wallet/test/walletdb_tests.cpp +++ b/src/wallet/test/walletdb_tests.cpp @@ -14,7 +14,8 @@ namespace { static std::unique_ptr LoadWallet(CWalletDB *db) { - std::unique_ptr wallet(new CWallet(Params())); + std::unique_ptr wallet( + new CWallet(Params(), "dummy", CWalletDBWrapper::CreateDummy())); DBErrors res = db->LoadWallet(wallet.get()); BOOST_CHECK(res == DBErrors::LOAD_OK); return wallet; @@ -24,7 +25,7 @@ BOOST_FIXTURE_TEST_SUITE(walletdb_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(write_erase_name) { - CWalletDB walletdb(pwalletMain->GetDBHandle(), "cr+"); + CWalletDB walletdb(m_wallet.GetDBHandle(), "cr+"); CTxDestination dst1 = CKeyID(uint160S("c0ffee")); CTxDestination dst2 = CKeyID(uint160S("f00d")); @@ -48,7 +49,7 @@ } BOOST_AUTO_TEST_CASE(write_erase_purpose) { - CWalletDB walletdb(pwalletMain->GetDBHandle(), "cr+"); + CWalletDB walletdb(m_wallet.GetDBHandle(), "cr+"); CTxDestination dst1 = CKeyID(uint160S("c0ffee")); CTxDestination dst2 = CKeyID(uint160S("f00d")); @@ -72,7 +73,7 @@ } BOOST_AUTO_TEST_CASE(write_erase_destdata) { - CWalletDB walletdb(pwalletMain->GetDBHandle(), "cr+"); + CWalletDB walletdb(m_wallet.GetDBHandle(), "cr+"); CTxDestination dst1 = CKeyID(uint160S("c0ffee")); CTxDestination dst2 = CKeyID(uint160S("f00d")); @@ -107,7 +108,7 @@ } BOOST_AUTO_TEST_CASE(no_dest_fails) { - CWalletDB walletdb(pwalletMain->GetDBHandle(), "cr+"); + CWalletDB walletdb(m_wallet.GetDBHandle(), "cr+"); CTxDestination dst = CNoDestination{}; BOOST_CHECK(!walletdb.WriteName(dst, "name")); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -729,6 +730,14 @@ */ bool AddWatchOnly(const CScript &dest) override; + /** + * Wallet filename from wallet= command line or config option. + * Used in debug logs and to send RPCs to the right wallet instance when + * more than one wallet is loaded. + */ + std::string m_name; + + /** Internal database handle. */ std::unique_ptr dbw; /** @@ -760,13 +769,7 @@ /** * Get a name for this wallet for logging/debugging purposes. */ - std::string GetName() const { - if (dbw) { - return dbw->GetName(); - } else { - return "dummy"; - } - } + std::string GetName() const { return m_name; } void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool); @@ -780,16 +783,11 @@ MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID; - // Create wallet with dummy database handle - explicit CWallet(const CChainParams &chainParamsIn) - : dbw(new CWalletDBWrapper()), chainParams(chainParamsIn) { - SetNull(); - } - - // Create wallet with passed-in database handle - CWallet(const CChainParams &chainParamsIn, + /** Construct wallet with specified name and database implementation. */ + CWallet(const CChainParams &chainParamsIn, std::string name, std::unique_ptr dbw_in) - : dbw(std::move(dbw_in)), chainParams(chainParamsIn) { + : m_name(std::move(name)), dbw(std::move(dbw_in)), + chainParams(chainParamsIn) { SetNull(); } @@ -1192,7 +1190,8 @@ * in case of an error. */ static CWallet *CreateWalletFromFile(const CChainParams &chainParams, - const std::string walletFile); + const std::string &name, + const fs::path &path); /** * Wallet post-init setup diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4198,17 +4198,18 @@ } CWallet *CWallet::CreateWalletFromFile(const CChainParams &chainParams, - const std::string walletFile) { + const std::string &name, + const fs::path &path) { + const std::string &walletFile = name; + // Needed to restore wallet transaction meta data after -zapwallettxes std::vector vWtx; if (gArgs.GetBoolArg("-zapwallettxes", false)) { uiInterface.InitMessage(_("Zapping all transactions from wallet...")); - std::unique_ptr dbw( - new CWalletDBWrapper(&bitdb, walletFile)); - std::unique_ptr tempWallet = - std::make_unique(chainParams, std::move(dbw)); + std::unique_ptr tempWallet = std::make_unique( + chainParams, name, CWalletDBWrapper::Create(path)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DBErrors::LOAD_OK) { InitError( @@ -4221,9 +4222,8 @@ int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::unique_ptr dbw( - new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *walletInstance = new CWallet(chainParams, std::move(dbw)); + CWallet *walletInstance = + new CWallet(chainParams, name, CWalletDBWrapper::Create(path)); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -224,14 +224,14 @@ std::vector &txIdsOut); /* Try to (very carefully!) recover wallet database (with a possible key * type filter) */ - static bool Recover(const std::string &filename, void *callbackDataIn, + static bool Recover(const fs::path &wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void *callbackData, CDataStream ssKey, CDataStream ssValue), std::string &out_backup_filename); /* Recover convenience-function to bypass the key filter callback, called * when verify fails, recovers everything */ - static bool Recover(const std::string &filename, + static bool Recover(const fs::path &wallet_path, std::string &out_backup_filename); /* Recover filter (used as callback), will only let keys (cryptographical * keys) as KV/key-type pass through */ @@ -241,12 +241,10 @@ * key) type */ static bool IsKeyType(const std::string &strType); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string &walletFile, - const fs::path &walletDir, + static bool VerifyEnvironment(const fs::path &wallet_path, std::string &errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string &walletFile, - const fs::path &walletDir, + static bool VerifyDatabaseFile(const fs::path &wallet_path, std::string &warningStr, std::string &errorStr); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -787,20 +787,21 @@ // // Try to (very carefully!) recover wallet file if there is a problem. // -bool CWalletDB::Recover(const std::string &filename, void *callbackDataIn, +bool CWalletDB::Recover(const fs::path &wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void *callbackData, CDataStream ssKey, CDataStream ssValue), std::string &out_backup_filename) { - return CDB::Recover(filename, callbackDataIn, recoverKVcallback, + return CDB::Recover(wallet_path, callbackDataIn, recoverKVcallback, out_backup_filename); } -bool CWalletDB::Recover(const std::string &filename, +bool CWalletDB::Recover(const fs::path &wallet_path, std::string &out_backup_filename) { // recover without a key filter callback // results in recovering all record types - return CWalletDB::Recover(filename, nullptr, nullptr, out_backup_filename); + return CWalletDB::Recover(wallet_path, nullptr, nullptr, + out_backup_filename); } bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, @@ -827,17 +828,15 @@ return true; } -bool CWalletDB::VerifyEnvironment(const std::string &walletFile, - const fs::path &walletDir, +bool CWalletDB::VerifyEnvironment(const fs::path &wallet_path, std::string &errorStr) { - return CDB::VerifyEnvironment(walletFile, walletDir, errorStr); + return CDB::VerifyEnvironment(wallet_path, errorStr); } -bool CWalletDB::VerifyDatabaseFile(const std::string &walletFile, - const fs::path &walletDir, +bool CWalletDB::VerifyDatabaseFile(const fs::path &wallet_path, std::string &warningStr, std::string &errorStr) { - return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, + return CDB::VerifyDatabaseFile(wallet_path, warningStr, errorStr, CWalletDB::Recover); }