From 116419e58dddef8fe3ff9806a1d8ceebe64ae3e6 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 10 Jan 2017 16:53:10 -0500 Subject: [PATCH] Don't overwrite validation state with corruption check AcceptToMemoryPool has several classes of return false statements. - return state.Invalid or state.DoS directly itself - return false and set fMissingInputs (state is valid) - return false and state is set by failed CheckTransaction - return false and state is set by failed CheckInputs. This commit patches the last case where the state variable was reused for additional calls to CheckInputs to identify witness stripping as cause of validation failure. After this commit, it should be the case that if !fMissingInputs, state is always Invalid if AcceptToMemoryPool returns false. --- src/validation.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 37a4186e0..9c06a3d94 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -919,12 +919,13 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // 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. - if (!tx.HasWitness() && CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) && - !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) { + CValidationState stateDummy; // Want reported failures to be from first CheckInputs + if (!tx.HasWitness() && CheckInputs(tx, stateDummy, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) && + !CheckInputs(tx, stateDummy, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) { // Only the witness is missing, so the transaction itself may be fine. state.SetCorruptionPossible(); } - return false; + return false; // state filled in by CheckInputs } // Check again against just the consensus-critical mandatory script