Skip to content

Conversation

@furszy
Copy link
Member

@furszy furszy commented Jan 3, 2026

We currently fail migration if the wallet does not contain the best block locator.
This is a problem for wallets created before #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.

Note:
Originally I wrote the test using a BDB library (bsddb3) but I wasn't happy with the
new dependency, and ended up zeroing the record length manually. I'm not sure this
will work on every platform. If it doesn't, we can just just drop the test as this can be
tested manually on any ancient wallet too.

@DrahtBot DrahtBot added the Wallet label Jan 3, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 3, 2026

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/34198.

Reviews

See the guideline for information on the review process.

Type Reviewers
Stale ACK bensig

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> 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_migration.py: assert idx != -1, f"{key!r} not found in wallet.dat" -> Use assert_not_equal(idx, -1, f"{key!r} not found in wallet.dat")

2026-01-07

@bensig
Copy link
Contributor

bensig commented Jan 5, 2026

ACK 63a079e

Code review ACK. The fix makes sense - ancient pre-#152 wallets shouldn't fail migration just because they lack the bestblock record.

Couldn't run the test locally (requires previous releases for BDB wallet support), but the logic is straightforward.

furszy added 2 commits January 7, 2026 11:25
The best block locator was introduced in bitcoin#152, previously created
wallets do not have these record.
Pre-bitcoin#152 wallets have no best block stored. Test we
can migrate them.
@furszy furszy force-pushed the 2026_wallet_migration_ancient_wallets branch from 63a079e to 785eadf Compare January 7, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants