From ab14d1d6a4a8ef5fe5013150e6c5ebcb5f5e4ea9 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Mon, 20 May 2024 22:32:32 +0200
Subject: validation: Don't error if maxsigcachesize exceeds uint32::max

Instead clamp it to uint32::max if it exceeds it.

Co-authored-by: Anthony Towns <aj@erisian.com.au>
---
 src/cuckoocache.h       | 14 ++++++--------
 src/init.cpp            |  7 ++-----
 src/script/sigcache.cpp |  8 ++------
 src/script/sigcache.h   |  1 -
 src/validation.cpp      |  5 +----
 5 files changed, 11 insertions(+), 24 deletions(-)

(limited to 'src')

diff --git a/src/cuckoocache.h b/src/cuckoocache.h
index df320ed465..8370179395 100644
--- a/src/cuckoocache.h
+++ b/src/cuckoocache.h
@@ -14,7 +14,6 @@
 #include <cstring>
 #include <limits>
 #include <memory>
-#include <optional>
 #include <utility>
 #include <vector>
 
@@ -360,16 +359,15 @@ public:
      * structure
      * @returns A pair of the maximum number of elements storable (see setup()
      * documentation for more detail) and the approximate total size of these
-     * elements in bytes or std::nullopt if the size requested is too large.
+     * elements in bytes.
      */
-    std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t bytes)
+    std::pair<uint32_t, size_t> setup_bytes(size_t bytes)
     {
-        size_t requested_num_elems = bytes / sizeof(Element);
-        if (std::numeric_limits<uint32_t>::max() < requested_num_elems) {
-            return std::nullopt;
-        }
+        uint32_t requested_num_elems(std::min<size_t>(
+            bytes / sizeof(Element),
+            std::numeric_limits<uint32_t>::max()));
 
-        auto num_elems = setup(bytes/sizeof(Element));
+        auto num_elems = setup(requested_num_elems);
 
         size_t approx_size_bytes = num_elems * sizeof(Element);
         return std::make_pair(num_elems, approx_size_bytes);
diff --git a/src/init.cpp b/src/init.cpp
index cb294ba84f..985b8d3d63 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1156,11 +1156,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     ValidationCacheSizes validation_cache_sizes{};
     ApplyArgsManOptions(args, validation_cache_sizes);
-    if (!InitSignatureCache(validation_cache_sizes.signature_cache_bytes)
-        || !InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes))
-    {
-        return InitError(strprintf(_("Unable to allocate memory for -maxsigcachesize: '%s' MiB"), args.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_BYTES >> 20)));
-    }
+    (void)InitSignatureCache(validation_cache_sizes.signature_cache_bytes);
+    (void)InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes);
 
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 7c6c282cc4..666601d174 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -15,7 +15,6 @@
 
 #include <algorithm>
 #include <mutex>
-#include <optional>
 #include <shared_mutex>
 #include <vector>
 
@@ -77,7 +76,7 @@ public:
         std::unique_lock<std::shared_mutex> lock(cs_sigcache);
         setValid.insert(entry);
     }
-    std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t n)
+    std::pair<uint32_t, size_t> setup_bytes(size_t n)
     {
         return setValid.setup_bytes(n);
     }
@@ -96,10 +95,7 @@ static CSignatureCache signatureCache;
 // signatureCache.
 bool InitSignatureCache(size_t max_size_bytes)
 {
-    auto setup_results = signatureCache.setup_bytes(max_size_bytes);
-    if (!setup_results) return false;
-
-    const auto [num_elems, approx_size_bytes] = *setup_results;
+    const auto [num_elems, approx_size_bytes] = signatureCache.setup_bytes(max_size_bytes);
     LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
               approx_size_bytes >> 20, max_size_bytes >> 20, num_elems);
     return true;
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index d33d60d5bc..26b8b9cd2a 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -10,7 +10,6 @@
 #include <span.h>
 #include <util/hasher.h>
 
-#include <optional>
 #include <vector>
 
 // DoS prevention: limit cache size to 32MiB (over 1000000 entries on 64-bit
diff --git a/src/validation.cpp b/src/validation.cpp
index 3e9ba08bb1..a9d26daf99 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2100,10 +2100,7 @@ bool InitScriptExecutionCache(size_t max_size_bytes)
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
 
-    auto setup_results = g_scriptExecutionCache.setup_bytes(max_size_bytes);
-    if (!setup_results) return false;
-
-    const auto [num_elems, approx_size_bytes] = *setup_results;
+    const auto [num_elems, approx_size_bytes] = g_scriptExecutionCache.setup_bytes(max_size_bytes);
     LogPrintf("Using %zu MiB out of %zu MiB requested for script execution cache, able to store %zu elements\n",
               approx_size_bytes >> 20, max_size_bytes >> 20, num_elems);
     return true;
-- 
cgit v1.2.3


From 13a3661aba95b54b822c99ecbb695b14a22536d2 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 17 May 2024 23:33:25 +0200
Subject: kernel: De-globalize script execution cache

Move its ownership to the ChainstateManager class.

Next to simplifying usage of the kernel library by no longer requiring
manual setup of the cache prior to using validation code, it also slims
down the amount of memory allocated by BasicTestingSetup.
---
 src/bitcoin-chainstate.cpp           |  1 -
 src/init.cpp                         |  1 -
 src/kernel/chainstatemanager_opts.h  |  2 ++
 src/kernel/validation_cache_sizes.h  |  1 -
 src/node/chainstatemanager_args.cpp  |  9 +++++++++
 src/node/validation_cache_args.cpp   |  1 -
 src/script/sigcache.h                |  1 +
 src/test/txvalidationcache_tests.cpp | 36 ++++++++++++++++++-----------------
 src/test/util/setup_common.cpp       |  1 -
 src/validation.cpp                   | 37 +++++++++++++++++++++---------------
 src/validation.h                     | 21 ++++++++++++++++----
 11 files changed, 70 insertions(+), 41 deletions(-)

(limited to 'src')

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index ecbdcd48bb..d11d793d21 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -68,7 +68,6 @@ int main(int argc, char* argv[])
     // performing the check with the signature cache.
     kernel::ValidationCacheSizes validation_cache_sizes{};
     Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
-    Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
 
     ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};
 
diff --git a/src/init.cpp b/src/init.cpp
index 985b8d3d63..5accc63f37 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1157,7 +1157,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     ValidationCacheSizes validation_cache_sizes{};
     ApplyArgsManOptions(args, validation_cache_sizes);
     (void)InitSignatureCache(validation_cache_sizes.signature_cache_bytes);
-    (void)InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes);
 
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h
index 076841c3c9..1b08eeeca7 100644
--- a/src/kernel/chainstatemanager_opts.h
+++ b/src/kernel/chainstatemanager_opts.h
@@ -9,6 +9,7 @@
 
 #include <arith_uint256.h>
 #include <dbwrapper.h>
+#include <script/sigcache.h>
 #include <txdb.h>
 #include <uint256.h>
 #include <util/time.h>
@@ -48,6 +49,7 @@ struct ChainstateManagerOpts {
     ValidationSignals* signals{nullptr};
     //! Number of script check worker threads. Zero means no parallel verification.
     int worker_threads_num{0};
+    size_t script_execution_cache_bytes{DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES};
 };
 
 } // namespace kernel
diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h
index 72e4d1a52c..1774d34226 100644
--- a/src/kernel/validation_cache_sizes.h
+++ b/src/kernel/validation_cache_sizes.h
@@ -13,7 +13,6 @@
 namespace kernel {
 struct ValidationCacheSizes {
     size_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
-    size_t script_execution_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
 };
 }
 
diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp
index bc4a815a3e..14dd8cd97b 100644
--- a/src/node/chainstatemanager_args.cpp
+++ b/src/node/chainstatemanager_args.cpp
@@ -56,6 +56,15 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
     opts.worker_threads_num = std::clamp(script_threads - 1, 0, MAX_SCRIPTCHECK_THREADS);
     LogPrintf("Script verification uses %d additional threads\n", opts.worker_threads_num);
 
+    if (auto max_size = args.GetIntArg("-maxsigcachesize")) {
+        // 1. When supplied with a max_size of 0, both the signature cache and
+        //    script execution cache create the minimum possible cache (2
+        //    elements). Therefore, we can use 0 as a floor here.
+        // 2. Multiply first, divide after to avoid integer truncation.
+        size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
+        opts.script_execution_cache_bytes = clamped_size_each;
+    }
+
     return {};
 }
 } // namespace node
diff --git a/src/node/validation_cache_args.cpp b/src/node/validation_cache_args.cpp
index ddf24f798d..b491dd9b37 100644
--- a/src/node/validation_cache_args.cpp
+++ b/src/node/validation_cache_args.cpp
@@ -27,7 +27,6 @@ void ApplyArgsManOptions(const ArgsManager& argsman, ValidationCacheSizes& cache
         size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
         cache_sizes = {
             .signature_cache_bytes = clamped_size_each,
-            .script_execution_cache_bytes = clamped_size_each,
         };
     }
 }
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index 26b8b9cd2a..d2723b47d2 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -16,6 +16,7 @@
 // systems). Due to how we count cache size, actual memory usage is slightly
 // more (~32.25 MiB)
 static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
+static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
 
 class CPubKey;
 
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 78ef96a15d..e3b215ad83 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -5,6 +5,7 @@
 #include <consensus/validation.h>
 #include <key.h>
 #include <random.h>
+#include <script/sigcache.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <test/util/setup_common.h>
@@ -22,6 +23,7 @@ struct Dersig100Setup : public TestChain100Setup {
 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) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)
@@ -118,7 +120,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
 // should fail.
 // Capture this interaction with the upgraded_nop argument: set it when evaluating
 // any script flag that is implemented as an upgraded NOP code.
-static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
+static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip, ValidationCache& validation_cache) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
     PrecomputedTransactionData txdata;
 
@@ -140,7 +142,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
             // WITNESS requires P2SH
             test_flags |= SCRIPT_VERIFY_P2SH;
         }
-        bool ret = CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, nullptr);
+        bool ret = CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, nullptr);
         // CheckInputScripts should succeed iff test_flags doesn't intersect with
         // failing_flags
         bool expected_return_value = !(test_flags & failing_flags);
@@ -150,13 +152,13 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
         if (ret && add_to_cache) {
             // Check that we get a cache hit if the tx was valid
             std::vector<CScriptCheck> scriptchecks;
-            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, &scriptchecks));
+            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
             BOOST_CHECK(scriptchecks.empty());
         } else {
             // Check that we get script executions to check, if the transaction
             // was invalid, or we didn't add to cache.
             std::vector<CScriptCheck> scriptchecks;
-            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, &scriptchecks));
+            BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
             BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size());
         }
     }
@@ -214,20 +216,20 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         TxValidationState state;
         PrecomputedTransactionData ptd_spend_tx;
 
-        BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr));
+        BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, 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<CScriptCheck> scriptchecks;
-        BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks));
+        BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, &scriptchecks));
         BOOST_CHECK_EQUAL(scriptchecks.size(), 1U);
 
         // Test that CheckInputScripts returns true iff DERSIG-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(CTransaction(spend_tx), SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC, false, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(spend_tx), SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC, false, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
     }
 
     // And if we produce a block with this tx, it should be valid (DERSIG not
@@ -253,7 +255,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         std::vector<unsigned char> vchSig2(p2pk_scriptPubKey.begin(), p2pk_scriptPubKey.end());
         invalid_under_p2sh_tx.vin[0].scriptSig << vchSig2;
 
-        ValidateCheckInputsForAllFlags(CTransaction(invalid_under_p2sh_tx), SCRIPT_VERIFY_P2SH, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(invalid_under_p2sh_tx), SCRIPT_VERIFY_P2SH, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
     }
 
     // Test CHECKLOCKTIMEVERIFY
@@ -276,13 +278,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         vchSig.push_back((unsigned char)SIGHASH_ALL);
         invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 101;
 
-        ValidateCheckInputsForAllFlags(CTransaction(invalid_with_cltv_tx), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(invalid_with_cltv_tx), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
 
         // Make it valid, and check again
         invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
         TxValidationState state;
         PrecomputedTransactionData txdata;
-        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr));
+        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
     }
 
     // TEST CHECKSEQUENCEVERIFY
@@ -304,13 +306,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         vchSig.push_back((unsigned char)SIGHASH_ALL);
         invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 101;
 
-        ValidateCheckInputsForAllFlags(CTransaction(invalid_with_csv_tx), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(invalid_with_csv_tx), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
 
         // Make it valid, and check again
         invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
         TxValidationState state;
         PrecomputedTransactionData txdata;
-        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr));
+        BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
     }
 
     // TODO: add tests for remaining script flags
@@ -333,11 +335,11 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         UpdateInput(valid_with_witness_tx.vin[0], sigdata);
 
         // This should be valid under all script flags.
-        ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
 
         // Remove the witness, and check that it is now invalid.
         valid_with_witness_tx.vin[0].scriptWitness.SetNull();
-        ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), SCRIPT_VERIFY_WITNESS, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), SCRIPT_VERIFY_WITNESS, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
     }
 
     {
@@ -362,7 +364,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         }
 
         // This should be valid under all script flags
