kernel: De-globalize signature cache

Move its ownership to the ChainstateManager class.

Next to simplifying usage of the kernel library by no longer requiring
manual setup of the cache prior to using validation code, it also slims
down the amount of memory allocated by BasicTestingSetup.

Use this opportunity to make SignatureCache RAII styled

Co-authored-by: Ryan Ofsky <[email protected]>

Author: TheCharlatan

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/v3_policy.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,12 +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));
-
     ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};
 
     class KernelNotifications : public kernel::Notifications

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,10 +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);
-    (void)InitSignatureCache(validation_cache_sizes.signature_cache_bytes);
-
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
     auto& scheduler = *node.scheduler;

src/kernel/chainstatemanager_opts.h

@@ -50,6 +50,7 @@ struct ChainstateManagerOpts {
     //! 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

@@ -63,6 +63,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
         // 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 {};

src/node/chainstatemanager_args.cpp

@@ -17,7 +17,7 @@
 #include <shared_mutex>
 #include <vector>
 
-SignatureCache::SignatureCache()
+SignatureCache::SignatureCache(const size_t max_size_bytes)
 {
     uint256 nonce = GetRandHash();
     // We want the nonce to be 64 bytes long to force the hasher to process
@@ -30,6 +30,10 @@ SignatureCache::SignatureCache()
     m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
     m_salted_hasher_schnorr.Write(nonce.begin(), 32);
     m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
+
+    const auto [num_elems, approx_size_bytes] = setValid.setup_bytes(max_size_bytes);
+    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);
 }
 
 void SignatureCache::ComputeEntryECDSA(uint256& entry, const uint256& hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const
@@ -56,48 +60,25 @@ void SignatureCache::Set(const uint256& entry)
     setValid.insert(entry);
 }
 
-std::pair<uint32_t, size_t> SignatureCache::setup_bytes(size_t n)
-{
-    return setValid.setup_bytes(n);
-}
-
-/* In previous versions of this code, signatureCache was a local static variable
- * in CachingTransactionSignatureChecker::VerifySignature.  We initialize
- * signatureCache outside of VerifySignature to avoid the atomic operation per
- * call overhead associated with local static variables even though
- * signatureCache could be made local to VerifySignature.
-*/
-static SignatureCache signatureCache;
-
-// To be called once in AppInitMain/BasicTestingSetup to initialize the
-// signatureCache.
-bool InitSignatureCache(size_t max_size_bytes)
-{
-    const auto [num_elems, approx_size_bytes] = signatureCache.setup_bytes(max_size_bytes);
-    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::VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
 {
     uint256 entry;
-    signatureCache.ComputeEntryECDSA(entry, sighash, vchSig, pubkey);
-    if (signatureCache.Get(entry, !store))
+    m_signature_cache.ComputeEntryECDSA(entry, sighash, vchSig, pubkey);
+    if (m_signature_cache.Get(entry, !store))
         return true;
     if (!TransactionSignatureChecker::VerifyECDSASignature(vchSig, pubkey, sighash))
         return false;
     if (store)
-        signatureCache.Set(entry);
+        m_signature_cache.Set(entry);
     return true;
 }
 
 bool CachingTransactionSignatureChecker::VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const
 {
     uint256 entry;
-    signatureCache.ComputeEntrySchnorr(entry, sighash, sig, pubkey);
-    if (signatureCache.Get(entry, !store)) return true;
+    m_signature_cache.ComputeEntrySchnorr(entry, sighash, sig, pubkey);
+    if (m_signature_cache.Get(entry, !store)) return true;
     if (!TransactionSignatureChecker::VerifySchnorrSignature(sig, pubkey, sighash)) return false;
-    if (store) signatureCache.Set(entry);
+    if (store) m_signature_cache.Set(entry);
     return true;
 }

src/node/validation_cache_args.cpp

@@ -15,21 +15,20 @@
 #include <util/hasher.h>
 
 #include <cstddef>
-#include <cstdint>
 #include <shared_mutex>
-#include <utility>
 #include <vector>
 
+class CPubKey;
 class CTransaction;
 class XOnlyPubKey;
 
 // 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 MiB)
-static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
-static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
-
-class CPubKey;
+static constexpr size_t DEFAULT_VALIDATION_CACHE_BYTES{32 << 20};
+static constexpr size_t DEFAULT_SIGNATURE_CACHE_BYTES{DEFAULT_VALIDATION_CACHE_BYTES / 2};
+static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_VALIDATION_CACHE_BYTES / 2};
+static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES + DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES);
 
 /**
  * Valid signature cache, to avoid doing expensive ECDSA signature checking
@@ -47,7 +46,10 @@ class SignatureCache
     std::shared_mutex cs_sigcache;
 
 public:
-    SignatureCache();
+    SignatureCache(size_t max_size_bytes);
+
+    SignatureCache(const SignatureCache&) = delete;
+    SignatureCache& operator=(const SignatureCache&) = delete;
 
     void ComputeEntryECDSA(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& vchSig, const CPubKey& pubkey) const;
 
@@ -56,22 +58,19 @@ class SignatureCache
     bool Get(const uint256& entry, const bool erase);
 
     void Set(const uint256& entry);
-
-    std::pair<uint32_t, size_t> setup_bytes(size_t n);
 };
 
 class CachingTransactionSignatureChecker : public TransactionSignatureChecker
 {
 private:
     bool store;
+    SignatureCache& m_signature_cache;
 
 public:
-    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn) {}
+    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, SignatureCache& signature_cache, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn), m_signature_cache(signature_cache)  {}
 
     bool VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override;
     bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
 };
 
-[[nodiscard]] bool InitSignatureCache(size_t max_size_bytes);
-
 #endif // BITCOIN_SCRIPT_SIGCACHE_H