Remove -mempoolreplacement to prevent needless block prop slowness.

At this point there is no reasonable excuse to disable opt-in RBF,
and, unlike when this option was added, there are now significant
issues created when disabling it (in the form of compact block
reconstruction failures). Further, it breaks a lot of modern wallet
behavior.
This commit is contained in:
Matt Corallo 2019-06-08 08:58:46 -04:00
parent 5d2ccf0ce9
commit 8053e5cdad
6 changed files with 5 additions and 41 deletions

View File

@ -488,9 +488,6 @@ Relay and mine data carrier transactions (default: 1)
Maximum size of data in data carrier transactions we relay and mine Maximum size of data in data carrier transactions we relay and mine
(default: 83) (default: 83)
.HP .HP
\fB\-mempoolreplacement\fR
.IP
Enable transaction replacement in the memory pool (default: 1)
.HP .HP
\fB\-minrelaytxfee=\fR<amt> \fB\-minrelaytxfee=\fR<amt>
.IP .IP

View File

@ -488,10 +488,6 @@ Relay and mine data carrier transactions (default: 1)
Maximum size of data in data carrier transactions we relay and mine Maximum size of data in data carrier transactions we relay and mine
(default: 83) (default: 83)
.HP .HP
\fB\-mempoolreplacement\fR
.IP
Enable transaction replacement in the memory pool (default: 1)
.HP
\fB\-minrelaytxfee=\fR<amt> \fB\-minrelaytxfee=\fR<amt>
.IP .IP
Fees (in BTC/kB) smaller than this are considered zero fee for relaying, Fees (in BTC/kB) smaller than this are considered zero fee for relaying,

View File

@ -524,7 +524,6 @@ void SetupServerArgs()
gArgs.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), false, OptionsCategory::NODE_RELAY); gArgs.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), false, OptionsCategory::NODE_RELAY);
gArgs.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), false, OptionsCategory::NODE_RELAY); gArgs.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), false, OptionsCategory::NODE_RELAY);
gArgs.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), false, OptionsCategory::NODE_RELAY); gArgs.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), false, OptionsCategory::NODE_RELAY);
gArgs.AddArg("-mempoolreplacement", strprintf("Enable transaction replacement in the memory pool (default: %u)", DEFAULT_ENABLE_REPLACEMENT), false, OptionsCategory::NODE_RELAY);
gArgs.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", gArgs.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)",
CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), false, OptionsCategory::NODE_RELAY); CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), false, OptionsCategory::NODE_RELAY);
gArgs.AddArg("-whitelistforcerelay", strprintf("Force relay of transactions from whitelisted peers even if the transactions were already in the mempool or violate local relay policy (default: %d)", DEFAULT_WHITELISTFORCERELAY), false, OptionsCategory::NODE_RELAY); gArgs.AddArg("-whitelistforcerelay", strprintf("Force relay of transactions from whitelisted peers even if the transactions were already in the mempool or violate local relay policy (default: %d)", DEFAULT_WHITELISTFORCERELAY), false, OptionsCategory::NODE_RELAY);
@ -1176,15 +1175,6 @@ bool AppInitParameterInteraction()
nMaxTipAge = gArgs.GetArg("-maxtipage", DEFAULT_MAX_TIP_AGE); nMaxTipAge = gArgs.GetArg("-maxtipage", DEFAULT_MAX_TIP_AGE);
fEnableReplacement = gArgs.GetBoolArg("-mempoolreplacement", DEFAULT_ENABLE_REPLACEMENT);
if ((!fEnableReplacement) && gArgs.IsArgSet("-mempoolreplacement")) {
// Minimal effort at forwards compatibility
std::string strReplacementModeList = gArgs.GetArg("-mempoolreplacement", ""); // default is impossible
std::vector<std::string> vstrReplacementModes;
boost::split(vstrReplacementModes, strReplacementModeList, boost::is_any_of(","));
fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "fee") != vstrReplacementModes.end());
}
return true; return true;
} }

View File

