diff --git a/doc/release-notes.md b/doc/release-notes.md index ac49dc79095..8fcd2a9163e 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -63,6 +63,36 @@ RPC changes - The `fundrawtransaction` rpc will reject the previously deprecated `reserveChangeKey` option. +External wallet files +--------------------- + +The `-wallet=` option now accepts full paths instead of requiring wallets +to be located in the -walletdir directory. + +Newly created wallet format +--------------------------- + +If `-wallet=` is specified with a path that does not exist, it will now +create a wallet directory at the specified location (containing a wallet.dat +data file, a db.log file, and database/log.?????????? files) instead of just +creating a data file at the path and storing log files in the parent +directory. This should make backing up wallets more straightforward than +before because the specified wallet path can just be directly archived without +having to look in the parent directory for transaction log files. + +For backwards compatibility, wallet paths that are names of existing data files +in the `-walletdir` directory will continue to be accepted and interpreted the +same as before. + +Low-level RPC changes +--------------------- + +- When bitcoin is not started with any `-wallet=` options, the name of + the default wallet returned by `getwalletinfo` and `listwallets` RPCs is + now the empty string `""` instead of `"wallet.dat"`. If bitcoin is started + with any `-wallet=` options, there is no change in behavior, and the + name of any wallet is just its `` string. + Credits ======= diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 6f438b60e97..98965840c7b 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -32,7 +32,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector vCoins; LOCK(wallet.cs_wallet); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index a60d3b3b6d0..41f1e5786c9 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -46,7 +46,7 @@ std::string HelpMessageCli() strUsage += HelpMessageOpt("-rpcport=", strprintf(_("Connect to JSON-RPC on (default: %u or testnet: %u)"), defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort())); strUsage += HelpMessageOpt("-rpcuser=", _("Username for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcwait", _("Wait for RPC server to start")); - strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (argument is wallet filename in bitcoind directory, required if bitcoind/-Qt runs with multiple wallets)")); + strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)")); strUsage += HelpMessageOpt("-stdin", _("Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.")); strUsage += HelpMessageOpt("-stdinrpcpass", strprintf(_("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password."))); @@ -339,8 +339,8 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // check if we should use a special wallet endpoint std::string endpoint = "/"; - std::string walletName = gArgs.GetArg("-rpcwallet", ""); - if (!walletName.empty()) { + if (!gArgs.GetArgs("-rpcwallet").empty()) { + std::string walletName = gArgs.GetArg("-rpcwallet", ""); char *encodedURI = evhttp_uriencode(walletName.c_str(), walletName.size(), false); if (encodedURI) { endpoint = "/wallet/"+ std::string(encodedURI); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 5ba75cc91de..976aadc0afe 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -158,9 +158,7 @@ void TestGUI() for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - bitdb.MakeMock(); - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - CWallet wallet(std::move(dbw)); + CWallet wallet("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet.LoadWallet(firstRun); { @@ -261,9 +259,6 @@ void TestGUI() QPushButton* removeRequestButton = receiveCoinsDialog.findChild("removeRequestButton"); removeRequestButton->click(); QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1); - - bitdb.Flush(true); - bitdb.Reset(); } } diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 23c62791287..ebe7b48da06 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -52,20 +52,55 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) } } } + +CCriticalSection cs_db; +std::map g_dbenvs; //!< Map from directory name to open db environment. } // namespace +CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) +{ + fs::path env_directory; + if (fs::is_regular_file(wallet_path)) { + // Special case for backwards compatibility: if wallet path points to an + // existing file, treat it as the path to a BDB data file in a parent + // directory that also contains BDB log files. + env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + } else { + // Normal case: Interpret wallet path as a directory path containing + // data and log files. + env_directory = wallet_path; + database_filename = "wallet.dat"; + } + 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 environment: %s\n", ret, DbEnv::strerror(ret)); @@ -80,29 +115,25 @@ void CDBEnv::Reset() 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; + TryCreateDirectories(pathIn); if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); return false; @@ -150,7 +181,7 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) // failure is ok (well, not really, but it's not worse than what 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; } @@ -209,12 +240,15 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, recoverFunc_type return RECOVER_FAIL; // Try to recover: - bool fRecovered = (*recoverFunc)(strFile, out_backup_filename); + bool fRecovered = (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename); return (fRecovered ? RECOVER_OK : RECOVER_FAIL); } -bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +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 @@ -225,7 +259,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco int64_t now = GetTime(); newFilename = strprintf("%s.%d.bak", filename, now); - int result = bitdb.dbenv->dbrename(nullptr, filename.c_str(), nullptr, + 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); @@ -236,7 +270,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } 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); @@ -244,7 +278,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - std::unique_ptr pdbCopy = MakeUnique(bitdb.dbenv.get(), 0); + std::unique_ptr pdbCopy = MakeUnique(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -257,7 +291,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return false; } - DbTxn* ptxn = bitdb.TxnBegin(); + DbTxn* ptxn = env->TxnBegin(); for (CDBEnv::KeyValPair& row : salvagedData) { if (recoverKVcallback) @@ -279,8 +313,12 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco 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); @@ -291,7 +329,7 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return false; } - if (!bitdb.Open(walletDir, true)) { + if (!env->Open(true /* retry */)) { errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); return false; } @@ -299,12 +337,16 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return true; } -bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) +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); + CDBEnv::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename); if (r == CDBEnv::RECOVER_OK) { warningStr = strprintf(_("Warning: Wallet file corrupt, data salvaged!" @@ -414,8 +456,8 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb nFlags |= DB_CREATE; { - 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."); pdb = env->mapDb[strFilename]; @@ -442,7 +484,25 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (ret != 0) { 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& env : g_dbenvs) { + CheckUniqueFileid(env.second, strFilename, *pdb_temp); + } pdb = pdb_temp.release(); env->mapDb[strFilename] = pdb; @@ -490,7 +550,7 @@ void CDB::Close() Flush(); { - LOCK(env->cs_db); + LOCK(cs_db); --env->mapFileUseCount[strFile]; } } @@ -518,7 +578,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) 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 env->CloseDb(strFile); @@ -646,7 +706,7 @@ bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) 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 @@ -694,7 +754,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) 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/db.h b/src/wallet/db.h index 787135e400c..b1ce4515345 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -32,20 +33,19 @@ private: // shutdown problems/crashes caused by a static initialized internal 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, @@ -56,7 +56,7 @@ public: enum VerifyResult { VERIFY_OK, RECOVER_OK, RECOVER_FAIL }; - typedef bool (*recoverFunc_type)(const std::string& strFile, std::string& out_backup_filename); + typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename); VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename); /** * Salvage data from a file that Verify says is bad. @@ -68,7 +68,7 @@ public: typedef std::pair, std::vector > KeyValPair; 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); @@ -85,7 +85,8 @@ public: } }; -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. * For BerkeleyDB this is just a (env, strFile) tuple. @@ -100,9 +101,33 @@ public: } /** Create DB handle to real database */ - CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in) : - nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0), env(env_in), strFile(strFile_in) + CWalletDBWrapper(const fs::path& wallet_path, bool mock = false) : + nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), 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 MakeUnique(path); + } + + /** Return object for accessing dummy database with no read/write capabilities. */ + static std::unique_ptr CreateDummy() + { + return MakeUnique(); + } + + /** Return object for accessing temporary in-memory database. */ + static std::unique_ptr CreateMock() + { + return MakeUnique("", true /* mock */); } /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero @@ -113,10 +138,6 @@ public: */ 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); @@ -161,15 +182,15 @@ public: void Flush(); void Close(); - static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* flush the wallet passively (TRY_LOCK) 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, std::string& errorStr); + 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, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); public: template @@ -329,7 +350,7 @@ public: { if (!pdb || activeTxn) return false; - DbTxn* ptxn = bitdb.TxnBegin(); + DbTxn* ptxn = env->TxnBegin(); if (!ptxn) return false; activeTxn = ptxn; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 9ac48bff77c..e028cf4210b 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -35,7 +35,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup")); - strUsage += HelpMessageOpt("-wallet=", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in .)")); strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST)); strUsage += HelpMessageOpt("-walletdir=", _("Specify directory to hold wallets (default: /wallets if it exists, otherwise )")); strUsage += HelpMessageOpt("-walletnotify=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); @@ -66,7 +66,7 @@ bool WalletParameterInteraction() return true; } - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + gArgs.SoftSetArg("-wallet", ""); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { @@ -230,18 +230,22 @@ bool VerifyWallets() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - if (boost::filesystem::path(walletFile).filename() != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. -wallet parameter must only specify a filename (not a path)."), walletFile)); - } - - if (SanitizeString(walletFile, SAFE_CHARS_FILENAME) != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. Invalid characters in -wallet filename."), walletFile)); - } - + // Do some checking on wallet path. It should be either a: + // + // 1. Path where a directory can be created. + // 2. Path to an existing directory. + // 3. Path to a symlink to a directory. + // 4. For backwards compatibility, the name of a data file in -walletdir. fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); - - if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { - return InitError(strprintf(_("Error loading wallet %s. -wallet filename must be a regular file."), walletFile)); + fs::file_type path_type = fs::symlink_status(wallet_path).type(); + if (!(path_type == fs::file_not_found || path_type == fs::directory_file || + (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || + (path_type == fs::regular_file && fs::path(walletFile).filename() == walletFile))) { + return InitError(strprintf( + _("Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " + "database/log.?????????? files can be stored, a location where such a directory could be created, " + "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)"), + walletFile, GetWalletDir())); } if (!wallet_paths.insert(wallet_path).second) { @@ -249,21 +253,21 @@ bool VerifyWallets() } 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; + CWallet dummyWallet("dummy", CWalletDBWrapper::CreateDummy()); std::string backup_filename; - if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { + if (!CWalletDB::Recover(wallet_path, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { return false; } } 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); } @@ -284,7 +288,7 @@ bool OpenWallets() } for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile); + CWallet * const pwallet = CWallet::CreateWalletFromFile(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 index e6510cc214b..7b20bd7b021 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -13,13 +13,13 @@ BOOST_FIXTURE_TEST_SUITE(accounting_tests, WalletTestingSetup) static void -GetResults(CWallet *wallet, std::map& results) +GetResults(CWallet& wallet, std::map& results) { std::list aes; results.clear(); - BOOST_CHECK(wallet->ReorderTransactions() == DB_LOAD_OK); - wallet->ListAccountCreditDebit("", aes); + BOOST_CHECK(wallet.ReorderTransactions() == DB_LOAD_OK); + wallet.ListAccountCreditDebit("", aes); for (CAccountingEntry& ae : aes) { results[ae.nOrderPos] = ae; @@ -33,28 +33,28 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) CAccountingEntry ae; std::map results; - LOCK(pwalletMain->cs_wallet); + LOCK(m_wallet.cs_wallet); ae.strAccount = ""; ae.nCreditDebit = 1; 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[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); 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[0].nTime == 1333333333); BOOST_CHECK(results[0].strComment.empty()); @@ -65,13 +65,13 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) 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[0].nTime == 1333333333); BOOST_CHECK(1 == vpwtx[0]->nOrderPos); BOOST_CHECK(results[2].nTime == 1333333336); @@ -85,8 +85,8 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ++tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -95,15 +95,15 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ++tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); 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[1].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); @@ -116,12 +116,12 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) 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[1].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 18abf9a9db3..77ccd0b8d8d 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -6,26 +6,21 @@ #include #include +#include WalletTestingSetup::WalletTestingSetup(const std::string& chainName): - TestingSetup(chainName) + TestingSetup(chainName), m_wallet("mock", CWalletDBWrapper::CreateMock()) { - bitdb.MakeMock(); bool fFirstRun; g_address_type = OUTPUT_TYPE_DEFAULT; g_change_type = OUTPUT_TYPE_DEFAULT; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = MakeUnique(std::move(dbw)); - pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain.get()); + m_wallet.LoadWallet(fFirstRun); + RegisterValidationInterface(&m_wallet); RegisterWalletRPCCommands(tableRPC); } WalletTestingSetup::~WalletTestingSetup() { - UnregisterValidationInterface(pwalletMain.get()); - - bitdb.Flush(true); - bitdb.Reset(); + UnregisterValidationInterface(&m_wallet); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index c03aec7f878..663836a9552 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -15,7 +15,7 @@ struct WalletTestingSetup: public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr pwalletMain; + CWallet m_wallet; }; #endif diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 9db5d639227..41348b50a4a 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -36,7 +36,7 @@ typedef std::set CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet; +static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); static std::vector vCoins; static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) @@ -382,7 +382,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -397,7 +397,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -409,7 +409,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); vpwallets.insert(vpwallets.begin(), &wallet); UniValue keys; keys.setArray(); @@ -471,7 +471,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); LOCK(wallet.cs_wallet); wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); @@ -486,7 +486,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); JSONRPCRequest request; request.params.setArray(); @@ -516,7 +516,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // debit functions. BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); CWalletTx wtx(&wallet, MakeTransactionRef(coinbaseTxns.back())); LOCK2(cs_main, wallet.cs_wallet); wtx.hashBlock = chainActive.Tip()->GetBlockHash(); @@ -562,27 +562,25 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 // expanded to cover more corner cases of smart time logic. BOOST_AUTO_TEST_CASE(ComputeTimeSmart) { - CWallet wallet; - // 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); @@ -591,12 +589,12 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) 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"); @@ -608,10 +606,9 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - ::bitdb.MakeMock(); g_address_type = OUTPUT_TYPE_DEFAULT; g_change_type = OUTPUT_TYPE_DEFAULT; - wallet.reset(new CWallet(std::unique_ptr(new CWalletDBWrapper(&bitdb, "wallet_test.dat")))); + wallet = MakeUnique("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); @@ -623,8 +620,6 @@ public: ~ListCoinsTestingSetup() { wallet.reset(); - ::bitdb.Flush(true); - ::bitdb.Reset(); } CWalletTx& AddTx(CRecipient recipient) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a014f5b2a0d..0c468878e11 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -45,7 +45,6 @@ OutputType g_address_type = OUTPUT_TYPE_NONE; OutputType g_change_type = OUTPUT_TYPE_NONE; bool g_wallet_allow_fallback_fee = true; // CWallet::GetDestValues(const std::string& prefix) const return values; } -CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) +CWallet* CWallet::CreateWalletFromFile(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 = MakeUnique(std::move(dbw)); + std::unique_ptr tempWallet = MakeUnique(name, CWalletDBWrapper::Create(path)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); @@ -3929,8 +3929,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *walletInstance = new CWallet(std::move(dbw)); + CWallet *walletInstance = new CWallet(name, CWalletDBWrapper::Create(path)); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DB_LOAD_OK) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4db45f16ef5..3e2d1794d89 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -15,6 +15,7 @@ #include #include