-        ValidateCheckInputsForAllFlags(CTransaction(tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip());
+        ValidateCheckInputsForAllFlags(CTransaction(tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
 
         // Check that if the second input is invalid, but the first input is
         // valid, the transaction is not cached.
@@ -372,12 +374,12 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
         TxValidationState state;
         PrecomputedTransactionData txdata;
         // This transaction is now invalid under segwit, because of the second input.
-        BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, nullptr));
+        BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
 
         std::vector<CScriptCheck> scriptchecks;
         // Make sure this transaction was not cached (ie because the first
         // input was valid)
-        BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, &scriptchecks));
+        BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, &scriptchecks));
         // Should get 2 script checks back -- caching is on a whole-transaction basis.
         BOOST_CHECK_EQUAL(scriptchecks.size(), 2U);
     }
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index cc7b2d6546..7408fd8ec4 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -191,7 +191,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     ValidationCacheSizes validation_cache_sizes{};
     ApplyArgsManOptions(*m_node.args, validation_cache_sizes);
     Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
-    Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
 
     m_node.chain = interfaces::MakeChain(m_node);
     static bool noui_connected = false;
diff --git a/src/validation.cpp b/src/validation.cpp
index a9d26daf99..b0d383f131 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -134,6 +134,7 @@ const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locato
 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)
                        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
@@ -394,7 +395,8 @@ void Chainstate::MaybeUpdateMempoolForReorg(
 * */
 static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationState& state,
                 const CCoinsViewCache& view, const CTxMemPool& pool,
-                unsigned int flags, PrecomputedTransactionData& txdata, CCoinsViewCache& coins_tip)
+                unsigned int flags, PrecomputedTransactionData& txdata, CCoinsViewCache& coins_tip,
+                ValidationCache& validation_cache)
                 EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs)
 {
     AssertLockHeld(cs_main);
@@ -426,7 +428,7 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationS
     }
 
     // Call CheckInputScripts() to cache signature and script validity against current tip consensus rules.
-    return CheckInputScripts(tx, state, view, flags, /* cacheSigStore= */ true, /* cacheFullScriptStore= */ true, txdata);
+    return CheckInputScripts(tx, state, view, flags, /* cacheSigStore= */ true, /* cacheFullScriptStore= */ true, txdata, validation_cache);
 }
 
 namespace {
@@ -716,6 +718,11 @@ private:
         return true;
     }
 
+    ValidationCache& GetValidationCache()
+    {
+        return m_active_chainstate.m_chainman.m_validation_cache;
+    }
+
 private:
     CTxMemPool& m_pool;
     CCoinsViewCache m_view;
@@ -1231,13 +1238,13 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws)
 
     // Check input scripts and signatures.
     // This is done last to help prevent CPU exhaustion denial-of-service attacks.
-    if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata)) {
+    if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
         // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
         // need to turn both off, and compare against just turning off CLEANSTACK
         // to see if the failure is specifically due to witness validation.
         TxValidationState state_dummy; // Want reported failures to be from first CheckInputScripts
-        if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata) &&
-                !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata)) {
+        if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache()) &&
+                !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
             // Only the witness is missing, so the transaction itself may be fine.
             state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
                     state.GetRejectReason(), state.GetDebugMessage());
@@ -1273,7 +1280,7 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws)
     // transactions into the mempool can be exploited as a DoS attack.
     unsigned int currentBlockScriptVerifyFlags{GetBlockScriptFlags(*m_active_chainstate.m_chain.Tip(), m_active_chainstate.m_chainman)};
     if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags,
-                                        ws.m_precomputed_txdata, m_active_chainstate.CoinsTip())) {
+                                        ws.m_precomputed_txdata, m_active_chainstate.CoinsTip(), GetValidationCache())) {
         LogPrintf("BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s\n", hash.ToString(), state.ToString());
         return Assume(false);
     }
@@ -2087,10 +2094,9 @@ bool CScriptCheck::operator()() {
     return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *txdata), &error);
 }
 
-static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
-bool InitScriptExecutionCache(size_t max_size_bytes)
+ValidationCache::ValidationCache(const size_t script_execution_cache_bytes)
 {
     // Setup the salted hasher
     uint256 nonce = GetRandHash();
@@ -2100,10 +2106,9 @@ bool InitScriptExecutionCache(size_t max_size_bytes)
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
 
-    const auto [num_elems, approx_size_bytes] = g_scriptExecutionCache.setup_bytes(max_size_bytes);
+    const auto [num_elems, approx_size_bytes] = m_script_execution_cache.setup_bytes(script_execution_cache_bytes);
     LogPrintf("Using %zu MiB out of %zu MiB requested for script execution cache, able to store %zu elements\n",
-              approx_size_bytes >> 20, max_size_bytes >> 20, num_elems);
-    return true;
+              approx_size_bytes >> 20, script_execution_cache_bytes >> 20, num_elems);
 }
 
 /**
@@ -2128,6 +2133,7 @@ bool InitScriptExecutionCache(size_t max_size_bytes)
 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)
 {
     if (tx.IsCoinBase()) return true;
@@ -2145,7 +2151,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     CSHA256 hasher = g_scriptExecutionCacheHasher;
     hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
     AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks
-    if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) {
+    if (validation_cache.m_script_execution_cache.contains(hashCacheEntry, !cacheFullScriptStore)) {
         return true;
     }
 
@@ -2206,7 +2212,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     if (cacheFullScriptStore && !pvChecks) {
         // We executed all of the provided scripts, and were told to
         // cache the result. Do so now.
-        g_scriptExecutionCache.insert(hashCacheEntry);
+        validation_cache.m_script_execution_cache.insert(hashCacheEntry);
     }
 
     return true;
@@ -2664,7 +2670,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], parallel_script_checks ? &vChecks : nullptr)) {
+            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());
@@ -6226,7 +6232,8 @@ ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Opt
     : m_script_check_queue{/*batch_size=*/128, options.worker_threads_num},
       m_interrupt{interrupt},
       m_options{Flatten(std::move(options))},
-      m_blockman{interrupt, std::move(blockman_options)}
+      m_blockman{interrupt, std::move(blockman_options)},
+      m_validation_cache{m_options.script_execution_cache_bytes}
 {
 }
 
diff --git a/src/validation.h b/src/validation.h
index ab7891539a..efe7661b5f 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -10,9 +10,10 @@
 #include <attributes.h>
 #include <chain.h>
 #include <checkqueue.h>
-#include <kernel/chain.h>
 #include <consensus/amount.h>
+#include <cuckoocache.h>
 #include <deploymentstatus.h>
+#include <kernel/chain.h>
 #include <kernel/chainparams.h>
 #include <kernel/chainstatemanager_opts.h>
 #include <kernel/cs_main.h> // IWYU pragma: export
@@ -360,8 +361,19 @@ static_assert(std::is_nothrow_move_assignable_v<CScriptCheck>);
 static_assert(std::is_nothrow_move_constructible_v<CScriptCheck>);
 static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
 
-/** Initializes the script-execution cache */
-[[nodiscard]] bool InitScriptExecutionCache(size_t max_size_bytes);
+/**
+ * Convenience class for initializing and passing the script execution cache.
+ */
+class ValidationCache
+{
+public:
+    CuckooCache::cache<uint256, SignatureCacheHasher> m_script_execution_cache;
+
+    ValidationCache(size_t script_execution_cache_bytes);
+
+    ValidationCache(const ValidationCache&) = delete;
+    ValidationCache& operator=(const ValidationCache&) = delete;
+};
 
 /** Functions for validating blocks and updating the block tree */
 
@@ -796,7 +808,6 @@ private:
     friend ChainstateManager;
 };
 
-
 enum class SnapshotCompletionResult {
     SUCCESS,
     SKIPPED,
@@ -970,6 +981,8 @@ public:
     //! chainstate to avoid duplicating block metadata.
     node::BlockManager m_blockman;
 
+    ValidationCache m_validation_cache;
+
     /**
      * Whether initial block download has ended and IsInitialBlockDownload
      * should return false from now on.
-- 
cgit v1.2.3


From 021d38822c0e6a1b9497bcb20401c5c37e1bb84d Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sat, 18 May 2024 21:06:16 +0200
Subject: kernel: De-globalize script execution cache hasher

Move it to the ChainstateManager class.
---
 src/validation.cpp | 8 +++-----
 src/validation.h   | 7 +++++++
 2 files changed, 10 insertions(+), 5 deletions(-)

(limited to 'src')

diff --git a/src/validation.cpp b/src/validation.cpp
index b0d383f131..86dd11e41e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2094,8 +2094,6 @@ bool CScriptCheck::operator()() {
     return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *txdata), &error);
 }
 
-static CSHA256 g_scriptExecutionCacheHasher;
-
 ValidationCache::ValidationCache(const size_t script_execution_cache_bytes)
 {
     // Setup the salted hasher
@@ -2103,8 +2101,8 @@ ValidationCache::ValidationCache(const size_t script_execution_cache_bytes)
     // We want the nonce to be 64 bytes long to force the hasher to process
     // this chunk, which makes later hash computations more efficient. We
     // just write our 32-byte entropy twice to fill the 64 bytes.
-    g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
-    g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
+    m_script_execution_cache_hasher.Write(nonce.begin(), 32);
+    m_script_execution_cache_hasher.Write(nonce.begin(), 32);
 
     const auto [num_elems, approx_size_bytes] = m_script_execution_cache.setup_bytes(script_execution_cache_bytes);
     LogPrintf("Using %zu MiB out of %zu MiB requested for script execution cache, able to store %zu elements\n",
@@ -2148,7 +2146,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     // properly commits to the scriptPubKey in the inputs view of that
     // transaction).
     uint256 hashCacheEntry;
-    CSHA256 hasher = g_scriptExecutionCacheHasher;
+    CSHA256 hasher = validation_cache.ScriptExecutionCacheHasher();
     hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
     AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks
     if (validation_cache.m_script_execution_cache.contains(hashCacheEntry, !cacheFullScriptStore)) {
diff --git a/src/validation.h b/src/validation.h
index efe7661b5f..d1a3223832 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -366,6 +366,10 @@ static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
  */
 class ValidationCache
 {
+private:
+    //! Pre-initialized hasher to avoid having to recreate it for every hash calculation.
+    CSHA256 m_script_execution_cache_hasher;
+
 public:
     CuckooCache::cache<uint256, SignatureCacheHasher> m_script_execution_cache;
 
@@ -373,6 +377,9 @@ public:
 
     ValidationCache(const ValidationCache&) = delete;
     ValidationCache& operator=(const ValidationCache&) = delete;
+
+    //! Return a copy of the pre-initialized hasher.
+    CSHA256 ScriptExecutionCacheHasher() const { return m_script_execution_cache_hasher; }
 };
 
 /** Functions for validating blocks and updating the block tree */
-- 
cgit v1.2.3


From 66d74bfc45ae0f743084475ac3bbfb4355bb6ec2 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sat, 18 May 2024 21:39:48 +0200
Subject: Expose CSignatureCache class in header

This is done in preparation for the following commit. Also rename it to
SignatureCache.
---
 src/script/sigcache.cpp | 104 +++++++++++++++++++-----------------------------
 src/script/sigcache.h   |  40 +++++++++++++++++++
 2 files changed, 81 insertions(+), 63 deletions(-)

(limited to 'src')

diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 666601d174..64d092f555 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -5,82 +5,61 @@
 
 #include <script/sigcache.h>
 
-#include <common/system.h>
+#include <crypto/sha256.h>
 #include <logging.h>
 #include <pubkey.h>
 #include <random.h>
+#include <script/interpreter.h>
+#include <span.h>
 #include <uint256.h>
 
-#include <cuckoocache.h>
-
-#include <algorithm>
 #include <mutex>
 #include <shared_mutex>
 #include <vector>
 
-namespace {
-/**
- * Valid signature cache, to avoid doing expensive ECDSA signature checking
- * twice for every transaction (once when accepted into memory pool, and
- * again when accepted into the block chain)
- */
-class CSignatureCache
+SignatureCache::SignatureCache()
 {
-private:
-     //! Entries are SHA256(nonce || 'E' or 'S' || 31 zero bytes || signature hash || public key || signature):
-    CSHA256 m_salted_hasher_ecdsa;
-    CSHA256 m_salted_hasher_schnorr;
-    typedef CuckooCache::cache<uint256, SignatureCacheHasher> map_type;
-    map_type setValid;
-    std::shared_mutex cs_sigcache;
+    uint256 nonce = GetRandHash();
+    // We want the nonce to be 64 bytes long to force the hasher to process
+    // this chunk, which makes later hash computations more efficient. We
+    // just write our 32-byte entropy, and then pad with 'E' for ECDSA and
+    // 'S' for Schnorr (followed by 0 bytes).
+    static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
+    static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
+    m_salted_hasher_ecdsa.Write(nonce.begin(), 32);
+    m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
+    m_salted_hasher_schnorr.Write(nonce.begin(), 32);
+    m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
+}
 
-public:
-    CSignatureCache()
-    {
-        uint256 nonce = GetRandHash();
-        // We want the nonce to be 64 bytes long to force the hasher to process
-        // this chunk, which makes later hash computations more efficient. We
-        // just write our 32-byte entropy, and then pad with 'E' for ECDSA and
-        // 'S' for Schnorr (followed by 0 bytes).
-        static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
-        static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
-        m_salted_hasher_ecdsa.Write(nonce.begin(), 32);
-        m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
-        m_salted_hasher_schnorr.Write(nonce.begin(), 32);
-        m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
-    }
+void SignatureCache::ComputeEntryECDSA(uint256& entry, const uint256& hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const
+{
+    CSHA256 hasher = m_salted_hasher_ecdsa;
+    hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(vchSig.data(), vchSig.size()).Finalize(entry.begin());
+}
 
-    void
-    ComputeEntryECDSA(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const
-    {
-        CSHA256 hasher = m_salted_hasher_ecdsa;
-        hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(vchSig.data(), vchSig.size()).Finalize(entry.begin());
-    }
+void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const
+{
+    CSHA256 hasher = m_salted_hasher_schnorr;
+    hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(sig.data(), sig.size()).Finalize(entry.begin());
+}
 
-    void
-    ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const
-    {
-        CSHA256 hasher = m_salted_hasher_schnorr;
-        hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(sig.data(), sig.size()).Finalize(entry.begin());
-    }
+bool SignatureCache::Get(const uint256& entry, const bool erase)
+{
+    std::shared_lock<std::shared_mutex> lock(cs_sigcache);
+    return setValid.contains(entry, erase);
+}
 
-    bool
-    Get(const uint256& entry, const bool erase)
-    {
-        std::shared_lock<std::shared_mutex> lock(cs_sigcache);
-        return setValid.contains(entry, erase);
-    }
+void SignatureCache::Set(const uint256& entry)
+{
+    std::unique_lock<std::shared_mutex> lock(cs_sigcache);
+    setValid.insert(entry);
+}
 
-    void Set(const uint256& entry)
-    {
-        std::unique_lock<std::shared_mutex> lock(cs_sigcache);
-        setValid.insert(entry);
-    }
-    std::pair<uint32_t, size_t> setup_bytes(size_t n)
-    {
-        return setValid.setup_bytes(n);
-    }
-};
+std::pair<uint32_t, size_t> SignatureCache::setup_bytes(size_t n)
+{
+    return setValid.setup_bytes(n);
+}
 
 /* In previous versions of this code, signatureCache was a local static variable
  * in CachingTransactionSignatureChecker::VerifySignature.  We initialize
@@ -88,8 +67,7 @@ public:
  * call overhead associated with local static variables even though
  * signatureCache could be made local to VerifySignature.
 */
-static CSignatureCache signatureCache;
-} // namespace
+static SignatureCache signatureCache;
 
 // To be called once in AppInitMain/BasicTestingSetup to initialize the
 // signatureCache.
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index d2723b47d2..8b1c4519ef 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -6,12 +6,23 @@
 #ifndef BITCOIN_SCRIPT_SIGCACHE_H
 #define BITCOIN_SCRIPT_SIGCACHE_H
 
+#include <consensus/amount.h>
+#include <crypto/sha256.h>
+#include <cuckoocache.h>
 #include <script/interpreter.h>
 #include <span.h>
+#include <uint256.h>
 #include <util/hasher.h>
 
+#include <cstddef>
+#include <cstdint>
+#include <shared_mutex>
+#include <utility>
 #include <vector>
 
+class CTransaction;
+class XOnlyPubKey;
+
 // DoS prevention: limit cache size to 32MiB (over 1000000 entries on 64-bit
 // systems). Due to how we count cache size, actual memory usage is slightly
 // more (~32.25 MiB)
@@ -20,6 +31,35 @@ static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_MAX_SIG_CAC
 
 class CPubKey;
 
+/**
+ * Valid signature cache, to avoid doing expensive ECDSA signature checking
+ * twice for every transaction (once when accepted into memory pool, and
+ * again when accepted into the block chain)
+ */
+class SignatureCache
+{
+private:
+    //! Entries are SHA256(nonce || 'E' or 'S' || 31 zero bytes || signature hash || public key || signature):
+    CSHA256 m_salted_hasher_ecdsa;
+    CSHA256 m_salted_hasher_schnorr;
+    typedef CuckooCache::cache<uint256, SignatureCacheHasher> map_type;
+    map_type setValid;
+    std::shared_mutex cs_sigcache;
+
+public:
+    SignatureCache();
+
+    void ComputeEntryECDSA(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const;
+
+    void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const;
+
+    bool Get(const uint256& entry, const bool erase);
+
+    void Set(const uint256& entry);
+
+    std::pair<uint32_t, size_t> setup_bytes(size_t n);
+};
+
 class CachingTransactionSignatureChecker : public TransactionSignatureChecker
 {
 private:
-- 
cgit v1.2.3


From 606a7ab862470413ced400aa68a94fd37c8ad3d3 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sat, 18 May 2024 11:18:44 +0200
Subject: kernel: De-globalize signature cache

Move its ownership to the ChainstateManager class.

Next to simplifying usage of the kernel library by no longer requiring
manual setup of the cache prior to using validation code, it also slims
down the amount of memory allocated by BasicTestingSetup.

Use this opportunity to make SignatureCache RAII styled

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/Makefile.am                     |  3 ---
 src/bitcoin-chainstate.cpp          |  7 -------
 src/init.cpp                        |  9 +-------
 src/kernel/chainstatemanager_opts.h |  1 +
 src/kernel/validation_cache_sizes.h | 19 -----------------
 src/node/chainstatemanager_args.cpp |  1 +
 src/node/validation_cache_args.cpp  | 33 -----------------------------
 src/node/validation_cache_args.h    | 17 ---------------
 src/script/sigcache.cpp             | 41 ++++++++++---------------------------
 src/script/sigcache.h               | 23 ++++++++++-----------
 src/test/fuzz/script_sigcache.cpp   |  5 ++++-
 src/test/script_p2sh_tests.cpp      |  3 ++-
 src/test/script_tests.cpp           |  9 ++++----
 src/test/transaction_tests.cpp      |  5 ++++-
 src/test/util/setup_common.cpp      |  8 --------
 src/validation.cpp                  | 11 +++++-----
 src/validation.h                    | 12 +++++++----
 17 files changed, 54 insertions(+), 153 deletions(-)
 delete mode 100644 src/kernel/validation_cache_sizes.h
 delete mode 100644 src/node/validation_cache_args.cpp
 delete mode 100644 src/node/validation_cache_args.h

(limited to 'src')

diff --git a/src/Makefile.am b/src/Makefile.am
index 52d4aae893..c1c9b17c9d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -195,7 +195,6 @@ BITCOIN_CORE_H = \
   kernel/mempool_removal_reason.h \
   kernel/messagestartchars.h \
   kernel/notifications_interface.h \
-  kernel/validation_cache_sizes.h \
   kernel/warning.h \
   key.h \
   key_io.h \
@@ -240,7 +239,6 @@ BITCOIN_CORE_H = \
   node/txreconciliation.h \
   node/types.h \
   node/utxo_snapshot.h \
-  node/validation_cache_args.h \
   node/warnings.h \
   noui.h \
   outputtype.h \
@@ -445,7 +443,6 @@ libbitcoin_node_a_SOURCES = \
   node/transaction.cpp \
   node/txreconciliation.cpp \
   node/utxo_snapshot.cpp \
-  node/validation_cache_args.cpp \
   node/warnings.cpp \
   noui.cpp \
   policy/v3_policy.cpp \
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index d11d793d21..98af162b4d 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -15,7 +15,6 @@
 #include <kernel/chainstatemanager_opts.h>
 #include <kernel/checks.h>
 #include <kernel/context.h>
-#include <kernel/validation_cache_sizes.h>
 #include <kernel/warning.h>
 
 #include <consensus/validation.h>
@@ -63,12 +62,6 @@ int main(int argc, char* argv[])
     // properly
     assert(kernel::SanityChecks(kernel_context));
 
-    // Necessary for CheckInputScripts (eventually called by ProcessNewBlock),
-    // which will try the script cache first and fall back to actually
-    // performing the check with the signature cache.
-    kernel::ValidationCacheSizes validation_cache_sizes{};
-    Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
-
     ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};
 
     class KernelNotifications : public kernel::Notifications
diff --git a/src/init.cpp b/src/init.cpp
index 5accc63f37..c368c9c3dc 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -8,7 +8,6 @@
 #include <init.h>
 
 #include <kernel/checks.h>
-#include <kernel/validation_cache_sizes.h>
 
 #include <addrman.h>
 #include <banman.h>
@@ -54,7 +53,6 @@
 #include <node/mempool_persist_args.h>
 #include <node/miner.h>
 #include <node/peerman_args.h>
-#include <node/validation_cache_args.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
 #include <policy/fees_args.h>
@@ -119,7 +117,6 @@
 using common::AmountErrMsg;
 using common::InvalidPortErrMsg;
 using common::ResolveErrMsg;
-using kernel::ValidationCacheSizes;
 
 using node::ApplyArgsManOptions;
 using node::BlockManager;
@@ -619,7 +616,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-test=<option>", "Pass a test-only option. Options include : " + Join(TEST_OPTIONS_DOC, ", ") + ".", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
-    argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
+    argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_VALIDATION_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-maxtipage=<n>",
                    strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)",
                              Ticks<std::chrono::seconds>(DEFAULT_MAX_TIP_AGE)),
@@ -1154,10 +1151,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   args.GetArg("-datadir", ""), fs::PathToString(fs::current_path()));
     }
 
-    ValidationCacheSizes validation_cache_sizes{};
-    ApplyArgsManOptions(args, validation_cache_sizes);
-    (void)InitSignatureCache(validation_cache_sizes.signature_cache_bytes);
-
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
     auto& scheduler = *node.scheduler;
diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h
index 1b08eeeca7..1b605f3d55 100644
--- a/src/kernel/chainstatemanager_opts.h
+++ b/src/kernel/chainstatemanager_opts.h
@@ -50,6 +50,7 @@ struct ChainstateManagerOpts {
     //! Number of script check worker threads. Zero means no parallel verification.
     int worker_threads_num{0};
     size_t script_execution_cache_bytes{DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES};
+    size_t signature_cache_bytes{DEFAULT_SIGNATURE_CACHE_BYTES};
 };
 
 } // namespace kernel
diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h
deleted file mode 100644
index 1774d34226..0000000000
--- a/src/kernel/validation_cache_sizes.h
+++ /dev/null
@@ -1,19 +0,0 @@
-// Copyright (c) 2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H
-#define BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H
-
-#include <script/sigcache.h>
-
-#include <cstddef>
-#include <limits>
-
-namespace kernel {
-struct ValidationCacheSizes {
-    size_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
-};
-}
-
-#endif // BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H
diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp
index 14dd8cd97b..39b5f3ad3e 100644
--- a/src/node/chainstatemanager_args.cpp
+++ b/src/node/chainstatemanager_args.cpp
@@ -63,6 +63,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
         // 2. Multiply first, divide after to avoid integer truncation.
         size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
         opts.script_execution_cache_bytes = clamped_size_each;
