From e7bbeb42480bdbc1d130f7b36b72c9b5e27051e9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 7 Jan 2026 11:08:57 +0000 Subject: [PATCH 1/4] partial Merge bitcoin/bitcoin#34156: wallet: fix unnamed legacy wallet migration failure BACKPORT NOTE: Code taken from the 28.x adaptation (bitcoin#34223: e1e9d71da9, e47af69222, 1d4662441f, 75b59e5aba, fb4406e63a, 86eaf71e60, 77622e000b) because master's commits target the BERKELEY_RO-only migration path. Dropped test_pruned_wallet_backup (needs bitcoin#30678). The backup filename keeps Dash's basename reduction for path-style wallet names (see the TODO for bitcoin#32273) and only adopts MigrationPrefixName for the unnamed wallet. b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb test: coverage for migration failure when last sync is beyond prune height (furszy) 82caa8193a3e36f248dcc949e0cd41def191efac wallet: migration, fix watch-only and solvables wallets names (furszy) d70b159c42008ac3b63d1c43d99d4f1316d2f1ef wallet: improve post-migration logging (furszy) f011e0f0680a8c39988ae57dae57eb86e92dd449 test: restorewallet, coverage for existing dirs, unnamed wallet and prune failure (furszy) 36093bde63286e19821a9e62cdff1712b6245dc7 test: add coverage for unnamed wallet migration failure (furszy) f4c7e28e80bf9af50b03a770b641fd309a801589 wallet: fix unnamed wallet migration failure (furszy) 4ed0693a3f2a427ef9e7ad016930ec29fa244995 wallet: RestoreWallet failure, erase only what was created (furszy) Pull request description: Minimal fix for #34128. The issue occurs during the migration of a legacy unnamed wallet (the legacy "default" wallet). When the migration fails, the cleanup logic is triggered to roll back the state, which involves erasing the newly created descriptor wallets directories. Normally, this only affects the parent directories of named wallets, since they each reside in their own directories. However, because the unnamed wallet resides directly in the top-level `/wallets/` folder, this logic accidentally deletes the main directory. The fix ensures that only the wallet.dat file of the unnamed wallet is touched and restored, preserving the wallet in BDB format and leaving the main `/wallets/` directory intact. #### Story Line: #32273 fixed a different set of issues and, in doing so, uncovered this one. Before the mentioned PR, backups were stored in the same directory as the wallet.dat file. On a migration failure, the backup was then copied to the top-level `/wallets/` directory. For the unnamed legacy wallet, the wallet directory is the `/wallets/` directory, so the source and destination paths were identical. As a result, we threw early in the `fs::copy_file` call ([here](https://github.com/bitcoin/bitcoin/blob/29.x/src/wallet/wallet.cpp#L4572)) because the file already existed, as we were trying to copy the file onto itself. This caused the cleanup logic to abort early on and never reach the removal line. #### Testing Notes: Cherry-pick the test commit on top of master and run it. You will see the failure and realize the reason by reading the test code. ACKs for top commit: achow101: ACK b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb davidgumberg: crACK https://github.com/bitcoin/bitcoin/commit/b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb w0xlt: ACK https://github.com/bitcoin/bitcoin/commit/b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb willcl-ark: ACK b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb Tree-SHA512: d0be14c0ed6417f999c3f2f429652c2407097d0cc18453c91653e57ae4b5375b327ad3b2553d9ea6ff46a3ae00cdbd5ab325b94eba763072c4fc5a773b85618b Co-authored-by: merge-script --- src/wallet/wallet.cpp | 138 +++++++++++++++++++++------- test/functional/wallet_backup.py | 83 ++++++++++++++++- test/functional/wallet_migration.py | 109 +++++++++++++++++++++- 3 files changed, 287 insertions(+), 43 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ba76e246f8ba..65b8246664cb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -517,6 +517,7 @@ std::shared_ptr RestoreWallet(WalletContext& context, const fs::path& b const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::u8path(wallet_name)); auto wallet_file = wallet_path / "wallet.dat"; std::shared_ptr wallet; + bool wallet_file_copied = false; bool created_parent_dir = false; try { @@ -526,14 +527,34 @@ std::shared_ptr RestoreWallet(WalletContext& context, const fs::path& b return nullptr; } - if (fs::exists(wallet_path) || !TryCreateDirectories(wallet_path)) { - error = Untranslated(strprintf("Failed to create database path '%s'. Database already exists.", fs::PathToString(wallet_path))); - status = DatabaseStatus::FAILED_ALREADY_EXISTS; - return nullptr; + // Wallet directories are allowed to exist, but must not contain a .dat file. + // Any existing wallet database is treated as a hard failure to prevent overwriting. + if (fs::exists(wallet_path)) { + // If this is a file, it is the db and we don't want to overwrite it. + if (!fs::is_directory(wallet_path)) { + error = Untranslated(strprintf("Failed to restore wallet. Database file exists '%s'.", fs::PathToString(wallet_path))); + status = DatabaseStatus::FAILED_ALREADY_EXISTS; + return nullptr; + } + + // Check we are not going to overwrite an existing db file + if (fs::exists(wallet_file)) { + error = Untranslated(strprintf("Failed to restore wallet. Database file exists in '%s'.", fs::PathToString(wallet_file))); + status = DatabaseStatus::FAILED_ALREADY_EXISTS; + return nullptr; + } + } else { + // The directory doesn't exist, create it + if (!TryCreateDirectories(wallet_path)) { + error = Untranslated(strprintf("Failed to restore database path '%s'.", fs::PathToString(wallet_path))); + status = DatabaseStatus::FAILED_ALREADY_EXISTS; + return nullptr; + } + created_parent_dir = true; } - created_parent_dir = true; fs::copy_file(backup_file, wallet_file, fs::copy_options::none); + wallet_file_copied = true; wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings); } catch (const std::exception& e) { @@ -542,8 +563,19 @@ std::shared_ptr RestoreWallet(WalletContext& context, const fs::path& b error += strprintf(Untranslated("Unexpected exception: %s"), e.what()); status = DatabaseStatus::FAILED_LOAD; } - if (!wallet && created_parent_dir) { - fs::remove_all(wallet_path); + + // Remove created wallet path only when loading fails + if (!wallet) { + if (wallet_file_copied) fs::remove(wallet_file); + // Clean up the parent directory if we created it during restoration. + // Verifying a corrupt BDB file leaves its environment files (db.log, + // .walletlock, database/) next to the wallet file, so the directory is + // not necessarily empty. It did not exist before this call, so removing + // it recursively only touches what this restore created. + // TODO(bitcoin#28710): once BDB is gone nothing but wallet.dat can be + // created here; restore upstream's `Assume(fs::is_empty(wallet_path)); + // fs::remove(wallet_path);` (bitcoin#34156, 4ed0693a3f). + if (created_parent_dir) fs::remove_all(wallet_path); } return wallet; @@ -5022,6 +5054,15 @@ bool CWallet::CanGrindR() const return !IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER); } +// Returns wallet prefix for migration. +// Used to name the backup file and newly created wallets. +// E.g. a watch-only wallet is named "_watchonly". +static std::string MigrationPrefixName(CWallet& wallet) +{ + const std::string& name{wallet.GetName()}; + return name.empty() ? "default_wallet" : name; +} + bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, MigrationResult& res) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { AssertLockHeld(wallet.cs_wallet); @@ -5053,7 +5094,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, DatabaseStatus status; std::vector warnings; - std::string wallet_name = wallet.GetName() + "_watchonly"; + std::string wallet_name = MigrationPrefixName(wallet) + "_watchonly"; std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); if (!database) { error = strprintf(_("Wallet file creation failed: %s"), error); @@ -5090,7 +5131,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, DatabaseStatus status; std::vector warnings; - std::string wallet_name = wallet.GetName() + "_solvables"; + std::string wallet_name = MigrationPrefixName(wallet) + "_solvables"; std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); if (!database) { error = strprintf(_("Wallet file creation failed: %s"), error); @@ -5171,7 +5212,12 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle std::string name = to_reload->GetName(); to_reload.reset(); to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); - return to_reload != nullptr; + 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); + return false; + } + return true; }; // Before anything else, check if there is something to migrate. @@ -5184,10 +5230,13 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle // Make a backup of the DB in the wallet's directory with a unique filename // based on the wallet name and current timestamp. The backup filename uses - // the basename of the wallet path (or "wallet" if the name is blank) so that - // path-style wallet names cannot escape the wallet directory. - // TODO: backport bitcoin/bitcoin#32273 to also move the backup into the - // top-level walletdir and unify with upstream. + // the basename of the wallet path (or "default_wallet" if the name is blank) + // so that path-style wallet names cannot escape the wallet directory. + // TODO(bitcoin#32273): backups move to the top-level walletdir and are named + // from the wallet path. When backported, drop the basename reduction + // below, replace test_wallet_name_with_slashes with upstream's path + // tests, re-derive the bitcoin#34226 path cases from master, then + // take bitcoin#33104 and bitcoin#33122. fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); std::string backup_prefix; if (!wallet_name.empty()) { @@ -5196,9 +5245,9 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle backup_prefix = fs::PathToString(legacy_wallet_path.filename()); } // Fall back when the name was blank or canonicalizes to an empty filename - // (e.g. "/", "."), so we never emit a file like "-1748432384.legacy.bak". - if (backup_prefix.empty()) backup_prefix = "wallet"; - fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", backup_prefix, GetTime())); + // (e.g. "/", "."), so we never emit a file like "_1748432384.legacy.bak". + if (backup_prefix.empty()) backup_prefix = MigrationPrefixName(*local_wallet); + fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", backup_prefix, GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { if (was_loaded) { @@ -5235,26 +5284,44 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle success = DoMigration(*local_wallet, context, error, res); } - // In case of reloading failure, we need to remember the wallet dirs to remove - // Set is used as it may be populated with the same wallet directory paths multiple times, - // both before and after reloading. This ensures the set is complete even if one of the wallets - // fails to reload. - std::set wallet_dirs; + // In case of loading failure, we need to remember the wallet files we have created to remove. + // A `set` is used as it may be populated with the same wallet directory paths multiple times, + // both before and after loading. This ensures the set is complete even if one of the wallets + // fails to load. + std::set wallet_files_to_remove; + std::set wallet_empty_dirs_to_remove; + + // Helper to track wallet files and directories for cleanup on failure. + // Only directories of wallets created during migration (not the main wallet) are tracked. + auto track_for_cleanup = [&](const CWallet& wallet) { + const auto files = wallet.GetDatabase().Files(); + wallet_files_to_remove.insert(files.begin(), files.end()); + if (wallet.GetName() != wallet_name) { + // If this isn’t the main wallet, mark its directory for removal. + // This applies to the watch-only and solvable wallets. + // Wallets stored directly as files in the top-level directory + // (e.g. default unnamed wallets) don’t have a removable parent directory. + wallet_empty_dirs_to_remove.insert(fs::PathFromString(wallet.GetDatabase().Filename()).parent_path()); + } + }; + + if (success) { // Migration successful, unload all wallets locally, then reload them. // Reload the main wallet - wallet_dirs.insert(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path()); + LogInfo("Loading new wallets after migration...\n"); + track_for_cleanup(*local_wallet); success = reload_wallet(local_wallet); res.wallet = local_wallet; res.wallet_name = wallet_name; if (success && res.watchonly_wallet) { // Reload watchonly - wallet_dirs.insert(fs::PathFromString(res.watchonly_wallet->GetDatabase().Filename()).parent_path()); + track_for_cleanup(*res.watchonly_wallet); success = reload_wallet(res.watchonly_wallet); } if (success && res.solvables_wallet) { // Reload solvables - wallet_dirs.insert(fs::PathFromString(res.solvables_wallet->GetDatabase().Filename()).parent_path()); + track_for_cleanup(*res.solvables_wallet); success = reload_wallet(res.solvables_wallet); } } @@ -5262,7 +5329,7 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle // Migration failed, cleanup // Copy the backup to the actual wallet dir fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); - fs::copy_file(backup_path, temp_backup_location, fs::copy_options::none); + fs::rename(backup_path, temp_backup_location); // Drop the second reference to the reloaded main wallet: UnloadWallet() below // waits until the last reference is gone, and this one would only die on return. @@ -5278,8 +5345,8 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet)); // Get the directories to remove after unloading - for (std::shared_ptr& w : created_wallets) { - wallet_dirs.emplace(fs::PathFromString(w->GetDatabase().Filename()).parent_path()); + for (std::shared_ptr& wallet : created_wallets) { + track_for_cleanup(*wallet); } // Unload the wallets @@ -5298,9 +5365,15 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle } } - // Delete the wallet directories - for (const fs::path& dir : wallet_dirs) { - fs::remove_all(dir); + // First, delete the db files we have created throughout this process and nothing else + for (const fs::path& file : wallet_files_to_remove) { + fs::remove(file); + } + + // Second, delete the created wallet directories and nothing else. They must be empty at this point. + for (const fs::path& dir : wallet_empty_dirs_to_remove) { + Assume(fs::is_empty(dir)); + fs::remove(dir); } // Restore the backup @@ -5312,8 +5385,7 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle } // Move the backup to the wallet dir - fs::copy_file(temp_backup_location, backup_path, fs::copy_options::none); - fs::remove(temp_backup_location); + fs::rename(temp_backup_location, backup_path); return util::Error{error}; } diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 8f8c2da40ea5..9048ec24d210 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -37,7 +37,11 @@ from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + sha256sum_file, +) class WalletBackupTest(BitcoinTestFramework): def add_options(self, parser): @@ -155,17 +159,84 @@ def restore_preserves_existing_directory(self): with open(sentinel, encoding='utf8') as f: assert_equal(f.read(), 'preserve existing data') assert_raises_rpc_error(-8, 'Backup file does not exist', node.restorewallet, wallet_name, backup_file + '.missing') - assert_raises_rpc_error(-36, 'Database already exists', node.restorewallet, wallet_name, backup_file) + existing_db = os.path.join(destination, 'wallet.dat') + with open(existing_db, 'w', encoding='utf8') as f: + f.write('not a wallet') + assert_raises_rpc_error(-36, 'Database file exists', node.restorewallet, wallet_name, backup_file) assert os.path.isfile(sentinel) + with open(existing_db, encoding='utf8') as f: + assert_equal(f.read(), 'not a wallet') def restore_wallet_existent_name(self): node = self.nodes[3] backup_file = os.path.join(self.nodes[0].datadir, 'wallet.bak') wallet_name = "res0" - wallet_file = os.path.join(node.wallets_path, wallet_name) - error_message = "Failed to create database path '{}'. Database already exists.".format(wallet_file) + wallet_file = node.wallets_path / wallet_name + error_message = "Failed to restore wallet. Database file exists in '{}'.".format(wallet_file / "wallet.dat") assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file) - assert os.path.exists(wallet_file) + assert wallet_file.exists() + + def test_restore_existent_dir(self): + self.log.info("Test restore on an existent empty directory") + node = self.nodes[3] + backup_file = self.nodes[0].datadir_path / 'wallet.bak' + wallet_name = "restored_wallet" + wallet_dir = node.wallets_path / wallet_name + os.mkdir(wallet_dir) + res = node.restorewallet(wallet_name, backup_file) + assert_equal(res['name'], wallet_name) + node.unloadwallet(wallet_name) + + self.log.info("Test restore succeeds when the target directory contains non-wallet files") + wallet_file = node.wallets_path / wallet_name / "wallet.dat" + os.remove(wallet_file) + extra_file = node.wallets_path / wallet_name / "not_a_wallet.txt" + extra_file.touch() + res = node.restorewallet(wallet_name, backup_file) + assert_equal(res['name'], wallet_name) + assert extra_file.exists() # extra file was not removed by mistake + node.unloadwallet(wallet_name) + + self.log.info("Test restore failure due to existing db file in the destination directory") + original_shasum = sha256sum_file(wallet_file) + error_message = "Failed to restore wallet. Database file exists in '{}'.".format(wallet_dir / "wallet.dat") + assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file) + # Ensure the wallet file remains untouched + assert wallet_dir.exists() + assert_equal(original_shasum, sha256sum_file(wallet_file)) + + self.log.info("Test restore succeeds when the .dat file in the destination has a different name") + second_wallet = wallet_dir / "hidden_storage.dat" + os.rename(wallet_dir / "wallet.dat", second_wallet) + original_shasum = sha256sum_file(second_wallet) + res = node.restorewallet(wallet_name, backup_file) + assert_equal(res['name'], wallet_name) + assert (wallet_dir / "hidden_storage.dat").exists() + assert_equal(original_shasum, sha256sum_file(second_wallet)) + node.unloadwallet(wallet_name) + + # Clean for follow-up tests + os.remove(wallet_file) + + def test_restore_into_unnamed_wallet(self): + self.log.info("Test restore into a default unnamed wallet") + # This is also useful to test the migration recovery after failure logic + node = self.nodes[3] + if not self.options.descriptors: + node.unloadwallet("") + os.rename(node.wallets_path / "wallet.dat", node.wallets_path / "default.wallet.dat") + backup_file = self.nodes[0].datadir_path / 'wallet.bak' + wallet_name = "" + res = node.restorewallet(wallet_name, backup_file) + assert_equal(res['name'], "") + assert (node.wallets_path / "wallet.dat").exists() + # Clean for follow-up tests + node.unloadwallet("") + os.remove(node.wallets_path / "wallet.dat") + if not self.options.descriptors: + os.rename(node.wallets_path / "default.wallet.dat", node.wallets_path / "wallet.dat") + node.loadwallet("") + def run_test(self): self.log.info("Generating initial blockchain") @@ -243,6 +314,8 @@ def run_test(self): assert_equal(res2_rpc.getbalance(), balance2) self.restore_wallet_existent_name() + self.test_restore_existent_dir() + self.test_restore_into_unnamed_wallet() if not self.options.descriptors: self.log.info("Restoring using dumped wallet") diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 78910199b0bb..aa00bebbea6a 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -4,11 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test Migrating a wallet from legacy to descriptor.""" +from pathlib import Path import os import random import shutil import struct -import time from test_framework.address import key_to_p2pkh, script_to_p2sh from test_framework.bdb import BTREE_MAGIC @@ -19,6 +19,7 @@ from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, find_vout_for_address, sha256sum_file, @@ -50,8 +51,8 @@ def assert_is_sqlite(self, wallet_name): assert_equal(file_magic, b'SQLite format 3\x00') assert_equal(self.nodes[0].get_wallet_rpc(wallet_name).getwalletinfo()["format"], "sqlite") - def create_legacy_wallet(self, wallet_name, disable_private_keys=False, blank=False): - self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, disable_private_keys=disable_private_keys, blank=blank) + def create_legacy_wallet(self, wallet_name, disable_private_keys=False, blank=False, **kwargs): + self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, disable_private_keys=disable_private_keys, blank=blank, **kwargs) wallet = self.nodes[0].get_wallet_rpc(wallet_name) info = wallet.getwalletinfo() assert_equal(info["descriptors"], False) @@ -574,19 +575,91 @@ def test_wallet_name_with_slashes(self): # Basename must use the sanitized prefix derived from the wallet # name's last component, not the full "nested/..." path. backup_basename = os.path.basename(backup_path) - assert backup_basename.startswith("subdir_wallet-"), backup_basename + assert backup_basename.startswith("subdir_wallet_"), backup_basename assert backup_basename.endswith(".legacy.bak"), backup_basename self.assert_is_sqlite(nested_name) + def clear_default_wallet(self, backup_file): + # Test cleanup: Clear unnamed default wallet for subsequent tests + (self.nodes[0].wallets_path / "wallet.dat").unlink() + shutil.rmtree(self.nodes[0].wallets_path / "default_wallet_watchonly", ignore_errors=True) + shutil.rmtree(self.nodes[0].wallets_path / "default_wallet_solvables", ignore_errors=True) + backup_file.unlink() + def test_default_wallet(self): self.log.info("Test migration of the wallet named as the empty string") wallet = self.create_legacy_wallet("") - self.migrate_wallet(wallet) + res = self.migrate_wallet(wallet) + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], True) + assert_equal(info["format"], "sqlite") + + wallet.unloadwallet() + self.clear_default_wallet(backup_file=Path(res["backup_path"])) + + def test_default_wallet_watch_only(self): + self.log.info("Test unnamed (default) watch-only wallet migration") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("") + wallet.importaddress(def_wallet.getnewaddress()) + + res = self.migrate_wallet(wallet) + wallet = self.nodes[0].get_wallet_rpc("") + watchonly_wallet = self.nodes[0].get_wallet_rpc("default_wallet_watchonly") + + info = watchonly_wallet.getwalletinfo() + assert_equal(info["descriptors"], True) + assert_equal(info["format"], "sqlite") + assert_equal(info["private_keys_enabled"], False) + assert_equal(info["walletname"], "default_wallet_watchonly") + + # The default wallet will still exist and have newly generated descriptors + assert (self.nodes[0].wallets_path / "wallet.dat").exists() info = wallet.getwalletinfo() assert_equal(info["descriptors"], True) assert_equal(info["format"], "sqlite") + assert_equal(info["private_keys_enabled"], True) + assert_equal(info["walletname"], "") + assert_greater_than(info["keypoolsize"], 0) + + wallet.unloadwallet() + watchonly_wallet.unloadwallet() + self.clear_default_wallet(backup_file=Path(res["backup_path"])) + + def test_default_wallet_failure(self): + self.log.info("Test failure during unnamed (default) wallet migration") + master_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("", blank=True) + wallet.importaddress(master_wallet.getnewaddress()) + + # Create wallet directory with the watch-only name and a wallet file. + # Because the wallet dir exists, this will cause migration to fail. + watch_only_dir = self.nodes[0].wallets_path / "default_wallet_watchonly" + os.mkdir(watch_only_dir) + shutil.copyfile(self.nodes[0].wallets_path / "wallet.dat", watch_only_dir / "wallet.dat") + + mocked_time = int(time.time()) + self.nodes[0].setmocktime(mocked_time) + assert_raises_rpc_error(-4, "Wallet file creation failed", self.nodes[0].migratewallet, "") + self.nodes[0].setmocktime(0) + + # Verify the /wallets/ path exists + assert self.nodes[0].wallets_path.exists() + # Check backup file exists. Because the wallet has no name, the backup is prefixed with 'default_wallet' + backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak" + assert backup_path.exists() + # Verify the original unnamed wallet was restored + assert (self.nodes[0].wallets_path / "wallet.dat").exists() + # And verify it is still a BDB wallet + with open(self.nodes[0].wallets_path / "wallet.dat", "rb") as f: + data = f.read(16) + _, _, magic = struct.unpack("QII", data) + assert_equal(magic, BTREE_MAGIC) + + wallet.unloadwallet() + self.clear_default_wallet(backup_path) def test_direct_file(self): self.log.info("Test migration of a wallet that is not in a wallet directory") @@ -930,6 +1003,27 @@ def test_failed_migration_cleanup(self): _, _, magic = struct.unpack("QII", data) assert_equal(magic, BTREE_MAGIC) + def unsynced_wallet_on_pruned_node_fails(self): + self.log.info("Test migration of an unsynced wallet on a pruned node fails gracefully") + shutil.rmtree(self.nodes[0].wallets_path / "database", ignore_errors=True) + wallet = self.create_legacy_wallet("", load_on_startup=False) + last_wallet_synced_block = wallet.getwalletinfo()['lastprocessedblock']['height'] + + # Generate blocks just so the wallet best block is pruned + self.restart_node(0, ["-fastprune", "-prune=1", "-nowallet"]) + self.generate(self.nodes[0], 450, sync_fun=self.no_op) + self.nodes[0].pruneblockchain(250) + # Ensure next block to sync is unavailable + assert_raises_rpc_error(-1, "Block not available (pruned data)", self.nodes[0].getblock, self.nodes[0].getblockhash(last_wallet_synced_block + 1)) + + # Check migration failure + mocked_time = self.mocktime + assert_raises_rpc_error(-4, "last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)\nUnable to restore backup of wallet.", self.nodes[0].migratewallet, wallet_name="") + + # The wallet will not have been restored, and the backup will be in the wallets directory. + backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak" + assert backup_path.exists() + def run_test(self): self.generate(self.nodes[0], 101) @@ -943,7 +1037,9 @@ def run_test(self): self.test_unloaded() self.test_unloaded_by_path() self.test_wallet_name_with_slashes() + self.test_default_wallet_failure() self.test_default_wallet() + self.test_default_wallet_watch_only() self.test_direct_file() self.test_migrate_raw_p2sh() self.test_hybrid_pubkey() @@ -951,5 +1047,8 @@ def run_test(self): self.test_addressbook() self.test_failed_migration_cleanup() + # Note: After this test the first 250 blocks of 'nodes[0]' are pruned + self.unsynced_wallet_on_pruned_node_fails() + if __name__ == '__main__': WalletMigrationTest().main() From d1d602b51992e717e0b997e73f2acc3bdb680314 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 9 Jan 2026 10:02:21 +0000 Subject: [PATCH 2/4] Merge bitcoin/bitcoin#34226: wallet: test: Relative wallet failed migration cleanup BACKPORT NOTE: Single-node test layout as on 28.x (4d219725a7). The expected backup path uses the last component of the resolved wallet path because Dash keeps the basename rule for path-style wallet names (see the TODO for bitcoin#32273 in wallet.cpp). eeaf28dbe0e09819ab0e95bb7762b29536bdeef6 wallet: test: Failed migration cleanup (David Gumberg) Pull request description: Prior to https://github.com/bitcoin/bitcoin/pull/34156, an issue existed where if migration of a wallet with a relative pathname failed, the relatively specified path where the legacy wallet is would be deleted. This issue predates #32273, because the relative pathnames get stacked together, e.g. "../../", the copy conflict bug that caused migration to abort early instead of getting far enough to attempt clean-up that was fixed in #32273 is avoided. This is a functional test demonstrating that we handle failed migration clean-up correctly for relatively-named wallets. To see the issue, you can backport this test onto 29.x: https://github.com/davidgumberg/bitcoin/tree/2026-01-07-rel-migration-test-backport I've also added an absolute path failed migration cleanup test. WRT this and #34156, absolute paths exhibit similar behavior to unnamed wallets. Because of the name-conflict bug prior to #32273 an absolute-path migration would fail no matter what because migration would attempt to copy a file to a destination that already exists. But after #32273, absolute-path migration gets past there, and if it fails for some other reason, the same behavior that's fixed in #34156 occurs where the directory containing the wallet file is deleted. ACKs for top commit: achow101: ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6 furszy: ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6 rkrux: lgtm ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6 Tree-SHA512: ee366fe526d0328654a86c2e9e6f228ca81554c8f8a78c259fa7aab90f024f9e5694ecf3f1d188938355f4e6d351c5a6a8ad236701bdd0ce63005e5d42c15e15 Co-authored-by: merge-script --- test/functional/wallet_migration.py | 84 +++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index aa00bebbea6a..dbf5e01fd740 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -628,38 +628,71 @@ def test_default_wallet_watch_only(self): watchonly_wallet.unloadwallet() self.clear_default_wallet(backup_file=Path(res["backup_path"])) - def test_default_wallet_failure(self): - self.log.info("Test failure during unnamed (default) wallet migration") + def test_migration_failure(self, wallet_name): + is_default = wallet_name == "" + wallet_pretty_name = "unnamed (default)" if is_default else f'"{wallet_name}"' + self.log.info(f"Test failure during migration of wallet named: {wallet_pretty_name}") + # Preface, set up legacy wallet and unload it master_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) - wallet = self.create_legacy_wallet("", blank=True) + wallet = self.create_legacy_wallet(wallet_name, blank=True) wallet.importaddress(master_wallet.getnewaddress()) + wallet.unloadwallet() - # Create wallet directory with the watch-only name and a wallet file. - # Because the wallet dir exists, this will cause migration to fail. - watch_only_dir = self.nodes[0].wallets_path / "default_wallet_watchonly" + if os.path.isabs(wallet_name): + wallet_path = Path(wallet_name) + else: + wallet_path = self.nodes[0].wallets_path / wallet_name + + # This will be the watch-only directory the migration tries to create, + # we make migration fail by placing a wallet.dat file there. + wo_prefix = wallet_name or "default_wallet" + # wo_prefix might have path characters in it, this corresponds with + # DoMigration(). + wo_dirname = f"{wo_prefix}_watchonly" + watch_only_dir = self.nodes[0].wallets_path / wo_dirname os.mkdir(watch_only_dir) - shutil.copyfile(self.nodes[0].wallets_path / "wallet.dat", watch_only_dir / "wallet.dat") + shutil.copyfile(wallet_path / "wallet.dat", watch_only_dir / "wallet.dat") - mocked_time = int(time.time()) - self.nodes[0].setmocktime(mocked_time) - assert_raises_rpc_error(-4, "Wallet file creation failed", self.nodes[0].migratewallet, "") - self.nodes[0].setmocktime(0) + # Make a file in the wallets dir that must still exist after migration + survive_path = self.nodes[0].wallets_path / "survive" + open(survive_path, "wb").close() + assert survive_path.exists() + + # The backup is named after the node's time, which is the framework's mocktime + mocked_time = self.mocktime + assert_raises_rpc_error(-4, "Failed to create database", self.nodes[0].migratewallet, wallet_name) + wallet.unloadwallet() - # Verify the /wallets/ path exists + # Verify the /wallets/ path exists. assert self.nodes[0].wallets_path.exists() - # Check backup file exists. Because the wallet has no name, the backup is prefixed with 'default_wallet' - backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak" + + # Verify survive is still there + assert survive_path.exists() + # Verify the wallet path exists. + assert Path(wallet_path / "wallet.dat").exists() + + # The backup is named after the last component of the wallet's path, so + # path-style wallet names cannot escape the wallet directory. + backup_prefix = "default_wallet" if is_default else wallet_path.resolve().name + backup_path = wallet_path / f"{backup_prefix}_{mocked_time}.legacy.bak" assert backup_path.exists() - # Verify the original unnamed wallet was restored - assert (self.nodes[0].wallets_path / "wallet.dat").exists() - # And verify it is still a BDB wallet - with open(self.nodes[0].wallets_path / "wallet.dat", "rb") as f: + + with open(self.nodes[0].wallets_path / wallet_name / self.wallet_data_filename, "rb") as f: data = f.read(16) _, _, magic = struct.unpack("QII", data) assert_equal(magic, BTREE_MAGIC) - wallet.unloadwallet() - self.clear_default_wallet(backup_path) + survive_path.unlink() + + # Cleanup + if is_default: + self.clear_default_wallet(backup_path) + else: + backup_path.unlink() + Path(watch_only_dir / "wallet.dat").unlink() + Path(watch_only_dir).rmdir() + Path(wallet_path / "wallet.dat").unlink() + def test_direct_file(self): self.log.info("Test migration of a wallet that is not in a wallet directory") @@ -1037,7 +1070,16 @@ def run_test(self): self.test_unloaded() self.test_unloaded_by_path() self.test_wallet_name_with_slashes() - self.test_default_wallet_failure() + + migration_failure_cases = [ + "", + "../", + os.path.abspath(self.nodes[0].datadir_path / "absolute_path"), + "normallynamedwallet" + ] + for wallet_name in migration_failure_cases: + self.test_migration_failure(wallet_name=wallet_name) + self.test_default_wallet() self.test_default_wallet_watch_only() self.test_direct_file() From 7db2696ce675178233201139e1a4cf4d14cb7f3b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 29 Jan 2026 17:48:10 +0100 Subject: [PATCH 3/4] Merge bitcoin/bitcoin#34370: [29.x] Fix #34222 backport bugs BACKPORT NOTE: These five commits have no master counterpart (they fix the BDB-era backport bitcoin#34222 / bitcoin#34223). The 28.x copies in bitcoin#34620 (29abedc97b, 619480bd78, 6494072295, c5d9f75c4b, 242b9ae3f3) were used as the code reference. Dash has no load_after_restore (bitcoin#31451) and no blank-wallet branch (bitcoin#28976); see the TODOs in MigrateLegacyToDescriptor. 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 Co-authored-by: merge-script --- src/wallet/bdb.cpp | 48 ++++++++++++++++++++++++++++++++++ src/wallet/bdb.h | 15 +---------- src/wallet/wallet.cpp | 32 +++++++++++++++-------- test/functional/tool_wallet.py | 19 ++++++++++++++ 4 files changed, 89 insertions(+), 25 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 58488fdbfaa4..c1b7bff8d880 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -331,6 +332,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 47f02f7112bb..2d9bf4ef0e33 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -129,20 +129,7 @@ class BerkeleyDatabase : public WalletDatabase /** 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 65b8246664cb..7378090c1050 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5214,7 +5214,7 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle 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; @@ -5280,6 +5280,15 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle // 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 + // TODO(bitcoin#32273): this block exists only on the 28.x/29.x branches. Remove + // it together with the backup naming TODO above: once backups live in + // the top-level walletdir they never need to be moved. + 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, and cleanup if it fails success = DoMigration(*local_wallet, context, error, res); } @@ -5327,9 +5336,6 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle } if (!success) { // Migration failed, cleanup - // Copy the backup to the actual wallet dir - fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); - fs::rename(backup_path, temp_backup_location); // Drop the second reference to the reloaded main wallet: UnloadWallet() below // waits until the last reference is gone, and this one would only die on return. @@ -5377,16 +5383,20 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle } // Restore the backup - DatabaseStatus status; - std::vector warnings; - if (!RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, error, warnings)) { - error += _("\nUnable to restore backup of wallet."); + // 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. + // TODO(bitcoin#31451): RestoreWallet loads the wallet even when it was not loaded + // before migration. When backporting bitcoin#31451, add its load_after_restore + // parameter to RestoreWallet, pass was_loaded here, assert that the wallet is + // reloaded only if was_loaded, and re-derive this function from the 29.x commits + // d91f56e1e3 and a074d36254 (bitcoin#34222), which already have that shape. + bilingual_str restore_error; + const auto& ptr_wallet = RestoreWallet(context, backup_path, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings); + if (!restore_error.empty()) { + error += restore_error + _("\nUnable to restore backup of wallet."); return util::Error{error}; } - // Move the backup to the wallet dir - fs::rename(temp_backup_location, backup_path); - return util::Error{error}; } return res; diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 98a1b6bf6bf4..b4c61be7846b 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -401,12 +401,31 @@ def test_dump_createfromdump(self): 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 (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") + (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") + + 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") From f7ba6da3123abdde12d063c1f91c601f74aecd19 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 2 Nov 2026 17:06:43 -0600 Subject: [PATCH 4/4] test: prune past Dash's first fastprune block file in wallet_migration unsynced_wallet_on_pruned_node_fails was taken from Bitcoin 28.x, which mines 450 blocks and prunes to height 250. Dash's early regtest blocks are small (~200 B), so the first 64 KiB -fastprune block file holds heights 0..~320. Manual pruning only removes whole files whose last block is at or below min(250, tip - 288), so nothing was pruned, the wallet's next block stayed available, and the test failed with "No exception raised" (CI: "Prune (Manual): prune_height=250 removed 0 blk/rev pairs"). Mine 600 blocks and prune to height 400 so the first block file, which holds the wallet's next block, is removed while the second one stays. Co-Authored-By: Claude Opus 5.5 --- test/functional/wallet_migration.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index dbf5e01fd740..3e11e8d27836 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -1042,10 +1042,12 @@ def unsynced_wallet_on_pruned_node_fails(self): wallet = self.create_legacy_wallet("", load_on_startup=False) last_wallet_synced_block = wallet.getwalletinfo()['lastprocessedblock']['height'] - # Generate blocks just so the wallet best block is pruned + # Generate blocks just so the wallet best block is pruned. + # Dash's small early regtest blocks fill the first -fastprune block file up + # to height ~320, so mine and prune further than upstream to remove it. self.restart_node(0, ["-fastprune", "-prune=1", "-nowallet"]) - self.generate(self.nodes[0], 450, sync_fun=self.no_op) - self.nodes[0].pruneblockchain(250) + self.generate(self.nodes[0], 600, sync_fun=self.no_op) + self.nodes[0].pruneblockchain(400) # Ensure next block to sync is unavailable assert_raises_rpc_error(-1, "Block not available (pruned data)", self.nodes[0].getblock, self.nodes[0].getblockhash(last_wallet_synced_block + 1)) @@ -1089,7 +1091,7 @@ def run_test(self): self.test_addressbook() self.test_failed_migration_cleanup() - # Note: After this test the first 250 blocks of 'nodes[0]' are pruned + # Note: After this test the first block file of 'nodes[0]' is pruned self.unsynced_wallet_on_pruned_node_fails() if __name__ == '__main__':