#35760 wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan

full analysis

https://github.com/bitcoin/bitcoin/pull/35760 · achow101 · +62/-23 in 8 files, 5 commits · labels: Wallet

Goal

  • Fail wallet loading immediately when transaction records are corrupted instead of forcing a rescan
  • Prevent repeated full rescans from genesis and inconsistent in-memory wallet state when DB records corrupt

This PR changes wallet loading so that a transaction record whose stored database key does not match its deserialized hash returns DBErrors::CORRUPT instead of DBErrors::NEED_RESCAN. It completely eliminates DBErrors::NEED_RESCAN and the rescan_required flag in CWallet::AttachChain, adds unit test coverage for corrupted transaction records, and adds documentation to doc/managing-wallets.md advising users on backup restoration.

Problem: Previously, a transaction record with a mismatched hash caused the wallet to rescan the entire blockchain from genesis on every startup because the rescan never repaired or purged the corrupted record. Additionally, the corrupted transaction was partially added to in-memory wallet structures without spend tracking or TXO caching, risking state inconsistency.

Category: Wallet (#41 of 84)

P3 · bug fix

  • P3 because it addresses a narrow reliability edge case seen only during database corruption
  • Prevents endless startup rescans from genesis and protects against inconsistent in-memory wallet state

P3 because it addresses a narrow reliability issue where corrupted transaction records caused a full blockchain rescan on every startup. The rescan never resolved the underlying corruption and left the wallet in a partially inconsistent in-memory state. Failing immediately and cleanly with DBErrors::CORRUPT is a solid improvement in error handling, but it affects an edge case seen only during database corruption.

Membership: Directly modifies wallet loading logic, wallet error types, and transaction database loading in src/wallet/.

Factors: security/stability 1, bug 1, performance 1, user value 1, leverage 1

Reviewability: Ready

  • Ready for review: code is small, passes CI, and includes unit test coverage

The code is small, passes CI, and includes dedicated unit test coverage. The minor error message swap flagged in review is not substantial enough to invalidate an assessment.

Author status: silent since 2026-08-26 after addressing previous test issues and documentation additions.

Open concerns:

  • polespinasa noted that disabling loading on corrupt transactions leaves users who lack backups with no way to access uncorrupted funds or export descriptors from Core.
  • jeanpablojp pointed out that the stored hash and calculated hash arguments are swapped in the strprintf error message in LoadTxRecords.

Resolved concerns:

  • pablomartin4btc noted the absence of recovery guidance, which was resolved by incorporating documentation on recovering from corrupted wallets into managing-wallets.md.
  • polespinasa identified that the initial unit test passed for the wrong reason due to a deserialization failure rather than a hash mismatch; this was corrected by adding a dummy input to the mock transaction.

Agreement: Mild

  • General agreement that failing fast is better than triggering broken rescans
  • Support because rescanning cannot repair corrupt records and wastes time (pablomartin4btc, rkrux)
  • Verified by reproducing corruption behavior on regtest (jeanpablojp)
  • Nonblocking objection: users without backups cannot access intact funds or export keys (polespinasa)

General agreement on failing fast on corruption, with an unresolved concern from polespinasa about recovery options for users without backups.

Most reviewers agree that attempting an automatic rescan on corrupted transactions was broken and misleading. However, polespinasa raised a standing nonblocking objection that completely refusing to load leaves users without backups stranded without a way to salvage uncorrupted funds or export keys.

  • pablomartin4btc supported failing hard with CORRUPT because rescan is not a reliable repair mechanism.
  • rkrux Concept ACKed, agreeing that the wallet should stop and fail fast instead of attempting automatic fixes on load.
  • jeanpablojp Approach ACKed after reproducing the corruption behavior on regtest.
  • polespinasa cautioned that users without backups are locked out of uncorrupted funds and suggested allowing descriptor exports or warning-only loads.
  • jeanpablojp noted swapped parameters in the strprintf corruption error message.

Review verdicts (DrahtBot): 0

Files

23 lines under test/bench/ci.

  • src/wallet/wallet.cpp +9/-18
  • src/wallet/test/walletload_tests.cpp +23/-0
  • doc/managing-wallets.md +19/-0
  • doc/release-notes-35760.md +6/-0
  • src/wallet/walletdb.cpp +3/-2
  • src/wallet/wallet.h +1/-1
  • src/wallet/wallettool.cpp +1/-1
  • src/wallet/walletdb.h +0/-1

Card

PR 35760 makes wallet loading fail immediately with DBErrors::CORRUPT when a transaction record's stored database key does not match its deserialized hash, completely removing the legacy DBErrors::NEED_RESCAN state. Previously, such corruption triggered a full rescan from genesis on every startup without actually repairing the record or properly populating spend maps. The PR includes unit tests, release notes, and documentation in managing-wallets.md detailing backup restoration. Reviewers support the fail-fast approach, though polespinasa raised a concern regarding users who lack backups losing the ability to access unaffected funds.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:20, confidence high, input hash b6c00b1b22d974ce