diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -23,8 +23,10 @@ * per MB in a block (network rule). */ static const int64_t MAX_BLOCK_SIGOPS_PER_MB = 20000; -/** allowed number of signature check operations per transaction. */ +/** allowed number of signature check operations (sigops) per transaction. */ static const uint64_t MAX_TX_SIGOPS_COUNT = 20000; +/** Allowed number of signature check operations per transaction. */ +static const uint64_t MAX_TX_SIGCHEKS = 3000; /** * The ratio between the maximum allowable block size and the maximum allowable * SigChecks (executed signature check operations) in the block. (network rule). 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 @@ -93,6 +93,21 @@ BOOST_CHECK_EQUAL(g_mempool.size(), 0U); } +static inline 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, + CheckInputsLimiter *pBlockLimitSigChecks = nullptr) + EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + static TxSigCheckLimiter nSigChecksTxLimiter; + nSigChecksTxLimiter = TxSigCheckLimiter(); + return CheckInputs(tx, state, view, fScriptChecks, flags, sigCacheStore, + scriptCacheStore, txdata, nSigChecksOut, + nSigChecksTxLimiter, pBlockLimitSigChecks, pvChecks); +} + // Run CheckInputs (using pcoinsTip) 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. diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -456,6 +456,7 @@ * CScriptCheck), atomic so as to be compatible with parallel validation. */ class CheckInputsLimiter { +protected: std::atomic remaining; public: @@ -469,6 +470,20 @@ bool check() { return remaining >= 0; } }; +class TxSigCheckLimiter : public CheckInputsLimiter { +public: + TxSigCheckLimiter() : CheckInputsLimiter(MAX_TX_SIGCHEKS) {} + + // Let's make this bad boy copiable. + TxSigCheckLimiter(const TxSigCheckLimiter &rhs) + : CheckInputsLimiter(rhs.remaining.load()) {} + + TxSigCheckLimiter &operator=(const TxSigCheckLimiter &rhs) { + remaining = rhs.remaining.load(); + return *this; + } +}; + /** * Check whether all inputs of this transaction are valid (no double spends, * scripts & sigs, amounts). This does not modify the UTXO set. @@ -500,10 +515,26 @@ const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, - std::vector *pvChecks = nullptr, - CheckInputsLimiter *pLimitSigChecks = nullptr) + TxSigCheckLimiter &txLimitSigChecks, + CheckInputsLimiter *pBlockLimitSigChecks, + std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +/** + * Handy shortcut for to full fledged CheckInputs call. + */ +static inline 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) + EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + TxSigCheckLimiter nSigChecksTxLimiter; + return CheckInputs(tx, state, view, fScriptChecks, flags, sigCacheStore, + scriptCacheStore, txdata, nSigChecksOut, + nSigChecksTxLimiter, nullptr, nullptr); +} + /** Get the BIP9 state for a given deployment at the current tip. */ ThresholdState VersionBitsTipState(const Consensus::Params ¶ms, Consensus::DeploymentPos pos); @@ -579,22 +610,26 @@ ScriptError error; ScriptExecutionMetrics metrics; PrecomputedTransactionData txdata; - CheckInputsLimiter *pLimitSigChecks; + TxSigCheckLimiter *pTxLimitSigChecks; + CheckInputsLimiter *pBlockLimitSigChecks; public: CScriptCheck() : amount(), ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false), - error(ScriptError::UNKNOWN), txdata(), pLimitSigChecks(nullptr) {} + error(ScriptError::UNKNOWN), txdata(), pTxLimitSigChecks(nullptr), + pBlockLimitSigChecks(nullptr) {} CScriptCheck(const CScript &scriptPubKeyIn, const Amount amountIn, const CTransaction &txToIn, unsigned int nInIn, uint32_t nFlagsIn, bool cacheIn, const PrecomputedTransactionData &txdataIn, - CheckInputsLimiter *pLimitSigChecksIn = nullptr) + TxSigCheckLimiter *pTxLimitSigChecksIn = nullptr, + CheckInputsLimiter *pBlockLimitSigChecksIn = nullptr) : scriptPubKey(scriptPubKeyIn), amount(amountIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(ScriptError::UNKNOWN), txdata(txdataIn), - pLimitSigChecks(pLimitSigChecksIn) {} + pTxLimitSigChecks(pTxLimitSigChecksIn), + pBlockLimitSigChecks(pBlockLimitSigChecksIn) {} bool operator()(); @@ -608,7 +643,8 @@ std::swap(error, check.error); std::swap(metrics, check.metrics); std::swap(txdata, check.txdata); - std::swap(pLimitSigChecks, check.pLimitSigChecks); + std::swap(pTxLimitSigChecks, check.pTxLimitSigChecks); + std::swap(pBlockLimitSigChecks, check.pBlockLimitSigChecks); } ScriptError GetScriptError() const { return error; } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1219,8 +1219,10 @@ metrics, &error)) { return false; } - if (pLimitSigChecks && - !pLimitSigChecks->consume_and_check(metrics.nSigChecks)) { + if ((pTxLimitSigChecks && + !pTxLimitSigChecks->consume_and_check(metrics.nSigChecks)) || + (pBlockLimitSigChecks && + !pBlockLimitSigChecks->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. @@ -1241,8 +1243,9 @@ const uint32_t flags, bool sigCacheStore, bool scriptCacheStore, const PrecomputedTransactionData &txdata, int &nSigChecksOut, - std::vector *pvChecks /*= nullptr*/, - CheckInputsLimiter *pLimitSigChecks /*= nullptr*/) { + TxSigCheckLimiter &txLimitSigChecks, + CheckInputsLimiter *pBlockLimitSigChecks, + std::vector *pvChecks) { AssertLockHeld(cs_main); assert(!tx.IsCoinBase()); @@ -1265,8 +1268,9 @@ // scriptPubKey in the inputs view of that transaction). ScriptCacheKey hashCacheEntry(tx, flags); if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore, nSigChecksOut)) { - if (pLimitSigChecks && - !pLimitSigChecks->consume_and_check(nSigChecksOut)) { + if (!txLimitSigChecks.consume_and_check(nSigChecksOut) || + (pBlockLimitSigChecks && + !pBlockLimitSigChecks->consume_and_check(nSigChecksOut))) { return state.Invalid(false, REJECT_NONSTANDARD, strprintf("too-many-sigchecks")); } @@ -1290,7 +1294,7 @@ // Verify signature CScriptCheck check(scriptPubKey, amount, tx, i, flags, sigCacheStore, - txdata, pLimitSigChecks); + txdata, &txLimitSigChecks, pBlockLimitSigChecks); if (pvChecks) { pvChecks->push_back(std::move(check)); } else if (!check()) { @@ -1901,6 +1905,9 @@ CheckInputsLimiter nSigChecksBlockLimiter( GetMaxBlockSigChecksCount(options.getExcessiveBlockSize())); + std::vector nSigChecksTxLimiters; + nSigChecksTxLimiters.resize(block.vtx.size() - 1); + CBlockUndo blockundo; blockundo.vtxundo.resize(block.vtx.size() - 1); @@ -1991,7 +1998,8 @@ int nSigChecksRet; if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, fCacheResults, PrecomputedTransactionData(tx), - nSigChecksRet, &vChecks, &nSigChecksBlockLimiter)) { + nSigChecksRet, nSigChecksTxLimiters.at(txIndex), + &nSigChecksBlockLimiter, &vChecks)) { // Parallel CheckInputs shouldn't fail except for this reason, which // is banworthy. Use "blk-bad-inputs" to mimic the parallel script // check error. diff --git a/test/functional/abc-block-sigchecks-activation.py b/test/functional/abc-block-sigchecks-activation.py --- a/test/functional/abc-block-sigchecks-activation.py +++ b/test/functional/abc-block-sigchecks-activation.py @@ -25,6 +25,7 @@ from test_framework.mininode import P2PDataStore from test_framework.script import ( CScript, + OP_CHECKDATASIG, OP_CHECKDATASIGVERIFY, OP_3DUP, OP_RETURN, @@ -54,6 +55,8 @@ # this error in log file: BLOCK_SIGCHECKS_PARALLEL_ERROR = "blk-bad-inputs, parallel script check failed" +MAX_TX_SIGCHECK = 3000 + def create_transaction(spendfrom, custom_script, amount=None): # Fund and sign a transaction to a given output. @@ -236,6 +239,75 @@ tx.rehash() submittxes_2.append(tx) + # Check high sigcheck transactions + self.log.info("Create transaction that have high sigchecks") + + fundings = [] + + def make_spend(sigcheckcount): + # Add a funding tx to fundings, and return a tx spending that using + # scriptsig. + self.log.debug( + "Gen tx with {} sigchecks.".format(sigcheckcount)) + + def get_script_with_sigcheck(count): + return CScript([cds_message, + cds_pubkey] + (count - 1) * [OP_3DUP, OP_CHECKDATASIGVERIFY] + [OP_CHECKDATASIG]) + + # get funds locked with OP_1 + sourcetx = self.spendable_outputs.popleft() + # make funding that forwards to scriptpubkey + last_sigcheck_count = ((sigcheckcount - 1) % 30) + 1 + fundtx = create_transaction( + sourcetx, get_script_with_sigcheck(last_sigcheck_count)) + + fill_sigcheck_script = get_script_with_sigcheck(30) + + remaining_sigcheck = sigcheckcount + while remaining_sigcheck > 30: + fundtx.vout[0].nValue -= 1000 + fundtx.vout.append(CTxOut(100, bytes(fill_sigcheck_script))) + remaining_sigcheck -= 30 + + fundtx.rehash() + fundings.append(fundtx) + + # make the spending + scriptsig = CScript([cds_signature]) + + tx = CTransaction() + tx.vin.append(CTxIn(COutPoint(fundtx.sha256, 1), scriptsig)) + + input_index = 2 + remaining_sigcheck = sigcheckcount + while remaining_sigcheck > 30: + tx.vin.append( + CTxIn( + COutPoint( + fundtx.sha256, + input_index), + scriptsig)) + remaining_sigcheck -= 30 + input_index += 1 + + tx.vout.append(CTxOut(0, CScript([OP_RETURN]))) + pad_tx(tx) + tx.rehash() + return tx + + # Create transactions with may sigchecks. + good_tx = make_spend(MAX_TX_SIGCHECK) + bad_tx = make_spend(MAX_TX_SIGCHECK + 1) + + tip = self.build_block(tip, fundings) + node.p2p.send_blocks_and_test([tip], node) + + # Both tx are accepted before the activation. + pre_activation_sigcheck_block = self.build_block( + tip, [good_tx, bad_tx]) + node.p2p.send_blocks_and_test([pre_activation_sigcheck_block], node) + node.invalidateblock(pre_activation_sigcheck_block.hash) + # Activation tests self.log.info("Approach to just before upgrade activation") @@ -285,6 +357,18 @@ assert_equal(node.getblockchaininfo()[ 'mediantime'], SIGCHECKS_ACTIVATION_TIME) + self.log.info( + "Try a block with a transaction going over the limit (limit: {})".format(MAX_TX_SIGCHECK)) + bad_tx_block = self.build_block(tip, [bad_tx]) + check_for_ban_on_rejected_block( + node, bad_tx_block, reject_reason=BLOCK_SIGCHECKS_PARALLEL_ERROR) + + self.log.info( + "Try a block with a transaction just under the limit (limit: {})".format(MAX_TX_SIGCHECK)) + good_tx_block = self.build_block(tip, [good_tx]) + node.p2p.send_blocks_and_test([good_tx_block], node) + node.invalidateblock(good_tx_block.hash) + # save this tip for later # ~ upgrade_block = tip