From 6b824c37dca841006189227bcf7b9a0a24f5bd4f Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 4 Aug 2022 16:40:56 +0100
Subject: [PATCH] Merge bitcoin/bitcoin#25527: [kernel 3c/n] Decouple
 validation cache initialization from `ArgsManager`

0f3a2532c38074dd9789d1c4c667db6ca46ff0ab validationcaches: Use size_t for sizes (Carl Dong)
41c5201a90bbc2893333e334e8945759ef24e7dd validationcaches: Add and use ValidationCacheSizes (Carl Dong)
82d3058539f54ebad745e2b02b61df01aa832a54 cuckoocache: Check for uint32 overflow in setup_bytes (Carl Dong)
b370164b319df1a500b70694b077f92265a777fb validationcaches: Abolish arbitrary limit (Carl Dong)
08dbc6ef72db48168dc03991f5f838dae42c8dfd cuckoocache: Return approximate memory size (Carl Dong)
0dbce4b1034b53d19b88af332385a006098b6d48 tests: Reduce calls to InitS*Cache() (Carl Dong)
---
 src/Makefile.am                      |  3 +++
 src/cuckoocache.h                    | 23 +++++++++++++-----
 src/init.cpp                         | 16 ++++++++++---
 src/kernel/validation_cache_sizes.h  | 20 ++++++++++++++++
 src/node/validation_cache_args.cpp   | 36 ++++++++++++++++++++++++++++
 src/node/validation_cache_args.h     | 17 +++++++++++++
 src/script/sigcache.cpp              | 18 +++++++-------
 src/script/sigcache.h                | 11 ++++-----
 src/test/fuzz/script_sigcache.cpp    | 10 +++++---
 src/test/txvalidationcache_tests.cpp |  5 ----
 src/test/util/setup_common.cpp       | 12 ++++++++--
 src/validation.cpp                   | 17 +++++++------
 src/validation.h                     |  2 +-
 13 files changed, 149 insertions(+), 41 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/src/Makefile.am b/src/Makefile.am
index f76c844973a4..2d3840d1395b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -257,6 +257,7 @@ BITCOIN_CORE_H = \
   instantsend/net_instantsend.h \
   instantsend/signing.h \
   kernel/coinstats.h \
+  kernel/validation_cache_sizes.h \
   key.h \
   key_io.h \
   limitedmap.h \
@@ -314,6 +315,7 @@ BITCOIN_CORE_H = \
   node/txreconciliation.h \
   node/interface_ui.h \
   node/utxo_snapshot.h \
+  node/validation_cache_args.h \
   noui.h \
   outputtype.h \
   policy/feerate.h \
@@ -572,6 +574,7 @@ libbitcoin_node_a_SOURCES = \
   node/transaction.cpp \
   node/txreconciliation.cpp \
   node/interface_ui.cpp \
+  node/validation_cache_args.cpp \
   noui.cpp \
   policy/fees.cpp \
   policy/packages.cpp \
diff --git a/src/cuckoocache.h b/src/cuckoocache.h
index 22c06d89735a..c61bf379d2d2 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>
 
@@ -326,7 +328,7 @@ class cache
     }
 
     /** 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.
      *
@@ -336,8 +338,8 @@ class cache
     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 +359,21 @@ class cache
      *
      * @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 or std::nullopt if the size requested is too large.
      */
-    uint32_t setup_bytes(size_t bytes)
+    std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t bytes)
     {
-        return setup(bytes/sizeof(Element));
+        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);
+        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 bfe1b42ac36e..edfe9277089e 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -10,6 +10,8 @@
 
 #include <init.h>
 
+#include <kernel/validation_cache_sizes.h>
+
 #include <addrman.h>
 #include <banman.h>
 #include <base58.h>
@@ -45,6 +47,7 @@
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <node/txreconciliation.h>
+#include <node/validation_cache_args.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
 #include <policy/policy.h>
@@ -138,7 +141,9 @@
 #endif
 
 using kernel::CoinStatsHashType;
+using kernel::ValidationCacheSizes;
 
+using node::ApplyArgsManOptions;
 using node::CacheSizes;
 using node::CalculateCacheSizes;
 using node::ChainstateLoadingError;
@@ -763,7 +768,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-watchquorums=<n>", strprintf("Watch and validate quorum communication (default: %u)", llmq::DEFAULT_WATCH_QUORUMS), 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("-disablegovernance", strprintf("Disable governance validation (0-1, default: %u)", 0), ArgsManager::ALLOW_ANY, 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("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-minsporkkeys=<n>", "Overrides minimum spork signers to change spork value. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
@@ -1518,8 +1523,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   args.GetArg("-datadir", ""), fs::PathToString(fs::current_path()));
     }
 
