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

w0xlt commented at 6:50 pm on August 13, 2025: contributor

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 commented at 6:50 pm on August 13, 2025: contributor

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

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

w0xlt force-pushed on Aug 13, 2025

DrahtBot added the label CI failed on Aug 13, 2025

DrahtBot commented at 6:57 pm on August 13, 2025: contributor

🚧 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.

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.

w0xlt force-pushed on Aug 13, 2025

Add test to non-HD and single chain wallets 3b1b154c31

w0xlt force-pushed on Aug 13, 2025

in test/functional/wallet_ancient_migration.py:62 in 3b1b154c31

57+        # Clean up any existing nodes and data
58+        self.cleanup_nodes()
59+
60+        # Setup fresh nodes for this test
61+        self.add_nodes(
62+            2,

pablomartin4btc commented at 7:45 pm on August 13, 2025:

0            self.num_nodes,

pablomartin4btc commented at 7:53 pm on August 13, 2025: member

Concept ACK

Left a couple of suggestions.

in test/functional/test_runner.py:433 in 3b1b154c31

428@@ -428,7 +429,12 @@ def main():
429     logging.basicConfig(format='%(message)s', level=logging_level)
430 
431     # Create base test directory
432-    tmpdir = "%s/test_runner_₿_🏃_%s" % (args.tmpdirprefix, datetime.datetime.now().strftime("%Y%m%d_%H%M%S"))
433+    # Special case for tests using old binaries (e.g. version v0.14.3) that don't handle Unicode on Windows
434+    has_migration_tests = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)

pablomartin4btc commented at 8:21 pm on August 13, 2025:

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)

w0xlt commented at 8:47 pm on August 13, 2025:

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.

pablomartin4btc commented at 9:43 pm on August 13, 2025:

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.

0    requires_ascii_datadir = any('wallet_ancient_migration' in script for script in BASE_SCRIPTS)
1    tmpdirprefix = "%s/test_runner_₿_🏃_%s"
2    if platform.system() == 'Windows' and requires_ascii_datadir:
3        tmpdir = "%s/test_runner_btc_run_%s"
4    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?

w0xlt commented at 9:46 pm on August 13, 2025:

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.

DrahtBot removed the label CI failed on Aug 13, 2025

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

Code Coverage & Benchmarks

Reviews