From 0e0d01913a881845fa95ed37b012f7e7edd2c65d Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Thu, 25 Jun 2026 15:10:01 +0200 Subject: [PATCH] mempool: disable full-RBF, require BIP125 opt-in signaling Bitcoin Core defaults to accepting any fee-bumping replacement regardless of signaling (full-RBF). Revert this for Elements: only replace mempool transactions that explicitly opt in via BIP125 nSequence signaling (or TRUC), and report fullrbf=false in getmempoolinfo. Update feature_rbf.py accordingly, and add a regression test for CVE-2021-31876 confirming that inherited signaling from an unconfirmed parent does not make a non-signaling child replaceable. --- src/rpc/mempool.cpp | 4 +- src/validation.cpp | 19 ++++++++- test/functional/feature_rbf.py | 73 +++++++++++++++++++++++++++++++--- 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index bc1ddb341e..14ff773af1 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -695,7 +695,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) ret.pushKV("minrelaytxfee", ValueFromAmount(pool.m_opts.min_relay_feerate.GetFeePerK())); ret.pushKV("incrementalrelayfee", ValueFromAmount(pool.m_opts.incremental_relay_feerate.GetFeePerK())); ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()}); - ret.pushKV("fullrbf", true); + ret.pushKV("fullrbf", false); return ret; } @@ -717,7 +717,7 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::STR_AMOUNT, "minrelaytxfee", "Current minimum relay fee for transactions"}, {RPCResult::Type::NUM, "incrementalrelayfee", "minimum fee rate increment for mempool limiting or replacement in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::NUM, "unbroadcastcount", "Current number of transactions that haven't passed initial broadcast yet"}, - {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection (DEPRECATED)"}, + {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection (DEPRECATED). Always false: this mempool requires BIP125 opt-in signaling for replacement, except for TRUC transactions."}, }}, RPCExamples{ HelpExampleCli("getmempoolinfo", "") diff --git a/src/validation.cpp b/src/validation.cpp index d32faa9ed4..0002c330be 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -986,7 +986,24 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Transaction conflicts with a mempool tx, but we're not allowing replacements in this context. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed"); } - ws.m_conflicts.insert(ptxConflicting->GetHash()); + if (!ws.m_conflicts.count(ptxConflicting->GetHash())) + { + // 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. + // + // All TRUC transactions are considered replaceable. + const bool allow_rbf{SignalsOptInRBF(*ptxConflicting) || ptxConflicting->version == TRUC_VERSION}; + if (!allow_rbf) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); + } + + ws.m_conflicts.insert(ptxConflicting->GetHash()); + } } } diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 9ef1cc9183..19e764560d 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -9,6 +9,7 @@ # from test_framework.script import CScript from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, + SEQUENCE_FINAL, COIN, # CTransaction, # CTxIn, @@ -82,6 +83,9 @@ def run_test(self): self.log.info("Running test full replace by fee...") self.test_fullrbf() + self.log.info("Running test no inherited signaling (CVE-2021-31876)...") + self.test_no_inherited_signaling() + self.log.info("Passed") def make_utxo(self, node, amount, *, confirmed=True, scriptPubKey=None): @@ -602,11 +606,13 @@ def test_replacement_relay_fee(self): def test_fullrbf(self): # BIP125 signaling is not respected + # ELEMENTS: disable mempoolfullrbf and keep BIP125 signaling confirmed_utxo = self.make_utxo(self.nodes[0], int(2 * COIN)) - assert self.nodes[0].getmempoolinfo()["fullrbf"] + assert not self.nodes[0].getmempoolinfo()["fullrbf"] # ELEMENTS # Create an explicitly opt-out BIP125 transaction, which will be ignored + # ELEMENTS: respect BIP125 signal optout_tx = self.wallet.send_self_transfer( from_node=self.nodes[0], utxo_to_spend=confirmed_utxo, @@ -620,12 +626,67 @@ def test_fullrbf(self): fee_rate=Decimal('0.02'), ) - # Send the replacement transaction, conflicting with the optout_tx. - self.nodes[0].sendrawtransaction(conflicting_tx['hex'], 0) + # ELEMENTS: The replacement is rejected because optout_tx did not signal replaceability. + assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, conflicting_tx['hex'], 0) + + # ELEMENTS: Optout_tx is still in the mempool; the replacement was rejected + assert optout_tx['txid'] in self.nodes[0].getrawmempool() + assert conflicting_tx['txid'] not in self.nodes[0].getrawmempool() + + # ELEMENTS + def test_no_inherited_signaling(self): + """Make sure we're not susceptible to CVE-2021-31876. + + BIP125 says a transaction is replaceable if it directly signals via + nSequence, *or* if it spends an output of an unconfirmed parent that + itself signals replaceability ("inherited signaling"). + Elements only enforces the direct-signaling half of that rule + when deciding whether to *accept* a replacement -- see the comment + next to the SignalsOptInRBF() call in MemPoolAccept::PreChecks(). + This is the discrepancy reported as CVE-2021-31876: a transaction + can be reported as `bip125-replaceable` (because it inherits the + signal from an unconfirmed ancestor) while an actual attempt to + replace it is still rejected. + + This test confirms that this divergence is consistent: a + non-signaling child of a signaling, unconfirmed parent must remain + non-replaceable. + """ + confirmed_utxo = self.make_utxo(self.nodes[0], int(2 * COIN)) + + # Parent explicitly signals replaceability... + parent_utxo = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=confirmed_utxo, + sequence=MAX_BIP125_RBF_SEQUENCE, + fee=Decimal("0.01"), + )["new_utxo"] + + # ...but the child spending it does not. + child_tx = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=parent_utxo, + sequence=SEQUENCE_FINAL, + fee=Decimal("0.01"), + ) + + # `bip125-replaceable` reports True: it is inherited from the + # unconfirmed, signaling parent (the BIP125-compliant half of the logic, + # implemented by IsRBFOptIn() for RPC reporting purposes). + assert_equal(True, self.nodes[0].getmempoolentry(child_tx['txid'])['bip125-replaceable']) + + # However, an actual replacement of the child is rejected: PreChecks() + # only consults the child's own sequence numbers, not its ancestors', + # so the inherited signal is *not* honoured for acceptance purposes. + conflicting_tx = self.wallet.create_self_transfer( + utxo_to_spend=parent_utxo, + fee=Decimal("0.05"), + ) + assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, conflicting_tx['hex'], 0) - # Optout_tx is not anymore in the mempool. - assert optout_tx['txid'] not in self.nodes[0].getrawmempool() - assert conflicting_tx['txid'] in self.nodes[0].getrawmempool() + # The non-signaling child is still in the mempool; the replacement never got in. + assert child_tx['txid'] in self.nodes[0].getrawmempool() + assert conflicting_tx['txid'] not in self.nodes[0].getrawmempool() if __name__ == '__main__': ReplaceByFeeTest(__file__).main()