From 8a4cfddf23a4575a1042dfa97d3478727775e8dd Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 15 Jul 2025 16:11:36 -0700 Subject: [PATCH 1/2] wallet: Set migrated wallet name only on success After a wallet is migrated and we are trying to load it, if it could not be loaded, don't try to set the wallet name. --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3d83f356f64..c3452f7814a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4306,7 +4306,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr success = (wallet != nullptr); // When no wallet is set, set the main wallet. - if (!res.wallet) { + if (success && !res.wallet) { res.wallet_name = wallet->GetName(); res.wallet = std::move(wallet); } From 060695c22ae7b2b0f2a1dd1417ed1b9d5a5ab542 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 16 Jul 2025 09:05:45 +0200 Subject: [PATCH 2/2] test: Failed load after migrate should restore backup --- test/functional/wallet_migration.py | 33 +++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index ecca81c7fdf..a1b319b90bb 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -65,6 +65,12 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(file_magic, b'SQLite format 3\x00') assert_equal(self.master_node.get_wallet_rpc(wallet_name).getwalletinfo()["format"], "sqlite") + def assert_is_bdb(self, wallet_name): + with open(self.master_node.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) + def create_legacy_wallet(self, wallet_name, **kwargs): self.old_node.createwallet(wallet_name=wallet_name, descriptors=False, **kwargs) wallet = self.old_node.get_wallet_rpc(wallet_name) @@ -916,10 +922,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(original_shasum, new_shasum) # Check the wallet we tried to migrate is still BDB - with open(self.master_node.wallets_path / "failed" / "wallet.dat", "rb") as f: - data = f.read(16) - _, _, magic = struct.unpack("QII", data) - assert_equal(magic, BTREE_MAGIC) + self.assert_is_bdb("failed") def test_blank(self): self.log.info("Test that a blank wallet is migrated") @@ -1372,6 +1375,27 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(addr_info["solvable"], True) assert "hex" in addr_info + def test_loading_failure_after_migration(self): + self.log.info("Test that a failed loading of the wallet at the end of migration restores the backup") + self.stop_node(self.old_node.index) + self.old_node.chain = "signet" + self.old_node.replace_in_config([("regtest=", "signet="), ("[regtest]", "[signet]")]) + # Disable network sync and prevent disk space warning on small (tmp)fs + self.start_node(self.old_node.index, extra_args=self.old_node.extra_args + ["-maxconnections=0", "-prune=550"]) + + wallet_name = "failed_load_after_migrate" + self.create_legacy_wallet(wallet_name) + assert_raises_rpc_error(-4, "Wallet loading failed. Wallet files should not be reused across chains.", lambda: self.migrate_and_get_rpc(wallet_name)) + + # Check the wallet we tried to migrate is still BDB + self.assert_is_bdb(wallet_name) + + self.stop_node(self.old_node.index) + self.old_node.chain = "regtest" + self.old_node.replace_in_config([("signet=", "regtest="), ("[signet]", "[regtest]")]) + self.start_node(self.old_node.index) + self.connect_nodes(1, 0) + def run_test(self): self.master_node = self.nodes[0] self.old_node = self.nodes[1] @@ -1404,6 +1428,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_miniscript() self.test_taproot() self.test_solvable_no_privs() + self.test_loading_failure_after_migration() if __name__ == '__main__': WalletMigrationTest(__file__).main()