+        opts.signature_cache_bytes = clamped_size_each;
     }
 
     return {};
diff --git a/src/node/validation_cache_args.cpp b/src/node/validation_cache_args.cpp
deleted file mode 100644
index b491dd9b37..0000000000
--- a/src/node/validation_cache_args.cpp
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright (c) 2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#include <node/validation_cache_args.h>
-
-#include <kernel/validation_cache_sizes.h>
-
-#include <common/args.h>
-
-#include <algorithm>
-#include <cstddef>
-#include <cstdint>
-#include <memory>
-#include <optional>
-
-using kernel::ValidationCacheSizes;
-
-namespace node {
-void ApplyArgsManOptions(const ArgsManager& argsman, ValidationCacheSizes& cache_sizes)
-{
-    if (auto max_size = argsman.GetIntArg("-maxsigcachesize")) {
-        // 1. When supplied with a max_size of 0, both InitSignatureCache and
-        //    InitScriptExecutionCache create the minimum possible cache (2
-        //    elements). Therefore, we can use 0 as a floor here.
-        // 2. Multiply first, divide after to avoid integer truncation.
-        size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
-        cache_sizes = {
-            .signature_cache_bytes = clamped_size_each,
-        };
-    }
-}
-} // namespace node
diff --git a/src/node/validation_cache_args.h b/src/node/validation_cache_args.h
deleted file mode 100644
index f447c13b49..0000000000
--- a/src/node/validation_cache_args.h
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright (c) 2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_NODE_VALIDATION_CACHE_ARGS_H
-#define BITCOIN_NODE_VALIDATION_CACHE_ARGS_H
-
-class ArgsManager;
-namespace kernel {
-struct ValidationCacheSizes;
-};
-
-namespace node {
-void ApplyArgsManOptions(const ArgsManager& argsman, kernel::ValidationCacheSizes& cache_sizes);
-} // namespace node
-
-#endif // BITCOIN_NODE_VALIDATION_CACHE_ARGS_H
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 64d092f555..33531e6bf5 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -17,7 +17,7 @@
 #include <shared_mutex>
 #include <vector>
 
-SignatureCache::SignatureCache()
+SignatureCache::SignatureCache(const size_t max_size_bytes)
 {
     uint256 nonce = GetRandHash();
     // We want the nonce to be 64 bytes long to force the hasher to process
@@ -30,6 +30,10 @@ SignatureCache::SignatureCache()
     m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
     m_salted_hasher_schnorr.Write(nonce.begin(), 32);
     m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
+
+    const auto [num_elems, approx_size_bytes] = setValid.setup_bytes(max_size_bytes);
+    LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
+              approx_size_bytes >> 20, max_size_bytes >> 20, num_elems);
 }
 
 void SignatureCache::ComputeEntryECDSA(uint256& entry, const uint256& hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const
@@ -56,48 +60,25 @@ void SignatureCache::Set(const uint256& entry)
     setValid.insert(entry);
 }
 
