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.

Author: dongcarl

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"\

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 \

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

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);

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

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

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

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;
 }
 

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 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker
     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

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;