Add txids with non-standard inputs to reject filter

Our policy checks for non-standard inputs depend only on the non-witness
portion of a transaction: we look up the scriptPubKey of the input being
spent from our UTXO set (which is covered by the input txid), and the p2sh
checks only rely on the scriptSig portion of the input.

Consequently it's safe to add txids of transactions that fail these checks to
the reject filter, as the witness is irrelevant to the failure. This is helpful
for any situation where we might request the transaction again via txid (either
from txid-relay peers, or if we might fetch the transaction via txid due to
parent-fetching of orphans).

Further, in preparation for future witness versions being deployed on the
network, ensure that WITNESS_UNKNOWN transactions are rejected in
AreInputsStandard(), so that transactions spending v1 (or greater) witness
outputs will fall into this category of having their txid added to the reject
filter.

Github-Pull: #19620
Rebased-From: 7989901c7e
This commit is contained in:
Suhas Daftuar 2020-07-29 11:07:23 -04:00
parent 28a9df7d76
commit 2ea826cfc4
4 changed files with 26 additions and 6 deletions

View file

@ -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 ||

View file

@ -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<uint256>& 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) {

View file

@ -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;
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<std::vector<unsigned char> > stack;

View file

@ -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))