diff options
| -rw-r--r-- | src/consensus/validation.h | 3 | ||||
| -rw-r--r-- | src/net_processing.cpp | 35 | ||||
| -rw-r--r-- | src/policy/policy.cpp | 8 | ||||
| -rw-r--r-- | src/validation.cpp | 5 |
4 files changed, 45 insertions, 6 deletions
diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 8de7a8f2d8..2a93a090d6 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -26,7 +26,8 @@ enum class TxValidationResult { * is uninteresting. */ TX_RECENT_CONSENSUS_CHANGE, - 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, //!< transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ca701a7e5b..ef7f486e0b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -187,7 +187,7 @@ namespace { * million to make it highly unlikely for users to have issues with this * filter. * - * We only need to add wtxids to this filter. For non-segwit + * We typically only add wtxids to this filter. For non-segwit * transactions, the txid == wtxid, so this only prevents us from * re-downloading non-segwit transactions when communicating with * non-wtxidrelay peers -- which is important for avoiding malleation @@ -196,6 +196,12 @@ namespace { * the reject filter store wtxids is exactly what we want to avoid * redownload of a rejected transaction. * + * In cases where we can tell that a segwit transaction will fail + * validation no matter the witness, we may add the txid of such + * transaction to the filter as well. This can be helpful when + * communicating with txid-relay peers or if we were to otherwise fetch a + * transaction via txid (eg in our orphan handling). + * * Memory used: 1.3 MB */ std::unique_ptr<CRollingBloomFilter> recentRejects GUARDED_BY(cs_main); @@ -1161,6 +1167,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, } // Conflicting (but not necessarily invalid) data or different policy: case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: + case TxValidationResult::TX_INPUTS_NOT_STANDARD: case TxValidationResult::TX_NOT_STANDARD: case TxValidationResult::TX_MISSING_INPUTS: case TxValidationResult::TX_PREMATURE_SPEND: @@ -2053,6 +2060,19 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin // if we start doing this too early. assert(recentRejects); recentRejects->insert(orphanTx.GetWitnessHash()); + // 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). + // Add the txid to the reject filter to prevent repeated + // processing of this transaction in the event that child + // transactions are later received (resulting in + // parent-fetching by txid via the orphan-handling logic). + if (orphan_state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && orphanTx.GetWitnessHash() != orphanTx.GetHash()) { + // We only add the txid if it differs from the wtxid, to + // avoid wasting entries in the rolling bloom filter. + recentRejects->insert(orphanTx.GetHash()); + } } EraseOrphanTx(orphanHash); done = true; @@ -2940,7 +2960,7 @@ void ProcessMessage( // We do the AlreadyHave() check using wtxid, rather than txid - in the // absence of witness malleation, this is strictly better, because the - // recent rejects filter may contain the wtxid but will never contain + // recent rejects filter may contain the wtxid but rarely contains // the txid of a segwit transaction that has been rejected. // In the presence of witness malleation, it's possible that by only // doing the check with wtxid, we could overlook a transaction which @@ -3034,6 +3054,17 @@ void ProcessMessage( // if we start doing this too early. assert(recentRejects); recentRejects->insert(tx.GetWitnessHash()); + // 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). + // Add the txid to the reject filter to prevent repeated + // processing of this transaction in the event that child + // transactions are later received (resulting in + // parent-fetching by txid via the orphan-handling logic). + if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.GetWitnessHash() != tx.GetHash()) { + recentRejects->insert(tx.GetHash()); + } if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index c56abaf6c9..0e9820da1e 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<std::vector<unsigned char> > vSolutions; TxoutType whichType = Solver(prev.scriptPubKey, vSolutions); - if (whichType == TxoutType::NONSTANDARD) { + if (whichType == TxoutType::NONSTANDARD || whichType == TxoutType::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 == TxoutType::SCRIPTHASH) { std::vector<std::vector<unsigned char> > stack; diff --git a/src/validation.cpp b/src/validation.cpp index 7efff8d4b4..f3e08d333b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -689,8 +689,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(TxValidationResult::TX_NOT_STANDARD, "bad-txns-nonstandard-inputs"); + if (fRequireStandard && !AreInputsStandard(tx, m_view)) { + return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); + } // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) |