From 0dbce4b1034b53d19b88af332385a006098b6d48 Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Thu, 30 Jun 2022 22:56:34 -0400
Subject: [PATCH 1/6] tests: Reduce calls to InitS*Cache()

In src/test/fuzz/script_sigcache.cpp, we should really be setting up a
full working BasicTestingSetup. The initialize_ function is only run
once anyway.

In src/test/txvalidationcache_tests.cpp, the Dersig100Setup inherits
from BasicTestingSetup, which should have already set up a global script
execution cache without the need to explicitly call
InitScriptExecutionCache.
---
 src/test/fuzz/script_sigcache.cpp    | 11 +++++++----
 src/test/txvalidationcache_tests.cpp |  5 -----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/test/fuzz/script_sigcache.cpp b/src/test/fuzz/script_sigcache.cpp
index f7e45d68897..f6af7947df0 100644
--- a/src/test/fuzz/script_sigcache.cpp
+++ b/src/test/fuzz/script_sigcache.cpp
@@ -10,18 +10,21 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 
 #include <cstdint>
 #include <optional>
 #include <string>
 #include <vector>
 
+namespace {
+const BasicTestingSetup* g_setup;
+} // namespace
+
 void initialize_script_sigcache()
 {
-    static const ECCVerifyHandle ecc_verify_handle;
-    ECC_Start();
-    SelectParams(CBaseChainParams::REGTEST);
-    InitSignatureCache();
+    static const auto testing_setup = MakeNoLogFileContext<>();
+    g_setup = testing_setup.get();
 }
 
 FUZZ_TARGET_INIT(script_sigcache, initialize_script_sigcache)
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index dd4bc5af75f..633f75ff4fa 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -161,11 +161,6 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
 {
     // Test that passing CheckInputScripts with one set of script flags doesn't imply
     // that we would pass again with a different set of flags.
-    {
-        LOCK(cs_main);
-        InitScriptExecutionCache();
-    }
-
     CScript p2pk_scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
     CScript p2sh_scriptPubKey = GetScriptForDestination(ScriptHash(p2pk_scriptPubKey));
     CScript p2pkh_scriptPubKey = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));

From 08dbc6ef72db48168dc03991f5f838dae42c8dfd Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Thu, 30 Jun 2022 23:10:55 -0400
Subject: [PATCH 2/6] cuckoocache: Return approximate memory size

Returning the approximate total size eliminates the need for
InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better
idea of this information.
---
 src/bitcoin-chainstate.cpp     |  4 ++--
 src/cuckoocache.h              | 14 +++++++++-----
 src/init.cpp                   |  5 +++--
 src/script/sigcache.cpp        | 12 ++++++++----
 src/script/sigcache.h          |  2 +-
 src/test/util/setup_common.cpp |  4 ++--
 src/validation.cpp             | 10 +++++++---
 src/validation.h               |  2 +-
 8 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 4656cb23e7c..9386936b811 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -62,8 +62,8 @@ int main(int argc, char* argv[])
     // 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.
-    InitSignatureCache();
-    InitScriptExecutionCache();
+    Assert(InitSignatureCache());
+    Assert(InitScriptExecutionCache());
 
 
     // SETUP: Scheduling and Background Signals