-std::pair<uint32_t, size_t> SignatureCache::setup_bytes(size_t n)
-{
-    return setValid.setup_bytes(n);
-}
-
-/* In previous versions of this code, signatureCache was a local static variable
- * in CachingTransactionSignatureChecker::VerifySignature.  We initialize
- * signatureCache outside of VerifySignature to avoid the atomic operation per
- * call overhead associated with local static variables even though
- * signatureCache could be made local to VerifySignature.
-*/
-static SignatureCache signatureCache;
-
-// To be called once in AppInitMain/BasicTestingSetup to initialize the
-// signatureCache.
-bool InitSignatureCache(size_t max_size_bytes)
-{
-    const auto [num_elems, approx_size_bytes] = signatureCache.setup_bytes(max_size_bytes);
-    LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
-              approx_size_bytes >> 20, max_size_bytes >> 20, num_elems);
-    return true;
-}
-
 bool CachingTransactionSignatureChecker::VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
 {
     uint256 entry;
-    signatureCache.ComputeEntryECDSA(entry, sighash, vchSig, pubkey);
-    if (signatureCache.Get(entry, !store))
+    m_signature_cache.ComputeEntryECDSA(entry, sighash, vchSig, pubkey);
+    if (m_signature_cache.Get(entry, !store))
         return true;
     if (!TransactionSignatureChecker::VerifyECDSASignature(vchSig, pubkey, sighash))
         return false;
     if (store)
-        signatureCache.Set(entry);
+        m_signature_cache.Set(entry);
     return true;
 }
 
 bool CachingTransactionSignatureChecker::VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const
 {
     uint256 entry;
-    signatureCache.ComputeEntrySchnorr(entry, sighash, sig, pubkey);
-    if (signatureCache.Get(entry, !store)) return true;
+    m_signature_cache.ComputeEntrySchnorr(entry, sighash, sig, pubkey);
+    if (m_signature_cache.Get(entry, !store)) return true;
     if (!TransactionSignatureChecker::VerifySchnorrSignature(sig, pubkey, sighash)) return false;
-    if (store) signatureCache.Set(entry);
+    if (store) m_signature_cache.Set(entry);
     return true;
 }
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index 8b1c4519ef..76802e6a7c 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -15,21 +15,20 @@
 #include <util/hasher.h>
 
 #include <cstddef>
-#include <cstdint>
 #include <shared_mutex>
-#include <utility>
 #include <vector>
 
+class CPubKey;
 class CTransaction;
 class XOnlyPubKey;
 
 // DoS prevention: limit cache size to 32MiB (over 1000000 entries on 64-bit
 // systems). Due to how we count cache size, actual memory usage is slightly
 // more (~32.25 MiB)
-static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
-static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
-
-class CPubKey;
+static constexpr size_t DEFAULT_VALIDATION_CACHE_BYTES{32 << 20};
+static constexpr size_t DEFAULT_SIGNATURE_CACHE_BYTES{DEFAULT_VALIDATION_CACHE_BYTES / 2};
+static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_VALIDATION_CACHE_BYTES / 2};
+static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES + DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES);
 
 /**
  * Valid signature cache, to avoid doing expensive ECDSA signature checking
@@ -47,7 +46,10 @@ private:
     std::shared_mutex cs_sigcache;
 
 public:
-    SignatureCache();
+    SignatureCache(size_t max_size_bytes);
+
+    SignatureCache(const SignatureCache&) = delete;
+    SignatureCache& operator=(const SignatureCache&) = delete;
 
     void ComputeEntryECDSA(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const;
 
@@ -56,22 +58,19 @@ public:
     bool Get(const uint256& entry, const bool erase);
 
     void Set(const uint256& entry);
-
-    std::pair<uint32_t, size_t> setup_bytes(size_t n);
 };
 
 class CachingTransactionSignatureChecker : public TransactionSignatureChecker
 {
 private:
     bool store;
+    SignatureCache& m_signature_cache;
 
 public:
-    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn) {}
+    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, SignatureCache& signature_cache, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn), m_signature_cache(signature_cache)  {}
 
     bool VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override;
     bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
 };
 
-[[nodiscard]] bool InitSignatureCache(size_t max_size_bytes);
-
 #endif // BITCOIN_SCRIPT_SIGCACHE_H
diff --git a/src/test/fuzz/script_sigcache.cpp b/src/test/fuzz/script_sigcache.cpp
index 5fdbc9e106..3248ebc4af 100644
--- a/src/test/fuzz/script_sigcache.cpp
+++ b/src/test/fuzz/script_sigcache.cpp
@@ -18,12 +18,15 @@
 
 namespace {
 const BasicTestingSetup* g_setup;
+SignatureCache* g_signature_cache;
 } // namespace
 
 void initialize_script_sigcache()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
+    static SignatureCache signature_cache{DEFAULT_SIGNATURE_CACHE_BYTES};
     g_setup = testing_setup.get();
+    g_signature_cache = &signature_cache;
 }
 
 FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
@@ -36,7 +39,7 @@ FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
     const CAmount amount = ConsumeMoney(fuzzed_data_provider);
     const bool store = fuzzed_data_provider.ConsumeBool();
     PrecomputedTransactionData tx_data;
-    CachingTransactionSignatureChecker caching_transaction_signature_checker{mutable_transaction ? &tx : nullptr, n_in, amount, store, tx_data};
+    CachingTransactionSignatureChecker caching_transaction_signature_checker{mutable_transaction ? &tx : nullptr, n_in, amount, store, *g_signature_cache, tx_data};
     if (fuzzed_data_provider.ConsumeBool()) {
         const auto random_bytes = fuzzed_data_provider.ConsumeBytes<unsigned char>(64);
         const XOnlyPubKey pub_key(ConsumeUInt256(fuzzed_data_provider));
diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp
index 54dcc218b9..f91203cc48 100644
--- a/src/test/script_p2sh_tests.cpp
+++ b/src/test/script_p2sh_tests.cpp
@@ -113,13 +113,14 @@ BOOST_AUTO_TEST_CASE(sign)
     }
     // All of the above should be OK, and the txTos have valid signatures
     // Check to make sure signature verification fails if we use the wrong ScriptSig:
+    SignatureCache signature_cache{DEFAULT_SIGNATURE_CACHE_BYTES};
     for (int i = 0; i < 8; i++) {
         PrecomputedTransactionData txdata(txTo[i]);
         for (int j = 0; j < 8; j++)
         {
             CScript sigSave = txTo[i].vin[0].scriptSig;
             txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig;
-            bool sigOK = CScriptCheck(txFrom.vout[txTo[i].vin[0].prevout.n], CTransaction(txTo[i]), 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)();
+            bool sigOK = CScriptCheck(txFrom.vout[txTo[i].vin[0].prevout.n], CTransaction(txTo[i]), signature_cache, 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)();
             if (i == j)
                 BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j));
             else
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 39b53295e7..0d309469ef 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -1526,7 +1526,7 @@ static std::vector<unsigned int> AllConsensusFlags()
 /** Precomputed list of all valid combinations of consensus-relevant script validation flags. */
 static const std::vector<unsigned int> ALL_CONSENSUS_FLAGS = AllConsensusFlags();
 
