From 5e8ad98163af9749e7a3c44a9107cc241c5bd7ab Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 27 Dec 2025 14:32:11 -0500 Subject: [PATCH] wallet: migration, fix watch-only and solvables wallets names Because the default wallet has no name, the watch-only and solvables wallets created during migration end up having no name either. This fixes it by applying the same prefix name we use for the backup file for an unnamed default wallet. Before: watch-only wallet named "_watchonly" After: watch-only wallet named "default_wallet_watchonly" Github-Pull: bitcoin/bitcoin#34156 Rebased-From: 82caa8193a3e36f248dcc949e0cd41def191efac --- src/wallet/wallet.cpp | 15 ++++++++-- test/functional/wallet_migration.py | 45 +++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b54cf0be0f7..2397d84a6f5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4326,6 +4326,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); @@ -4357,7 +4366,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); @@ -4394,7 +4403,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); @@ -4533,7 +4542,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Make a backup of the DB fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); - fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime())); + fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", MigrationPrefixName(*local_wallet), GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { if (was_loaded) { diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 3ca053043bb..c11986226df 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -5,6 +5,7 @@ """Test Migrating a wallet from legacy to descriptor.""" import os +from pathlib import Path import random import shutil import struct @@ -25,6 +26,7 @@ from test_framework.script import hash160 from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, script_to_p2sh_script, script_to_p2wsh_script from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, find_vout_for_address, sha256sum_file, @@ -523,6 +525,14 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(bals, wallet.getbalances()) + def clear_default_wallet(self, backup_file): + # Test cleanup: Clear unnamed default wallet for subsequent tests + (self.old_node.wallets_path / "wallet.dat").unlink() + (self.master_node.wallets_path / "wallet.dat").unlink(missing_ok=True) + shutil.rmtree(self.master_node.wallets_path / "default_wallet_watchonly", ignore_errors=True) + shutil.rmtree(self.master_node.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("") @@ -549,6 +559,36 @@ class WalletMigrationTest(BitcoinTestFramework): self.master_node.setmocktime(0) + 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") + master_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("", blank=True) + wallet.importaddress(master_wallet.getnewaddress(address_type="legacy")) + + res, def_wallet = self.migrate_and_get_rpc("") + wallet = self.master_node.get_wallet_rpc("default_wallet_watchonly") + + info = 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.master_node.wallets_path / "wallet.dat").exists() + def_wallet_info = def_wallet.getwalletinfo() + assert_equal(def_wallet_info["descriptors"], True) + assert_equal(def_wallet_info["format"], "sqlite") + assert_equal(def_wallet_info["private_keys_enabled"], True) + assert_equal(def_wallet_info["walletname"], "") + assert_greater_than(def_wallet_info["keypoolsize"], 0) + + 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.master_node.get_wallet_rpc(self.default_wallet_name) @@ -557,7 +597,7 @@ class WalletMigrationTest(BitcoinTestFramework): # 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.master_node.wallets_path / "_watchonly" + watch_only_dir = self.master_node.wallets_path / "default_wallet_watchonly" os.mkdir(watch_only_dir) shutil.copyfile(self.old_node.wallets_path / "wallet.dat", watch_only_dir / "wallet.dat") @@ -580,7 +620,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(magic, BTREE_MAGIC) # Test cleanup: clear default wallet for next test - os.remove(self.old_node.wallets_path / "wallet.dat") + 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") @@ -1408,6 +1448,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_unloaded_by_path() self.test_default_wallet_failure() self.test_default_wallet() + self.test_default_wallet_watch_only() self.test_direct_file() self.test_addressbook() self.test_migrate_raw_p2sh()