Merge bitcoin#13527: policy: Remove promiscuousmempoolflags

faa2444 policy: Remove promiscuousmempoolflags (MarcoFalke)

Pull request description:

  It seems odd to clutter validation code with features that can only ever be used for testing (testnet or regtest). Removing that test-only code makes the mempool logic less painful to understand and easier to reason about when changed or refactored in the future.

Tree-SHA512: 3b897aa9604ac8d82ebe9573c6efd468c93ddaa08d378ebc902e247b7aa6c68fcde71e5b449c08f17a067146cdc66dc50a67ce06d07607c27e5189a49c3fba3f

Author: laanwj

src/validation.cpp

@@ -704,6 +704,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
         // If we aren't going to actually accept it but just were verifying it, we are fine already
         if(fDryRun) return true;
 
+        constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
+
         // Check against previous transactions
         // This is done last to help prevent CPU exhaustion denial-of-service attacks.
         PrecomputedTransactionData txdata(tx);
@@ -726,22 +728,9 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
         // invalid blocks (using TestBlockValidity), however allowing such
         // transactions into the mempool can be exploited as a DoS attack.
         unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(chainActive.Tip(), Params().GetConsensus());
-        if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata))
-        {
-            // If we're using promiscuousmempoolflags, we may hit this normally
-            // Check if current block has some flags that scriptVerifyFlags
-            // does not before printing an ominous warning
-            if (!(~scriptVerifyFlags & currentBlockScriptVerifyFlags)) {
-                return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against latest-block but not STANDARD flags %s, %s",
+        if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) {
+            return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s",
                     __func__, hash.ToString(), FormatStateMessage(state));
-            } else {
-                if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, false, txdata)) {
-                    return error("%s: ConnectInputs failed against MANDATORY but not STANDARD flags due to promiscuous mempool %s, %s",
-                        __func__, hash.ToString(), FormatStateMessage(state));
-                } else {
-                    LogPrintf("Warning: -promiscuousmempool flags set to not include currently enforced soft forks, this may break mining or otherwise cause instability!\n");
-                }
-            }
         }
 
         // This transaction should only count for fee estimation if the

test/functional/bip65-cltv-p2p.py

@@ -64,7 +64,7 @@ class BIP65Test(BitcoinTestFramework):
     def __init__(self):
         super().__init__()
         self.num_nodes = 1
-        self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1', '-dip3params=9000:9000']]
+        self.extra_args = [['-whitelist=127.0.0.1', '-dip3params=9000:9000']]
         self.setup_clean_chain = True
 
     def run_test(self):
@@ -125,12 +125,10 @@ def run_test(self):
         spendtx.rehash()
 
         # First we show that this tx is valid except for CLTV by getting it
-        # accepted to the mempool (which we can achieve with
-        # -promiscuousmempoolflags).
-        node0.send_and_ping(msg_tx(spendtx))
-        assert spendtx.hash in self.nodes[0].getrawmempool()
+        # rejected from the mempool for exactly that reason.
+        assert_raises_jsonrpc(-26, '64: non-mandatory-script-verify-flag (Negative locktime)', self.nodes[0].sendrawtransaction, bytes_to_hex_str(spendtx.serialize()), True)
 
-        # Now we verify that a block with this transaction is invalid.
+        # Now we verify that a block with this transaction is also invalid.
         block.vtx.append(spendtx)
         block.hashMerkleRoot = block.calc_merkle_root()
         block.solve()

test/functional/bipdersig-p2p.py

@@ -47,12 +47,13 @@ def create_transaction(node, coinbase, to_address, amount):
     tx.deserialize(BytesIO(hex_str_to_bytes(signresult['hex'])))
     return tx
 
+
 class BIP66Test(BitcoinTestFramework):
 
     def __init__(self):
         super().__init__()
         self.num_nodes = 1
-        self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1', '-dip3params=9000:9000']]
+        self.extra_args = [['-whitelist=127.0.0.1', '-dip3params=9000:9000']]
         self.setup_clean_chain = True
 
     def run_test(self):
@@ -114,12 +115,10 @@ def run_test(self):
         spendtx.rehash()
 
         # First we show that this tx is valid except for DERSIG by getting it
-        # accepted to the mempool (which we can achieve with
-        # -promiscuousmempoolflags).
-        node0.send_and_ping(msg_tx(spendtx))
-        assert spendtx.hash in self.nodes[0].getrawmempool()
+        # rejected from the mempool for exactly that reason.
+        assert_raises_jsonrpc(-26, '64: non-mandatory-script-verify-flag (Non-canonical DER signature)', self.nodes[0].sendrawtransaction, bytes_to_hex_str(spendtx.serialize()), True)
 
-        # Now we verify that a block with this transaction is invalid.
+        # Now we verify that a block with this transaction is also invalid.
         block.vtx.append(spendtx)
         block.hashMerkleRoot = block.calc_merkle_root()
         block.rehash()