From 8b5e7297c02f3100a9cb27bfe206e3fc617ec173 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 4 Aug 2020 20:45:28 -0400 Subject: [PATCH] refactor: Pass wallet database into CWallet::Create No changes in behavior --- src/wallet/bdb.cpp | 1 - src/wallet/bdb.h | 3 +++ src/wallet/db.cpp | 8 -------- src/wallet/db.h | 8 ++++---- src/wallet/load.cpp | 8 ++++++-- src/wallet/test/wallet_tests.cpp | 5 ++++- src/wallet/wallet.cpp | 17 +++++++++-------- src/wallet/wallet.h | 2 +- 8 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 8f8652bb0b133..fbb3d2cac57dd 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -359,7 +359,6 @@ void BerkeleyDatabase::Open(const char* pszMode) if (ret != 0) { throw std::runtime_error(strprintf("BerkeleyDatabase: Error %d, can't open database %s", ret, strFile)); } - m_file_path = (env->Directory() / strFile).string(); // Call CheckUniqueFileid on the containing BDB environment to // avoid BDB data consistency bugs that happen when different data diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 9f24a2f10bc87..fd5a49acc3132 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -144,6 +144,9 @@ class BerkeleyDatabase : public WalletDatabase /** Verifies the environment and database file */ bool Verify(bilingual_str& error); + /** Return path to main database filename */ + std::string Filename() override { return (env->Directory() / strFile).string(); } + /** * Pointer to shared database environment. * diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 1eb82a03c7cd2..bd1d1147303f0 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -23,11 +23,3 @@ void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std:: database_filename = "wallet.dat"; } } - -fs::path WalletDataFilePath(const fs::path& wallet_path) -{ - fs::path env_directory; - std::string database_filename; - SplitWalletPath(wallet_path, env_directory, database_filename); - return env_directory / database_filename; -} diff --git a/src/wallet/db.h b/src/wallet/db.h index 6e11d7de88eb3..96d1f44d91f08 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -18,8 +18,6 @@ struct bilingual_str; -/** Given a wallet directory path or legacy file path, return path to main data file in the wallet database. */ -fs::path WalletDataFilePath(const fs::path& wallet_path); void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); /** RAII class that provides access to a WalletDatabase */ @@ -142,13 +140,14 @@ class WalletDatabase virtual void ReloadDbEnv() = 0; + /** Return path to main database file for logs and error messages. */ + virtual std::string Filename() = 0; + std::atomic nUpdateCounter; unsigned int nLastSeen; unsigned int nLastFlushed; int64_t nLastWalletUpdate; - std::string m_file_path; - /** Make a DatabaseBatch connected to this database */ virtual std::unique_ptr MakeBatch(const char* mode = "r+", bool flush_on_close = true) = 0; }; @@ -189,6 +188,7 @@ class DummyDatabase : public WalletDatabase bool PeriodicFlush() override { return true; } void IncrementUpdateCounter() override { ++nUpdateCounter; } void ReloadDbEnv() override {} + std::string Filename() override { return "dummy"; } std::unique_ptr MakeBatch(const char* mode = "r+", bool flush_on_close = true) override { return MakeUnique(); } }; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 84dc5adf662a2..c5d045e9ef641 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -68,10 +68,14 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal bool LoadWallets(interfaces::Chain& chain, const std::vector& wallet_files) { try { - for (const std::string& walletFile : wallet_files) { + for (const std::string& name : wallet_files) { + DatabaseOptions options; + DatabaseStatus status; + options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets() bilingual_str error; std::vector warnings; - std::shared_ptr pwallet = CWallet::CreateWalletFromFile(chain, walletFile, error, warnings); + std::unique_ptr database = MakeWalletDatabase(name, options, status, error); + std::shared_ptr pwallet = database ? CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b3001efd55db4..6b98482f9809c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -37,9 +37,12 @@ BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) static std::shared_ptr TestLoadWallet(interfaces::Chain& chain) { + DatabaseOptions options; + DatabaseStatus status; bilingual_str error; std::vector warnings; - auto wallet = CWallet::CreateWalletFromFile(chain, "", error, warnings); + auto database = MakeWalletDatabase("", options, status, error); + auto wallet = CWallet::Create(chain, "", std::move(database), options.create_flags, error, warnings); wallet->postInitProcess(); return wallet; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b5928f2b7e0d4..1751cbf5bc099 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -203,12 +203,13 @@ namespace { std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const std::string& name, Optional load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector& warnings) { try { - if (!MakeWalletDatabase(name, options, status, error)) { + std::unique_ptr database = MakeWalletDatabase(name, options, status, error); + if (!database) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return nullptr; } - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, name, error, warnings); + std::shared_ptr wallet = CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; return nullptr; @@ -260,7 +261,8 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, const std::strin } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. - if (!MakeWalletDatabase(name, options, status, error)) { + std::unique_ptr database = MakeWalletDatabase(name, options, status, error); + if (!database) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_VERIFY; return nullptr; @@ -274,7 +276,7 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, const std::strin } // Make the wallet - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, name, error, warnings, wallet_creation_flags); + std::shared_ptr wallet = CWallet::Create(chain, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -3803,10 +3805,9 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags) +std::shared_ptr CWallet::Create(interfaces::Chain& chain, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) { - fs::path path = fs::absolute(name, GetWalletDir()); - const std::string walletFile = WalletDataFilePath(path).string(); + const std::string& walletFile = database->Filename(); chain.initMessage(_("Loading wallet...").translated); @@ -3814,7 +3815,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, bool fFirstRun = true; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr walletInstance(new CWallet(&chain, name, CreateWalletDatabase(path)), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(&chain, name, std::move(database)), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c9ebb94dc14f1..c54480612ac3b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1146,7 +1146,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool MarkReplaced(const uint256& originalHash, const uint256& newHash); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags = 0); + static std::shared_ptr Create(interfaces::Chain& chain, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); /** * Wallet post-init setup