Wallet: add consistency check on wallet transactions stored on wallet database when loading the wallet. #35163

pull Luquitasjeffrey wants to merge 2 commits into bitcoin:master from Luquitasjeffrey:issue34599 changing 2 files +94 −5

Luquitasjeffrey commented at 4:16 AM on April 27, 2026: none
Fixes: #34599 When loading a corrupted wallet database there exists the chance where the transaction state is inconsistent. For example, in the wallet database a parent transaction could appear to be inactive while the child transaction was confirmed (of course that makes no sense, in the blockchain both transactions are confirmed). As a result, when the wallet attempts to abandon inactive transactions, it raises an assertion error. To fix the problem, if when loading the wallet it encounters an inconsistent transaction state, it will return NEED_RESCAN as db errors, because, it is not usable a wallet that has inconsistent transaction state.
- Adds a new test case in wallet_tests.cpp: test_crash_corrupt_wallet_abandon
Tests: added an unit test that reproduces the assertion error in AbandonTransaction call due to corruption on the wallet database file e9cdbd1ad0
wallet: Added a transaction consistency check when loading the wallet from db d61c0a9299
DrahtBot added the label Wallet on Apr 27, 2026
DrahtBot commented at 4:17 AM on April 27, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35115 (wallet: fix abandon crash on inconsistent state by tony-ku)
- #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
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.



LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
- [TxStateConfirmed{m_node.chainman->ActiveChain()[1]->GetBlockHash(), 1, 0}] in src/wallet/test/wallet_tests.cpp
- [TxStateConfirmed{block_child.GetHash(), 101, 1}] in src/wallet/test/wallet_tests.cpp
<sup>2026-04-27 04:17:21</sup>
in src/wallet/walletdb.cpp:1052 in d61c0a9299
```
1052 | +        for (const auto& txin : wtx.tx->vin) {
1053 | +            auto it = pwallet->mapWallet.find(txin.prevout.hash);
1054 | +            if (it != pwallet->mapWallet.end()) {
1055 | +                const CWalletTx& ptx = it->second;
1056 | +                if (wtx.isConfirmed() && !ptx.isConfirmed()) {
1057 | +                    result = DBErrors::NEED_RESCAN;
```
junbyjun1238 commented at 10:15 AM on April 27, 2026:

Question: Should these assignments preserve any more severe load error already stored in result?

Since DBErrors is severity-ordered, and this function already tracks the worst result via std::max, a plain assignment to DBErrors::NEED_RESCAN could silently overwrite a more severe error already stored in result. Would result = std::max(result, DBErrors::NEED_RESCAN) be safer here?
dergoegge commented at 1:10 PM on April 27, 2026: member

This PR looks LLM generated, without real understanding from the author.

LLM generated unit tests are not great for proving the existence of bugs, as LLMs can abuse the access to internal state to produce the wanted results.

Please write a functional test that demonstrates the bug, as that would at least increase the confidence in the LLMs analysis.
achow101 commented at 3:07 AM on April 28, 2026: member

I'm closing this PR as it is clearly LLM generated and there is no evidence that the author understands the mechanism of the issue.
achow101 closed this on Apr 28, 2026

Contributors

Labels

Linked (view graph)

#34599 wallet: Assertion `!wtx.isConfirmed()` failed

Wallet: add consistency check on wallet transactions stored on wallet database when loading the wallet. #35163

Reviews

Conflicts

LLM Linter (✨ experimental)