test: Bumping a transaction prevents bumping malleations
What the developer wrote
Authored by Ava Chow
test: Bumping a transaction prevents bumping malleations
What changed, and why it matters
This commit only adds and updates a test file. It does not change any production wallet code. The new test checks that when a user has both an original transaction and a malleated version of it in their wallet, bumping the fee on one of them correctly prevents the other from also being bumped. This is a regression test for existing behavior, not a security fix.
No action required. This is a test-only change. Reviewers may optionally run the updated functional test to confirm the existing wallet behavior is covered.
Security signals we found
transaction malleation handling
bumpfee replacement metadata
wallet metadata persistence
functional regression test only
Evidence from the diff
The diff modifies test/functional/wallet_txn_clone.py. It expands the test_malleated_metadata_synced functional test to create multiple legacy UTXOs, generate an original tx and a malleated clone, mine the clone, invalidate the block to return it to the mempool, then verify that bumpfee on either the original or the malleated tx marks both as replaced and that bumping the other one fails with ‘Cannot bump transaction … which was already bumped by transaction’. It also adds persistence checks across wallet unload/load and extends test_malleated_rbf_metadata_synced with a reload check. No C++ or Python wallet implementation code is changed.
Changed components
test/functional/wallet_txn_clone.pyInspect captured patch +53 / −5
### test/functional/wallet_txn_clone.py
@@ -8,6 +8,7 @@
from test_framework.util import (
assert_equal,
assert_not_equal,
+ assert_raises_rpc_error,
)
from test_framework.messages import (
COIN,
@@ -170,16 +171,56 @@ def test_malleated_metadata_synced(self):
wallet = self.nodes[0].get_wallet_rpc("metadata_clone")
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
- def_wallet.sendtoaddress(wallet.getnewaddress(address_type="legacy"), 1)
+ # Make non-segwit UTXOs that can be malleated. Smaller than the spending amount
+ # to create multiple inputs.
+ for _ in range(6):
+ def_wallet.sendtoaddress(wallet.getnewaddress(address_type="legacy"), 0.5)
self.generate(self.nodes[0], 1)
- original_txid = wallet.sendtoaddress(def_wallet.getnewaddress(), 0.5, comment="testing", fee_rate=1)
- malleated_tx, malleated_txid = self.malleate_tx(wallet, original_txid)
+ # Bumping either should prevent the other from being bumped as well
+ for bump_malleated in [False, True]:
+ original_txid = wallet.sendtoaddress(def_wallet.getnewaddress(), 0.9, comment="testing", fee_rate=1)
+ malleated_tx, malleated_txid = self.malleate_tx(wallet, original_txid)
- self.generateblock(self.nodes[0], def_wallet.getnewaddress(), [malleated_tx])
+ blockhash = self.generateblock(self.nodes[0], def_wallet.getnewaddress(), [malleated_tx])["hash"]
+
+ assert_equal(wallet.gettransaction(malleated_txid)["comment"], "testing")
+
+ # Check synced comment was written to disk
+ wallet.unloadwallet()
+ self.nodes[0].loadwallet("metadata_clone")
+ assert_equal(wallet.gettransaction(malleated_txid)["comment"], "testing")
+
+ # Put the malleated back into the mempol by invalidating the block
+ self.nodes[0].invalidateblock(blockhash)
+
+ if bump_malleated:
+ to_bump = malleated_txid
+ other_bump = original_txid
+ else:
+ to_bump = original_txid
+ other_bump = malleated_txid
+
+ bumped = wallet.bumpfee(to_bump, fee_rate=10)
+
+ def check_metadata():
+ original_txinfo = wallet.gettransaction(original_txid)
+ malleated_txinfo = wallet.gettransaction(malleated_txid)
+ assert_equal(original_txinfo["replaced_by_txid"], bumped["txid"])
+ assert_equal(malleated_txinfo["replaced_by_txid"], bumped["txid"])
- assert_equal(wallet.gettransaction(malleated_txid)["comment"], "testing")
+ assert_raises_rpc_error(-4, f"Cannot bump transaction {other_bump} which was already bumped by transaction", wallet.bumpfee, other_bump, fee_rate=20)
+
+ check_metadata()
+
+ # Check persistence
+ wallet.unloadwallet()
+ self.nodes[0].loadwallet("metadata_clone")
+
+ check_metadata()
+
+ self.nodes[0].reconsiderblock(blockhash)
def test_malleated_rbf_metadata_synced(self):
self.log.info("Test malleation of a rbf has copied user provided and replacement metadata")
@@ -201,6 +242,13 @@ def test_malleated_rbf_metadata_synced(self):
assert_equal(txinfo["comment"], "testing")
assert_equal(txinfo["replaces_txid"], orig_txid)
+ # Synced metadata must survive a reload
+ wallet.unloadwallet()
+ self.nodes[0].loadwallet("rbf_metadata_clone")
+ txinfo = wallet.gettransaction(malleated_txid)
+ assert_equal(txinfo["comment"], "testing")
+ assert_equal(txinfo["replaces_txid"], orig_txid)
+
if __name__ == '__main__':
TxnMallTest(__file__).main()Why this scored 15/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.