Page MenuHomePhabricator
Differential D5070 Diff 15794 src/script/interpreter.cpp

Changeset View

Standalone View

View Options

src/script/interpreter.cpp

Show First 20 Lines • Show All 1,719 Lines • ▼ Show 20 Linesbool VerifyScript(const CScript &scriptSig, const CScript &scriptPubKey,
} }
if (stack.empty()) { if (stack.empty()) {
return set_error(serror, ScriptError::EVAL_FALSE); return set_error(serror, ScriptError::EVAL_FALSE);
} }
if (CastToBool(stack.back()) == false) { if (CastToBool(stack.back()) == false) {
return set_error(serror, ScriptError::EVAL_FALSE); return set_error(serror, ScriptError::EVAL_FALSE);
} }
bool segwit_exempt = false;
deadalnixUnsubmitted
Not Done
Inline Actions

Can you avoid the double negative problem? It's really not clear what segwit_exempt means here, but it is clearly defining a negative, and then you check !segwit_exempt all over the place. Double negative are notoriously confusing and an endless source of bugs.

It seems to me like this is true when the script is a segwit program, so why not is_segwit_program ?

deadalnix: Can you avoid the double negative problem? It's really not clear what `segwit_exempt` means…
// Additional validation for spend-to-script-hash transactions: // Additional validation for spend-to-script-hash transactions:
if ((flags & SCRIPT_VERIFY_P2SH) && scriptPubKey.IsPayToScriptHash()) { if ((flags & SCRIPT_VERIFY_P2SH) && scriptPubKey.IsPayToScriptHash()) {
// scriptSig must be literals-only or validation fails // scriptSig must be literals-only or validation fails
if (!scriptSig.IsPushOnly()) { if (!scriptSig.IsPushOnly()) {
return set_error(serror, ScriptError::SIG_PUSHONLY); return set_error(serror, ScriptError::SIG_PUSHONLY);
} }
// Restore stack. // Restore stack.
swap(stack, stackCopy); swap(stack, stackCopy);
// stack cannot be empty here, because if it was the P2SH HASH <> EQUAL // stack cannot be empty here, because if it was the P2SH HASH <> EQUAL
// scriptPubKey would be evaluated with an empty stack and the // scriptPubKey would be evaluated with an empty stack and the
// EvalScript above would return false. // EvalScript above would return false.
assert(!stack.empty()); assert(!stack.empty());
const valtype &pubKeySerialized = stack.back(); const valtype &pubKeySerialized = stack.back();
CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end()); CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end());
popstack(stack); popstack(stack);
// Bail out early if SCRIPT_DISALLOW_SEGWIT_RECOVERY is not set, the // If SCRIPT_DISALLOW_SEGWIT_RECOVERY is not set and redeem script is a
// redeem script is a p2sh segwit program, and it was the only item // p2sh segwit program, and it was the only item pushed onto the stack,
// pushed onto the stack. // then mark for special exemptions from regular script rules.
if ((flags & SCRIPT_DISALLOW_SEGWIT_RECOVERY) == 0 && stack.empty() && if ((flags & SCRIPT_DISALLOW_SEGWIT_RECOVERY) == 0 && stack.empty() &&
pubKey2.IsWitnessProgram()) { pubKey2.IsWitnessProgram()) {
// must set metricsOut for all successful returns segwit_exempt = true;
// Prior to activation of this flag, all transactions will count as
// having a sigchecks count of 0 for accounting purposes outside of
// VerifyScript.
if (!(flags & SCRIPT_REPORT_SIGCHECKS)) {
metrics.nSigChecks = 0;
}
metricsOut = metrics;
return set_success(serror);
} }
if (!segwit_exempt) {
if (!EvalScript(stack, pubKey2, flags, checker, metrics, serror)) { if (!EvalScript(stack, pubKey2, flags, checker, metrics, serror)) {
// serror is set // serror is set
return false; return false;
} }
if (stack.empty()) { if (stack.empty()) {
return set_error(serror, ScriptError::EVAL_FALSE); return set_error(serror, ScriptError::EVAL_FALSE);
} }
if (!CastToBool(stack.back())) { if (!CastToBool(stack.back())) {
return set_error(serror, ScriptError::EVAL_FALSE); return set_error(serror, ScriptError::EVAL_FALSE);
} }
} }
}
// The CLEANSTACK check is only performed after potential P2SH evaluation, // The CLEANSTACK check is only performed after potential P2SH evaluation,
// as the non-P2SH evaluation of a P2SH script will obviously not result in // as the non-P2SH evaluation of a P2SH script will obviously not result in
// a clean stack (the P2SH inputs remain). The same holds for witness // a clean stack (the P2SH inputs remain). The same holds for witness
// evaluation. // evaluation.
if ((flags & SCRIPT_VERIFY_CLEANSTACK) != 0) { if (!segwit_exempt && (flags & SCRIPT_VERIFY_CLEANSTACK)) {
// Disallow CLEANSTACK without P2SH, as otherwise a switch // Disallow CLEANSTACK without P2SH, as otherwise a switch
// CLEANSTACK->P2SH+CLEANSTACK would be possible, which is not a // CLEANSTACK->P2SH+CLEANSTACK would be possible, which is not a
// softfork (and P2SH should be one). // softfork (and P2SH should be one).
assert((flags & SCRIPT_VERIFY_P2SH) != 0); assert((flags & SCRIPT_VERIFY_P2SH) != 0);
if (stack.size() != 1) { if (stack.size() != 1) {
return set_error(serror, ScriptError::CLEANSTACK); return set_error(serror, ScriptError::CLEANSTACK);
} }
} }
if (flags & SCRIPT_VERIFY_INPUT_SIGCHECKS) { if (!segwit_exempt && (flags & SCRIPT_VERIFY_INPUT_SIGCHECKS)) {
// This limit is intended for standard use, and is based on an // This limit is intended for standard use, and is based on an
// examination of typical and historical standard uses. // examination of typical and historical standard uses.
// - allowing P2SH ECDSA multisig with compressed keys, which at an // - allowing P2SH ECDSA multisig with compressed keys, which at an
// extreme (1-of-15) may have 15 SigChecks in ~590 bytes of scriptSig. // extreme (1-of-15) may have 15 SigChecks in ~590 bytes of scriptSig.
// - allowing Bare ECDSA multisig, which at an extreme (1-of-3) may have // - allowing Bare ECDSA multisig, which at an extreme (1-of-3) may have
// 3 sigchecks in ~72 bytes of scriptSig. // 3 sigchecks in ~72 bytes of scriptSig.
// - Since the size of an input is 41 bytes + length of scriptSig, then // - Since the size of an input is 41 bytes + length of scriptSig, then
// the most dense possible inputs satisfying this rule would be: // the most dense possible inputs satisfying this rule would be:
Show All 24 Lines