diff --git a/src/policy/policy.h b/src/policy/policy.h --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -69,7 +69,7 @@ /** * When transactions fail script evaluations under standard flags, this flagset * influences the decision of whether to drop them or to also ban the originator - * (see CheckInputs). + * (see CheckInputScripts). */ static constexpr uint32_t MANDATORY_SCRIPT_VERIFY_FLAGS = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC | 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 @@ -101,25 +101,25 @@ } static inline bool -CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, const uint32_t flags, - bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, int &nSigChecksOut, - std::vector *pvChecks, - CheckInputsLimiter *pBlockLimitSigChecks = nullptr) +CheckInputScripts(const CTransaction &tx, TxValidationState &state, + const CCoinsViewCache &view, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, + const PrecomputedTransactionData &txdata, int &nSigChecksOut, + std::vector *pvChecks, + CheckInputsLimiter *pBlockLimitSigChecks = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { // nSigChecksTxLimiter need to outlive this function call, because test // cases are using pvChecks, so the verification is done asynchronously. static TxSigCheckLimiter nSigChecksTxLimiter; nSigChecksTxLimiter = TxSigCheckLimiter(); - return CheckInputs(tx, state, view, flags, sigCacheStore, scriptCacheStore, - txdata, nSigChecksOut, nSigChecksTxLimiter, - pBlockLimitSigChecks, pvChecks); + return CheckInputScripts( + tx, state, view, flags, sigCacheStore, scriptCacheStore, txdata, + nSigChecksOut, nSigChecksTxLimiter, pBlockLimitSigChecks, pvChecks); } -// Run CheckInputs (using CoinsTip()) on the given transaction, for all script -// flags. Test that CheckInputs passes for all flags that don't overlap with the -// failing_flags argument, but otherwise fails. +// Run CheckInputScripts (using CoinsTip()) on the given transaction, for all +// script flags. Test that CheckInputScripts passes for all flags that don't +// overlap with the failing_flags argument, but otherwise fails. // CHECKLOCKTIMEVERIFY and CHECKSEQUENCEVERIFY (and future NOP codes that may // get reassigned) have an interaction with DISCOURAGE_UPGRADABLE_NOPS: if the // script flags used contain DISCOURAGE_UPGRADABLE_NOPS but don't contain @@ -148,12 +148,12 @@ } int nSigChecksDirect = 0xf00d; - bool ret = - CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), test_flags, - true, add_to_cache, txdata, nSigChecksDirect); + bool ret = CheckInputScripts( + tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, + add_to_cache, txdata, nSigChecksDirect); - // CheckInputs should succeed iff test_flags doesn't intersect with - // failing_flags + // CheckInputScripts 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); @@ -166,9 +166,9 @@ // Check that we get a cache hit if the tx was valid std::vector scriptchecks; int nSigChecksCached = 0xbeef; - BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), - test_flags, true, add_to_cache, txdata, - nSigChecksCached, &scriptchecks)); + BOOST_CHECK(CheckInputScripts( + tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, + add_to_cache, txdata, nSigChecksCached, &scriptchecks)); BOOST_CHECK(nSigChecksCached == nSigChecksDirect); BOOST_CHECK(scriptchecks.empty()); } else { @@ -176,9 +176,9 @@ // was invalid, or we didn't add to cache. std::vector scriptchecks; int nSigChecksUncached = 0xbabe; - BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), - test_flags, true, add_to_cache, txdata, - nSigChecksUncached, &scriptchecks)); + BOOST_CHECK(CheckInputScripts( + tx, state, &::ChainstateActive().CoinsTip(), test_flags, true, + add_to_cache, txdata, nSigChecksUncached, &scriptchecks)); BOOST_CHECK(!ret || nSigChecksUncached == 0); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); } @@ -186,8 +186,8 @@ } BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) { - // Test that passing CheckInputs with one set of script flags doesn't imply - // that we would pass again with a different set of flags. + // Test that passing CheckInputScripts with one set of script flags doesn't + // imply that we would pass again with a different set of flags. { LOCK(cs_main); InitScriptExecutionCache(); @@ -279,23 +279,25 @@ PrecomputedTransactionData ptd_spend_tx(tx); int nSigChecksDummy; - BOOST_CHECK(!CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - ptd_spend_tx, nSigChecksDummy, nullptr)); + BOOST_CHECK(!CheckInputScripts(tx, state, + &::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS, true, true, + 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 // not caching invalidity (if that changes, delete this test case). std::vector scriptchecks; - BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - ptd_spend_tx, nSigChecksDummy, &scriptchecks)); + BOOST_CHECK( + CheckInputScripts(tx, state, &::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS, true, true, + ptd_spend_tx, nSigChecksDummy, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); - // Test that CheckInputs returns true iff cleanstack-enforcing flags are - // not present. Don't add these checks to the cache, so that we can test - // later that block validation works fine in the absence of cached - // successes. + // Test that CheckInputScripts returns true iff cleanstack-enforcing + // flags are not present. Don't add these checks to the cache, so that + // we can test later that block validation works fine in the absence of + // cached successes. ValidateCheckInputsForAllFlags( tx, SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS, 0, false, 0); } @@ -362,10 +364,10 @@ PrecomputedTransactionData txdata(transaction); int nSigChecksRet; - BOOST_CHECK(CheckInputs(transaction, state, - ::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nSigChecksRet, nullptr)); + BOOST_CHECK(CheckInputScripts(transaction, state, + ::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS, true, true, + txdata, nSigChecksRet, nullptr)); BOOST_CHECK(nSigChecksRet == 1); } @@ -402,10 +404,10 @@ PrecomputedTransactionData txdata(transaction); int nSigChecksRet; - BOOST_CHECK(CheckInputs(transaction, state, - &::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nSigChecksRet, nullptr)); + BOOST_CHECK(CheckInputScripts(transaction, state, + &::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS, true, true, + txdata, nSigChecksRet, nullptr)); BOOST_CHECK(nSigChecksRet == 1); } @@ -466,10 +468,10 @@ */ std::vector scriptchecks1; CheckInputsLimiter sigchecklimiter1(1); - BOOST_CHECK(CheckInputs(transaction, state, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - &scriptchecks1, &sigchecklimiter1)); + BOOST_CHECK(CheckInputScripts( + transaction, state, &::ChainstateActive().CoinsTip(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks1, + &sigchecklimiter1)); // the first check passes but it did consume the limit. BOOST_CHECK(scriptchecks1[1]()); BOOST_CHECK(sigchecklimiter1.check()); @@ -482,10 +484,10 @@ // Serial validation fails with the limiter. CheckInputsLimiter sigchecklimiter2(1); TxValidationState state2; - BOOST_CHECK(!CheckInputs(transaction, state2, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - nullptr, &sigchecklimiter2)); + BOOST_CHECK(!CheckInputScripts( + transaction, state2, &::ChainstateActive().CoinsTip(), flags, + true, true, txdata, nSigChecksDummy, nullptr, + &sigchecklimiter2)); BOOST_CHECK(!sigchecklimiter2.check()); BOOST_CHECK_EQUAL(state2.GetRejectReason(), "non-mandatory-script-verify-flag (Validation " @@ -498,27 +500,27 @@ std::vector scriptchecks3; CheckInputsLimiter sigchecklimiter3(2); // first in parallel - BOOST_CHECK(CheckInputs(transaction, state, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - &scriptchecks3, &sigchecklimiter3)); + BOOST_CHECK(CheckInputScripts( + transaction, state, &::ChainstateActive().CoinsTip(), 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, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - nullptr, &sigchecklimiter4)); + BOOST_CHECK(CheckInputScripts( + transaction, state, &::ChainstateActive().CoinsTip(), 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, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - &scriptchecks5, &sigchecklimiter5)); + BOOST_CHECK(CheckInputScripts( + transaction, state, &::ChainstateActive().CoinsTip(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks5, + &sigchecklimiter5)); BOOST_CHECK(scriptchecks5.empty()); BOOST_CHECK(sigchecklimiter5.check()); @@ -528,10 +530,10 @@ */ CheckInputsLimiter sigchecklimiter6(1); TxValidationState state6; - BOOST_CHECK(!CheckInputs(transaction, state6, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - nullptr, &sigchecklimiter6)); + BOOST_CHECK(!CheckInputScripts( + transaction, state6, &::ChainstateActive().CoinsTip(), flags, + true, true, txdata, nSigChecksDummy, nullptr, + &sigchecklimiter6)); BOOST_CHECK_EQUAL(state6.GetRejectReason(), "too-many-sigchecks"); BOOST_CHECK_EQUAL(state6.GetResult(), TxValidationResult::TX_CONSENSUS); @@ -540,10 +542,10 @@ std::vector scriptchecks7; CheckInputsLimiter sigchecklimiter7(1); TxValidationState state7; - BOOST_CHECK(!CheckInputs(transaction, state7, - &::ChainstateActive().CoinsTip(), flags, - true, true, txdata, nSigChecksDummy, - &scriptchecks7, &sigchecklimiter7)); + BOOST_CHECK(!CheckInputScripts( + transaction, state7, &::ChainstateActive().CoinsTip(), flags, + true, true, txdata, nSigChecksDummy, &scriptchecks7, + &sigchecklimiter7)); BOOST_CHECK_EQUAL(state7.GetRejectReason(), "too-many-sigchecks"); BOOST_CHECK_EQUAL(state6.GetResult(), TxValidationResult::TX_CONSENSUS); @@ -563,18 +565,18 @@ // This transaction is now invalid because the second signature is // missing. int nSigChecksDummy; - BOOST_CHECK(!CheckInputs(transaction, state, - &::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS, true, true, - txdata, nSigChecksDummy, nullptr)); + BOOST_CHECK(!CheckInputScripts(transaction, state, + &::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS, true, true, + txdata, nSigChecksDummy, nullptr)); // Make sure this transaction was not cached (ie becausethe first input // was valid) std::vector scriptchecks; - BOOST_CHECK( - CheckInputs(transaction, state, &::ChainstateActive().CoinsTip(), - STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_ENFORCE_SIGCHECKS, - true, true, txdata, nSigChecksDummy, &scriptchecks)); + BOOST_CHECK(CheckInputScripts( + transaction, state, &::ChainstateActive().CoinsTip(), + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_ENFORCE_SIGCHECKS, 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 @@ -342,7 +342,7 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** - * Simple class for regulating resource usage during CheckInputs (and + * Simple class for regulating resource usage during CheckInputScripts (and * CScriptCheck), atomic so as to be compatible with parallel validation. */ class CheckInputsLimiter { @@ -385,8 +385,11 @@ class ConnectTrace; /** - * Check whether all inputs of this transaction are valid (no double spends, - * scripts & sigs, amounts). This does not modify the UTXO set. + * Check whether all of this transaction's input scripts succeed. + * + * This involves ECDSA signature checks so can be computationally intensive. + * This function should only be called after the cheap sanity checks in + * CheckTxInputs passed. * * If pvChecks is not nullptr, script checks are pushed onto it instead of being * performed inline. Any script checks which are not necessary (eg due to script @@ -410,28 +413,28 @@ * returned pvChecks must be executed exactly once in order to probe the limit * accurately. */ -bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, const uint32_t flags, - bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, int &nSigChecksOut, - TxSigCheckLimiter &txLimitSigChecks, - CheckInputsLimiter *pBlockLimitSigChecks, - std::vector *pvChecks) +bool CheckInputScripts(const CTransaction &tx, TxValidationState &state, + const CCoinsViewCache &view, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, + const PrecomputedTransactionData &txdata, + int &nSigChecksOut, TxSigCheckLimiter &txLimitSigChecks, + CheckInputsLimiter *pBlockLimitSigChecks, + std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** - * Handy shortcut to full fledged CheckInputs call. + * Handy shortcut to full fledged CheckInputScripts call. */ static inline bool -CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &view, const uint32_t flags, - bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, int &nSigChecksOut) +CheckInputScripts(const CTransaction &tx, TxValidationState &state, + 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, flags, sigCacheStore, scriptCacheStore, - txdata, nSigChecksOut, nSigChecksTxLimiter, nullptr, - nullptr); + return CheckInputScripts(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 @@ -295,21 +295,21 @@ // pool.cs should be locked already, but go ahead and re-take the lock here // to enforce that mempool doesn't change between when we check the view and - // when we actually call through to CheckInputs + // when we actually call through to CheckInputScripts LOCK(pool.cs); assert(!tx.IsCoinBase()); for (const CTxIn &txin : tx.vin) { const Coin &coin = view.AccessCoin(txin.prevout); - // At this point we haven't actually checked if the coins are all - // available (or shouldn't assume we have, since CheckInputs does). So - // we just return failure if the inputs are not available here, and then - // only have to check equivalence for available inputs. + // AcceptToMemoryPoolWorker has already checked that the coins are + // available, so this shouldn't fail. If the inputs are not available + // here then return false. if (coin.IsSpent()) { return false; } + // Check equivalence for available inputs. const CTransactionRef &txFrom = pool.get(txin.prevout.GetTxId()); if (txFrom) { assert(txFrom->GetId() == txin.prevout.GetTxId()); @@ -323,11 +323,11 @@ } } - // Call CheckInputs() to cache signature and script validity against current - // tip consensus rules. - return CheckInputs(tx, state, view, flags, /* cacheSigStore = */ true, - /* cacheFullScriptStore = */ true, txdata, - nSigChecksOut); + // Call CheckInputScripts() to cache signature and script validity against + // current tip consensus rules. + return CheckInputScripts(tx, state, view, flags, /* cacheSigStore = */ true, + /* cacheFullScriptStore = */ true, txdata, + nSigChecksOut); } namespace { @@ -596,9 +596,9 @@ const uint32_t scriptVerifyFlags = ws.m_next_block_script_verify_flags | STANDARD_SCRIPT_VERIFY_FLAGS; PrecomputedTransactionData txdata(tx); - if (!CheckInputs(tx, state, m_view, scriptVerifyFlags, true, false, txdata, - ws.m_sig_checks_standard)) { - // State filled in by CheckInputs. + if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, + txdata, ws.m_sig_checks_standard)) { + // State filled in by CheckInputScripts return false; } @@ -656,7 +656,7 @@ // 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). - return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed " + return error("%s: BUG! PLEASE REPORT THIS! CheckInputScripts failed " "against next-block but not STANDARD flags %s, %s", __func__, txid.ToString(), state.ToString()); } @@ -1133,13 +1133,13 @@ return pindexPrev->nHeight + 1; } -bool CheckInputs(const CTransaction &tx, TxValidationState &state, - const CCoinsViewCache &inputs, const uint32_t flags, - bool sigCacheStore, bool scriptCacheStore, - const PrecomputedTransactionData &txdata, int &nSigChecksOut, - TxSigCheckLimiter &txLimitSigChecks, - CheckInputsLimiter *pBlockLimitSigChecks, - std::vector *pvChecks) { +bool CheckInputScripts(const CTransaction &tx, TxValidationState &state, + const CCoinsViewCache &inputs, const uint32_t flags, + bool sigCacheStore, bool scriptCacheStore, + const PrecomputedTransactionData &txdata, + int &nSigChecksOut, TxSigCheckLimiter &txLimitSigChecks, + CheckInputsLimiter *pBlockLimitSigChecks, + std::vector *pvChecks) { AssertLockHeld(cs_main); assert(!tx.IsCoinBase()); @@ -1927,17 +1927,18 @@ int nSigChecksRet; TxValidationState tx_state; if (fScriptChecks && - !CheckInputs(tx, tx_state, view, flags, fCacheResults, - fCacheResults, PrecomputedTransactionData(tx), - nSigChecksRet, nSigChecksTxLimiters[txIndex], - &nSigChecksBlockLimiter, &vChecks)) { + !CheckInputScripts(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, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); - return error("ConnectBlock(): CheckInputs on %s failed with %s", - tx.GetId().ToString(), state.ToString()); + return error( + "ConnectBlock(): CheckInputScripts on %s failed with %s", + tx.GetId().ToString(), state.ToString()); } control.Add(vChecks);