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, furszy

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34418 (qa: Make wallet_multiwallet.py Windows-compatible by hodlinator)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

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

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

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,

w0xlt commented at 10:14 pm on January 16, 2026:

Done. Thanks.

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 outdated

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.

maflcko commented at 7:49 am on August 18, 2025:

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

furszy commented at 2:52 pm on December 5, 2025:

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.

w0xlt commented at 10:12 pm on January 16, 2026:

Applied the same approach as used in https://github.com/bitcoin/bitcoin/pull/34240

DrahtBot removed the label CI failed on Aug 13, 2025

in test/functional/wallet_ancient_migration.py:177 in 3b1b154c31 outdated

162+
163+        # Verify the blockchain was loaded correctly
164+        node1_info = self.nodes[1].getblockchaininfo()
165+        self.log.info(f"Modern node blockchain info: height={node1_info['blocks']}")
166+        assert_equal(node1_info['blocks'], 102)
167+

maflcko commented at 7:44 am on August 18, 2025:

I think you may be able to skip this by using dumb_sync_blocks from https://github.com/bitcoin/bitcoin/commit/c847dee1488a294c9a9632a00ba1134b21e41947#diff-e48d83ef13f93a84b5686eb797eeed80dd194cf39db7f2972cf82a5ed25415a7L78-L95

w0xlt commented at 10:11 pm on January 16, 2026:

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

maflcko commented at 10:02 am on January 19, 2026:

I couldn’t find reference to dumb_sync_blocks in that commit.

You should be able to use this command locally:

 0$ git show c847dee1488a294c9a9632a00ba1134b21e41947 -- ./test/functional/wallet_upgradewallet.py | grep -A19 'def dumb_sync'
 1-    def dumb_sync_blocks(self):
 2-        """
 3-        Little helper to sync older wallets.
 4-        Notice that v0.15.2's regtest is hardforked, so there is
 5-        no sync for it.
 6-        v0.15.2 is only being used to test for version upgrade
 7-        and master hash key presence.
 8-        v0.16.3 is being used to test for version upgrade and balances.
 9-        Further info: [#18774 (review)](/bitcoin-bitcoin/18774/#discussion_r416967844)
10-        """
11-        node_from = self.nodes[0]
12-        v16_3_node = self.nodes[1]
13-        to_height = node_from.getblockcount()
14-        height = self.nodes[1].getblockcount()
15-        for i in range(height, to_height+1):
16-            b = node_from.getblock(blockhash=node_from.getblockhash(i), verbose=0)
17-            v16_3_node.submitblock(b)
18-        assert_equal(v16_3_node.getblockcount(), to_height)
19-
20-    def test_upgradewallet(self, wallet, previous_version, requested_version=None, expected_version=None):

fanquake commented at 10:52 am on December 3, 2025: member

@achow101 can you leave a conceptual review?

furszy commented at 2:58 pm on December 3, 2025: member

Concept ACK, will review soon

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

90+
91+        # Verify wallet type
92+        if "non-HD" in wallet_type:
93+            assert('hdmasterkeyid' not in old_wallet_info)
94+        else:
95+            assert('hdmasterkeyid' in old_wallet_info)

furszy commented at 3:11 pm on December 5, 2025:

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)

w0xlt commented at 10:09 pm on January 16, 2026:

Done. Thanks.

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

190+        self.log.info(f"Backup created at: {migration_result['backup_path']}")
191+        assert(os.path.exists(migration_result['backup_path']))
192+
193+        # Rescan the blockchain
194+        self.log.info("Rescanning blockchain...")
195+        self.nodes[1].rescanblockchain()

furszy commented at 3:14 pm on December 5, 2025:

rescanning after migration shouldn’t be needed

w0xlt commented at 10:09 pm on January 16, 2026:

Done. Thanks.

furszy commented at 3:21 pm on December 5, 2025: member

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.

w0xlt force-pushed on Jan 7, 2026

w0xlt force-pushed on Jan 16, 2026

test: add wallet migration test for v0.14.3 legacy wallets

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.

1b6f3f1235

w0xlt force-pushed on Jan 16, 2026

DrahtBot added the label CI failed on Jan 16, 2026

DrahtBot removed the label CI failed on Jan 16, 2026

w0xlt commented at 10:13 pm on January 16, 2026: contributor

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

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

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)