Merge #13527: policy: Remove promiscuousmempoolflags
faa24441ec
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
This commit is contained in:
commit
e7ea858729
5 changed files with 29 additions and 42 deletions
|
@ -133,8 +133,11 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
|
||||||
|
|
||||||
// Decide whether to include witness transactions
|
// Decide whether to include witness transactions
|
||||||
// This is only needed in case the witness softfork activation is reverted
|
// This is only needed in case the witness softfork activation is reverted
|
||||||
// (which would require a very deep reorganization) or when
|
// (which would require a very deep reorganization).
|
||||||
// -promiscuousmempoolflags is used.
|
// 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
|
// TODO: replace this with a call to main to assess validity of a mempool
|
||||||
// transaction (which in most cases can be a no-op).
|
// transaction (which in most cases can be a no-op).
|
||||||
fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx;
|
fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx;
|
||||||
|
|
|
@ -894,10 +894,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
|
constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
|
||||||
if (!chainparams.RequireStandard()) {
|
|
||||||
scriptVerifyFlags = gArgs.GetArg("-promiscuousmempoolflags", scriptVerifyFlags);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check against previous transactions
|
// Check against previous transactions
|
||||||
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
|
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
|
||||||
|
@ -932,20 +929,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
|
||||||
// transactions into the mempool can be exploited as a DoS attack.
|
// transactions into the mempool can be exploited as a DoS attack.
|
||||||
unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(chainActive.Tip(), Params().GetConsensus());
|
unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(chainActive.Tip(), Params().GetConsensus());
|
||||||
if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) {
|
if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) {
|
||||||
// If we're using promiscuousmempoolflags, we may hit this normally
|
return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s",
|
||||||
// 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",
|
|
||||||
__func__, hash.ToString(), FormatStateMessage(state));
|
__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) {
|
if (test_accept) {
|
||||||
|
|
|
@ -62,7 +62,7 @@ def create_transaction(node, coinbase, to_address, amount):
|
||||||
class BIP65Test(BitcoinTestFramework):
|
class BIP65Test(BitcoinTestFramework):
|
||||||
def set_test_params(self):
|
def set_test_params(self):
|
||||||
self.num_nodes = 1
|
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
|
self.setup_clean_chain = True
|
||||||
|
|
||||||
def run_test(self):
|
def run_test(self):
|
||||||
|
@ -116,12 +116,13 @@ class BIP65Test(BitcoinTestFramework):
|
||||||
spendtx.rehash()
|
spendtx.rehash()
|
||||||
|
|
||||||
# First we show that this tx is valid except for CLTV by getting it
|
# First we show that this tx is valid except for CLTV by getting it
|
||||||
# accepted to the mempool (which we can achieve with
|
# rejected from the mempool for exactly that reason.
|
||||||
# -promiscuousmempoolflags).
|
assert_equal(
|
||||||
self.nodes[0].p2p.send_and_ping(msg_tx(spendtx))
|
[{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Negative locktime)'}],
|
||||||
assert spendtx.hash in self.nodes[0].getrawmempool()
|
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.vtx.append(spendtx)
|
||||||
block.hashMerkleRoot = block.calc_merkle_root()
|
block.hashMerkleRoot = block.calc_merkle_root()
|
||||||
block.solve()
|
block.solve()
|
||||||
|
|
|
@ -47,10 +47,11 @@ def create_transaction(node, coinbase, to_address, amount):
|
||||||
tx.deserialize(BytesIO(hex_str_to_bytes(signresult['hex'])))
|
tx.deserialize(BytesIO(hex_str_to_bytes(signresult['hex'])))
|
||||||
return tx
|
return tx
|
||||||
|
|
||||||
|
|
||||||
class BIP66Test(BitcoinTestFramework):
|
class BIP66Test(BitcoinTestFramework):
|
||||||
def set_test_params(self):
|
def set_test_params(self):
|
||||||
self.num_nodes = 1
|
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
|
self.setup_clean_chain = True
|
||||||
|
|
||||||
def run_test(self):
|
def run_test(self):
|
||||||
|
@ -108,12 +109,13 @@ class BIP66Test(BitcoinTestFramework):
|
||||||
spendtx.rehash()
|
spendtx.rehash()
|
||||||
|
|
||||||
# First we show that this tx is valid except for DERSIG by getting it
|
# First we show that this tx is valid except for DERSIG by getting it
|
||||||
# accepted to the mempool (which we can achieve with
|
# rejected from the mempool for exactly that reason.
|
||||||
# -promiscuousmempoolflags).
|
assert_equal(
|
||||||
self.nodes[0].p2p.send_and_ping(msg_tx(spendtx))
|
[{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Non-canonical DER signature)'}],
|
||||||
assert spendtx.hash in self.nodes[0].getrawmempool()
|
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.vtx.append(spendtx)
|
||||||
block.hashMerkleRoot = block.calc_merkle_root()
|
block.hashMerkleRoot = block.calc_merkle_root()
|
||||||
block.rehash()
|
block.rehash()
|
||||||
|
|
|
@ -43,8 +43,8 @@ class SegWitTest(BitcoinTestFramework):
|
||||||
self.num_nodes = 3
|
self.num_nodes = 3
|
||||||
# This test tests SegWit both pre and post-activation, so use the normal BIP9 activation.
|
# 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"],
|
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=4", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"],
|
||||||
["-blockversion=536870915", "-promiscuousmempoolflags=517", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]]
|
["-blockversion=536870915", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]]
|
||||||
|
|
||||||
def setup_network(self):
|
def setup_network(self):
|
||||||
super().setup_network()
|
super().setup_network()
|
||||||
|
@ -64,12 +64,8 @@ class SegWitTest(BitcoinTestFramework):
|
||||||
sync_blocks(self.nodes)
|
sync_blocks(self.nodes)
|
||||||
|
|
||||||
def fail_accept(self, node, error_msg, txid, sign, redeem_script=""):
|
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):
|
def run_test(self):
|
||||||
self.nodes[0].generate(161) #block 161
|
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()))
|
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.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_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_mine(self.nodes[2], wit_ids[NODE_2][WIT_V1][2], 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_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V0][2], False, witness_script(False, self.pubkey[2]))
|
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_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 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.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
|
self.success_mine(self.nodes[0], wit_ids[NODE_0][WIT_V0][0], True) #block 432
|
||||||
|
|
Loading…
Add table
Reference in a new issue