Restore -mempoolreplacement option to allow disabling opt-in RBF

This partially reverts commit 8053e5c.

Author: luke-jr

src/init.cpp

@@ -649,6 +649,7 @@ void SetupServerArgs(ArgsManager& argsman)
                              MAX_OP_RETURN_RELAY),
                    ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
     argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
+    argsman.AddArg("-mempoolreplacement", strprintf("Enable transaction replacement in the memory pool (default: %u)", DEFAULT_ENABLE_REPLACEMENT), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
     argsman.AddArg("-permitbaremultisig", strprintf("Relay transactions creating non-P2SH multisig outputs (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY,
                    OptionsCategory::NODE_RELAY);
     argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)",
@@ -1058,6 +1059,14 @@ bool AppInitParameterInteraction(const ArgsManager& args)
         }
     }
 
+    gEnableReplacement = args.GetBoolArg("-mempoolreplacement", DEFAULT_ENABLE_REPLACEMENT);
+    if ((!gEnableReplacement) && args.IsArgSet("-mempoolreplacement")) {
+        // Minimal effort at forwards compatibility
+        std::string replacement_opt = args.GetArg("-mempoolreplacement", "");  // default is impossible
+        std::vector<std::string> replacement_modes = util::SplitString(replacement_opt, ",");
+        gEnableReplacement = (std::find(replacement_modes.begin(), replacement_modes.end(), "fee") != replacement_modes.end());
+    }
+
     // Also report errors from parsing before daemonization
     {
         kernel::Notifications notifications{};

src/policy/policy.h

@@ -36,6 +36,8 @@ static constexpr unsigned int MAX_STANDARD_TX_SIGOPS_COST{MAX_BLOCK_SIGOPS_COST/
 static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{1000};
 /** Default for -bytespersigop */
 static constexpr unsigned int DEFAULT_BYTES_PER_SIGOP{20};
+/** Default for -mempoolreplacement */
+static constexpr bool DEFAULT_ENABLE_REPLACEMENT{true};
 /** Default for -permitbaremultisig */
 static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{true};
 /** The maximum number of witness stack items in a standard P2WSH script */

src/policy/settings.cpp

@@ -7,4 +7,5 @@
 
 #include <policy/policy.h>
 
+bool gEnableReplacement = DEFAULT_ENABLE_REPLACEMENT;
 unsigned int nBytesPerSigOp = DEFAULT_BYTES_PER_SIGOP;

src/policy/settings.h

@@ -6,6 +6,7 @@
 #ifndef BITCOIN_POLICY_SETTINGS_H
 #define BITCOIN_POLICY_SETTINGS_H
 
+extern bool gEnableReplacement;
 extern unsigned int nBytesPerSigOp;
 
 #endif // BITCOIN_POLICY_SETTINGS_H

src/validation.cpp

@@ -858,7 +858,14 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
                 //
                 // Replaceability signaling of the original transactions may be
                 // ignored due to node setting.
-                const bool allow_rbf{(m_pool.m_opts.full_rbf || ignore_rejects.count("txn-mempool-conflict")) || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->version == TRUC_VERSION};
+                bool allow_rbf;
+                if (m_pool.m_opts.full_rbf || ignore_rejects.count("txn-mempool-conflict")) {
+                    allow_rbf = true;
+                } else if (!gEnableReplacement) {
+                    allow_rbf = false;
+                } else {
+                    allow_rbf = SignalsOptInRBF(*ptxConflicting) || ptxConflicting->version == TRUC_VERSION;
+                }
                 if (!allow_rbf) {
                     return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
                 }

test/functional/feature_rbf.py

@@ -25,7 +25,7 @@ def add_options(self, parser):
         self.add_wallet_options(parser)
 
     def set_test_params(self):
-        self.num_nodes = 2
+        self.num_nodes = 3
         # both nodes disable full-rbf to test BIP125 signaling
         self.extra_args = [
             [
@@ -39,6 +39,10 @@ def set_test_params(self):
             [
                 "-mempoolfullrbf=0",
             ],
+            [
+                "-acceptnonstdtxn=1",
+                "-mempoolreplacement=0",
+            ],
         ]
         self.supports_cli = False
 
@@ -114,17 +118,24 @@ def test_simple_doublespend(self):
         # we use MiniWallet to create a transaction template with inputs correctly set,
         # and modify the output (amount, scriptPubKey) according to our needs
         tx = self.wallet.create_self_transfer()["tx"]
-        tx1a_txid = self.nodes[0].sendrawtransaction(tx.serialize().hex())
+        tx1a_hex = tx.serialize().hex()
+        tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex)
+        assert_equal(tx1a_txid, self.nodes[2].sendrawtransaction(tx1a_hex))
 
         # Should fail because we haven't changed the fee
         tx.vout[0].scriptPubKey[-1] ^= 1
 
         # This will raise an exception due to insufficient fee
-        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex(), 0)
+        tx1b_hex = tx.serialize().hex()
+        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, 0)
+        # This will raise an exception due to transaction replacement being disabled
+        assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[2].sendrawtransaction, tx1b_hex, 0)
 
         # Extra 0.1 BTC fee
         tx.vout[0].nValue -= int(0.1 * COIN)
         tx1b_hex = tx.serialize().hex()
+        # Replacement still disabled even with "enough fee"
+        assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[2].sendrawtransaction, tx1b_hex, 0)
         # Works when enabled
         tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, 0)
 
@@ -135,6 +146,11 @@ def test_simple_doublespend(self):
 
         assert_equal(tx1b_hex, self.nodes[0].getrawtransaction(tx1b_txid))
 
+        # Third node is running mempoolreplacement=0, will not replace originally-seen txn
+        mempool = self.nodes[2].getrawmempool()
+        assert tx1a_txid in mempool
+        assert tx1b_txid not in mempool
+
     def test_doublespend_chain(self):
         """Doublespend of a long chain"""