wallet, test: Ancient Wallet Migration from v0.14.3 (no-HD and Single Chain) #33186

w0xlt · 2025-08-13T18:50:30Z

This PR adds test coverage for migrating legacy Bitcoin Core wallets from v0.14.3 (released in 2017) to the descriptor wallet format. The test validates that users can safely upgrade their wallets while preserving all funds, transaction history, and addresses.

This test was originally developed on top of #32977, as it was requested in reviews.
However, since it also increases test coverage, it can be merged independently.

The test covers two wallet migration scenarios:

Non-HD Wallet Migration - Tests migration of non-HD wallets (created with -usehd=0)
Single Chain HD Wallet Migration - Tests migration of HD wallets from v0.14.3 (VERSION_HD_BASE)

The node v0.14.3 cannot be synced because it doesn't have the syncwithvalidationinterfacequeue RPC implemented (required by test_framework.py), so the solution is to copy the block folder to the modern node and start it with -reindex-chainstate. Because of this additional complexity, this testing is best managed in a separate file rather than in the existing migration test files.

DrahtBot · 2025-08-13T18:50:35Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33186.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	pablomartin4btc, furszy

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:

test/functional/wallet_ancient_migration.py "assert old_balance > 0" -> use assert_greater_than(old_balance, 0)
test/functional/wallet_ancient_migration.py "assert len(old_txs) > 0" -> use assert_greater_than(len(old_txs), 0)
test/functional/wallet_ancient_migration.py "assert len(txid) == 64" -> use assert_equal(len(txid), 64)

^2026-01-16

DrahtBot · 2025-08-13T18:57:07Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/48028643718}
_{LLM reason (✨ experimental): The CI failure is caused by a lint error due to trailing whitespace in the code.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

pablomartin4btc · 2025-08-13T19:45:25Z

test/functional/wallet_ancient_migration.py

+
+        # Setup fresh nodes for this test
+        self.add_nodes(
+            2,


Suggested change

2,

self.num_nodes,

Done. Thanks.

pablomartin4btc

Concept ACK

Left a couple of suggestions.

pablomartin4btc · 2025-08-13T20:21:03Z

test/functional/test_runner.py

    # Create base test directory
-    tmpdir = "%s/test_runner_₿_🏃_%s" % (args.tmpdirprefix, datetime.datetime.now().strftime("%Y%m%d_%H%M%S"))
+    # Special case for tests using old binaries (e.g. version v0.14.3) that don't handle Unicode on Windows
+    has_migration_tests = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)


more than has_migration_tests shouldn't indicate like it's an old binary? (there are other tests that uses migration and feature_unsupported_utxo_db.py uses a node v0.14.3 as well)

Without this, the Windows, test cross-built CI task will fail because it can't handle non-ASCII characters in -datadir.

feature_unsupported_utxo_db.py doesn't copy the directory from one node to another. This test needs to do this because version 0.14.3 doesn't have the syncwithvalidationinterfacequeue RPC, so it can't be synchronized.

Yeah, it's already well explained in the PR description. Then perhaps the variable name could be supports_unicode_datadir/ (requires_ascii_datadir) or something like that.

Suggested change

has_migration_tests = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)

requires_ascii_datadir = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)

tmpdirprefix = "%s/test_runner_₿_🏃_%s"

if platform.system() == 'Windows' and requires_ascii_datadir:

tmpdir = "%s/test_runner_btc_run_%s"

tmpdir = tmpdirprefix % (args.tmpdirprefix, datetime.datetime.now().strftime("%Y%m%d_%H%M%S"))

Wouldn't this affect the datadir for all the tests running on Windows platform?

Thanks for the suggestion. It's much clearer.
Yes, it affects all tests run on the Windows platform. As far as I know, there's no way to filter by this task specifically.

feature_unsupported_utxo_db.py doesn't copy the directory from one node to another. This test needs to do this because version 0.14.3 doesn't have the syncwithvalidationinterfacequeue RPC, so it can't be synchronized.

What's the error message? It seems odd that one test works fine with unicode and the other does not. See also the suggestion https://github.com/bitcoin/bitcoin/pull/33186/files#r2281561637

