cryptogera/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-02-01 03:01:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34370: [29.x] Fix #34222 backport bugs

Browse Source 
65173944ed60df3b9cffca95932aed8720921478 QA: tool_wallet: Check that db.log is deleted with a lone legacy wallet, but not with a shared db environment (Luke Dashjr)
69a6b9b1152ba0bb3edab6d2a54509fd416b24c8 Bugfix: Wallet/Migration: Move backup into wallet directory when migrating from non-directory (Luke Dashjr)
cef01d0be5223e9d33efc897d7fbe5d0a08692c0 Wallet/Migration: Skip moving the backup file back and forth for no reason (Luke Dashjr)
60f529027c6eacbdc298fab50192f8c60d7082a1 Wallet/Migration: If loading the new watchonly or solvables wallet fails, log the correct wallet name in error message (Luke Dashjr)
7475d134f6a3a6039ab6b9d39706ade47c764aa8 Wallet/bdb: Safely and correctly list files only used by the single wallet (Luke Dashjr)

Pull request description:

ACKs for top commit:
  achow101:
    ACK 65173944ed60df3b9cffca95932aed8720921478
  furszy:
    light ACK 65173944ed60df3b9cffca95932aed8720921478

Tree-SHA512: c10fe00dde512ca78cd6939a748b3875d0b40e9714997aedfd939a1dffdc7eaa2fd1779f3972a34b1c1d9a97d8f1ee1e082c970de15ac0e2ef5d9bbf3dc1d89a
This commit is contained in:
merge-script 2026-01-29 17:48:10 +01:00
commit 115172ceb8
No known key found for this signature in database
GPG Key ID: 9B79B45691DB4173
4 changed files with 72 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/wallet/bdb.cpp
View File

@ -16,6 +16,7 @@
#include <util/strencodings.h>
#include <util/translation.h>
#include <set>
#include <stdint.h>
#include <db_cxx.h>
@ -340,6 +341,53 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr)
return true;
}
std::vector<fs::path> BerkeleyDatabase::Files()
{
std::vector<fs::path> files;
// If the wallet is the *only* file, clean up the entire BDB environment
constexpr auto build_files_list = [](std::vector<fs::path>& files, const std::shared_ptr<BerkeleyEnvironment>& 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<fs::path> 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);

15
src/wallet/bdb.h
View File

@ -132,20 +132,7 @@ public:
/** Return path to main database filename */
std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); }
std::vector<fs::path> Files() override
{
std::vector<fs::path> 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<fs::path> Files() override;
std::string Format() override { return "bdb"; }
/**

16
src/wallet/wallet.cpp
View File

@ -4532,7 +4532,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
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<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
// 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<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
}
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<std::shared_ptr<CWallet>> created_wallets;
@ -4680,15 +4683,12 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
// 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;

17
test/functional/tool_wallet.py
View File

@ -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")