-    InitSignatureCache();
-    InitScriptExecutionCache();
+    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);
     if (script_threads <= 0) {
diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h
new file mode 100644
index 000000000000..72e4d1a52cc6
--- /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 <cstddef>
+#include <limits>
+
+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};
+};
+}
+
+#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 000000000000..f6f3577c8d73
--- /dev/null
+++ b/src/node/validation_cache_args.cpp
@@ -0,0 +1,36 @@
+// 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 <script/sigcache.h>
+
+#include <util/system.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 (argsman.IsArgSet("-maxsigcachesize")) {
+        int64_t max_size = argsman.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_BYTES >> 20);
+        // 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,
+            .script_execution_cache_bytes = clamped_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 000000000000..f447c13b49fb
--- /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 1e4b0be66ab4..4ad15c639dbe 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -13,6 +13,7 @@
 #include <cuckoocache.h>
 
 #include <mutex>
+#include <optional>
 #include <shared_mutex>
 #include <vector>
 
@@ -61,7 +62,7 @@ class CSignatureCache
         std::unique_lock<std::shared_mutex> lock(cs_sigcache);
         setValid.insert(entry);
     }
-    uint32_t setup_bytes(size_t n)
+    std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t n)
     {
         return setValid.setup_bytes(n);
     }
@@ -78,14 +79,15 @@ static CSignatureCache signatureCache;
 
 // To be called once in AppInitMain/BasicTestingSetup to initialize the
 // signatureCache.
-void InitSignatureCache()
+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::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);
-    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);
+    auto setup_results = signatureCache.setup_bytes(max_size_bytes);
+    if (!setup_results) return false;
+
+    const auto [num_elems, approx_size_bytes] = *setup_results;
+    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::VerifySignature(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 c9db28621a67..b2d30a1293de 100644
--- a/src/script/sigcache.h
+++ b/src/script/sigcache.h
@@ -9,14 +9,13 @@
 #include <script/interpreter.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;
-// Maximum sig cache size allowed
-static const int64_t MAX_MAX_SIG_CACHE_SIZE = 16384;
+// more (~32.25 MiB)
+static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
 
 class CPubKey;
 
@@ -31,6 +30,6 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker
     bool VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override;
 };
 
-void InitSignatureCache();
+[[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 1a72789e7582..eb469f181de1 100644
--- a/src/test/fuzz/script_sigcache.cpp
+++ b/src/test/fuzz/script_sigcache.cpp
@@ -10,17 +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()
 {
-    ECC_Start();
-    SelectParams(CBaseChainParams::REGTEST);
-    InitSignatureCache();
+    static const auto testing_setup = MakeNoLogFileContext<>();
+    g_setup = testing_setup.get();
 }
 
 FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 62eea17e34a0..bf040fee5e03 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -156,11 +156,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()));
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index c45177f82ba3..6f66a7a4416e 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>
@@ -23,6 +25,7 @@
 #include <node/blockstorage.h>
 #include <node/chainstate.h>
 #include <node/miner.h>
+#include <node/validation_cache_args.h>
 #include <policy/fees.h>
 #include <pow.h>
 #include <rpc/blockchain.h>
@@ -78,6 +81,8 @@
 #include <memory>
 #include <stdexcept>
 
+using kernel::ValidationCacheSizes;
+using node::ApplyArgsManOptions;
 using node::BlockAssembler;
 using node::CalculateCacheSizes;
 using node::DashChainstateSetup;
@@ -187,8 +192,11 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     BLSInit();
     SetupEnvironment();
     SetupNetworking();
-    InitSignatureCache();
-    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);
     m_node.netgroupman = std::make_unique<NetGroupManager>(/*asmap=*/std::vector<bool>());
diff --git a/src/validation.cpp b/src/validation.cpp
index 7686968416c2..0aa824b53cf6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1828,7 +1828,8 @@ bool CScriptCheck::operator()() {
 static CuckooCache::cache<uint256, SignatureCacheHasher> g_scriptExecutionCache;
 static CSHA256 g_scriptExecutionCacheHasher;
 
-void InitScriptExecutionCache() {
+bool InitScriptExecutionCache(size_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
@@ -1836,12 +1837,14 @@ void InitScriptExecutionCache() {
     // 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::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);
-    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);
+
+    auto setup_results = g_scriptExecutionCache.setup_bytes(max_size_bytes);
+    if (!setup_results) return false;
+
+    const auto [num_elems, approx_size_bytes] = *setup_results;
+    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 21a57c25ecf2..aa27823591d6 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -366,7 +366,7 @@ static_assert(std::is_nothrow_move_constructible_v<CScriptCheck>);
 static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
 
 /** Initializes the script-execution cache */
-void InitScriptExecutionCache();
+[[nodiscard]] bool InitScriptExecutionCache(size_t max_size_bytes);
 
 /** Functions for validating blocks and updating the block tree */