WIP: Assign one batch to each thread for parallel script checking

ConnectBlock benchmark before this commit:
|            ns/block |             block/s |    err% |       ins/block |       cyc/block |    IPC |      bra/block |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|      330,877,031.00 |                3.02 |    0.1% |  214,614,859.00 |   79,334,264.00 |  2.705 |  15,834,591.00 |    0.8% |      3.64 | `ConnectBlock`

ConnectBlock benchmark after this commit:
|            ns/block |             block/s |    err% |       ins/block |       cyc/block |    IPC |      bra/block |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|      115,439,963.00 |                8.66 |    0.2% |1,181,627,017.00 |  392,925,076.00 |  3.007 |  34,792,045.00 |    1.9% |      1.27 | `ConnectBlock`

src/checkqueue.h

@@ -5,14 +5,26 @@
 #ifndef BITCOIN_CHECKQUEUE_H
 #define BITCOIN_CHECKQUEUE_H
 
+#include <batchverify.h>
 #include <sync.h>
 #include <tinyformat.h>
 #include <util/threadnames.h>
 
 #include <algorithm>
+#include <concepts>
 #include <iterator>
 #include <vector>
 
+template <typename T, typename Obj>
+concept HasOperatorWithObj = requires(T t, Obj* obj, bool f) {
+    { t(obj, f) } -> std::same_as<bool>;
+};
+
+template <typename T>
+concept HasOperatorNoArgs = requires(T t) {
+    { t() } -> std::same_as<bool>;
+};
+
 /**
  * Queue for verifications that have to be performed.
   * The verifications are represented by a type T, which must provide an
@@ -24,6 +36,7 @@
   * as an N'th worker, until all jobs are done.
   */
 template <typename T>
+requires HasOperatorWithObj<T, BatchSchnorrVerifier> || HasOperatorNoArgs<T>
 class CCheckQueue
 {
 private:
@@ -66,6 +79,7 @@ class CCheckQueue
     bool Loop(bool fMaster) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
         std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv;
+        BatchSchnorrVerifier batch;
         std::vector<T> vChecks;
         vChecks.reserve(nBatchSize);
         unsigned int nNow = 0;
@@ -115,9 +129,16 @@ class CCheckQueue
                 fOk = fAllOk;
             }
             // execute work
-            for (T& check : vChecks)
+            for (size_t i = 0; i < vChecks.size(); i++) {
+                bool fLastCheck = i == vChecks.size() - 1;
                 if (fOk)
-                    fOk = check();
+                {
+                    if constexpr (HasOperatorWithObj<T, BatchSchnorrVerifier>)
+                        fOk = vChecks[i](&batch, fLastCheck);
+                    else
+                        fOk = vChecks[i]();
+                }
+            }
             vChecks.clear();
         } while (true);
     }

src/script/script_error.h

@@ -78,6 +78,9 @@ typedef enum ScriptError_t
     SCRIPT_ERR_TAPSCRIPT_CHECKMULTISIG,
     SCRIPT_ERR_TAPSCRIPT_MINIMALIF,
 
+    /* Batch validation */
+    SCRIPT_ERR_BATCH_VALIDATION_FAILED,
+
     /* Constant scriptCode */
     SCRIPT_ERR_OP_CODESEPARATOR,
     SCRIPT_ERR_SIG_FINDANDDELETE,

src/test/txvalidationcache_tests.cpp

@@ -24,8 +24,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
-                       std::vector<CScriptCheck>* pvChecks,
-                       BatchSchnorrVerifier* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+                       std::vector<CScriptCheck>* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)
 

src/validation.cpp

@@ -133,8 +133,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
-                       std::vector<CScriptCheck>* pvChecks = nullptr,
-                       BatchSchnorrVerifier* batch = nullptr)
+                       std::vector<CScriptCheck>* pvChecks = nullptr)
                        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx)
@@ -2086,11 +2085,18 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
     AddCoins(inputs, tx, nHeight);
 }
 