@ -111,7 +111,6 @@ bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED;
size_t nCoinCacheUsage = 5000 * 300; size_t nCoinCacheUsage = 5000 * 300;
uint64_t nPruneTarget = 0; uint64_t nPruneTarget = 0;
int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE;
bool fEnableReplacement = DEFAULT_ENABLE_REPLACEMENT;
uint256 hashAssumeValid; uint256 hashAssumeValid;
arith_uint256 nMinimumChainWork; arith_uint256 nMinimumChainWork;
@ -486,15 +485,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// unconfirmed ancestors anyway; doing otherwise is hopelessly // unconfirmed ancestors anyway; doing otherwise is hopelessly
// insecure. // insecure.
bool fReplacementOptOut = true; bool fReplacementOptOut = true;
if (fEnableReplacement) for (const CTxIn &_txin : ptxConflicting->vin)
{ {
for (const CTxIn &_txin : ptxConflicting->vin) if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE)
{ {
if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE) fReplacementOptOut = false;
{ break;
fReplacementOptOut = false;
break;
}
} }
} }
if (fReplacementOptOut) { if (fReplacementOptOut) {

View File

@ -116,8 +116,6 @@ static const char* const DEFAULT_BLOCKFILTERINDEX = "0";
static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100; static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100;
/** Default for -persistmempool */ /** Default for -persistmempool */
static const bool DEFAULT_PERSIST_MEMPOOL = true; static const bool DEFAULT_PERSIST_MEMPOOL = true;
/** Default for -mempoolreplacement */
static const bool DEFAULT_ENABLE_REPLACEMENT = true;
/** Default for using fee filter */ /** Default for using fee filter */
static const bool DEFAULT_FEEFILTER = true; static const bool DEFAULT_FEEFILTER = true;
@ -160,7 +158,6 @@ extern size_t nCoinCacheUsage;
extern CFeeRate minRelayTxFee; extern CFeeRate minRelayTxFee;
/** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */
extern int64_t nMaxTipAge; extern int64_t nMaxTipAge;
extern bool fEnableReplacement;
/** Block hash whose ancestors we will assume to have valid scripts without checking them. */ /** Block hash whose ancestors we will assume to have valid scripts without checking them. */
extern uint256 hashAssumeValid; extern uint256 hashAssumeValid;

View File

@ -64,7 +64,7 @@ def make_utxo(node, amount, confirmed=True, scriptPubKey=CScript([1])):
class ReplaceByFeeTest(BitcoinTestFramework): class ReplaceByFeeTest(BitcoinTestFramework):
def set_test_params(self): def set_test_params(self):
self.num_nodes = 2 self.num_nodes = 1
self.extra_args = [ self.extra_args = [
[ [
"-maxorphantx=1000", "-maxorphantx=1000",
@ -74,9 +74,6 @@ class ReplaceByFeeTest(BitcoinTestFramework):
"-limitdescendantcount=200", "-limitdescendantcount=200",
"-limitdescendantsize=101", "-limitdescendantsize=101",
], ],
[
"-mempoolreplacement=0",
],
] ]
def skip_test_if_missing_module(self): def skip_test_if_missing_module(self):
@ -148,16 +145,12 @@ class ReplaceByFeeTest(BitcoinTestFramework):
# 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, tx1b_hex, 0) 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[1].sendrawtransaction, tx1b_hex, 0)
# Extra 0.1 BTC fee # Extra 0.1 BTC fee
tx1b = CTransaction() tx1b = CTransaction()
tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)]
tx1b.vout = [CTxOut(int(0.9 * COIN), CScript([b'b' * 35]))] tx1b.vout = [CTxOut(int(0.9 * COIN), CScript([b'b' * 35]))]
tx1b_hex = txToHex(tx1b) tx1b_hex = txToHex(tx1b)
# Replacement still disabled even with "enough fee"
assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[1].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)
@ -168,11 +161,6 @@ class ReplaceByFeeTest(BitcoinTestFramework):
assert_equal(tx1b_hex, self.nodes[0].getrawtransaction(tx1b_txid)) assert_equal(tx1b_hex, self.nodes[0].getrawtransaction(tx1b_txid))
# Second node is running mempoolreplacement=0, will not replace originally-seen txn
mempool = self.nodes[1].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"""