diff options
author | Carl Dong <contact@carldong.me> | 2022-06-30 23:47:41 -0400 |
---|---|---|
committer | Carl Dong <contact@carldong.me> | 2022-08-03 12:03:27 -0400 |
commit | 41c5201a90bbc2893333e334e8945759ef24e7dd (patch) | |
tree | 3c8e26fa4b9769b7a2fa4fa75dd059c84d89fad8 | |
parent | 82d3058539f54ebad745e2b02b61df01aa832a54 (diff) |
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.
-rwxr-xr-x | ci/test/06_script_b.sh | 1 | ||||
-rw-r--r-- | src/Makefile.am | 3 | ||||
-rw-r--r-- | src/bitcoin-chainstate.cpp | 6 | ||||
-rw-r--r-- | src/init.cpp | 14 | ||||
-rw-r--r-- | src/kernel/validation_cache_sizes.h | 20 | ||||
-rw-r--r-- | src/node/validation_cache_args.cpp | 28 | ||||
-rw-r--r-- | src/node/validation_cache_args.h | 17 | ||||
-rw-r--r-- | src/script/sigcache.cpp | 8 | ||||
-rw-r--r-- | src/script/sigcache.h | 9 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 13 | ||||
-rw-r--r-- | src/validation.cpp | 9 | ||||
-rw-r--r-- | src/validation.h | 2 |
12 files changed, 110 insertions, 20 deletions
diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 3c02909212..f1b0c1ac15 100755 --- a/ci/test/06_script_b.sh +++ b/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"\ 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 */ |