Merge bitcoin/bitcoin#30141: kernel: De-globalize validation caches

606a7ab kernel: De-globalize signature cache (TheCharlatan)
66d74bf Expose CSignatureCache class in header (TheCharlatan)
021d388 kernel: De-globalize script execution cache hasher (TheCharlatan)
13a3661 kernel: De-globalize script execution cache (TheCharlatan)
ab14d1d validation: Don't error if maxsigcachesize exceeds uint32::max (TheCharlatan)

Pull request description:

  The validation caches are currently setup independently from where the rest of the validation code is initialized. This makes their ownership semantics unclear. There is also no clear enforcement on when and in what order they need to be initialized. The caches are always initialized in the `BasicTestingSetup` although a number of tests don't actually need them.

  Solve this by moving the caches from global scope into the `ChainstateManager` class. This simplifies the usage of the kernel library by no longer requiring manual setup of the caches prior to using the `ChainstateManager`. Tests that need to access the caches can instantiate them independently.

  ---
  This pull request is part of the [libbitcoinkernel project](bitcoin/bitcoin#27587).

ACKs for top commit:
  stickies-v:
    re-ACK 606a7ab
  glozow:
    reACK 606a7ab
  ryanofsky:
    Code review ACK 606a7ab. Just small formatting, include, and static_assert changes since last review.

Tree-SHA512: e7f3ee41406e3b233832bb67dc3a63c4203b5367e5daeed383df9cb590f227fcc62eae31311029c077d5e81b273a37a88a364db3dee2efe91bb3b9c9ddc8a42e

Author: ryanofsky

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/fees.cpp \

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,13 +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));
-    Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
-
     ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};
 
     class KernelNotifications : public kernel::Notifications

src/cuckoocache.h

@@ -14,7 +14,6 @@
 #include <cstring>
 #include <limits>
 #include <memory>
-#include <optional>
 #include <utility>
 #include <vector>
 
@@ -360,16 +359,15 @@ class cache
      * 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);

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,14 +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);
-    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)));
-    }
-
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
     auto& scheduler = *node.scheduler;

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,8 @@ 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};
+    size_t signature_cache_bytes{DEFAULT_SIGNATURE_CACHE_BYTES};
 };
 
 } // namespace kernel

src/kernel/validation_cache_sizes.h

@@ -56,6 +56,16 @@ 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;
+        opts.signature_cache_bytes = clamped_size_each;
+    }
+
     return {};
 }
 } // namespace node