removeprunedfunds removes all entries from mapTxSpends for the inputs of the pruned tx. However, this is incorrect, because there could be multiple entries from conflicting transactions (that shouldn’t be removed as well). This could lead to the wallet creating invalid transactions, trying to double spend utxos.
The bug persists when the conflicting tx was mined, because the wallet trusts its internal accounting instead of calling AddToSpends again.
The added test should fail on master.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34358.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | fjahr |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
removeprunedfunds removes all entries from mapTxSpends for the
inputs of the pruned tx. However, this is incorrect, because there could be
multiple entries from conflicting transactions (that shouldn't be
removed as well). This could lead to the wallet creating invalid
transactions, trying to double spend utxos.
The bug persists when the conflicting tx was mined, because
the wallet trusts its internal accounting instead of calling
AddToSpends again.
Concept ACK
Confirmed that the new test fails on master and reviewed the test code. Need to spend some extra time with the wallet code.