Restore -mempoolreplacement option to allow disabling opt-in RBF

This partially reverts commit 8053e5cdad.
This commit is contained in:
Luke Dashjr 2019-10-30 11:23:57 +00:00
parent 6678de6021
commit 9da96bcd19
6 changed files with 41 additions and 4 deletions

View File

@ -600,6 +600,7 @@ void SetupServerArgs(ArgsManager& argsman)
MAX_OP_RETURN_RELAY), MAX_OP_RETURN_RELAY),
ArgsManager::ALLOW_ANY, OptionsCategory::NODE_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("-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, argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY,
OptionsCategory::NODE_RELAY); 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)", argsman.AddArg("-minrelaytxfee=<amt>", 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.")); 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<std::string> 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 // Also report errors from parsing before daemonization
{ {
kernel::Notifications notifications{}; kernel::Notifications notifications{};

View File

@ -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}; static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{1000};
/** Default for -bytespersigop */ /** Default for -bytespersigop */
static constexpr unsigned int DEFAULT_BYTES_PER_SIGOP{20}; static constexpr unsigned int DEFAULT_BYTES_PER_SIGOP{20};
/** Default for -mempoolreplacement */
static constexpr bool DEFAULT_ENABLE_REPLACEMENT{true};
/** Default for -permitbaremultisig */ /** Default for -permitbaremultisig */
static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{true}; static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{true};
/** The maximum number of witness stack items in a standard P2WSH script */ /** The maximum number of witness stack items in a standard P2WSH script */

View File

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

View File

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

View File

@ -781,7 +781,15 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// //
// If replaceability signaling is ignored due to node setting, // If replaceability signaling is ignored due to node setting,
// replacement is always allowed. // 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"); return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
} }

View File

@ -25,7 +25,7 @@ class ReplaceByFeeTest(BitcoinTestFramework):
self.add_wallet_options(parser) self.add_wallet_options(parser)
def set_test_params(self): def set_test_params(self):
self.num_nodes = 2 self.num_nodes = 3
self.extra_args = [ self.extra_args = [
[ [
"-maxorphantx=1000", "-maxorphantx=1000",
@ -37,6 +37,10 @@ class ReplaceByFeeTest(BitcoinTestFramework):
# second node has default mempool parameters # second node has default mempool parameters
[ [
], ],
[
"-acceptnonstdtxn=1",
"-mempoolreplacement=0",
],
] ]
self.supports_cli = False self.supports_cli = False
@ -112,17 +116,24 @@ class ReplaceByFeeTest(BitcoinTestFramework):
# we use MiniWallet to create a transaction template with inputs correctly set, # we use MiniWallet to create a transaction template with inputs correctly set,
# and modify the output (amount, scriptPubKey) according to our needs # and modify the output (amount, scriptPubKey) according to our needs
tx = self.wallet.create_self_transfer()["tx"] 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 # Should fail because we haven't changed the fee
tx.vout[0].scriptPubKey[-1] ^= 1 tx.vout[0].scriptPubKey[-1] ^= 1
# This will raise an exception due to insufficient fee # 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 # Extra 0.1 BTC fee
tx.vout[0].nValue -= int(0.1 * COIN) tx.vout[0].nValue -= int(0.1 * COIN)
tx1b_hex = tx.serialize().hex() 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 # Works when enabled
tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, 0) 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)) 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): def test_doublespend_chain(self):
"""Doublespend of a long chain""" """Doublespend of a long chain"""