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: 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.
---
 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 +-
 11 files changed, 109 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

(limited to 'src')

diff --git a/src/Makefile.am b/src/Makefile.am
index 22390ef2bf..576a448a0d 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 9386936b81..fc3f91d492 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 53139924b6..4606b77e9f 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 0000000000..cf92cdbd64
--- /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 0000000000..ed183c24fe
--- /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 0000000000..f447c13b49
--- /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 507754ad7d..8e6971d3f3 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 0e3f900acf..4c1cad38a7 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 ef2c6af660..30d26ecf79 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 73a6e46cc4..7a03adf558 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 92ade4cf94..6e072a49d9 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 */
 
-- 
cgit v1.2.3