-bool CScriptCheck::operator()() {
+bool CScriptCheck::operator()(BatchSchnorrVerifier* batch, bool fVerifyBatch) {
     const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
     const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness;
-    if (m_batch) {
-        return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, BatchingCachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata, m_batch), &error);
+    if (batch) {
+        if (!VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, BatchingCachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata, batch), &error)) return false;
+        if (!fVerifyBatch) return true;
+        if (!batch->Verify()) {
+            LogPrintf("ERROR: %s: Batch verification failed\n", __func__);
+            error = SCRIPT_ERR_BATCH_VALIDATION_FAILED;
+            return false;
+        }
+        return true;
     }
     return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata), &error);
 }
@@ -2134,8 +2140,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
                        ValidationCache& validation_cache,
-                       std::vector<CScriptCheck>* pvChecks,
-                       BatchSchnorrVerifier* batch)
+                       std::vector<CScriptCheck>* pvChecks)
 {
     if (tx.IsCoinBase()) return true;
 
@@ -2179,7 +2184,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
         // spent being checked as a part of CScriptCheck.
 
         // Verify signature
-        CScriptCheck check(txdata.m_spent_outputs[i], tx, validation_cache.m_signature_cache, i, flags, cacheSigStore, &txdata, batch);
+        CScriptCheck check(txdata.m_spent_outputs[i], tx, validation_cache.m_signature_cache, i, flags, cacheSigStore, &txdata);
         if (pvChecks) {
             pvChecks->emplace_back(std::move(check));
         } else if (!check()) {
@@ -2195,7 +2200,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
                 // non-upgraded nodes by banning CONSENSUS-failing
                 // data providers.
                 CScriptCheck check2(txdata.m_spent_outputs[i], tx, validation_cache.m_signature_cache, i,
-                        flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata, batch);
+                        flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata);
                 if (check2())
                     return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
 
@@ -2621,8 +2626,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     int64_t nSigOpsCost = 0;
     blockundo.vtxundo.reserve(block.vtx.size() - 1);
 
-    BatchSchnorrVerifier batch{};
-
     for (unsigned int i = 0; i < block.vtx.size(); i++)
     {
         const CTransaction &tx = *(block.vtx[i]);
@@ -2675,7 +2678,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             std::vector<CScriptCheck> vChecks;
             bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
             TxValidationState tx_state;
-            if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, parallel_script_checks ? &vChecks : nullptr, &batch)) {
+            if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache, parallel_script_checks ? &vChecks : nullptr)) {
                 // Any transaction validation failure in ConnectBlock is a block consensus failure
                 state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,
                               tx_state.GetRejectReason(), tx_state.GetDebugMessage());
@@ -2711,11 +2714,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-validation-failed");
     }
 
-    if (!batch.Verify()) {
-        LogPrintf("ERROR: %s: Batch verification failed\n", __func__);
-        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-batch-verify-failed");
-    }
-
     const auto time_4{SteadyClock::now()};
     m_chainman.time_verify += time_4 - time_2;
     LogDebug(BCLog::BENCH, "    - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1,

src/validation.h

@@ -8,7 +8,6 @@
 
 #include <arith_uint256.h>
 #include <attributes.h>
-#include <batchverify.h>
 #include <chain.h>
 #include <checkqueue.h>
 #include <consensus/amount.h>
@@ -339,18 +338,17 @@ class CScriptCheck
     ScriptError error{SCRIPT_ERR_UNKNOWN_ERROR};
     PrecomputedTransactionData *txdata;
     SignatureCache* m_signature_cache;
-    BatchSchnorrVerifier* m_batch;
 
 public:
-    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, SignatureCache& signature_cache, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn, BatchSchnorrVerifier* batchIn = nullptr) :
-        m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn), m_signature_cache(&signature_cache), m_batch(batchIn) { }
+    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, SignatureCache& signature_cache, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
+        m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn), m_signature_cache(&signature_cache) { }
 
     CScriptCheck(const CScriptCheck&) = delete;
     CScriptCheck& operator=(const CScriptCheck&) = delete;
     CScriptCheck(CScriptCheck&&) = default;
     CScriptCheck& operator=(CScriptCheck&&) = default;
 
-    bool operator()();
+    bool operator()(BatchSchnorrVerifier* batch = nullptr, bool fVerifyBatch = false);
 
     ScriptError GetScriptError() const { return error; }
 };