From b6369329ce0c1eca929ff91ff960ee995dd0ea50 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 6 Jan 2026 16:09:38 -0800 Subject: [PATCH] wallettool: do not use fs::remove_all in createfromdump cleanup --- src/wallet/bdb.h | 17 +++++++++++++++-- src/wallet/dump.cpp | 8 +++++++- test/functional/tool_wallet.py | 7 +++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 81531c00a6..e757b93769 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -141,11 +141,24 @@ public: /** Verifies the environment and database file */ bool Verify(bilingual_str& error); - std::vector Files() override { return {}; } - /** Return path to main database filename */ std::string Filename() override { return fs::PathToString(env->Directory() / strFile); } + std::vector Files() override + { + std::vector files; + files.emplace_back(env->Directory() / strFile); + 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::string Format() override { return "bdb"; } /** * Pointer to shared database environment. diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index 6d8508fc72..d8e72a9309 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -284,11 +284,17 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling dump_file.close(); } + // On failure, gather the paths to remove + std::vector paths_to_remove = wallet->GetDatabase().Files(); + if (!name.empty()) paths_to_remove.push_back(wallet_path); + wallet.reset(); // The pointer deleter will close the wallet for us. // Remove the wallet dir if we have a failure if (!ret) { - fs::remove_all(wallet_path); + for (const auto& p : paths_to_remove) { + fs::remove(p); + } } return ret; diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index b1c4f34012..82fa3d4dff 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -396,7 +396,14 @@ class ToolWalletTest(BitcoinTestFramework): self.write_dump(dump_data, bad_sum_wallet_dump) self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "elementsregtest/wallets", "badload")) + if not self.options.descriptors: + os.rename(self.nodes[0].datadir + "/" + "elementsregtest/wallets/wallet.dat", self.nodes[0].datadir + "/" + "elementsregtest/wallets/default.wallet.dat") + self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') + assert os.path.exists(self.nodes[0].datadir + "/" + "elementsregtest/wallets") + assert not os.path.exists(self.nodes[0].datadir + "/" + "elementsregtest/wallets/wallet.dat") + self.log.info('Checking createfromdump with an unnamed wallet') + self.do_tool_createfromdump("", "wallet.dump") def run_test(self): self.wallet_path = os.path.join(self.nodes[0].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)