diff options
| -rw-r--r-- | src/validation.cpp | 11 | ||||
| -rwxr-xr-x | test/functional/feature_rbf.py | 67 |
2 files changed, 74 insertions, 4 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index e6e6aadb17..5e3d429c2e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -629,10 +629,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // is for the sake of multi-party protocols, where we don't // want a single party to be able to disable replacement. // - // The opt-out ignores descendants as anyone relying on - // first-seen mempool behavior should be checking all - // unconfirmed ancestors anyway; doing otherwise is hopelessly - // insecure. + // Transactions that don't explicitly signal replaceability are + // *not* replaceable with the current logic, even if one of their + // unconfirmed ancestors signals replaceability. This diverges + // from BIP125's inherited signaling description (see CVE-2021-31876). + // Applications relying on first-seen mempool behavior should + // check all unconfirmed ancestors; otherwise an opt-in ancestor + // might be replaced, causing removal of this descendant. bool fReplacementOptOut = true; for (const CTxIn &_txin : ptxConflicting->vin) { diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 344db5f652..0bb04ae267 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -116,6 +116,9 @@ class ReplaceByFeeTest(BitcoinTestFramework): self.log.info("Running test prioritised transactions...") self.test_prioritised_transactions() + self.log.info("Running test no inherited signaling...") + self.test_no_inherited_signaling() + self.log.info("Passed") def test_simple_doublespend(self): @@ -564,5 +567,69 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert_equal(json0["vin"][0]["sequence"], 4294967293) assert_equal(json1["vin"][0]["sequence"], 4294967294) + def test_no_inherited_signaling(self): + # Send tx from which to conflict outputs later + base_txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) + self.nodes[0].generate(1) + self.sync_blocks() + + # Create an explicitly opt-in parent transaction + optin_parent_tx = self.nodes[0].createrawtransaction([{ + 'txid': base_txid, + 'vout': 0, + "sequence": 0xfffffffd, + }], {self.nodes[0].getnewaddress(): Decimal("9.99998")}) + + optin_parent_tx = self.nodes[0].signrawtransactionwithwallet(optin_parent_tx) + + # Broadcast parent tx + optin_parent_txid = self.nodes[0].sendrawtransaction(hexstring=optin_parent_tx["hex"], maxfeerate=0) + assert optin_parent_txid in self.nodes[0].getrawmempool() + + replacement_parent_tx = self.nodes[0].createrawtransaction([{ + 'txid': base_txid, + 'vout': 0, + "sequence": 0xfffffffd, + }], {self.nodes[0].getnewaddress(): Decimal("9.90000")}) + + replacement_parent_tx = self.nodes[0].signrawtransactionwithwallet(replacement_parent_tx) + + # Test if parent tx can be replaced. + res = self.nodes[0].testmempoolaccept(rawtxs=[replacement_parent_tx['hex']], maxfeerate=0)[0] + + # Parent can be replaced. + assert_equal(res['allowed'], True) + + # Create an opt-out child tx spending the opt-in parent + optout_child_tx = self.nodes[0].createrawtransaction([{ + 'txid': optin_parent_txid, + 'vout': 0, + "sequence": 0xffffffff, + }], {self.nodes[0].getnewaddress(): Decimal("9.99990")}) + + optout_child_tx = self.nodes[0].signrawtransactionwithwallet(optout_child_tx) + + # Broadcast child tx + optout_child_txid = self.nodes[0].sendrawtransaction(hexstring=optout_child_tx["hex"], maxfeerate=0) + assert optout_child_txid in self.nodes[0].getrawmempool() + + replacement_child_tx = self.nodes[0].createrawtransaction([{ + 'txid': optin_parent_txid, + 'vout': 0, + "sequence": 0xffffffff, + }], {self.nodes[0].getnewaddress(): Decimal("9.00000")}) + + replacement_child_tx = self.nodes[0].signrawtransactionwithwallet(replacement_child_tx) + + # Broadcast replacement child tx + # BIP 125 : + # 1. The original transactions signal replaceability explicitly or through inheritance as described in the above + # Summary section. + # The original transaction (`optout_child_tx`) doesn't signal RBF but its parent (`optin_parent_txid`) does. + # The replacement transaction (`replacement_child_tx`) should be able to replace the original transaction. + # See CVE-2021-31876 for further explanations. + assert optin_parent_txid in self.nodes[0].getrawmempool() + assert_raises_rpc_error(-26, 'txn-mempool-conflict', self.nodes[0].sendrawtransaction, replacement_child_tx["hex"], 0) + if __name__ == '__main__': ReplaceByFeeTest().main() |