-static void AssetTest(const UniValue& test)
+static void AssetTest(const UniValue& test, SignatureCache& signature_cache)
 {
     BOOST_CHECK(test.isObject());
 
@@ -1543,7 +1543,7 @@ static void AssetTest(const UniValue& test)
         CTransaction tx(mtx);
         PrecomputedTransactionData txdata;
         txdata.Init(tx, std::vector<CTxOut>(prevouts));
-        CachingTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, true, txdata);
+        CachingTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, true, signature_cache, txdata);
 
         for (const auto flags : ALL_CONSENSUS_FLAGS) {
             // "final": true tests are valid for all flags. Others are only valid with flags that are
@@ -1561,7 +1561,7 @@ static void AssetTest(const UniValue& test)
         CTransaction tx(mtx);
         PrecomputedTransactionData txdata;
         txdata.Init(tx, std::vector<CTxOut>(prevouts));
-        CachingTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, true, txdata);
+        CachingTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, true, signature_cache, txdata);
 
         for (const auto flags : ALL_CONSENSUS_FLAGS) {
             // If a test is supposed to fail with test_flags, it should also fail with any superset thereof.
@@ -1577,6 +1577,7 @@ BOOST_AUTO_TEST_CASE(script_assets_test)
 {
     // See src/test/fuzz/script_assets_test_minimizer.cpp for information on how to generate
     // the script_assets_test.json file used by this test.
+    SignatureCache signature_cache{DEFAULT_SIGNATURE_CACHE_BYTES};
 
     const char* dir = std::getenv("DIR_UNIT_TEST_DATA");
     BOOST_WARN_MESSAGE(dir != nullptr, "Variable DIR_UNIT_TEST_DATA unset, skipping script_assets_test");
@@ -1597,7 +1598,7 @@ BOOST_AUTO_TEST_CASE(script_assets_test)
     BOOST_CHECK(tests.size() > 0);
 
     for (size_t i = 0; i < tests.size(); i++) {
-        AssetTest(tests[i]);
+        AssetTest(tests[i], signature_cache);
     }
     file.close();
 }
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 34176626f0..a7fda5865c 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -17,6 +17,7 @@
 #include <policy/settings.h>
 #include <script/script.h>
 #include <script/script_error.h>
+#include <script/sigcache.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <script/solver.h>
@@ -578,9 +579,11 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
         coins.emplace_back(std::move(coin));
     }
 
+    SignatureCache signature_cache{DEFAULT_SIGNATURE_CACHE_BYTES};
+
     for(uint32_t i = 0; i < mtx.vin.size(); i++) {
         std::vector<CScriptCheck> vChecks;
-        vChecks.emplace_back(coins[tx.vin[i].prevout.n].out, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata);
+        vChecks.emplace_back(coins[tx.vin[i].prevout.n].out, tx, signature_cache, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata);
         control.Add(std::move(vChecks));
     }
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 7408fd8ec4..3bf1f76019 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -6,8 +6,6 @@
 
 #include <test/util/setup_common.h>
 
-#include <kernel/validation_cache_sizes.h>
-
 #include <addrman.h>
 #include <banman.h>
 #include <chainparams.h>
@@ -30,7 +28,6 @@
 #include <node/mempool_args.h>
 #include <node/miner.h>
 #include <node/peerman_args.h>
-#include <node/validation_cache_args.h>
 #include <node/warnings.h>
 #include <noui.h>
 #include <policy/fees.h>
@@ -68,7 +65,6 @@
 #include <stdexcept>
 
 using kernel::BlockTreeDB;
-using kernel::ValidationCacheSizes;
 using node::ApplyArgsManOptions;
 using node::BlockAssembler;
 using node::BlockManager;
@@ -188,10 +184,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     m_node.ecc_context = std::make_unique<ECC_Context>();
     SetupEnvironment();
 
-    ValidationCacheSizes validation_cache_sizes{};
-    ApplyArgsManOptions(*m_node.args, validation_cache_sizes);
-    Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
-
     m_node.chain = interfaces::MakeChain(m_node);
     static bool noui_connected = false;
     if (!noui_connected) {
diff --git a/src/validation.cpp b/src/validation.cpp
index 86dd11e41e..21d3366086 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2091,10 +2091,11 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
 bool CScriptCheck::operator()() {
     const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
     const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness;
-    return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *txdata), &error);
+    return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata), &error);
 }
 
-ValidationCache::ValidationCache(const size_t script_execution_cache_bytes)
+ValidationCache::ValidationCache(const size_t script_execution_cache_bytes, const size_t signature_cache_bytes)
+    : m_signature_cache{signature_cache_bytes}
 {
     // Setup the salted hasher
     uint256 nonce = GetRandHash();
@@ -2176,7 +2177,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, i, flags, cacheSigStore, &txdata);
+        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()) {
@@ -2189,7 +2190,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
                 // splitting the network between upgraded and
                 // non-upgraded nodes by banning CONSENSUS-failing
                 // data providers.
-                CScriptCheck check2(txdata.m_spent_outputs[i], tx, i,
+                CScriptCheck check2(txdata.m_spent_outputs[i], tx, validation_cache.m_signature_cache, i,
                         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())));
@@ -6231,7 +6232,7 @@ ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Opt
       m_interrupt{interrupt},
       m_options{Flatten(std::move(options))},
       m_blockman{interrupt, std::move(blockman_options)},
-      m_validation_cache{m_options.script_execution_cache_bytes}
+      m_validation_cache{m_options.script_execution_cache_bytes, m_options.signature_cache_bytes}
 {
 }
 
diff --git a/src/validation.h b/src/validation.h
index d1a3223832..142b5c93f0 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -22,6 +22,7 @@
 #include <policy/packages.h>
 #include <policy/policy.h>
 #include <script/script_error.h>
+#include <script/sigcache.h>
 #include <sync.h>
 #include <txdb.h>
 #include <txmempool.h> // For CTxMemPool::cs
@@ -341,10 +342,11 @@ private:
     bool cacheStore;
     ScriptError error{SCRIPT_ERR_UNKNOWN_ERROR};
     PrecomputedTransactionData *txdata;
+    SignatureCache* m_signature_cache;
 
 public:
-    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
-        m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn) { }
+    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;
@@ -362,7 +364,8 @@ static_assert(std::is_nothrow_move_constructible_v<CScriptCheck>);
 static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
 
 /**
- * Convenience class for initializing and passing the script execution cache.
+ * Convenience class for initializing and passing the script execution cache
+ * and signature cache.
  */
 class ValidationCache
 {
@@ -372,8 +375,9 @@ private:
 
 public:
     CuckooCache::cache<uint256, SignatureCacheHasher> m_script_execution_cache;
+    SignatureCache m_signature_cache;
 
-    ValidationCache(size_t script_execution_cache_bytes);
+    ValidationCache(size_t script_execution_cache_bytes, size_t signature_cache_bytes);
 
     ValidationCache(const ValidationCache&) = delete;
     ValidationCache& operator=(const ValidationCache&) = delete;
-- 
cgit v1.2.3