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 @@ -102,8 +102,8 @@ static inline bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, bool fScriptChecks, - const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, + const CCoinsViewCache &view, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, std::vector *pvChecks, CheckInputsLimiter *pBlockLimitSigChecks = nullptr) @@ -112,9 +112,9 @@ // cases are using pvChecks, so the verification is done asynchronously. static TxSigCheckLimiter nSigChecksTxLimiter; nSigChecksTxLimiter = TxSigCheckLimiter(); - return CheckInputs(tx, state, view, fScriptChecks, flags, sigCacheStore, - scriptCacheStore, txdata, nSigChecksOut, - nSigChecksTxLimiter, pBlockLimitSigChecks, pvChecks); + return CheckInputs(tx, state, view, flags, sigCacheStore, scriptCacheStore, + txdata, nSigChecksOut, nSigChecksTxLimiter, + pBlockLimitSigChecks, pvChecks); } // Run CheckInputs (using pcoinsTip) on the given transaction, for all script @@ -148,8 +148,8 @@ } int nSigChecksDirect = 0xf00d; - bool ret = CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, - true, add_to_cache, txdata, nSigChecksDirect); + bool ret = CheckInputs(tx, state, pcoinsTip.get(), test_flags, true, + add_to_cache, txdata, nSigChecksDirect); // CheckInputs should succeed iff test_flags doesn't intersect with // failing_flags @@ -165,8 +165,8 @@ // 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, + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), test_flags, + true, add_to_cache, txdata, nSigChecksCached, &scriptchecks)); BOOST_CHECK(nSigChecksCached == nSigChecksDirect); BOOST_CHECK(scriptchecks.empty()); @@ -175,8 +175,8 @@ // 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, + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), test_flags, + true, add_to_cache, txdata, nSigChecksUncached, &scriptchecks)); BOOST_CHECK(!ret || nSigChecksUncached == 0); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); @@ -276,7 +276,7 @@ PrecomputedTransactionData ptd_spend_tx(tx); int nSigChecksDummy; - BOOST_CHECK(!CheckInputs(tx, state, pcoinsTip.get(), true, + BOOST_CHECK(!CheckInputs(tx, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS, true, true, ptd_spend_tx, nSigChecksDummy, nullptr)); @@ -284,7 +284,7 @@ // ConnectBlock), we should add a script check object for this -- we're // not caching invalidity (if that changes, delete this test case). std::vector scriptchecks; - BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, + BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS, true, true, ptd_spend_tx, nSigChecksDummy, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); @@ -358,7 +358,7 @@ PrecomputedTransactionData txdata(transaction); int nSigChecksRet; - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS, true, true, txdata, nSigChecksRet, nullptr)); BOOST_CHECK(nSigChecksRet == 1); @@ -397,7 +397,7 @@ PrecomputedTransactionData txdata(transaction); int nSigChecksRet; - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS, true, true, txdata, nSigChecksRet, nullptr)); BOOST_CHECK(nSigChecksRet == 1); @@ -460,8 +460,8 @@ */ std::vector scriptchecks1; CheckInputsLimiter sigchecklimiter1(1); - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - flags, true, true, txdata, nSigChecksDummy, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks1, &sigchecklimiter1)); // the first check passes but it did consume the limit. BOOST_CHECK(scriptchecks1[1]()); @@ -475,7 +475,7 @@ // Serial validation fails with the limiter. CheckInputsLimiter sigchecklimiter2(1); TxValidationState state2; - BOOST_CHECK(!CheckInputs(transaction, state2, pcoinsTip.get(), true, + BOOST_CHECK(!CheckInputs(transaction, state2, pcoinsTip.get(), flags, true, true, txdata, nSigChecksDummy, nullptr, &sigchecklimiter2)); BOOST_CHECK(!sigchecklimiter2.check()); @@ -490,23 +490,23 @@ std::vector scriptchecks3; CheckInputsLimiter sigchecklimiter3(2); // first in parallel - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - flags, true, true, txdata, nSigChecksDummy, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks3, &sigchecklimiter3)); BOOST_CHECK(scriptchecks3[1]()); BOOST_CHECK(scriptchecks3[0]()); BOOST_CHECK(sigchecklimiter3.check()); // then in serial, caching the result. CheckInputsLimiter sigchecklimiter4(2); - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - flags, true, true, txdata, nSigChecksDummy, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), flags, + true, true, txdata, nSigChecksDummy, nullptr, &sigchecklimiter4)); BOOST_CHECK(sigchecklimiter4.check()); // now in parallel again, grabbing the cached result. std::vector scriptchecks5; CheckInputsLimiter sigchecklimiter5(2); - BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, - flags, true, true, txdata, nSigChecksDummy, + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks5, &sigchecklimiter5)); BOOST_CHECK(scriptchecks5.empty()); BOOST_CHECK(sigchecklimiter5.check()); @@ -517,7 +517,7 @@ */ CheckInputsLimiter sigchecklimiter6(1); TxValidationState state6; - BOOST_CHECK(!CheckInputs(transaction, state6, pcoinsTip.get(), true, + BOOST_CHECK(!CheckInputs(transaction, state6, pcoinsTip.get(), flags, true, true, txdata, nSigChecksDummy, nullptr, &sigchecklimiter6)); BOOST_CHECK_EQUAL(state6.GetRejectReason(), "too-many-sigchecks"); @@ -528,7 +528,7 @@ std::vector scriptchecks7; CheckInputsLimiter sigchecklimiter7(1); TxValidationState state7; - BOOST_CHECK(!CheckInputs(transaction, state7, pcoinsTip.get(), true, + BOOST_CHECK(!CheckInputs(transaction, state7, pcoinsTip.get(), flags, true, true, txdata, nSigChecksDummy, &scriptchecks7, &sigchecklimiter7)); BOOST_CHECK_EQUAL(state7.GetRejectReason(), "too-many-sigchecks"); @@ -550,7 +550,7 @@ // This transaction is now invalid because the second signature is // missing. int nSigChecksDummy; - BOOST_CHECK(!CheckInputs(transaction, state, pcoinsTip.get(), true, + BOOST_CHECK(!CheckInputs(transaction, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS, true, true, txdata, nSigChecksDummy, nullptr)); @@ -558,7 +558,7 @@ // was valid) std::vector scriptchecks; BOOST_CHECK( - CheckInputs(transaction, state, pcoinsTip.get(), true, + CheckInputs(transaction, state, pcoinsTip.get(), STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_ENFORCE_SIGCHECKS, true, true, txdata, nSigChecksDummy, &scriptchecks)); // Should get 2 script checks back -- caching is on a whole-transaction diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -487,9 +487,8 @@ * accurately. */ bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, bool fScriptChecks, - const uint32_t flags, bool sigCacheStore, - bool scriptCacheStore, + const CCoinsViewCache &view, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, TxSigCheckLimiter &txLimitSigChecks, CheckInputsLimiter *pBlockLimitSigChecks, @@ -501,14 +500,14 @@ */ static inline bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, bool fScriptChecks, - const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, + const CCoinsViewCache &view, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { TxSigCheckLimiter nSigChecksTxLimiter; - return CheckInputs(tx, state, view, fScriptChecks, flags, sigCacheStore, - scriptCacheStore, txdata, nSigChecksOut, - nSigChecksTxLimiter, nullptr, nullptr); + return CheckInputs(tx, state, view, flags, sigCacheStore, scriptCacheStore, + txdata, nSigChecksOut, nSigChecksTxLimiter, nullptr, + nullptr); } /** Get the BIP9 state for a given deployment at the current tip. */ diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -330,8 +330,8 @@ } } - return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, - txdata, nSigChecksOut); + return CheckInputs(tx, state, view, flags, cacheSigStore, true, txdata, + nSigChecksOut); } /** @@ -518,7 +518,7 @@ nextBlockScriptVerifyFlags | STANDARD_SCRIPT_VERIFY_FLAGS; PrecomputedTransactionData txdata(tx); int nSigChecksStandard; - if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, + if (!CheckInputs(tx, state, view, scriptVerifyFlags, true, false, txdata, nSigChecksStandard)) { // State filled in by CheckInputs. return false; @@ -1029,9 +1029,8 @@ } bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &inputs, bool fScriptChecks, - const uint32_t flags, bool sigCacheStore, - bool scriptCacheStore, + const CCoinsViewCache &inputs, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, TxSigCheckLimiter &txLimitSigChecks, CheckInputsLimiter *pBlockLimitSigChecks, @@ -1043,15 +1042,6 @@ pvChecks->reserve(tx.vin.size()); } - // Skip script verification when connecting blocks under the assumevalid - // block. Assuming the assumevalid block is valid this is safe because - // block merkle hashes are still computed and checked, of course, if an - // assumed valid block is invalid due to false scriptSigs this optimization - // would allow an invalid chain to be accepted. - if (!fScriptChecks) { - return true; - } - // First check if script executions have been cached with the same flags. // Note that this assumes that the inputs provided are correct (ie that the // transaction hash which is in tx's prevouts properly commits to the @@ -1572,18 +1562,24 @@ pindexBestHeader->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->nChainWork >= nMinimumChainWork) { // This block is a member of the assumed verified chain and an - // ancestor of the best header. The equivalent time check - // discourages hash power from extorting the network via DOS - // attack into accepting an invalid block through telling users - // they must manually set assumevalid. Requiring a software - // change or burying the invalid block, regardless of the - // setting, makes it hard to hide the implication of the demand. - // This also avoids having release candidates that are hardly - // doing any signature verification at all in testing without - // having to artificially set the default assumed verified block - // further back. The test against nMinimumChainWork prevents the - // skipping when denied access to any chain at least as good as - // the expected chain. + // ancestor of the best header. + // Script verification is skipped when connecting blocks under + // the assumevalid block. Assuming the assumevalid block is + // valid this is safe because block merkle hashes are still + // computed and checked, Of course, if an assumed valid block is + // invalid due to false scriptSigs this optimization would allow + // an invalid chain to be accepted. + // The equivalent time check discourages hash power from + // extorting the network via DOS attack into accepting an + // invalid block through telling users they must manually set + // assumevalid. Requiring a software change or burying the + // invalid block, regardless of the setting, makes it hard to + // hide the implication of the demand. This also avoids having + // release candidates that are hardly doing any signature + // verification at all in testing without having to artificially + // set the default assumed verified block further back. The test + // against nMinimumChainWork prevents the skipping when denied + // access to any chain at least as good as the expected chain. fScriptChecks = (GetBlockProofEquivalentTime( *pindexBestHeader, *pindex, *pindexBestHeader, @@ -1777,11 +1773,11 @@ // deferred into vChecks). int nSigChecksRet; TxValidationState tx_state; - if (!CheckInputs(tx, tx_state, view, fScriptChecks, flags, - fCacheResults, fCacheResults, - PrecomputedTransactionData(tx), nSigChecksRet, - nSigChecksTxLimiters[txIndex], &nSigChecksBlockLimiter, - &vChecks)) { + if (fScriptChecks && + !CheckInputs(tx, tx_state, view, flags, fCacheResults, + fCacheResults, PrecomputedTransactionData(tx), + nSigChecksRet, nSigChecksTxLimiters[txIndex], + &nSigChecksBlockLimiter, &vChecks)) { // Any transaction validation failure in ConnectBlock is a block // consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,