diff --git a/src/cuckoocache.h b/src/cuckoocache.h
index d0dc61c7e62..5fc852439fe 100644
--- a/src/cuckoocache.h
+++ b/src/cuckoocache.h
@@ -336,8 +336,8 @@ public:
     uint32_t setup(uint32_t new_size)
     {
         // depth_limit must be at least one otherwise errors can occur.
-        depth_limit = static_cast<uint8_t>(std::log2(static_cast<float>(std::max((uint32_t)2, new_size))));
         size = std::max<uint32_t>(2, new_size);
+        depth_limit = static_cast<uint8_t>(std::log2(static_cast<float>(size)));
         table.resize(size);
         collection_flags.setup(size);
         epoch_flags.resize(size);
@@ -357,12 +357,16 @@ public:
      *
      * @param bytes the approximate number of bytes to use for this data
      * structure
-     * @returns the maximum number of elements storable (see setup()
-     * documentation for more detail)
+     * @returns A pair of the maximum number of elements storable (see setup()
+     * documentation for more detail) and the approxmiate total size of these
+     * elements in bytes.
      */
-    uint32_t setup_bytes(size_t bytes)
+    std::pair<uint32_t, size_t> setup_bytes(size_t bytes)
     {
-        return setup(bytes/sizeof(Element));
+        auto num_elems = setup(bytes/sizeof(Element));
+
+        size_t approx_size_bytes = num_elems * sizeof(Element);
+        return std::make_pair(num_elems, approx_size_bytes);
     }
 
     /** insert loops at most depth_limit times trying to insert a hash
diff --git a/src/init.cpp b/src/init.cpp
index e3d13ad972a..53139924b6b 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1115,8 +1115,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   args.GetArg("-datadir", ""), fs::PathToString(fs::current_path()));
     }
 
-    InitSignatureCache();
-    InitScriptExecutionCache();
+    if (!InitSignatureCache() || !InitScriptExecutionCache()) {
+        return InitError(strprintf(_("Unable to allocate memory for -maxsigcachesize: '%s' MiB"), args.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE)));
+    }
 
     int script_threads = args.GetIntArg("-par", DEFAULT_SCRIPTCHECK_THREADS);
     if (script_threads <= 0) {
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 4e2acc463a9..7cee55a4312 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -75,7 +75,7 @@ public:
         std::unique_lock<std::shared_mutex> lock(cs_sigcache);
         setValid.insert(entry);
     }
-    uint32_t setup_bytes(size_t n)
+    std::pair<uint32_t, size_t> setup_bytes(size_t n)
     {
         return setValid.setup_bytes(n);
     }
@@ -92,14 +92,18 @@ static CSignatureCache signatureCache;
 
 // To be called once in AppInitMain/BasicTestingSetup to initialize the
 // signatureCache.
-void InitSignatureCache()
+bool InitSignatureCache()
 {
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
     size_t nMaxCacheSize = std::min(std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2), MAX_MAX_SIG_CACHE_SIZE) * ((size_t) 1 << 20);
-    size_t nElems = signatureCache.setup_bytes(nMaxCacheSize);
+
+    auto setup_results = signatureCache.setup_bytes(nMaxCacheSize);
+
+    const auto [num_elems, approx_size_bytes] = setup_results;
     LogPrintf("Using %zu MiB out of %zu/2 requested for signature cache, able to store %zu elements\n",
-            (nElems*sizeof(uint256)) >>20, (nMaxCacheSize*2)>>20, nElems);
+              approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
+    return true;
 }
 
 bool CachingTransactionSignatureChecker::VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index 1e21d6f3402..edcba2543c8 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -33,6 +33,6 @@ public:
     bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
 };
 
-void InitSignatureCache();
+[[nodiscard]] bool InitSignatureCache();
 
 #endif // BITCOIN_SCRIPT_SIGCACHE_H
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index dc6e000c65a..ef2c6af6601 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -133,8 +133,8 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     m_node.kernel = std::make_unique<kernel::Context>();
     SetupEnvironment();
     SetupNetworking();
-    InitSignatureCache();
-    InitScriptExecutionCache();
+    Assert(InitSignatureCache());
+    Assert(InitScriptExecutionCache());
     m_node.chain = interfaces::MakeChain(m_node);
     fCheckBlockIndex = true;
     static bool noui_connected = false;
diff --git a/src/validation.cpp b/src/validation.cpp
index d018d185e91..d0c84ba3175 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1656,7 +1656,7 @@ bool CScriptCheck::operator()() {
 static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
-void InitScriptExecutionCache() {
+bool InitScriptExecutionCache() {
     // Setup the salted hasher
     uint256 nonce = GetRandHash();
     // We want the nonce to be 64 bytes long to force the hasher to process
@@ -1667,9 +1667,13 @@ void InitScriptExecutionCache() {
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
     size_t nMaxCacheSize = std::min(std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2), MAX_MAX_SIG_CACHE_SIZE) * ((size_t) 1 << 20);
-    size_t nElems = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
+
+    auto setup_results = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
+
+    const auto [num_elems, approx_size_bytes] = setup_results;
     LogPrintf("Using %zu MiB out of %zu/2 requested for script execution cache, able to store %zu elements\n",
-            (nElems*sizeof(uint256)) >>20, (nMaxCacheSize*2)>>20, nElems);
+              approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
+    return true;
 }
 
 /**
diff --git a/src/validation.h b/src/validation.h
index df9146852f8..92ade4cf943 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -323,7 +323,7 @@ public:
 };
 
 /** Initializes the script-execution cache */
-void InitScriptExecutionCache();
+[[nodiscard]] bool InitScriptExecutionCache();
 
 /** Functions for validating blocks and updating the block tree */
 

From b370164b319df1a500b70694b077f92265a777fb Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Mon, 25 Jul 2022 16:07:56 -0400
Subject: [PATCH 3/6] validationcaches: Abolish arbitrary limit

1. -maxsigcachesize is a DEBUG_ONLY option

2. Almost 7 years has passed since its semantics change in
   830e3f3d027ba5c8121eed0f6a9ce99961352572 from "number of entries" to
   "number of mebibytes"

3. A std::new_handler was added to the codebase after the original PR
   which introduced this limit, which will terminate immediately instead
   of causing trouble by being caught somewhere unexpected.
---
 src/script/sigcache.cpp | 2 +-
 src/script/sigcache.h   | 2 --
 src/validation.cpp      | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 7cee55a4312..43b594f3cb6 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -96,7 +96,7 @@ bool InitSignatureCache()
 {
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::min(std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2), MAX_MAX_SIG_CACHE_SIZE) * ((size_t) 1 << 20);
+    size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
 
     auto setup_results = signatureCache.setup_bytes(nMaxCacheSize);
 
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index edcba2543c8..0e3f900acf5 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -16,8 +16,6 @@
 // systems). Due to how we count cache size, actual memory usage is slightly
 // more (~32.25 MB)
 static const unsigned int DEFAULT_MAX_SIG_CACHE_SIZE = 32;
-// Maximum sig cache size allowed
-static const int64_t MAX_MAX_SIG_CACHE_SIZE = 16384;
 
 class CPubKey;
 
diff --git a/src/validation.cpp b/src/validation.cpp
index d0c84ba3175..4d174a4b166 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1666,7 +1666,7 @@ bool InitScriptExecutionCache() {
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::min(std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2), MAX_MAX_SIG_CACHE_SIZE) * ((size_t) 1 << 20);
+    size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
 
     auto setup_results = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
 

From 82d3058539f54ebad745e2b02b61df01aa832a54 Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Fri, 1 Jul 2022 19:53:04 -0400
Subject: [PATCH 4/6] cuckoocache: Check for uint32 overflow in setup_bytes

This fixes an potential overflow which existed prior to this patchset.

If CuckooCache::cache<Element, Hash>::setup_bytes is called with a
`size_t bytes` which, when divided by sizeof(Element), does not fit into
an uint32_t, the implicit conversion to uint32_t in the call to setup
will result in an overflow.

At least on x86_64, this overflow is possible:

static_assert(std::numeric_limits<size_t>::max() / 32 <= std::numeric_limits<uint32_t>::max());
static_assert(std::numeric_limits<size_t>::max() / 4 <= std::numeric_limits<uint32_t>::max());

This commit detects such cases and signals to callers that the `size_t
bytes` input is too large.
---
 src/cuckoocache.h       | 11 +++++++++--
 src/script/sigcache.cpp |  6 ++++--
 src/validation.cpp      |  3 ++-
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/cuckoocache.h b/src/cuckoocache.h
index 5fc852439fe..b5da87136d9 100644
--- a/src/cuckoocache.h
+++ b/src/cuckoocache.h
@@ -12,7 +12,9 @@
 #include <atomic>
 #include <cmath>
 #include <cstring>
+#include <limits>
 #include <memory>
+#include <optional>
 #include <utility>
 #include <vector>
 
@@ -359,10 +361,15 @@ public:
      * structure
      * @returns A pair of the maximum number of elements storable (see setup()
      * documentation for more detail) and the approxmiate total size of these
-     * elements in bytes.
+     * elements in bytes or std::nullopt if the size requested is too large.
      */
-    std::pair<uint32_t, size_t> setup_bytes(size_t bytes)
+    std::optional<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;
+        }
+
         auto num_elems = setup(bytes/sizeof(Element));
 
         size_t approx_size_bytes = num_elems * sizeof(Element);
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 43b594f3cb6..507754ad7d9 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -14,6 +14,7 @@
 
 #include <algorithm>
 #include <mutex>
