wallet: fix removeprunedfunds bug with conflicting transactions
What the developer wrote
Authored by Martin Zumsande
wallet: fix removeprunedfunds bug with conflicting transactions
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.
What changed, and why it matters
This commit fixes a bug in Bitcoin Core's wallet where removing a pruned transaction could accidentally delete records of other conflicting transactions that spend the same coin. As a result, the wallet might mistakenly believe a coin was still available and create an invalid double-spend transaction. The fix ensures only the specific pruned transaction's spend records are removed, leaving conflicting transactions intact.
Apply the patch to ensure RemoveTxs only deletes the specific mapTxSpends entries belonging to the removed transaction. Users running wallet software with removeprunedfunds should upgrade to a version containing this fix, especially if they use RBF/conflicting transactions or pruning workflows.
Security signals we found
Incorrect multimap erase logic leading to loss of conflicting spend tracking
Wallet internal accounting could be corrupted, enabling creation of invalid double-spend transactions
Bug persists even when conflicting transaction is mined because wallet trusts corrupted internal state
Fix includes regression test covering conflicting transaction scenario
Evidence from the diff
In CWallet::RemoveTxs, the previous code called mapTxSpends.erase(txin.prevout) for every input of each removed transaction. Because mapTxSpends is a multimap keyed by COutPoint, this erased all entries for that outpoint, including entries belonging to other conflicting transactions that also spend it. The patch changes the logic to iterate the equal_range for the outpoint and erase only the entry whose value matches the transaction being removed. A functional test is added demonstrating that after removeprunedfunds(tx1), a conflicting tx2 still keeps the UTXO marked as spent.
Changed components
src/wallet/wallet.cppCWallet::RemoveTxsmapTxSpendsRPC removeprunedfundstest/functional/wallet_importprunedfunds.pyInspect captured patch +37 / −2
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index abbb6c12..a24fc908 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2398,8 +2398,15 @@ util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs
for (const auto& it : erased_txs) {
const Txid hash{it->first};
wtxOrdered.erase(it->second.m_it_wtxOrdered);
- for (const auto& txin : it->second.tx->vin)
- mapTxSpends.erase(txin.prevout);
+ for (const auto& txin : it->second.tx->vin) {
+ auto range = mapTxSpends.equal_range(txin.prevout);
+ for (auto iter = range.first; iter != range.second; ++iter) {
+ if (iter->second == hash) {
+ mapTxSpends.erase(iter);
+ break;
+ }
+ }
+ }
for (unsigned int i = 0; i < it->second.tx->vout.size(); ++i) {
m_txos.erase(COutPoint(hash, i));
}
diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py
index 72896052..1948cb92 100755
--- a/test/functional/wallet_importprunedfunds.py
+++ b/test/functional/wallet_importprunedfunds.py
@@ -14,6 +14,7 @@ from test_framework.messages import (
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
+ assert_not_equal,
assert_raises_rpc_error,
wallet_importprivkey,
)
@@ -129,6 +130,33 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
mb.header.nTime += 1 # modify arbitrary block header field to change block hash
assert_raises_rpc_error(-5, "Block not found in chain", w1.importprunedfunds, rawtxn1, mb.serialize().hex())
+ self.log.info("Test removeprunedfunds with conflicting transactions")
+ node = self.nodes[0]
+
+ # Create a transaction
+ utxo = node.listunspent()[0]
+ addr = node.getnewaddress()
+ tx1_id = node.send(outputs=[{addr: 1}], inputs=[utxo])["txid"]
+ tx1_fee = node.gettransaction(tx1_id)["fee"]
+
+ # Create a conflicting tx with a larger fee (tx1_fee is negative)
+ output_value = utxo["amount"] + tx1_fee - Decimal("0.00001")
+ raw_tx2 = node.createrawtransaction(inputs=[utxo], outputs=[{addr: output_value}])
+ signed_tx2 = node.signrawtransactionwithwallet(raw_tx2)
+ tx2_id = node.sendrawtransaction(signed_tx2["hex"])
+ assert_not_equal(tx2_id, tx1_id)
+
+ # Both txs should be in the wallet, tx2 replaced tx1 in mempool
+ assert tx1_id in [tx["txid"] for tx in node.listtransactions()]
+ assert tx2_id in [tx["txid"] for tx in node.listtransactions()]
+
+ # Remove the replaced tx from wallet
+ node.removeprunedfunds(tx1_id)
+
+ # The UTXO should still be considered spent (by tx2)
+ available_utxos = [u["txid"] for u in node.listunspent(minconf=0)]
+ assert utxo["txid"] not in available_utxos, "UTXO should still be spent by conflicting tx"
+
if __name__ == '__main__':
ImportPrunedFundsTest(__file__).main()
Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.