diff --git a/src/script/script_error.h b/src/script/script_error.h --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -80,6 +80,9 @@ ILLEGAL_FORKID, MUST_USE_FORKID, + /* Auxiliary errors (unused by interpreter) */ + SIGCHECKS_LIMIT_EXCEEDED, + ERROR_COUNT, }; diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -104,6 +104,8 @@ return "Illegal use of SIGHASH_FORKID"; case ScriptError::MUST_USE_FORKID: return "Signature must use SIGHASH_FORKID"; + case ScriptError::SIGCHECKS_LIMIT_EXCEEDED: + return "Validation resources exceeded (SigChecks)"; case ScriptError::UNKNOWN: case ScriptError::ERROR_COUNT: default: 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 @@ -425,6 +425,95 @@ CTransaction(tx), SCRIPT_ENABLE_REPLAY_PROTECTION, SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_VERIFY_P2SH, true, 2); + { + // Try checking this valid transaction with sigchecks limiter + // supplied. Each input consumes 1 sigcheck. + + CValidationState state; + CTransaction transaction(tx); + PrecomputedTransactionData txdata(transaction); + uint32_t flags = + STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_REPORT_SIGCHECKS; + int nSigChecksDummy; + + /** + * Parallel validation initially works (no cached value), but + * evaluation of the script checks produces a failure. + */ + std::vector scriptchecks1; + CheckInputsLimiter sigchecklimiter1(1); + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, + 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()); + // the second check (the first input) fails due to the limiter. + BOOST_CHECK(!scriptchecks1[0]()); + BOOST_CHECK_EQUAL(scriptchecks1[0].GetScriptError(), + ScriptError::SIGCHECKS_LIMIT_EXCEEDED); + BOOST_CHECK(!sigchecklimiter1.check()); + + // Serial validation fails with the limiter. + CheckInputsLimiter sigchecklimiter2(1); + CValidationState state2; + BOOST_CHECK(!CheckInputs(transaction, state2, pcoinsTip.get(), true, + flags, true, true, txdata, nSigChecksDummy, + nullptr, &sigchecklimiter2)); + BOOST_CHECK(!sigchecklimiter2.check()); + BOOST_CHECK_EQUAL(state2.GetRejectReason(), "too-many-sigchecks"); + + /** + * A slightly more permissive limiter (just enough) passes, and + * allows caching the result. + */ + std::vector scriptchecks3; + CheckInputsLimiter sigchecklimiter3(2); + // first in parallel + BOOST_CHECK(CheckInputs(transaction, state, pcoinsTip.get(), true, + 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, + 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, + &scriptchecks5, &sigchecklimiter5)); + BOOST_CHECK(scriptchecks5.empty()); + BOOST_CHECK(sigchecklimiter5.check()); + + /** + * Going back to the lower limit, we now fail immediately due to the + * caching. + */ + CheckInputsLimiter sigchecklimiter6(1); + CValidationState state6; + BOOST_CHECK(!CheckInputs(transaction, state6, pcoinsTip.get(), true, + flags, true, true, txdata, nSigChecksDummy, + nullptr, &sigchecklimiter6)); + BOOST_CHECK_EQUAL(state6.GetRejectReason(), "too-many-sigchecks"); + BOOST_CHECK(!sigchecklimiter6.check()); + // even in parallel validation, immediate fail from the cache. + std::vector scriptchecks7; + CheckInputsLimiter sigchecklimiter7(1); + CValidationState state7; + BOOST_CHECK(!CheckInputs(transaction, state7, pcoinsTip.get(), true, + flags, true, true, txdata, nSigChecksDummy, + &scriptchecks7, &sigchecklimiter7)); + BOOST_CHECK_EQUAL(state7.GetRejectReason(), "too-many-sigchecks"); + BOOST_CHECK(!sigchecklimiter7.check()); + BOOST_CHECK(scriptchecks7.empty()); + } + // Check that if the second input is invalid, but the first input is // valid, the transaction is not cached. // Invalidate vin[1] diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -465,6 +465,24 @@ /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state); +/** + * Simple class for regulating resource usage during CheckInputs (and + * CScriptCheck), atomic so as to be compatible with parallel validation. + */ +class CheckInputsLimiter { + std::atomic remaining; + +public: + CheckInputsLimiter(int64_t limit) : remaining(limit) {} + + bool consume_and_check(int consumed) { + auto newvalue = (remaining -= consumed); + return newvalue >= 0; + } + + bool check() { return remaining >= 0; } +}; + /** * Check whether all inputs of this transaction are valid (no double spends, * scripts & sigs, amounts). This does not modify the UTXO set. @@ -482,13 +500,22 @@ * 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. + * + * pLimitSigChecks can be passed to limit the sigchecks count either in parallel + * or serial validation. With pvChecks null (serial validation), breaking the + * pLimitSigChecks limit will abort evaluation early and return false. With + * pvChecks not-null (parallel validation): the cached nSigChecks may itself + * break the limit in which case false is returned, OR, each entry in the + * returned pvChecks must be executed exactly once in order to probe the limit + * accurately. */ bool CheckInputs(const CTransaction &tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, - std::vector *pvChecks = nullptr) + std::vector *pvChecks = nullptr, + CheckInputsLimiter *pLimitSigChecks = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get the BIP9 state for a given deployment at the current tip. */ @@ -551,6 +578,9 @@ /** * Closure representing one script verification. * Note that this stores references to the spending transaction. + * + * Note that if pLimitSigChecks is passed, then failure does not imply that + * scripts have failed. */ class CScriptCheck { private: @@ -563,19 +593,22 @@ ScriptError error; ScriptExecutionMetrics metrics; PrecomputedTransactionData txdata; + CheckInputsLimiter *pLimitSigChecks; public: CScriptCheck() : amount(), ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false), - error(ScriptError::UNKNOWN), txdata() {} + error(ScriptError::UNKNOWN), txdata(), pLimitSigChecks(nullptr) {} CScriptCheck(const CScript &scriptPubKeyIn, const Amount amountIn, const CTransaction &txToIn, unsigned int nInIn, uint32_t nFlagsIn, bool cacheIn, - const PrecomputedTransactionData &txdataIn) + const PrecomputedTransactionData &txdataIn, + CheckInputsLimiter *pLimitSigChecksIn = nullptr) : scriptPubKey(scriptPubKeyIn), amount(amountIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), - error(ScriptError::UNKNOWN), txdata(txdataIn) {} + error(ScriptError::UNKNOWN), txdata(txdataIn), + pLimitSigChecks(pLimitSigChecksIn) {} bool operator()(); @@ -588,6 +621,7 @@ std::swap(cacheStore, check.cacheStore); std::swap(error, check.error); std::swap(txdata, check.txdata); + std::swap(pLimitSigChecks, check.pLimitSigChecks); } ScriptError GetScriptError() const { return error; } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1191,10 +1191,21 @@ bool CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; - return VerifyScript(scriptSig, scriptPubKey, nFlags, - CachingTransactionSignatureChecker(ptxTo, nIn, amount, - cacheStore, txdata), - metrics, &error); + if (!VerifyScript(scriptSig, scriptPubKey, nFlags, + CachingTransactionSignatureChecker(ptxTo, nIn, amount, + cacheStore, txdata), + metrics, &error)) { + return false; + } + if (pLimitSigChecks && + !pLimitSigChecks->consume_and_check(metrics.nSigChecks)) { + // we can't assign a meaningful script error (since the script + // succeeded), but remove the ScriptError::OK which could be + // misinterpreted. + error = ScriptError::SIGCHECKS_LIMIT_EXCEEDED; + return false; + } + return true; } int GetSpendHeight(const CCoinsViewCache &inputs) { @@ -1208,7 +1219,8 @@ const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, - std::vector *pvChecks) { + std::vector *pvChecks /*= nullptr*/, + CheckInputsLimiter *pLimitSigChecks /*= nullptr*/) { AssertLockHeld(cs_main); assert(!tx.IsCoinBase()); @@ -1231,6 +1243,11 @@ // scriptPubKey in the inputs view of that transaction). ScriptCacheKey hashCacheEntry(tx, flags); if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, nSigChecksOut)) { + if (pLimitSigChecks && + !pLimitSigChecks->consume_and_check(nSigChecksOut)) { + return state.Invalid(false, REJECT_NONSTANDARD, + strprintf("too-many-sigchecks")); + } return true; } @@ -1251,10 +1268,17 @@ // Verify signature CScriptCheck check(scriptPubKey, amount, tx, i, flags, sigCacheStore, - txdata); + txdata, pLimitSigChecks); if (pvChecks) { pvChecks->push_back(std::move(check)); } else if (!check()) { + if (pLimitSigChecks && !pLimitSigChecks->check()) { + // It's not a script error to overrun the limit, and we just + // reject it as nonstandard. + return state.Invalid(false, REJECT_NONSTANDARD, + strprintf("too-many-sigchecks")); + } + ScriptError scriptError = check.GetScriptError(); // Compute flags without the optional standardness flags. // This differs from MANDATORY_SCRIPT_VERIFY_FLAGS as it contains @@ -1917,7 +1941,7 @@ bool fCacheResults = fJustCheck; std::vector vChecks; - // metricsRet may be accurate (found in cache) or 0 (checks were + // nSigChecksRet may be accurate (found in cache) or 0 (checks were // deferred into vChecks). int nSigChecksRet; if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults,