validation: detect witness stripping without re-running Script checks
What the developer wrote
Authored by Antoine Poinsot
validation: detect witness stripping without re-running Script checks
Since it was introduced in 4eb515574e1012bc8ea5dafc3042dcdf4c766f26 (#18044), the detection of a
stripped witness relies on running the Script checks 3 times. In the worst case, this consists in
running Script validation 3 times for every single input.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's
wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with
txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so
doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot,
P2WPKH, P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus).
For undefined program types, Script validation is always going to fail regardless of the witness (by
standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation.
However this might lead to more "false positives" (cases where we return witness stripping for an
otherwise invalid transaction) than the existing implementation. For instance a transaction with one
P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing
implementation would treat it as consensus invalid while the implementation in this commit would
always consider it witness stripped.
What changed, and why it matters
This commit changes how Bitcoin Core detects when a transaction's witness data has been stripped away. Previously, the software ran expensive script checks up to three times to figure out if a transaction failed only because its witness was missing. The new code detects the same condition with a cheaper check, but it may label some transactions as 'witness stripped' even when they are invalid for other reasons. The change is a performance and correctness refactor, not a fix for an active exploit, though the underlying behavior affects how the network handles invalid transactions.
Review the SpendsNonAnchorWitnessProg helper for correctness and ensure the new heuristic does not allow transactions to bypass intended rejection caching. Monitor for any edge cases involving mixed input types. No urgent deployment action is indicated; this is a defensive refactor.
Security signals we found
Removes repeated script validation to avoid CPU exhaustion DoS vector in mempool acceptance
Changes rejection classification from consensus-invalid to witness-stripped for some multi-input transactions
Affects reject filter behavior and 1p1c package relay orphan resolution
Introduces a heuristic that may produce false positives compared to prior implementation
Evidence from the diff
The patch removes the triple CheckInputScripts invocation used to detect witness stripping in MemPoolAccept::PolicyScriptChecks. Instead, after a script check failure, it checks whether the transaction has no witness and whether any input spends a non-anchor witness program (via SpendsNonAnchorWitnessProg). For defined witness programs, an empty witness always fails consensus; for undefined programs it always fails standardness; and for the pay-to-anchor carve-out an empty witness never causes failure. Therefore the new check is sufficient to detect stripped witnesses without re-running script validation. The trade-off is increased false positives: a transaction with both a non-witness invalid input and a stripped witness input will now be classified as TX_WITNESS_STRIPPED rather than consensus invalid.
Changed components
src/validation.cppMemPoolAccept::PolicyScriptChecksmempool transaction acceptancewitness stripping detectionP2P reject filter / 1p1c package relayInspect captured patch +2 / −7
diff --git a/src/validation.cpp b/src/validation.cpp
index 09e04ff0..078f88d1 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1254,13 +1254,8 @@ 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.
+ // Detect a failure due to a missing witness so that p2p code can handle rejection caching appropriately.
+ if (!tx.HasWitness() && SpendsNonAnchorWitnessProg(tx, m_view)) {
state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
state.GetRejectReason(), state.GetDebugMessage());
}
Why this scored 48/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.