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 @@ -106,7 +106,8 @@ // evaluating any script flag that is implemented as an upgraded NOP code. static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, - uint32_t required_flags, bool add_to_cache) + uint32_t required_flags, bool add_to_cache, + int expected_sigchecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { PrecomputedTransactionData txdata(tx); @@ -122,29 +123,42 @@ test_flags |= SCRIPT_VERIFY_P2SH; } + int nSigChecksDirect = 0xf00d; bool ret = CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, - true, add_to_cache, txdata, nullptr); + true, add_to_cache, txdata, nSigChecksDirect); // CheckInputs should succeed iff test_flags doesn't intersect with // failing_flags bool expected_return_value = !(test_flags & failing_flags); BOOST_CHECK_EQUAL(ret, expected_return_value); + if (ret) { + if (test_flags & SCRIPT_REPORT_SIGCHECKS) { + BOOST_CHECK(nSigChecksDirect == expected_sigchecks); + } else { + BOOST_CHECK(nSigChecksDirect == 0); + } + } + // Test the caching if (ret && add_to_cache) { // Check that we get a cache hit if the tx was valid std::vector scriptchecks; + int nSigChecksCached = 0xbeef; BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, true, add_to_cache, txdata, - &scriptchecks)); + nSigChecksCached, &scriptchecks)); + BOOST_CHECK(nSigChecksCached == nSigChecksDirect); BOOST_CHECK(scriptchecks.empty()); } else { // Check that we get script executions to check, if the transaction // was invalid, or we didn't add to cache. std::vector scriptchecks; + int nSigChecksUncached = 0xbabe; BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, true, add_to_cache, txdata, - &scriptchecks)); + nSigChecksUncached, &scriptchecks)); + BOOST_CHECK(!ret || nSigChecksUncached == 0); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); } } @@ -240,10 +254,11 @@ CValidationState state; PrecomputedTransactionData ptd_spend_tx(tx); + int nSigChecksDummy; BOOST_CHECK(!CheckInputs(tx, state, pcoinsTip.get(), true, STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - ptd_spend_tx, nullptr)); + ptd_spend_tx, nSigChecksDummy, nullptr)); // If we call again asking for scriptchecks (as happens in // ConnectBlock), we should add a script check object for this -- we're @@ -251,7 +266,7 @@ std::vector scriptchecks; BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - ptd_spend_tx, &scriptchecks)); + ptd_spend_tx, nSigChecksDummy, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); // Test that CheckInputs returns true iff cleanstack-enforcing flags are @@ -259,7 +274,7 @@ // later that block validation works fine in the absence of cached // successes. ValidateCheckInputsForAllFlags( - tx, SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS, 0, false); + tx, SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS, 0, false, 0); } // And if we produce a block with this tx, it should be valid, even though @@ -286,7 +301,7 @@ invalid_under_p2sh_tx.vin[0].scriptSig << vchSig2; ValidateCheckInputsForAllFlags(CTransaction(invalid_under_p2sh_tx), - SCRIPT_VERIFY_P2SH, 0, true); + SCRIPT_VERIFY_P2SH, 0, true, 0); } // Test CHECKLOCKTIMEVERIFY @@ -313,7 +328,7 @@ ValidateCheckInputsForAllFlags(CTransaction(invalid_with_cltv_tx), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | SCRIPT_ENABLE_REPLAY_PROTECTION, - SCRIPT_ENABLE_SIGHASH_FORKID, true); + SCRIPT_ENABLE_SIGHASH_FORKID, true, 1); // Make it valid, and check again invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100; @@ -322,9 +337,12 @@ CTransaction transaction(invalid_with_cltv_tx); PrecomputedTransactionData txdata(transaction); - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nullptr)); + int nSigChecksRet; + BOOST_CHECK( + CheckInputs(transaction, state, pcoinsTip.get(), true, + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, + true, true, txdata, nSigChecksRet, nullptr)); + BOOST_CHECK(nSigChecksRet == 1); } // TEST CHECKSEQUENCEVERIFY @@ -350,7 +368,7 @@ ValidateCheckInputsForAllFlags(CTransaction(invalid_with_csv_tx), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_ENABLE_REPLAY_PROTECTION, - SCRIPT_ENABLE_SIGHASH_FORKID, true); + SCRIPT_ENABLE_SIGHASH_FORKID, true, 1); // Make it valid, and check again invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; @@ -359,9 +377,12 @@ CTransaction transaction(invalid_with_csv_tx); PrecomputedTransactionData txdata(transaction); - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nullptr)); + int nSigChecksRet; + BOOST_CHECK( + CheckInputs(transaction, state, pcoinsTip.get(), true, + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, + true, true, txdata, nSigChecksRet, nullptr)); + BOOST_CHECK(nSigChecksRet == 1); } // TODO: add tests for remaining script flags @@ -402,7 +423,7 @@ // convention. ValidateCheckInputsForAllFlags( CTransaction(tx), SCRIPT_ENABLE_REPLAY_PROTECTION, - SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_VERIFY_P2SH, true); + SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_VERIFY_P2SH, true, 2); // Check that if the second input is invalid, but the first input is // valid, the transaction is not cached. @@ -415,10 +436,11 @@ // This transaction is now invalid because the second signature is // missing. + int nSigChecksDummy; BOOST_CHECK( !CheckInputs(transaction, state, pcoinsTip.get(), true, STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, - true, true, txdata, nullptr)); + true, true, txdata, nSigChecksDummy, nullptr)); // Make sure this transaction was not cached (ie becausethe first input // was valid) @@ -426,7 +448,7 @@ BOOST_CHECK( CheckInputs(transaction, state, pcoinsTip.get(), true, STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS, - true, true, txdata, &scriptchecks)); + true, true, txdata, nSigChecksDummy, &scriptchecks)); // Should get 2 script checks back -- caching is on a whole-transaction // basis. BOOST_CHECK_EQUAL(scriptchecks.size(), 2U); diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -473,6 +473,12 @@ * performed inline. Any script checks which are not necessary (eg due to script * execution cache hits) are, obviously, not pushed onto pvChecks/run. * + * Upon success nSigChecksOut will be filled in with either: + * - correct total for all inputs, or, + * - 0, in the case when checks were pushed onto pvChecks (i.e., a cache miss + * with pvChecks non-null), in which case the total can be found by executing + * pvChecks and adding the results. + * * Setting sigCacheStore/scriptCacheStore to false will remove elements from the * corresponding cache which are matched. This is useful for checking blocks * where we will likely never need the cache entry again. @@ -481,7 +487,7 @@ const CCoinsViewCache &view, bool fScriptChecks, const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, + const PrecomputedTransactionData &txdata, int &nSigChecksOut, std::vector *pvChecks = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -495,7 +495,7 @@ static bool CheckInputsFromMempoolAndCache( const CTransaction &tx, CValidationState &state, const CCoinsViewCache &view, const CTxMemPool &pool, const uint32_t flags, - bool cacheSigStore, PrecomputedTransactionData &txdata) + bool cacheSigStore, PrecomputedTransactionData &txdata, int &nSigChecksOut) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -529,7 +529,7 @@ } return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, - txdata); + txdata, nSigChecksOut); } static bool @@ -756,8 +756,9 @@ // Check against previous transactions. This is done last to help // prevent CPU exhaustion denial-of-service attacks. PrecomputedTransactionData txdata(tx); + int nSigChecksStandard; if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, - txdata)) { + txdata, nSigChecksStandard)) { // State filled in by CheckInputs. return false; } @@ -773,9 +774,10 @@ // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. + int nSigChecksConsensus; if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, nextBlockScriptVerifyFlags, true, - txdata)) { + txdata, nSigChecksConsensus)) { // This can occur under some circumstances, if the node receives an // unrequested tx which is invalid due to new consensus rules not // being activated yet (during IBD). @@ -1205,7 +1207,7 @@ const CCoinsViewCache &inputs, bool fScriptChecks, const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, + const PrecomputedTransactionData &txdata, int &nSigChecksOut, std::vector *pvChecks) { AssertLockHeld(cs_main); assert(!tx.IsCoinBase()); @@ -1228,12 +1230,12 @@ // transaction hash which is in tx's prevouts properly commits to the // scriptPubKey in the inputs view of that transaction). ScriptCacheKey hashCacheEntry(tx, flags); - int nSigChecksDummy; - if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, - nSigChecksDummy)) { + if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, nSigChecksOut)) { return true; } + int nSigChecksTotal = 0; + for (size_t i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; const Coin &coin = inputs.AccessCoin(prevout); @@ -1288,12 +1290,16 @@ strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(scriptError))); } + + nSigChecksTotal += check.GetScriptExecutionMetrics().nSigChecks; } + nSigChecksOut = nSigChecksTotal; + if (scriptCacheStore && !pvChecks) { // We executed all of the provided scripts, and were told to cache the // result. Do so now. - AddKeyInScriptCache(hashCacheEntry, 0); + AddKeyInScriptCache(hashCacheEntry, nSigChecksTotal); } return true; @@ -1888,9 +1894,12 @@ bool fCacheResults = fJustCheck; std::vector vChecks; + // metricsRet may be accurate (found in cache) or 0 (checks were + // deferred into vChecks). + int nSigChecksRet; if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, fCacheResults, PrecomputedTransactionData(tx), - &vChecks)) { + nSigChecksRet, &vChecks)) { return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetId().ToString(), FormatStateMessage(state)); }