Repository: bitcoin/bitcoin
File: test/functional/wallet_migration.py (Python)

Code before:

        # The multisig should not be in the spendable wallet
        addr_info = wallet.getaddressinfo(addr)
        assert_equal(addr_info["ismine"], False)
        assert "hex" not in addr_info

        # The multisig address should be in the solvables wallet
        addr_info = solvables.getaddressinfo(addr)
        assert_equal(addr_info["ismine"], True)
        assert_equal(addr_info["solvable"], True)
        assert "hex" in addr_info

    def run_test(self):
        self.master_node = self.nodes[0]
        self.old_node = self.nodes[1]

        self.generate(self.master_node, 101)

        # TODO: Test the actual records in the wallet for these tests too. The behavior may be correct, but the data written may not be what we actually want
        self.test_basic()
        self.test_multisig()
        self.test_other_watchonly()
        self.test_no_privkeys()
        self.test_pk_coinbases()
        self.test_encrypted()
        self.test_nonexistent()
        self.test_unloaded_by_path()
        self.test_default_wallet()
        self.test_direct_file()
        self.test_addressbook()
        self.test_migrate_raw_p2sh()
        self.test_conflict_txs()
        self.test_hybrid_pubkey()
        self.test_failed_migration_cleanup()
        self.test_avoidreuse()
        self.test_preserve_tx_extra_info()
        self.test_blank()
        self.test_migrate_simple_watch_only()
        self.test_manual_keys_import()
        self.test_p2wsh()
        self.test_disallowed_p2wsh()
        self.test_miniscript()
        self.test_taproot()
        self.test_solvable_no_privs()

if __name__ == '__main__':
    WalletMigrationTest(__file__).main()


Review comment (bug):
In 060695c22ae7b2b0f2a1dd1417ed1b9d5a5ab542 "test: Failed load after migrate should restore backup"

Adding a review note because it took me some time to understand why this error pops up only after migration is complete and not during the process as this particular error comes from inside `CWallet::AttachChain` that is a part of `CWallet::Create`. The reason I found is that all the `CWallet::Create` calls inside the migration process pass an empty context that doesn't contain the `chain` due to which the chain doesn't need to be attached. Only while loading the wallet after migration, the proper wallet context is passed that triggers this error.

https://github.com/bitcoin/bitcoin/blob/9f713b83dcf7480e6c4447f0f263bef096d55cc7/src/wallet/wallet.cpp#L4206-L4210

I verified that the migration-before-loading was triggered/completed by using the below patch in the `migrate_and_get_rpc` function. The below patch can be added separately as well (not necessary in this PR) because all the tests pass.

```diff
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index a1b319b90b..890478da26 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -125,7 +125,7 @@ class WalletMigrationTest(BitcoinTestFramework):
             if w["name"] == wallet_name:
                 assert_equal(w["warnings"], ["This wallet is a legacy wallet and will need to be migrated with migratewallet before it can be loaded"])
         # Migrate, checking that rescan does not occur
-        with self.master_node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Rescanning"]):
+        with self.master_node.assert_debug_log(expected_msgs=["Migrating wallet storage database from BerkeleyDB to SQLite."], unexpected_msgs=["Rescanning"]):
             migrate_info = self.master_node.migratewallet(wallet_name=wallet_name, **kwargs)
         # Update wallet name in case the initial wallet was completely migrated to a watch-only wallet
         # (in which case the wallet name would be suffixed by the 'watchonly' term)
```

Code after:
        migrate_res, wallet = self.migrate_and_get_rpc("multisig_noprivs")
        assert_equal(migrate_res["solvables_name"], "multisig_noprivs_solvables")
        solvables = self.master_node.get_wallet_rpc(migrate_res["solvables_name"])

        # The multisig should not be in the spendable wallet
        addr_info = wallet.getaddressinfo(addr)
        assert_equal(addr_info["ismine"], False)
        assert "hex" not in addr_info

        # The multisig address should be in the solvables wallet
        addr_info = solvables.getaddressinfo(addr)
        assert_equal(addr_info["ismine"], True)
        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]

        self.generate(self.master_node, 101)

        # TODO: Test the actual records in the wallet for these tests too. The behavior may be correct, but the data written may not be what we actually want
        self.test_basic()
        self.test_multisig()
        self.test_other_watchonly()
        self.test_no_privkeys()
        self.test_pk_coinbases()
        self.test_encrypted()
        self.test_nonexistent()
        self.test_unloaded_by_path()