diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 79851dff33f..f5a18266edb 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -340,6 +341,53 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr) return true; } +std::vector BerkeleyDatabase::Files() +{ + std::vector files; + // If the wallet is the *only* file, clean up the entire BDB environment + constexpr auto build_files_list = [](std::vector& files, const std::shared_ptr& env, const fs::path& filename) { + if (env->m_databases.size() != 1) return false; + + const auto env_dir = env->Directory(); + const auto db_subdir = env_dir / "database"; + if (fs::exists(db_subdir)) { + if (!fs::is_directory(db_subdir)) return false; + for (const auto& entry : fs::directory_iterator(db_subdir)) { + const auto& path = entry.path().filename(); + if (!fs::PathToString(path).starts_with("log.")) { + return false; + } + files.emplace_back(entry.path()); + } + } + const std::set allowed_paths = { + filename, + "db.log", + ".walletlock", + "database" + }; + for (const auto& entry : fs::directory_iterator(env_dir)) { + const auto& path = entry.path().filename(); + if (allowed_paths.contains(path)) { + files.emplace_back(entry.path()); + } else if (fs::is_directory(entry.path())) { + // Subdirectories can't possibly be using this db env, and is expected if this is a non-directory wallet + // Do not include them in Files, but still allow the env cleanup + } else { + return false; + } + } + return true; + }; + try { + if (build_files_list(files, env, m_filename)) return files; + } catch (...) { + // Give up building the comprehensive file list if any error occurs + } + // Otherwise, it's only really safe to delete the one wallet file + return {env->Directory() / m_filename}; +} + void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile) { dbenv->txn_checkpoint(0, 0, 0); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index ec773fd1770..a7cf953ed21 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -132,20 +132,7 @@ public: /** Return path to main database filename */ std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); } - std::vector Files() override - { - std::vector files; - files.emplace_back(env->Directory() / m_filename); - if (env->m_databases.size() == 1) { - files.emplace_back(env->Directory() / "db.log"); - files.emplace_back(env->Directory() / ".walletlock"); - files.emplace_back(env->Directory() / "database" / "log.0000000001"); - files.emplace_back(env->Directory() / "database"); - // Note that this list is not exhaustive as BDB may create more log files, and possibly other ones too - // However it should be good enough for the only calls to Files() - } - return files; - } + std::vector Files() override; std::string Format() override { return "bdb"; } /** diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 913c7453205..45adda65ead 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4532,7 +4532,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); if (!to_reload) { LogError("Failed to load wallet '%s' after migration. Rolling back migration to preserve consistency. " - "Error cause: %s\n", wallet_name, error.original); + "Error cause: %s\n", name, error.original); return false; } return true; @@ -4581,6 +4581,12 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // First change to using SQLite if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; + // In case we're migrating from file to directory, move the backup into it + this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); + backup_path = this_wallet_dir / backup_filename; + fs::rename(res.backup_path, backup_path); + res.backup_path = backup_path; + // Do the migration of keys and scripts for non-blank wallets, and cleanup if it fails success = local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); if (!success) { @@ -4634,9 +4640,6 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } if (!success) { // Migration failed, cleanup - // Before deleting the wallet's directory, copy the backup file to the top-level wallets dir - fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); - fs::rename(backup_path, temp_backup_location); // Make list of wallets to cleanup std::vector> created_wallets; @@ -4680,15 +4683,12 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Convert the backup file to the wallet db file by renaming it and moving it into the wallet's directory. // Reload it into memory if the wallet was previously loaded. bilingual_str restore_error; - const auto& ptr_wallet = RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/was_loaded); + const auto& ptr_wallet = RestoreWallet(context, backup_path, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/was_loaded); if (!restore_error.empty()) { error += restore_error + _("\nUnable to restore backup of wallet."); return util::Error{error}; } - // The wallet directory has been restored, but just in case, copy the previously created backup to the wallet dir - fs::rename(temp_backup_location, backup_path); - // Verify that there is no dangling wallet: when the wallet wasn't loaded before, expect null. // This check is performed after restoration to avoid an early error before saving the backup. bool wallet_reloaded = ptr_wallet != nullptr; diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 979804a5fea..788d9b0ee8d 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -410,17 +410,30 @@ class ToolWalletTest(BitcoinTestFramework): self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') assert not (self.nodes[0].wallets_path / "badload").is_dir() if not self.options.descriptors: - os.rename(self.nodes[0].wallets_path / "wallet.dat", self.nodes[0].wallets_path / "default.wallet.dat") + os.rename(self.nodes[0].wallets_path / "wallet.dat", self.nodes[0].wallets_path / "../default.wallet.dat") + (self.nodes[0].wallets_path / "db.log").unlink(missing_ok=True) self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') assert self.nodes[0].wallets_path.exists() assert not (self.nodes[0].wallets_path / "wallet.dat").exists() + if not self.options.descriptors: + assert not (self.nodes[0].wallets_path / "db.log").exists() self.log.info('Checking createfromdump with an unnamed wallet') self.do_tool_createfromdump("", "wallet.dump") assert (self.nodes[0].wallets_path / "wallet.dat").exists() os.unlink(self.nodes[0].wallets_path / "wallet.dat") if not self.options.descriptors: - os.rename(self.nodes[0].wallets_path / "default.wallet.dat", self.nodes[0].wallets_path / "wallet.dat") + os.rename(self.nodes[0].wallets_path / "../default.wallet.dat", self.nodes[0].wallets_path / "wallet.dat") + + self.log.info('Checking createfromdump with multiple non-directory wallets') + assert not (self.nodes[0].wallets_path / "wallet.dat").is_dir() + assert (self.nodes[0].wallets_path / "db.log").exists() + os.rename(self.nodes[0].wallets_path / "wallet.dat", self.nodes[0].wallets_path / "test.dat") + self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') + assert not (self.nodes[0].wallets_path / "wallet.dat").exists() + assert (self.nodes[0].wallets_path / "test.dat").exists() + assert (self.nodes[0].wallets_path / "db.log").exists() + os.rename(self.nodes[0].wallets_path / "test.dat", self.nodes[0].wallets_path / "wallet.dat") def test_chainless_conflicts(self): self.log.info("Test wallet tool when wallet contains conflicting transactions")