diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 2e23f4b3a4f..dbef958e194 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -48,7 +48,8 @@ enum class ValidationInvalidReason { BLOCK_TIME_FUTURE, //!< block timestamp was > 2 hours in the future (or our clock is bad) BLOCK_CHECKPOINT, //!< the block failed to meet one of our checkpoints // Only loose txn: - TX_NOT_STANDARD, //!< didn't meet our local policy rules + TX_INPUTS_NOT_STANDARD, //!< inputs (covered by txid) failed policy rules + TX_NOT_STANDARD, //!< otherwise didn't meet our local policy rules TX_MISSING_INPUTS, //!< a transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** @@ -72,6 +73,7 @@ inline bool IsTransactionReason(ValidationInvalidReason r) return r == ValidationInvalidReason::NONE || r == ValidationInvalidReason::CONSENSUS || r == ValidationInvalidReason::RECENT_CONSENSUS_CHANGE || + r == ValidationInvalidReason::TX_INPUTS_NOT_STANDARD || r == ValidationInvalidReason::TX_NOT_STANDARD || r == ValidationInvalidReason::TX_PREMATURE_SPEND || r == ValidationInvalidReason::TX_MISSING_INPUTS || diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3d0efa041df..4a183f0c9ac 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1055,6 +1055,7 @@ static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool v return true; case ValidationInvalidReason::RECENT_CONSENSUS_CHANGE: case ValidationInvalidReason::BLOCK_TIME_FUTURE: + case ValidationInvalidReason::TX_INPUTS_NOT_STANDARD: case ValidationInvalidReason::TX_NOT_STANDARD: case ValidationInvalidReason::TX_MISSING_INPUTS: case ValidationInvalidReason::TX_PREMATURE_SPEND: @@ -1846,10 +1847,15 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se // Probably non-standard or insufficient fee LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString()); assert(IsTransactionReason(orphan_state.GetReason())); - if (!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) { + if ((!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) || + orphan_state.GetReason() == ValidationInvalidReason::TX_INPUTS_NOT_STANDARD) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + // However, if the transaction failed for TX_INPUTS_NOT_STANDARD, + // then we know that the witness was irrelevant to the policy + // failure, since this check depends only on the txid + // (the scriptPubKey being spent is covered by the txid). assert(recentRejects); recentRejects->insert(orphanHash); } @@ -2574,10 +2580,15 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } } else { assert(IsTransactionReason(state.GetReason())); - if (!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) { + if ((!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) || + state.GetReason() == ValidationInvalidReason::TX_INPUTS_NOT_STANDARD) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + // However, if the transaction failed for TX_INPUTS_NOT_STANDARD, + // then we know that the witness was irrelevant to the policy + // failure, since this check depends only on the txid + // (the scriptPubKey being spent is covered by the txid). assert(recentRejects); recentRejects->insert(tx.GetHash()); if (RecursiveDynamicUsage(*ptx) < 100000) { diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 51de5841ec7..636a916602a 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -152,6 +152,8 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR * script can be anything; an attacker could use a very * expensive-to-check-upon-redemption script like: * DUP CHECKSIG DROP ... repeated 100 times... OP_1 + * + * Note that only the non-witness portion of the transaction is checked here. */ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) { @@ -164,7 +166,11 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) std::vector > vSolutions; txnouttype whichType = Solver(prev.scriptPubKey, vSolutions); - if (whichType == TX_NONSTANDARD) { + if (whichType == TX_NONSTANDARD || whichType == TX_WITNESS_UNKNOWN) { + // WITNESS_UNKNOWN failures are typically also caught with a policy + // flag in the script interpreter, but it can be helpful to catch + // this type of NONSTANDARD transaction earlier in transaction + // validation. return false; } else if (whichType == TX_SCRIPTHASH) { std::vector > stack; diff --git a/src/validation.cpp b/src/validation.cpp index ac98bd61c7a..bdf3c128cc9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -678,8 +678,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } // Check for non-standard pay-to-script-hash in inputs - if (fRequireStandard && !AreInputsStandard(tx, m_view)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + if (fRequireStandard && !AreInputsStandard(tx, m_view)) { + return state.Invalid(ValidationInvalidReason::TX_INPUTS_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + } // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view))