diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1750,6 +1750,13 @@ if ((flags & SCRIPT_DISALLOW_SEGWIT_RECOVERY) == 0 && stack.empty() && pubKey2.IsWitnessProgram()) { // 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)) { + metrics.nSigChecks = 0; + } metricsOut = metrics; return set_success(serror); } @@ -1804,6 +1811,12 @@ } } + // 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); } diff --git a/src/script/script_flags.h b/src/script/script_flags.h --- a/src/script/script_flags.h +++ b/src/script/script_flags.h @@ -111,6 +111,12 @@ // Note: The Segwit Recovery feature is a (currently moot) exception to // VERIFY_INPUT_SIGCHECKS SCRIPT_VERIFY_INPUT_SIGCHECKS = (1U << 22), + + // A utility flag to decide whether VerifyScript should output the correct + // sigchecks value or to report zero. + // This has no effect on script success / failure, and will be removed + // after cleanup of the SigChecks upgrade. + SCRIPT_REPORT_SIGCHECKS = (1U << 31), }; #endif // BITCOIN_SCRIPT_SCRIPT_FLAGS_H diff --git a/src/test/sigcheckcount_tests.cpp b/src/test/sigcheckcount_tests.cpp --- a/src/test/sigcheckcount_tests.cpp +++ b/src/test/sigcheckcount_tests.cpp @@ -312,37 +312,47 @@ // make sure that verifyscript is correctly resetting and accumulating // sigchecks for the input. - ScriptExecutionMetrics metrics; +#define CHECK_VERIFYSCRIPT(scriptSig, scriptPubKey, flags, expected_sigchecks) \ + { \ + ScriptExecutionMetrics metricsRet; \ + metricsRet.nSigChecks = 12345 ^ 0; \ + BOOST_CHECK(VerifyScript(scriptSig, scriptPubKey, \ + flags & ~SCRIPT_REPORT_SIGCHECKS, \ + dummysigchecker, metricsRet)); \ + BOOST_CHECK_EQUAL(metricsRet.nSigChecks, 0); \ + metricsRet.nSigChecks = 12345 ^ expected_sigchecks; \ + BOOST_CHECK(VerifyScript(scriptSig, scriptPubKey, \ + flags | SCRIPT_REPORT_SIGCHECKS, \ + dummysigchecker, metricsRet)); \ + BOOST_CHECK_EQUAL(metricsRet.nSigChecks, expected_sigchecks); \ + } + + // Simplest example + CHECK_VERIFYSCRIPT(CScript() << OP_1, CScript(), SCRIPT_VERIFY_NONE, 0); - // regardless of the initial value, it gets zeroed out - metrics.nSigChecks = 12345; - BOOST_CHECK(VerifyScript(CScript() << OP_1, CScript(), 0, dummysigchecker, - metrics)); - BOOST_CHECK_EQUAL(metrics.nSigChecks, 0); + // Common example + CHECK_VERIFYSCRIPT(CScript() << sigschnorr, CScript() << pub << OP_CHECKSIG, + SCRIPT_VERIFY_NONE, 1); - // it even gets zeroed out for segwit recovery special case (which returns - // true from an alternative location) - metrics.nSigChecks = 12345; + // Correct behaviour occurs for segwit recovery special case (which returns + // success from an alternative location) CScript swscript; swscript << OP_0 << std::vector(20); - BOOST_CHECK(VerifyScript(CScript() << ToByteVector(swscript), - CScript() << OP_HASH160 - << ToByteVector(CScriptID(swscript)) - << OP_EQUAL, - SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_CLEANSTACK, - dummysigchecker, metrics)); - BOOST_CHECK_EQUAL(metrics.nSigChecks, 0); + CHECK_VERIFYSCRIPT(CScript() << ToByteVector(swscript), + CScript() + << OP_HASH160 << ToByteVector(CScriptID(swscript)) + << OP_EQUAL, + SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_CLEANSTACK, 0); // If signature checks somehow occur in scriptsig, they do get counted. // This can happen in historical blocks pre SIGPUSHONLY, even with CHECKSIG. // (an analogous check for P2SH is not possible since it enforces // sigpushonly). - BOOST_CHECK(VerifyScript(CScript() << sigschnorr << msg << pub - << OP_CHECKDATASIG /* scriptSig */, - CScript() << sigecdsa << msg << pub - << OP_CHECKDATASIG /* scriptPubKey */, - 0, dummysigchecker, metrics)); - BOOST_CHECK_EQUAL(metrics.nSigChecks, 2); + CHECK_VERIFYSCRIPT(CScript() << sigschnorr << msg << pub + << OP_CHECKDATASIG /* scriptSig */, + CScript() << sigecdsa << msg << pub + << OP_CHECKDATASIG /* scriptPubKey */, + SCRIPT_VERIFY_NONE, 2); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -415,16 +415,18 @@ // This transaction is now invalid because the second signature is // missing. - BOOST_CHECK(!CheckInputs(transaction, state, pcoinsTip.get(), true, - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nullptr)); + BOOST_CHECK( + !CheckInputs(transaction, state, pcoinsTip.get(), true, + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, + true, true, txdata, nullptr)); // Make sure this transaction was not cached (ie becausethe first input // was valid) std::vector scriptchecks; - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, &scriptchecks)); + BOOST_CHECK( + CheckInputs(transaction, state, pcoinsTip.get(), true, + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, + true, true, txdata, &scriptchecks)); // Should get 2 script checks back -- caching is on a whole-transaction // basis. BOOST_CHECK_EQUAL(scriptchecks.size(), 2U);