diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -57,9 +57,6 @@ void MakeMock(); bool IsMock() const { return fMockDb; } bool IsInitialized() const { return fDbEnvInit; } - bool IsDatabaseLoaded(const std::string &db_filename) const { - return m_databases.find(db_filename) != m_databases.end(); - } fs::path Directory() const { return strPath; } bool Open(bilingual_str &error); @@ -84,9 +81,6 @@ std::shared_ptr GetWalletEnv(const fs::path &wallet_path, std::string &database_filename); -/** Return whether a BDB wallet database is currently loaded. */ -bool IsBDBWalletLoaded(const fs::path &wallet_path); - /** Check format of database file */ bool IsBerkeleyBtree(const fs::path &path); @@ -161,7 +155,7 @@ void ReloadDbEnv() override; /** Verifies the environment and database file */ - bool Verify(bilingual_str &error) override; + bool Verify(bilingual_str &error); /** * Pointer to shared database environment. diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -61,21 +61,6 @@ return memcmp(value, &rhs.value, sizeof(value)) == 0; } -bool IsBDBWalletLoaded(const fs::path &wallet_path) { - fs::path env_directory; - std::string database_filename; - SplitWalletPath(wallet_path, env_directory, database_filename); - - LOCK(cs_db); - auto env = g_dbenvs.find(env_directory.string()); - if (env == g_dbenvs.end()) { - return false; - } - - auto database = env->second.lock(); - return database && database->IsDatabaseLoaded(database_filename); -} - /** * @param[in] wallet_path Path to wallet directory. Or (for backwards * compatibility only) a path to a berkeley btree data file inside a wallet diff --git a/src/wallet/db.h b/src/wallet/db.h --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -160,9 +160,6 @@ unsigned int nLastFlushed; int64_t nLastWalletUpdate; - /** Verifies the environment and database file */ - virtual bool Verify(bilingual_str &error) = 0; - std::string m_file_path; /** Make a DatabaseBatch connected to this database */ @@ -214,7 +211,6 @@ bool PeriodicFlush() override { return true; } void IncrementUpdateCounter() override { ++nUpdateCounter; } void ReloadDbEnv() override {} - bool Verify(bilingual_str &errorStr) override { return true; } std::unique_ptr MakeBatch(const char *mode = "r+", bool flush_on_close = true) override { return std::make_unique(); diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -62,14 +62,11 @@ return false; } + DatabaseOptions options; + DatabaseStatus status; + options.verify = true; bilingual_str error_string; - std::vector warnings; - bool verify_success = - CWallet::Verify(chain, wallet_file, error_string, warnings); - if (!warnings.empty()) { - chain.initWarning(Join(warnings, Untranslated("\n"))); - } - if (!verify_success) { + if (!MakeWalletDatabase(wallet_file, options, status, error_string)) { chain.initError(error_string); return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -73,6 +73,9 @@ DatabaseStatus &status, bilingual_str &error, std::vector &warnings); std::unique_ptr HandleLoadWallet(LoadWalletFn load_wallet); +std::unique_ptr +MakeWalletDatabase(const std::string &name, const DatabaseOptions &options, + DatabaseStatus &status, bilingual_str &error); //! -paytxfee default constexpr Amount DEFAULT_PAY_TX_FEE = Amount::zero(); @@ -1378,11 +1381,6 @@ */ bool AbandonTransaction(const TxId &txid); - //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(interfaces::Chain &chain, const std::string &name, - bilingual_str &error_string, - std::vector &warnings); - /** * Initializes the wallet, returns a new CWallet instance or a null pointer * in case of an error. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -227,7 +227,7 @@ const DatabaseOptions &options, DatabaseStatus &status, bilingual_str &error, std::vector &warnings) { try { - if (!CWallet::Verify(chain, name, error, warnings)) { + if (!MakeWalletDatabase(name, options, status, error)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return nullptr; @@ -299,7 +299,7 @@ // Wallet::Verify will check if we're trying to create a wallet with a // duplicate name. - if (!CWallet::Verify(chain, name, error, warnings)) { + if (!MakeWalletDatabase(name, options, status, error)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_VERIFY; @@ -4214,16 +4214,15 @@ return values; } -bool CWallet::Verify(interfaces::Chain &chain, const std::string &name, - bilingual_str &error_string, - std::vector &warnings) { +std::unique_ptr +MakeWalletDatabase(const std::string &name, const DatabaseOptions &options, + DatabaseStatus &status, bilingual_str &error_string) { // 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. - LOCK(cs_wallets); const fs::path &wallet_path = fs::absolute(name, GetWalletDir()); fs::file_type path_type = fs::symlink_status(wallet_path).type(); if (!(path_type == fs::file_not_found || path_type == fs::directory_file || @@ -4238,30 +4237,10 @@ "or (for backwards compatibility) the name of an " "existing data file in -walletdir (%s)", name, GetWalletDir())); - return false; - } - - // Make sure that the wallet path doesn't clash with an existing wallet path - if (IsWalletLoaded(wallet_path)) { - error_string = Untranslated(strprintf( - "Error loading wallet %s. Duplicate -wallet filename specified.", - name)); - return false; - } - - // Keep same database environment instance across Verify/Recover calls - // below. - std::unique_ptr database = - CreateWalletDatabase(wallet_path); - - try { - return database->Verify(error_string); - } catch (const fs::filesystem_error &e) { - error_string = - Untranslated(strprintf("Error loading wallet %s. %s", name, - fsbridge::get_filesystem_error_message(e))); - return false; + status = DatabaseStatus::FAILED_BAD_PATH; + return nullptr; } + return MakeDatabase(wallet_path, options, status, error_string); } std::shared_ptr CWallet::CreateWalletFromFile( diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -301,9 +301,6 @@ std::string &strType, std::string &strErr, const KeyFilterFn &filter_fn = nullptr); -/** Return whether a wallet database is currently loaded. */ -bool IsWalletLoaded(const fs::path &wallet_path); - /** Return object for accessing database at specified path. */ std::unique_ptr CreateWalletDatabase(const fs::path &path); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1158,10 +1158,6 @@ return MakeBerkeleyDatabase(path, options, status, error); } -bool IsWalletLoaded(const fs::path &wallet_path) { - return IsBDBWalletLoaded(wallet_path); -} - /** Return object for accessing database at specified path. */ std::unique_ptr CreateWalletDatabase(const fs::path &path) { std::string filename; diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -282,17 +282,32 @@ self.nodes[0].loadwallet, 'wallets') # Fail to load duplicate wallets + path = os.path.join( + self.options.tmpdir, + "node0", + "regtest", + "wallets", + "w1", + "wallet.dat") assert_raises_rpc_error( -4, - 'Wallet file verification failed. Error loading wallet w1. Duplicate -wallet filename specified.', + "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format( + path), self.nodes[0].loadwallet, wallet_names[0]) # Fail to load duplicate wallets by different ways (directory and # filepath) + path = os.path.join( + self.options.tmpdir, + "node0", + "regtest", + "wallets", + "wallet.dat") assert_raises_rpc_error( -4, - "Wallet file verification failed. Error loading wallet wallet.dat. Duplicate -wallet filename specified.", + "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format( + path), self.nodes[0].loadwallet, 'wallet.dat')