Merge bitcoin/bitcoin#31097: validation: Improve input script check error reporting

86e2a6b749 [test] A non-standard transaction which is also consensus-invalid should return the consensus error (Antoine Poinsot)
f859ff8a4e [validation] Improve script check error reporting (dergoegge)

Pull request description:

  An input script might be invalid for multiple reasons. For example, it might fail both a standardness check and a consensus check, which can lead to a `mandatory-script-verify-flag-failed` error being reported that includes the script error string from the standardness failure (e.g. `mandatory-script-verify-flag-failed (Using OP_CODESEPARATOR in non-witness script)`), which is confusing.

ACKs for top commit:
  darosior:
    re-ACK 86e2a6b749
  ariard:
    Re-Code Review ACK 86e2a6b7
  instagibbs:
    ACK 86e2a6b749

Tree-SHA512: 053939107c0bcd6643e9006b2518ddc3a6de47d2c6c66af71a04e8af5cf9ec207f19e54583b7a056efd77571edf5fd4f36c31ebe80d1f0777219c756c055eb42
This commit is contained in:
merge-script
2024-10-21 14:58:44 +01:00
4 changed files with 24 additions and 2 deletions

View File

@@ -2179,6 +2179,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
if (pvChecks) {
pvChecks->emplace_back(std::move(check));
} else if (!check()) {
ScriptError error{check.GetScriptError()};
if (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) {
// Check whether the failure was caused by a
// non-mandatory script verification check, such as
@@ -2192,6 +2194,14 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
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(check.GetScriptError())));
// If the second check failed, it failed due to a mandatory script verification
// flag, but the first check might have failed on a non-mandatory script
// verification flag.
//
// Avoid reporting a mandatory script check failure with a non-mandatory error
// string by reporting the error from the second check.
error = check2.GetScriptError();
}
// MANDATORY flag failures correspond to
// TxValidationResult::TX_CONSENSUS. Because CONSENSUS
@@ -2202,7 +2212,7 @@ 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(check.GetScriptError())));
return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(error)));
}
}