Merge bitcoin/bitcoin#34198: wallet: fix ancient wallets migration

b86c1c443d test: add coverage for migrating ancient wallets (furszy)
fd44d48b24 wallet: fix ancient wallets migration (furszy)

Pull request description:

  We currently fail migration if the wallet does not contain the best block locator.
  This is a problem for wallets created before https://github.com/bitcoin/bitcoin/pull/152, which are not storing such record.

  Missing this record is not an error. it simply means the wallet will scan the chain prior
  to finish migration.

ACKs for top commit:
  achow101:
    ACK b86c1c443d
  w0xlt:
    reACK b86c1c443d
  sedited:
    Re-ACK b86c1c443d

Tree-SHA512: 5226934e16d32f3337c432a84e1adce9985518e52c62abfa4a8d6b3d857d4b5c6aa99ac90e84ae6772983ceaf7a67e128ff7e0e174843fcb892728b9be4653cf
This commit is contained in:
Ava Chow
2026-05-28 11:57:39 -07:00
2 changed files with 35 additions and 5 deletions

View File

@@ -4010,10 +4010,9 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
}
// Get best block locator so that we can copy it to the watchonly and solvables
// Note: The best block locator was introduced in #152 so ancient wallets do not have it
CBlockLocator best_block_locator;
if (!local_wallet_batch.ReadBestBlock(best_block_locator)) {
return util::Error{_("Error: Unable to read wallet's best block locator record")};
}
(void)local_wallet_batch.ReadBestBlock(best_block_locator);
// Update m_txos to match the descriptors remaining in this wallet
m_txos.clear();
@@ -4030,7 +4029,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
LOCK(data.watchonly_wallet->cs_wallet);
data.watchonly_wallet->nOrderPosNext = nOrderPosNext;
watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext);
// Write the best block locator to avoid rescanning on reload
// Write the locator record. An empty locator is valid and triggers rescan on load.
if (!watchonly_batch->WriteBestBlock(best_block_locator)) {
return util::Error{_("Error: Unable to write watchonly wallet best block locator record")};
}
@@ -4039,7 +4038,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
if (data.solvable_wallet) {
solvables_batch = std::make_unique<WalletBatch>(data.solvable_wallet->GetDatabase());
if (!solvables_batch->TxnBegin()) return util::Error{strprintf(_("Error: database transaction cannot be executed for wallet %s"), data.solvable_wallet->GetName())};
// Write the best block locator to avoid rescanning on reload
// Write the locator record. An empty locator is valid and triggers rescan on load.
if (!solvables_batch->WriteBestBlock(best_block_locator)) {
return util::Error{_("Error: Unable to write solvable wallet best block locator record")};
}

View File

@@ -1573,6 +1573,36 @@ class WalletMigrationTest(BitcoinTestFramework):
self.clear_default_wallet(backup_path)
@staticmethod
def erase_bdb_record(wallet_dat_path, key):
data = bytearray(wallet_dat_path.read_bytes())
idx = data.find(key)
assert idx != -1, f"{key!r} not found in wallet.dat"
for i in range(idx, idx + len(key)):
data[i] = 0
wallet_dat_path.write_bytes(data)
def test_missing_bestblock(self):
self.log.info("Test migrating legacy BDB wallet without bestblock record")
wallet_name = "nobestblock"
wallet = self.create_legacy_wallet(wallet_name)
wallet.unloadwallet()
# Erase block locator records like if this would be a pre-#152 wallet
self.erase_bdb_record(self.old_node.wallets_path / wallet_name / "wallet.dat", b"bestblock_nomerkle")
self.erase_bdb_record(self.old_node.wallets_path / wallet_name / "wallet.dat", b"bestblock")
shutil.copytree(self.old_node.wallets_path / wallet_name, self.master_node.wallets_path / wallet_name, dirs_exist_ok=True)
# Migrate, checking that rescan occurs
with self.master_node.assert_debug_log(expected_msgs=["Rescanning"], unexpected_msgs=[]):
self.master_node.migratewallet(wallet_name)
wallet = self.master_node.get_wallet_rpc(wallet_name)
info = wallet.getwalletinfo()
assert_equal(info["descriptors"], True)
assert_equal(info["format"], "sqlite")
def run_test(self):
self.master_node = self.nodes[0]
@@ -1619,6 +1649,7 @@ class WalletMigrationTest(BitcoinTestFramework):
self.test_taproot()
self.test_solvable_no_privs()
self.test_loading_failure_after_migration()
self.test_missing_bestblock()
# Note: After this test the first 250 blocks of 'master_node' are pruned
self.unsynced_wallet_on_pruned_node_fails()