feature_unsupported_utxo_db.py doesn't copy the directory from one node to another. This test needs to do this because version 0.14.3 doesn't have the syncwithvalidationinterfacequeue RPC, so it can't be synchronized.

What's the error message? It seems odd that one test works fine with unicode and the other does not. See also the suggestion https://github.com/bitcoin/bitcoin/pull/33186/files#r2281561637

@w0xlt can you please check this ^^ ?

Also, aren't we essentially removing the unicode coverage from the CI with this change? wallet_ancient_migration.py test should always be enabled there.

Applied the same approach as used in #34240

maflcko · 2025-08-18T07:44:30Z

test/functional/wallet_ancient_migration.py

+        # Stop the old node
+        self.log.info("Stopping old node...")
+        self.stop_node(0)
+
+        # Copy blockchain data from node0 to node1
+        self.log.info("Copying blockchain data from old node to modern node...")
+        old_blocks_dir = os.path.join(node0_datadir, self.chain, "blocks")
+        new_blocks_dir = os.path.join(node1_datadir, self.chain, "blocks")
+
+        # Clean up any existing blocks directory in node1
+        if os.path.exists(new_blocks_dir):
+            shutil.rmtree(new_blocks_dir)
+
+        if os.path.exists(old_blocks_dir):
+            shutil.copytree(old_blocks_dir, new_blocks_dir)
+            self.log.info("  Copied blocks directory")
+        assert(os.path.exists(new_blocks_dir))
+
+        # Start the modern node with -reindex-chainstate
+        self.log.info("Starting modern node with -reindex-chainstate...")
+        self.start_node(1, extra_args=["-reindex-chainstate"])
+
+        # Wait for reindex to complete
+        self.log.info("Waiting for chainstate reindex to complete...")
+        self.wait_until(
+            lambda: self.nodes[1].getblockcount() == 102,
+            timeout=30
+        )
+
+        # Verify the blockchain was loaded correctly
+        node1_info = self.nodes[1].getblockchaininfo()
+        self.log.info(f"Modern node blockchain info: height={node1_info['blocks']}")
+        assert_equal(node1_info['blocks'], 102)
+


I think you may be able to skip this by using dumb_sync_blocks from c847dee#diff-e48d83ef13f93a84b5686eb797eeed80dd194cf39db7f2972cf82a5ed25415a7L78-L95

I couldn't find reference to dumb_sync_blocks in that commit.
I used the same approach as in feature_unsupported_utxo_db.py

fanquake · 2025-12-03T10:52:24Z

@achow101 can you leave a conceptual review?

furszy · 2025-12-03T14:58:10Z

Concept ACK, will review soon

furszy

The node v0.14.3 cannot be synced because it doesn't have the syncwithvalidationinterfacequeue RPC implemented (required by test_framework.py), so the solution is to copy the block folder to the modern node and start it with -reindex-chainstate. Because of this additional complexity, this testing is best managed in a separate file rather than in the existing migration test files.

In order to know when the node is synced, cannot you just wait for a specific log line? See busy_wait_for_debug_log.

furszy · 2025-12-05T14:52:33Z

test/functional/test_runner.py

    # Create base test directory
-    tmpdir = "%s/test_runner_₿_🏃_%s" % (args.tmpdirprefix, datetime.datetime.now().strftime("%Y%m%d_%H%M%S"))
+    # Special case for tests using old binaries (e.g. version v0.14.3) that don't handle Unicode on Windows
+    has_migration_tests = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)


feature_unsupported_utxo_db.py doesn't copy the directory from one node to another. This test needs to do this because version 0.14.3 doesn't have the syncwithvalidationinterfacequeue RPC, so it can't be synchronized.

What's the error message? It seems odd that one test works fine with unicode and the other does not. See also the suggestion https://github.com/bitcoin/bitcoin/pull/33186/files#r2281561637

@w0xlt can you please check this ^^ ?

Also, aren't we essentially removing the unicode coverage from the CI with this change? wallet_ancient_migration.py test should always be enabled there.

furszy · 2025-12-05T15:11:45Z

test/functional/wallet_ancient_migration.py

