wallet: fix ancient wallets migration
What the developer wrote
Authored by furszy
wallet: fix ancient wallets migration
The best block locator was introduced in #152, previously created
wallets do not have these record.
What changed, and why it matters
This patch fixes a bug where migrating very old Bitcoin wallets would fail entirely. The problem was that the migration code required a 'best block locator' record that did not exist in wallets created before that feature was added. The fix makes that record optional: if it is missing, migration continues and writes an empty locator to the new wallet files, which simply causes them to rescan the blockchain on next load. There is no attacker-controlled path here; it is a robustness fix for a user-initiated wallet migration.
No immediate security action required. Treat as a normal bug-fix commit. Users with pre-PR-#152 wallets who run descriptor-wallet migration will now succeed instead of erroring out, at the cost of a rescan. Reviewers may want to confirm that an empty locator is indeed handled safely downstream in rescan logic.
Security signals we found
Missing-record handling changed from fatal error to graceful continuation
Empty CBlockLocator explicitly treated as valid by updated comments
No input validation, parsing, or cryptographic changes
No privilege boundary crossed
Evidence from the diff
In CWallet::ApplyMigrationData, the code previously called local_wallet_batch.ReadBestBlock(best_block_locator) and returned an error if it failed. Because the best-block-locator record was only introduced later (PR #152), wallets predating it fail migration. The change ignores the read result, allowing best_block_locator to remain empty, then writes that (possibly empty) locator to the watchonly and solvable child wallets. Comments are updated to note that an empty locator is valid and will trigger a rescan on load. This is a defensive fix for a missing-data edge case, not a memory-safety or cryptographic bug.
Changed components
src/wallet/wallet.cppCWallet::ApplyMigrationDatawallet migration (descriptor wallet upgrade)Inspect captured patch +4 / −5
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index bc27018c..0b20b37e 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3950,10 +3950,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();
@@ -3970,7 +3969,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")};
}
@@ -3979,7 +3978,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")};
}
Why this scored 22/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.