mirror of
https://github.com/ElementsProject/elements.git
synced 2026-08-17 13:07:54 +02:00
Merge bitcoin/bitcoin#33050: net, validation: don't punish peers for consensus-invalid txs
876dbdfb4702410dfd4037614dc9298a0c09c63e tests: drop expect_disconnect behaviour for tx relay (Anthony Towns)
b29ae9efdfeeff774e32ee433ce67d8ed8ecd49f validation: only check input scripts once (Anthony Towns)
266dd0e10d08c0bfde63205db15d6c210a021b90 net_processing: drop MaybePunishNodeForTx (Anthony Towns)
Pull request description:
Because we do not discourage nodes for transactions we consider non-standard, we don't get any DoS protection from this check in adversarial scenarios, so remove the check entirely both to simplify the code and reduce the risk of splitting the network due to changes in tx relay policy.
Then, because we no longer make use of the distinction between consensus and standardness failures during script validation, don't re-validate each script with only-consensus rules, reducing the cost to us of transactions that we won't relay.
ACKs for top commit:
achow101:
ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
darosior:
re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
sipa:
re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
glozow:
ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
Tree-SHA512: 8bb0395766dde54fc48f7077b80b88e35581aa6e3054d6d65735965147abefffa7348f0850bb3d46f6c2541fd384ecd40a00a57fa653adabff8a35582e2d1811
This commit is contained in:
parent
53dc403306
commit
4b301666ff
12 changed files with 57 additions and 123 deletions
|
|
@ -1793,21 +1793,15 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
|
|||
CCheck* check = new CScriptCheck(txdata.m_spent_outputs[i], tx, i, flags, cacheSigStore, &txdata);
|
||||
ScriptError serror = QueueCheck(pvChecks, check);
|
||||
if (serror != SCRIPT_ERR_OK) {
|
||||
// Tx failures never trigger disconnections/bans.
|
||||
// This is so that network splits aren't triggered
|
||||
// either due to non-consensus relay policies (such as
|
||||
// non-standard DER encodings or non-null dummy
|
||||
// arguments) or due to new consensus rules introduced in
|
||||
// soft forks.
|
||||
if (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) {
|
||||
// Check whether the failure was caused by a
|
||||
// non-mandatory script verification check, such as
|
||||
// non-standard DER encodings or non-null dummy
|
||||
// arguments; if so, ensure we return NOT_STANDARD
|
||||
// instead of CONSENSUS to avoid downstream users
|
||||
// splitting the network between upgraded and
|
||||
// non-upgraded nodes by banning CONSENSUS-failing
|
||||
// data providers.
|
||||
CScriptCheck check2(txdata.m_spent_outputs[i], tx, i,
|
||||
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata);
|
||||
if (check2()) {
|
||||
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(serror)));
|
||||
}
|
||||
}
|
||||
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("mempool-script-verify-flag-failed (%s)", ScriptErrorString(serror)));
|
||||
} else {
|
||||
// MANDATORY flag failures correspond to
|
||||
// TxValidationResult::TX_CONSENSUS. Because CONSENSUS
|
||||
// failures are the most serious case of validation
|
||||
|
|
@ -1817,7 +1811,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
|
|||
// support, to avoid splitting the network (but this
|
||||
// depends on the details of how net_processing handles
|
||||
// such errors).
|
||||
return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror)));
|
||||
return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue