Page MenuHomePhabricator
Differential D4940 Diff 15681 src/script/interpreter.cpp

Changeset View

Standalone View

View Options

src/script/interpreter.cpp

Show First 20 Lines • Show All 1,744 Lines • ▼ Show 20 Lines if ((flags & SCRIPT_VERIFY_P2SH) && scriptPubKey.IsPayToScriptHash()) {
popstack(stack); popstack(stack);
// Bail out early if SCRIPT_DISALLOW_SEGWIT_RECOVERY is not set, the // Bail out early if SCRIPT_DISALLOW_SEGWIT_RECOVERY is not set, the
// redeem script is a p2sh segwit program, and it was the only item // redeem script is a p2sh segwit program, and it was the only item
// pushed onto the stack. // pushed onto the stack.
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 // must set metricsOut for all successful returns
// 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)) {
deadalnixUnsubmitted
Not Done
Inline Actions

I would reverse the logic as follow:

1/ Set metricsOut as 0.
2/ Compute things using metrics.
3/ If the flag is set, metricsOut = metrics; before success.

This is more straightforward.

deadalnix: I would reverse the logic as follow: 1/ Set `metricsOut` as 0. 2/ Compute things using…
markblundebergAuthorUnsubmitted
Done
Inline Actions

If there were other things in metrics, I wouldn't be wanting to zero them all with this flag. It's the same for now yeah, but I think the current way is more clear.

markblundeberg: If there were other things in metrics, I wouldn't be wanting to zero them all with this flag.
deadalnixUnsubmitted
Not Done
Inline Actions

I won't block for this, but I still think the double negation is a bit confusing.

deadalnix: I won't block for this, but I still think the double negation is a bit confusing.
metrics.nSigChecks = 0;
}
metricsOut = metrics; metricsOut = metrics;
return set_success(serror); return set_success(serror);
} }
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;
} }
Show All 38 Lines if (flags & SCRIPT_VERIFY_INPUT_SIGCHECKS) {
static_assert(INT_MAX / 43 / 3 > MAX_OPS_PER_SCRIPT, static_assert(INT_MAX / 43 / 3 > MAX_OPS_PER_SCRIPT,
"overflow sanity check on maximum possible sigchecks " "overflow sanity check on maximum possible sigchecks "
"from sig+redeem+pub scripts"); "from sig+redeem+pub scripts");
if (int(scriptSig.size()) < metrics.nSigChecks * 43 - 60) { if (int(scriptSig.size()) < metrics.nSigChecks * 43 - 60) {
return set_error(serror, ScriptError::INPUT_SIGCHECKS); return set_error(serror, ScriptError::INPUT_SIGCHECKS);
} }
} }
// 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; metricsOut = metrics;
return set_success(serror); return set_success(serror);
} }