From 9da96bcd191426c2885dae3609afcdec637604c7 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 30 Oct 2019 11:23:57 +0000 Subject: [PATCH] Restore -mempoolreplacement option to allow disabling opt-in RBF This partially reverts commit 8053e5cdade87550f0381d51feab81dedfec6c46. --- src/init.cpp | 9 +++++++++ src/policy/policy.h | 2 ++ src/policy/settings.cpp | 1 + src/policy/settings.h | 1 + src/validation.cpp | 10 +++++++++- test/functional/feature_rbf.py | 22 +++++++++++++++++++--- 6 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 33152df32e..4d0f06691e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -600,6 +600,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 non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", @@ -999,6 +1000,14 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(Untranslated("-rpcserialversion=0 is deprecated and will be removed in the future. Specify -deprecatedrpc=serialversion to allow anyway.")); } + 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 replacement_modes = 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{}; diff --git a/src/policy/policy.h b/src/policy/policy.h index 3d1ebad13a..c0832a035c 100644 --- a/src/policy/policy.h +++ b/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 */ diff --git a/src/policy/settings.cpp b/src/policy/settings.cpp index 722b12acf5..4a47ebdfea 100644 --- a/src/policy/settings.cpp +++ b/src/policy/settings.cpp @@ -7,4 +7,5 @@ #include +bool gEnableReplacement = DEFAULT_ENABLE_REPLACEMENT; unsigned int nBytesPerSigOp = DEFAULT_BYTES_PER_SIGOP; diff --git a/src/policy/settings.h b/src/policy/settings.h index 145454b0dd..da33c53575 100644 --- a/src/policy/settings.h +++ b/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 diff --git a/src/validation.cpp b/src/validation.cpp index 776a080123..3eb7addd23 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -781,7 +781,15 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // // If replaceability signaling is ignored due to node setting, // replacement is always allowed. - if (!(m_pool.m_full_rbf || ignore_rejects.count("txn-mempool-conflict")) && !SignalsOptInRBF(*ptxConflicting)) { + bool allow_replacement; + if (m_pool.m_full_rbf || ignore_rejects.count("txn-mempool-conflict")) { + allow_replacement = true; + } else if (!gEnableReplacement) { + allow_replacement = false; + } else { + allow_replacement = SignalsOptInRBF(*ptxConflicting); + } + if (!allow_replacement) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index c5eeaf66e0..60dea3dce4 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -25,7 +25,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): self.add_wallet_options(parser) def set_test_params(self): - self.num_nodes = 2 + self.num_nodes = 3 self.extra_args = [ [ "-maxorphantx=1000", @@ -37,6 +37,10 @@ class ReplaceByFeeTest(BitcoinTestFramework): # second node has default mempool parameters [ ], + [ + "-acceptnonstdtxn=1", + "-mempoolreplacement=0", + ], ] self.supports_cli = False @@ -112,17 +116,24 @@ class ReplaceByFeeTest(BitcoinTestFramework): # 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) @@ -133,6 +144,11 @@ class ReplaceByFeeTest(BitcoinTestFramework): 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"""