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

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:wallet-rm-needs-rescan changing 5 files +13 −25

achow101 commented at 10:11 PM on July 20, 2026: member

In wallet loading, the NEED_RESCAN enum was only ever returned if the stored hash of a transaction did not match the calculated hash of the transaction. This would then trigger a rescan from genesis during loading, and then allow the wallet to be operational as if it were normal.

However, it seems incorrect to be treating such corruption as acceptable. That transaction would be inserted to mapWallet, but not be added to mapTxSpends or any of the conflict tracking or to TXO caching. Furthermore, the rescan doesn't actually fix the problem, the incorrect record would persist, which means that the wallet would be rescanning the entire chain every time it is loaded. The only situation this could occur is if the wallet actually experienced corruption, so it's reasonable to return CORRUPT instead and force the user to load a backup or otherwise un-corrupt the wallet before it can be used.

Implements the idea from #35501 (review)

walletdb: LoadToWallet failure is wallet corruption

If the wallet fails to load a transaction from the database, the wallet
is corrupted.

9d8d6f1cab

wallet: Remove DBErrors::NEED_RESCAN and rescan_required

DBErrors::NEED_RESCAN is no longer returned by any part of wallet
loading, so it can be deleted. AttachChain's rescan_required parameter
was only ever set to true when NEED_RESCAN was returned, so this can be
removed as well.

78d0f8338d

DrahtBot added the label Wallet on Jul 20, 2026
DrahtBot commented at 10:11 PM on July 20, 2026: 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/35760.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35716 (wallet: Replace mapWallet and wtxOrdered with a boost::multi_index by achow101)
- #35501 (wallet: store all witness variants of a transaction by achow101)
- #33034 (wallet: Store transactions in a separate sqlite table by achow101)
- #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
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.

Contributors

achow101

DrahtBot

Labels

Wallet

Linked (view graph)

#35501 wallet: store all witness variants of a transaction

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

Code Coverage & Benchmarks

Reviews

Conflicts