From faa24441ec047ec336b86f586016b9d318c1c0ad Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 23 Jun 2018 16:16:54 -0400 Subject: [PATCH] policy: Remove promiscuousmempoolflags --- src/miner.cpp | 7 +++++-- src/validation.cpp | 19 ++----------------- test/functional/feature_cltv.py | 13 +++++++------ test/functional/feature_dersig.py | 14 ++++++++------ test/functional/feature_segwit.py | 18 +++++++----------- 5 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index d4527a1d6..db618f88f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -133,8 +133,11 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc // Decide whether to include witness transactions // This is only needed in case the witness softfork activation is reverted - // (which would require a very deep reorganization) or when - // -promiscuousmempoolflags is used. + // (which would require a very deep reorganization). + // Note that the mempool would accept transactions with witness data before + // IsWitnessEnabled, but we would only ever mine blocks after IsWitnessEnabled + // unless there is a massive block reorganization with the witness softfork + // not activated. // TODO: replace this with a call to main to assess validity of a mempool // transaction (which in most cases can be a no-op). fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx; diff --git a/src/validation.cpp b/src/validation.cpp index baf083b15..7312be3ab 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -897,10 +897,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } - unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; - if (!chainparams.RequireStandard()) { - scriptVerifyFlags = gArgs.GetArg("-promiscuousmempoolflags", scriptVerifyFlags); - } + 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. @@ -935,20 +932,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // 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", + 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"); - } - } } if (test_accept) { diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index e9a8945e7..b0ea4ae6f 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -62,7 +62,7 @@ def create_transaction(node, coinbase, to_address, amount): class BIP65Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1']] + self.extra_args = [['-whitelist=127.0.0.1']] self.setup_clean_chain = True def run_test(self): @@ -120,12 +120,13 @@ class BIP65Test(BitcoinTestFramework): 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). - self.nodes[0].p2p.send_and_ping(msg_tx(spendtx)) - assert spendtx.hash in self.nodes[0].getrawmempool() + # rejected from the mempool for exactly that reason. + assert_equal( + [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Negative locktime)'}], + self.nodes[0].testmempoolaccept(rawtxs=[bytes_to_hex_str(spendtx.serialize())], allowhighfees=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() diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 02dcc3e55..d5a5ea688 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -47,10 +47,11 @@ def create_transaction(node, coinbase, to_address, amount): tx.deserialize(BytesIO(hex_str_to_bytes(signresult['hex']))) return tx + class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-promiscuousmempoolflags=1', '-whitelist=127.0.0.1']] + self.extra_args = [['-whitelist=127.0.0.1']] self.setup_clean_chain = True def run_test(self): @@ -110,12 +111,13 @@ class BIP66Test(BitcoinTestFramework): 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). - self.nodes[0].p2p.send_and_ping(msg_tx(spendtx)) - assert spendtx.hash in self.nodes[0].getrawmempool() + # rejected from the mempool for exactly that reason. + assert_equal( + [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Non-canonical DER signature)'}], + self.nodes[0].testmempoolaccept(rawtxs=[bytes_to_hex_str(spendtx.serialize())], allowhighfees=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() diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index b10306b28..99ad2d522 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -43,8 +43,8 @@ class SegWitTest(BitcoinTestFramework): self.num_nodes = 3 # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. self.extra_args = [["-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"], - ["-blockversion=4", "-promiscuousmempoolflags=517", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"], - ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]] + ["-blockversion=4", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"], + ["-blockversion=536870915", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]] def setup_network(self): super().setup_network() @@ -64,12 +64,8 @@ class SegWitTest(BitcoinTestFramework): sync_blocks(self.nodes) def fail_accept(self, node, error_msg, txid, sign, redeem_script=""): - assert_raises_rpc_error(-26, error_msg, send_to_witness, 1, node, getutxo(txid), self.pubkey[0], False, Decimal("49.998"), sign, redeem_script) + assert_raises_rpc_error(-26, error_msg, send_to_witness, use_p2wsh=1, node=node, utxo=getutxo(txid), pubkey=self.pubkey[0], encode_p2sh=False, amount=Decimal("49.998"), sign=sign, insert_redeem_script=redeem_script) - def fail_mine(self, node, txid, sign, redeem_script=""): - send_to_witness(1, node, getutxo(txid), self.pubkey[0], False, Decimal("49.998"), sign, redeem_script) - assert_raises_rpc_error(-1, "CreateNewBlock: TestBlockValidity failed", node.generate, 1) - sync_blocks(self.nodes) def run_test(self): self.nodes[0].generate(161) #block 161 @@ -171,10 +167,10 @@ class SegWitTest(BitcoinTestFramework): assert(self.nodes[0].getrawtransaction(segwit_tx_list[i]) == bytes_to_hex_str(tx.serialize_without_witness())) self.log.info("Verify witness txs without witness data are invalid after the fork") - self.fail_mine(self.nodes[2], wit_ids[NODE_2][WIT_V0][2], False) - self.fail_mine(self.nodes[2], wit_ids[NODE_2][WIT_V1][2], False) - self.fail_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V0][2], False, witness_script(False, self.pubkey[2])) - self.fail_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V1][2], False, witness_script(True, self.pubkey[2])) + self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)', wit_ids[NODE_2][WIT_V0][2], sign=False) + self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)', wit_ids[NODE_2][WIT_V1][2], sign=False) + self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)', p2sh_ids[NODE_2][WIT_V0][2], sign=False, redeem_script=witness_script(False, self.pubkey[2])) + self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)', p2sh_ids[NODE_2][WIT_V1][2], sign=False, redeem_script=witness_script(True, self.pubkey[2])) self.log.info("Verify default node can now use witness txs") self.success_mine(self.nodes[0], wit_ids[NODE_0][WIT_V0][0], True) #block 432