Merge #16171: Remove -mempoolreplacement to prevent needless block prop slowness.

8053e5cdad Remove -mempoolreplacement to prevent needless block prop slowness. (Matt Corallo)

Pull request description:

  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 removes an option that is:

  * (a) only useful when a large portion of (other) miners enforce it as well
  * (b) is detrimental to everyone (income for miners, RBF notifications for others) who uses it individually otherwise
  * (c) is effectively unused
  * (d) is often confused with disabling RBF (rather than just remaining stubbornly unaware of it while the rest of the network lets it through)

ACKs for commit 8053e5:
  practicalswift:
    utACK 8053e5cdade87550f0381d51feab81dedfec6c46
  promag:
    Deprecation would save from unlikely rantings, still ACK 8053e5c.
  jtimon:
    utACK 8053e5cdade87550f0381d51feab81dedfec6c46
  ajtowns:
    ACK 8053e5cdade87550f0381d51feab81dedfec6c46 -- quick code review, checked tests work
  MarcoFalke:
    ACK 8053e5cdade87550f0381d51feab81dedfec6c46

Tree-SHA512: 01aee8905b2487fc38a3a86649d422d2d2345bc60f878889ebda4b8680783e1f1a97c2000c27ef086719501be2abc2911b2039a259a5e5c04f3b24ff02b0427e

3 files changed, 4 insertions, 21 deletions

diff --git a/src/init.cpp b/src/init.cpp
index 93aa6f21b5..2a10f28a84 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -523,7 +523,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("-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("-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)",
         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);
@@ -1175,15 +1174,6 @@ bool AppInitParameterInteraction()
 
     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;
 }
 
diff --git a/src/validation.cpp b/src/validation.cpp
index b2925efe32..3026df34fe 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -111,7 +111,6 @@ bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED;
 size_t nCoinCacheUsage = 5000 * 300;
 uint64_t nPruneTarget = 0;
 int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE;
-bool fEnableReplacement = DEFAULT_ENABLE_REPLACEMENT;
 
 uint256 hashAssumeValid;
 arith_uint256 nMinimumChainWork;
@@ -486,15 +485,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
                 // unconfirmed ancestors anyway; doing otherwise is hopelessly
                 // insecure.
                 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) {
diff --git a/src/validation.h b/src/validation.h
index 963439d35b..638229952d 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -116,8 +116,6 @@ static const char* const DEFAULT_BLOCKFILTERINDEX = "0";
 static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100;
 /** Default for -persistmempool */
 static const bool DEFAULT_PERSIST_MEMPOOL = true;
-/** Default for -mempoolreplacement */
-static const bool DEFAULT_ENABLE_REPLACEMENT = true;
 /** Default for using fee filter */
 static const bool DEFAULT_FEEFILTER = true;
 
@@ -160,7 +158,6 @@ extern size_t nCoinCacheUsage;
 extern CFeeRate minRelayTxFee;
 /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */
 extern int64_t nMaxTipAge;
-extern bool fEnableReplacement;
 
 /** Block hash whose ancestors we will assume to have valid scripts without checking them. */
 extern uint256 hashAssumeValid;