validation: avoid redundant scriptsig validation in PolicyScriptChecks

This was performed to identify transactions whose witness was stripped and is not necessary anymore.
This commit is contained in:
Antoine Poinsot 2025-04-28 16:55:32 -04:00
parent 07018a44f8
commit f5f074eacc
2 changed files with 11 additions and 18 deletions

View file

@ -1235,21 +1235,7 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws)
// Check input scripts and signatures.
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
// SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
// need to turn both off, and compare against just turning off CLEANSTACK
// to see if the failure is specifically due to witness validation.
TxValidationState state_dummy; // Want reported failures to be from first CheckInputScripts
if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache()) &&
!CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
// Only the witness is missing, so the transaction itself may be fine.
state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
state.GetRejectReason(), state.GetDebugMessage());
}
return false; // state filled in by CheckInputScripts
}
return true;
return CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache());
}
bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws)

View file

@ -130,13 +130,16 @@ class OrphanHandlingTest(BitcoinTestFramework):
child = self.wallet.create_self_transfer(utxo_to_spend=parent['new_utxo'])
return child["tx"].getwtxid(), child["tx"], parent["tx"]
def relay_transaction(self, peer, tx):
def relay_transaction(self, peer, tx, with_ping=True):
"""Relay transaction using MSG_WTX"""
wtxid = int(tx.getwtxid(), 16)
peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=wtxid)]))
self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
peer.wait_for_getdata([wtxid])
if with_ping:
peer.send_and_ping(msg_tx(tx))
else:
peer.send_without_ping(msg_tx(tx))
def create_malleated_version(self, tx):
"""
@ -250,10 +253,14 @@ class OrphanHandlingTest(BitcoinTestFramework):
child_invalid_witness = self.wallet.create_self_transfer(utxo_to_spend=parent_normal["new_utxo"])
# Relay the parent with witness stripped. It should not be accepted.
self.relay_transaction(peer1, parent1_witness_stripped)
self.relay_transaction(peer1, parent1_witness_stripped, with_ping=False)
assert_equal(parent_normal["txid"], parent1_witness_stripped.rehash())
assert parent1_witness_stripped.rehash() not in node.getrawmempool()
# Sending the transaction without witness is a consensus failure and will cause the peer to disconnect.
peer1.wait_for_disconnect()
peer1 = node.add_p2p_connection(PeerTxRelayer())
# Relay the child. It should not be accepted because it has missing inputs.
self.relay_transaction(peer2, child_invalid_witness["tx"])
assert child_invalid_witness["txid"] not in node.getrawmempool()