+#include <optional>
 #include <shared_mutex>
 #include <vector>
 
@@ -75,7 +76,7 @@ public:
         std::unique_lock<std::shared_mutex> lock(cs_sigcache);
         setValid.insert(entry);
     }
-    std::pair<uint32_t, size_t> setup_bytes(size_t n)
+    std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t n)
     {
         return setValid.setup_bytes(n);
     }
@@ -99,8 +100,9 @@ bool InitSignatureCache()
     size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
 
     auto setup_results = signatureCache.setup_bytes(nMaxCacheSize);
+    if (!setup_results) return false;
 
-    const auto [num_elems, approx_size_bytes] = setup_results;
+    const auto [num_elems, approx_size_bytes] = *setup_results;
     LogPrintf("Using %zu MiB out of %zu/2 requested for signature cache, able to store %zu elements\n",
               approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
     return true;
diff --git a/src/validation.cpp b/src/validation.cpp
index 4d174a4b166..73a6e46cc49 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1669,8 +1669,9 @@ bool InitScriptExecutionCache() {
     size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
 
     auto setup_results = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
+    if (!setup_results) return false;
 
-    const auto [num_elems, approx_size_bytes] = setup_results;
+    const auto [num_elems, approx_size_bytes] = *setup_results;
     LogPrintf("Using %zu MiB out of %zu/2 requested for script execution cache, able to store %zu elements\n",
               approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
     return true;

From 41c5201a90bbc2893333e334e8945759ef24e7dd Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Thu, 30 Jun 2022 23:47:41 -0400
Subject: [PATCH 5/6] validationcaches: Add and use ValidationCacheSizes

Also:

- Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr
  DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer
  arithmetic overflow checking available to constexpr.
- Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES.
- Pass in max_size_bytes parameter to InitS*Cache(), modify log line to
  no longer allude to maxsigcachesize being split evenly between the two
  validation caches.
- Fix possible integer truncation and add a comment.

[META] I've kept the integer types as int64_t in order to not introduce
       unintended behaviour changes, in the next commit we will make
       them size_t.
---
 ci/test/06_script_b.sh              |  1 +
 src/Makefile.am                     |  3 +++
 src/bitcoin-chainstate.cpp          |  6 ++++--
 src/init.cpp                        | 14 +++++++++++---
 src/kernel/validation_cache_sizes.h | 20 ++++++++++++++++++++
 src/node/validation_cache_args.cpp  | 28 ++++++++++++++++++++++++++++
 src/node/validation_cache_args.h    | 17 +++++++++++++++++
 src/script/sigcache.cpp             |  8 ++++----
 src/script/sigcache.h               |  9 +++++----
 src/test/util/setup_common.cpp      | 13 +++++++++++--
 src/validation.cpp                  |  9 +++++----
 src/validation.h                    |  2 +-
 12 files changed, 110 insertions(+), 20 deletions(-)
 create mode 100644 src/kernel/validation_cache_sizes.h
 create mode 100644 src/node/validation_cache_args.cpp
 create mode 100644 src/node/validation_cache_args.h

diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
index 3c02909212d..f1b0c1ac159 100755
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@@ -46,6 +46,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
           " src/kernel"\
           " src/node/chainstate.cpp"\
           " src/node/mempool_args.cpp"\
+          " src/node/validation_cache_args.cpp"\
           " src/policy/feerate.cpp"\
           " src/policy/packages.cpp"\
           " src/policy/settings.cpp"\
diff --git a/src/Makefile.am b/src/Makefile.am
index 22390ef2bf5..576a448a0d4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -177,6 +177,7 @@ BITCOIN_CORE_H = \
   kernel/mempool_limits.h \
   kernel/mempool_options.h \
   kernel/mempool_persist.h \
+  kernel/validation_cache_sizes.h \
   key.h \
   key_io.h \
   logging.h \
@@ -207,6 +208,7 @@ BITCOIN_CORE_H = \
   node/psbt.h \
   node/transaction.h \
   node/utxo_snapshot.h \
+  node/validation_cache_args.h \
   noui.h \
   outputtype.h \
   policy/feerate.h \
@@ -390,6 +392,7 @@ libbitcoin_node_a_SOURCES = \
   node/minisketchwrapper.cpp \
   node/psbt.cpp \
   node/transaction.cpp \
+  node/validation_cache_args.cpp \
   noui.cpp \
   policy/fees.cpp \
   policy/fees_args.cpp \
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 9386936b811..fc3f91d4924 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -13,6 +13,7 @@
 
 #include <kernel/checks.h>
 #include <kernel/context.h>
+#include <kernel/validation_cache_sizes.h>
 
 #include <chainparams.h>
 #include <consensus/validation.h>
@@ -62,8 +63,9 @@ int main(int argc, char* argv[])
     // 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.
-    Assert(InitSignatureCache());
-    Assert(InitScriptExecutionCache());
+    kernel::ValidationCacheSizes validation_cache_sizes{};
+    Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
+    Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
 
 
     // SETUP: Scheduling and Background Signals
diff --git a/src/init.cpp b/src/init.cpp
index 53139924b6b..4606b77e9ff 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -11,6 +11,7 @@
 
 #include <kernel/checks.h>
 #include <kernel/mempool_persist.h>
+#include <kernel/validation_cache_sizes.h>
 
 #include <addrman.h>
 #include <banman.h>
@@ -44,6 +45,7 @@
 #include <node/mempool_args.h>
 #include <node/mempool_persist_args.h>
 #include <node/miner.h>
+#include <node/validation_cache_args.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
 #include <policy/fees_args.h>
@@ -105,7 +107,9 @@
 #endif
 
 using kernel::DumpMempool;
+using kernel::ValidationCacheSizes;
 
+using node::ApplyArgsManOptions;
 using node::CacheSizes;
 using node::CalculateCacheSizes;
 using node::DEFAULT_PERSIST_MEMPOOL;
@@ -548,7 +552,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-addrmantest", "Allows to test address relay on localhost", 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_SIZE), 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("-maxtipage=<n>", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-printpriority", strprintf("Log transaction fee rate in " + CURRENCY_UNIT + "/kvB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-uacomment=<cmt>", "Append comment to the user agent string", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
@@ -1115,8 +1119,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   args.GetArg("-datadir", ""), fs::PathToString(fs::current_path()));
     }
 
-    if (!InitSignatureCache() || !InitScriptExecutionCache()) {
-        return InitError(strprintf(_("Unable to allocate memory for -maxsigcachesize: '%s' MiB"), args.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE)));
+    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)));
     }
 
     int script_threads = args.GetIntArg("-par", DEFAULT_SCRIPTCHECK_THREADS);
diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h
new file mode 100644
index 00000000000..cf92cdbd64f
--- /dev/null
+++ b/src/kernel/validation_cache_sizes.h
@@ -0,0 +1,20 @@
+// 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 <cstdint>
+#include <limits>
+
+namespace kernel {
+struct ValidationCacheSizes {
+    int64_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
+    int64_t script_execution_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
+};
+}
+
+#endif // BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H
diff --git a/src/node/validation_cache_args.cpp b/src/node/validation_cache_args.cpp
new file mode 100644
index 00000000000..ed183c24fe6
--- /dev/null
+++ b/src/node/validation_cache_args.cpp
@@ -0,0 +1,28 @@
+// 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 <util/system.h>
+
+#include <memory>
+#include <optional>
+
+using kernel::ValidationCacheSizes;
+
+namespace node {
+void ApplyArgsManOptions(const ArgsManager& argsman, ValidationCacheSizes& cache_sizes)
+{
+    if (auto max_size = argsman.GetIntArg("-maxsigcachesize")) {
+        // Multiply first, divide after to avoid integer truncation
+        int64_t size_each = *max_size * (1 << 20) / 2;
+        cache_sizes = {
+            .signature_cache_bytes = size_each,
+            .script_execution_cache_bytes = size_each,
+        };
+    }
+}
+} // namespace node
diff --git a/src/node/validation_cache_args.h b/src/node/validation_cache_args.h
new file mode 100644
index 00000000000..f447c13b49f
--- /dev/null
+++ b/src/node/validation_cache_args.h
@@ -0,0 +1,17 @@
+// 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 507754ad7d9..8e6971d3f37 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -93,18 +93,18 @@ static CSignatureCache signatureCache;
 
 // To be called once in AppInitMain/BasicTestingSetup to initialize the
 // signatureCache.