+        old_wallet_info = self.nodes[0].getwalletinfo()
+        hdmasterkeyid = old_wallet_info.get('hdmasterkeyid', 'Not HD')
+        self.log.info(f"Old wallet info: HD master key = {hdmasterkeyid}")
+        assert('keypoolsize_hd_internal' not in old_wallet_info) # Ensure we are testing non-HD or single-chain (VERSION_HD_BASE) wallets
+
+        # Verify wallet type
+        if "non-HD" in wallet_type:
+            assert('hdmasterkeyid' not in old_wallet_info)
+        else:
+            assert('hdmasterkeyid' in old_wallet_info)


Could check the wallet version here.
The non-hd one should have version 6000 (WalletFeature::FEATURE_COMPRPUBKEY) and the hd one should have version 130000 (WalletFeature::FEATURE_HD)

Done. Thanks.

furszy · 2025-12-05T15:14:44Z

test/functional/wallet_ancient_migration.py

+        # Rescan the blockchain
+        self.log.info("Rescanning blockchain...")
+        self.nodes[1].rescanblockchain()


rescanning after migration shouldn't be needed

Done. Thanks.

Test migratewallet RPC on ancient wallet formats from v0.14.3: - Non-HD wallet (FEATURE_COMPRPUBKEY, version 60000) - HD single-chain wallet (VERSION_HD_BASE, version 130000) Verifies balance, transactions, and address ownership are preserved after migration to descriptor wallets.

w0xlt · 2026-01-16T22:13:38Z

Updated the PR with the same solution used in #34240.

w0xlt mentioned this pull request Aug 13, 2025

wallet: Remove wallet version and several legacy related functions #32977

Merged

w0xlt force-pushed the test_no_or_base_hd_wallet_migration branch from 224915a to a305380 Compare August 13, 2025 18:56

DrahtBot added the CI failed label Aug 13, 2025

w0xlt force-pushed the test_no_or_base_hd_wallet_migration branch 3 times, most recently from 5237377 to 3b1b154 Compare August 13, 2025 19:12

pablomartin4btc reviewed Aug 13, 2025

View reviewed changes

DrahtBot removed the CI failed label Aug 13, 2025

maflcko reviewed Aug 18, 2025

View reviewed changes

furszy reviewed Dec 5, 2025

View reviewed changes

w0xlt force-pushed the test_no_or_base_hd_wallet_migration branch from 3b1b154 to 8b376c9 Compare January 7, 2026 21:49

w0xlt mentioned this pull request Jan 8, 2026

test: fix Windows CI failures in wallet_multiwallet and old binary tests (ancient wallets) #34233

Closed

w0xlt force-pushed the test_no_or_base_hd_wallet_migration branch from 8b376c9 to 17d091e Compare January 16, 2026 20:47

w0xlt force-pushed the test_no_or_base_hd_wallet_migration branch from 17d091e to 1b6f3f1 Compare January 16, 2026 20:49

DrahtBot added CI failed and removed CI failed labels Jan 16, 2026

-    has_migration_tests = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)
+    requires_ascii_datadir = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)
+    tmpdirprefix = "%s/test_runner_₿_🏃_%s"
+    if platform.system() == 'Windows' and requires_ascii_datadir:
+        tmpdir = "%s/test_runner_btc_run_%s"
+    tmpdir = tmpdirprefix % (args.tmpdirprefix, datetime.datetime.now().strftime("%Y%m%d_%H%M%S"))

wallet, test: Ancient Wallet Migration from v0.14.3 (no-HD and Single Chain) #33186

Are you sure you want to change the base?

wallet, test: Ancient Wallet Migration from v0.14.3 (no-HD and Single Chain) #33186

Conversation

w0xlt commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

DrahtBot commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furszy Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanquake commented Dec 3, 2025

Uh oh!

furszy commented Dec 3, 2025

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

furszy Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

w0xlt commented Aug 13, 2025 •

edited

Loading

DrahtBot commented Aug 13, 2025 •

edited

Loading

furszy Dec 5, 2025 •

edited

Loading

furszy Dec 5, 2025 •

edited

Loading