-bool InitSignatureCache()
+bool InitSignatureCache(int64_t max_size_bytes)
 {
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
+    size_t nMaxCacheSize = std::max<int64_t>(max_size_bytes, 0);
 
     auto setup_results = signatureCache.setup_bytes(nMaxCacheSize);
     if (!setup_results) return false;
 
     const auto [num_elems, approx_size_bytes] = *setup_results;
-    LogPrintf("Using %zu MiB out of %zu/2 requested for signature cache, able to store %zu elements\n",
-              approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
+    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 0e3f900acf5..4c1cad38a71 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -10,12 +10,13 @@
 #include <span.h>
 #include <util/hasher.h>
 
+#include <optional>
 #include <vector>
 
-// DoS prevention: limit cache size to 32MB (over 1000000 entries on 64-bit
+// 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 MB)
-static const unsigned int DEFAULT_MAX_SIG_CACHE_SIZE = 32;
+// more (~32.25 MiB)
+static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
 
 class CPubKey;
 
@@ -31,6 +32,6 @@ public:
     bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
 };
 
-[[nodiscard]] bool InitSignatureCache();
+[[nodiscard]] bool InitSignatureCache(int64_t max_size_bytes);
 
 #endif // BITCOIN_SCRIPT_SIGCACHE_H
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index ef2c6af6601..30d26ecf798 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -4,6 +4,8 @@
 
 #include <test/util/setup_common.h>
 
+#include <kernel/validation_cache_sizes.h>
+
 #include <addrman.h>
 #include <banman.h>
 #include <chainparams.h>
@@ -21,6 +23,7 @@
 #include <node/context.h>
 #include <node/mempool_args.h>
 #include <node/miner.h>
+#include <node/validation_cache_args.h>
 #include <noui.h>
 #include <policy/fees.h>
 #include <policy/fees_args.h>
@@ -52,6 +55,8 @@
 #include <functional>
 #include <stdexcept>
 
+using kernel::ValidationCacheSizes;
+using node::ApplyArgsManOptions;
 using node::BlockAssembler;
 using node::CalculateCacheSizes;
 using node::LoadChainstate;
@@ -133,8 +138,12 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     m_node.kernel = std::make_unique<kernel::Context>();
     SetupEnvironment();
     SetupNetworking();
-    Assert(InitSignatureCache());
-    Assert(InitScriptExecutionCache());
+
+    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);
     fCheckBlockIndex = true;
     static bool noui_connected = false;
diff --git a/src/validation.cpp b/src/validation.cpp
index 73a6e46cc49..7a03adf5582 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1656,7 +1656,8 @@ bool CScriptCheck::operator()() {
 static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
-bool InitScriptExecutionCache() {
+bool InitScriptExecutionCache(int64_t max_size_bytes)
+{
     // Setup the salted hasher
     uint256 nonce = GetRandHash();
     // We want the nonce to be 64 bytes long to force the hasher to process
@@ -1666,14 +1667,14 @@ bool InitScriptExecutionCache() {
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
     // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
     // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::max((int64_t)0, gArgs.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2) * ((size_t) 1 << 20);
+    size_t nMaxCacheSize = std::max<int64_t>(max_size_bytes, 0);
 
     auto setup_results = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
     if (!setup_results) return false;
 
     const auto [num_elems, approx_size_bytes] = *setup_results;
-    LogPrintf("Using %zu MiB out of %zu/2 requested for script execution cache, able to store %zu elements\n",
-              approx_size_bytes >> 20, (nMaxCacheSize * 2) >> 20, num_elems);
+    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;
 }
 
diff --git a/src/validation.h b/src/validation.h
index 92ade4cf943..6e072a49d97 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -323,7 +323,7 @@ public:
 };
 
 /** Initializes the script-execution cache */
-[[nodiscard]] bool InitScriptExecutionCache();
+[[nodiscard]] bool InitScriptExecutionCache(int64_t max_size_bytes);
 
 /** Functions for validating blocks and updating the block tree */
 

From 0f3a2532c38074dd9789d1c4c667db6ca46ff0ab Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Fri, 1 Jul 2022 00:08:14 -0400
Subject: [PATCH 6/6] validationcaches: Use size_t for sizes

...also move the 0-clamping logic to ApplyArgsManOptions, where it
   belongs.
---
 src/cuckoocache.h                   |  2 +-
 src/kernel/validation_cache_sizes.h |  6 +++---
 src/node/validation_cache_args.cpp  | 14 ++++++++++----
 src/script/sigcache.cpp             |  8 ++------
 src/script/sigcache.h               |  2 +-
 src/validation.cpp                  |  7 ++-----
 src/validation.h                    |  2 +-
 7 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/cuckoocache.h b/src/cuckoocache.h
index b5da87136d9..61f553806e1 100644
--- a/src/cuckoocache.h
+++ b/src/cuckoocache.h
@@ -328,7 +328,7 @@ public:
     }
 
     /** setup initializes the container to store no more than new_size
-     * elements.
+     * elements and no less than 2 elements.
      *
      * setup should only be called once.
      *
diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h
index cf92cdbd64f..72e4d1a52cc 100644
--- a/src/kernel/validation_cache_sizes.h
+++ b/src/kernel/validation_cache_sizes.h
@@ -7,13 +7,13 @@
 
 #include <script/sigcache.h>
 
-#include <cstdint>
+#include <cstddef>
 #include <limits>
 
 namespace kernel {
 struct ValidationCacheSizes {
-    int64_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
-    int64_t script_execution_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
+    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/validation_cache_args.cpp b/src/node/validation_cache_args.cpp
index ed183c24fe6..5ea0a8ca0a8 100644
--- a/src/node/validation_cache_args.cpp
+++ b/src/node/validation_cache_args.cpp
@@ -8,6 +8,9 @@
 
 #include <util/system.h>
 
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
 #include <memory>
 #include <optional>
 
@@ -17,11 +20,14 @@ namespace node {
 void ApplyArgsManOptions(const ArgsManager& argsman, ValidationCacheSizes& cache_sizes)
 {
     if (auto max_size = argsman.GetIntArg("-maxsigcachesize")) {
-        // Multiply first, divide after to avoid integer truncation
-        int64_t size_each = *max_size * (1 << 20) / 2;
+        // 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 = size_each,
-            .script_execution_cache_bytes = size_each,
+            .signature_cache_bytes = clamped_size_each,
+            .script_execution_cache_bytes = clamped_size_each,
         };
     }
 }
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 8e6971d3f37..e507ec75281 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -93,13 +93,9 @@ static CSignatureCache signatureCache;
 
 // To be called once in AppInitMain/BasicTestingSetup to initialize the
 // signatureCache.
-bool InitSignatureCache(int64_t max_size_bytes)
+bool InitSignatureCache(size_t max_size_bytes)
 {
-    // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
-    // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::max<int64_t>(max_size_bytes, 0);
-
-    auto setup_results = signatureCache.setup_bytes(nMaxCacheSize);
+    auto setup_results = signatureCache.setup_bytes(max_size_bytes);
     if (!setup_results) return false;
 
     const auto [num_elems, approx_size_bytes] = *setup_results;
diff --git a/src/script/sigcache.h b/src/script/sigcache.h
index 4c1cad38a71..290955090d6 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -32,6 +32,6 @@ public:
     bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
 };
 
-[[nodiscard]] bool InitSignatureCache(int64_t max_size_bytes);
+[[nodiscard]] bool InitSignatureCache(size_t max_size_bytes);
 
 #endif // BITCOIN_SCRIPT_SIGCACHE_H
diff --git a/src/validation.cpp b/src/validation.cpp
index 7a03adf5582..c924bb3c349 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1656,7 +1656,7 @@ bool CScriptCheck::operator()() {
 static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
-bool InitScriptExecutionCache(int64_t max_size_bytes)
+bool InitScriptExecutionCache(size_t max_size_bytes)
 {
     // Setup the salted hasher
     uint256 nonce = GetRandHash();
@@ -1665,11 +1665,8 @@ bool InitScriptExecutionCache(int64_t max_size_bytes)
     // just write our 32-byte entropy twice to fill the 64 bytes.
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
     g_scriptExecutionCacheHasher.Write(nonce.begin(), 32);
-    // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
-    // setup_bytes creates the minimum possible cache (2 elements).
-    size_t nMaxCacheSize = std::max<int64_t>(max_size_bytes, 0);
 
-    auto setup_results = g_scriptExecutionCache.setup_bytes(nMaxCacheSize);
+    auto setup_results = g_scriptExecutionCache.setup_bytes(max_size_bytes);
     if (!setup_results) return false;
 
     const auto [num_elems, approx_size_bytes] = *setup_results;
diff --git a/src/validation.h b/src/validation.h
index 6e072a49d97..ec9043ad7a2 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -323,7 +323,7 @@ public:
 };
 
 /** Initializes the script-execution cache */
-[[nodiscard]] bool InitScriptExecutionCache(int64_t max_size_bytes);
+[[nodiscard]] bool InitScriptExecutionCache(size_t max_size_bytes);
 
 /** Functions for validating blocks and updating the block tree */