Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ci/dash/lint-tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ iwyu_tool.py \
"src/init" \
"src/kernel" \
"src/node/chainstate.cpp" \
"src/node/chainstatemanager_args.cpp" \
"src/node/mempool_args.cpp" \
"src/node/validation_cache_args.cpp" \
"src/node/minisketchwrapper.cpp" \
"src/policy/feerate.cpp" \
"src/policy/packages.cpp" \
Expand Down
5 changes: 5 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,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 \
limitedmap.h \
Expand Down Expand Up @@ -342,6 +343,7 @@ BITCOIN_CORE_H = \
node/blockstorage.h \
node/caches.h \
node/chainstate.h \
node/chainstatemanager_args.h \
node/coin.h \
node/connection_types.h \
node/context.h \
Expand All @@ -356,6 +358,7 @@ BITCOIN_CORE_H = \
node/transaction.h \
node/txreconciliation.h \
node/utxo_snapshot.h \
node/validation_cache_args.h \
noui.h \
outputtype.h \
policy/feerate.h \
Expand Down Expand Up @@ -612,6 +615,7 @@ libbitcoin_node_a_SOURCES = \
node/blockstorage.cpp \
node/caches.cpp \
node/chainstate.cpp \
node/chainstatemanager_args.cpp \
node/coin.cpp \
node/connection_types.cpp \
node/context.cpp \
Expand All @@ -627,6 +631,7 @@ libbitcoin_node_a_SOURCES = \
node/transaction.cpp \
node/txreconciliation.cpp \
node/utxo_snapshot.cpp \
node/validation_cache_args.cpp \
noui.cpp \
policy/fees.cpp \
policy/fees_args.cpp \
Expand Down
6 changes: 4 additions & 2 deletions src/bitcoin-chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <kernel/checks.h>
#include <kernel/context.h>
#include <kernel/validation_cache_sizes.h>

#include <chainlock/chainlock.h>
#include <chainparams.h>
Expand Down Expand Up @@ -72,8 +73,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.
InitSignatureCache();
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
Expand Down
3 changes: 2 additions & 1 deletion src/checkqueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,12 @@ class CCheckQueue
WITH_LOCK(m_mutex, m_request_stop = false);
}

bool HasThreads() const { return !m_worker_threads.empty(); }

~CCheckQueue()
{
assert(m_worker_threads.empty());
}

};

/**
Expand Down
23 changes: 17 additions & 6 deletions src/cuckoocache.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
#include <atomic>
#include <cmath>
#include <cstring>
#include <limits>
#include <memory>
#include <optional>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -325,7 +327,7 @@ class cache
}

/** setup initializes the container to store no more than new_size
* elements.
* elements and no less than 2 elements.
*
* setup should only be called once.
*
Expand All @@ -335,8 +337,8 @@ class cache
uint32_t setup(uint32_t new_size)
{
// depth_limit must be at least one otherwise errors can occur.
depth_limit = static_cast<uint8_t>(std::log2(static_cast<float>(std::max((uint32_t)2, new_size))));
size = std::max<uint32_t>(2, new_size);
depth_limit = static_cast<uint8_t>(std::log2(static_cast<float>(size)));
table.resize(size);
collection_flags.setup(size);
epoch_flags.resize(size);
Expand All @@ -356,12 +358,21 @@ class cache
*
* @param bytes the approximate number of bytes to use for this data
* structure
* @returns the maximum number of elements storable (see setup()
* documentation for more detail)
* @returns A pair of the maximum number of elements storable (see setup()
* documentation for more detail) and the approxmiate total size of these
* elements in bytes or std::nullopt if the size requested is too large.
*/
uint32_t setup_bytes(size_t bytes)
std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t bytes)
{
return setup(bytes/sizeof(Element));
size_t requested_num_elems = bytes / sizeof(Element);
if (std::numeric_limits<uint32_t>::max() < requested_num_elems) {
return std::nullopt;
}

auto num_elems = setup(bytes/sizeof(Element));

size_t approx_size_bytes = num_elems * sizeof(Element);
return std::make_pair(num_elems, approx_size_bytes);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/** insert loops at most depth_limit times trying to insert a hash
Expand Down
56 changes: 31 additions & 25 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <kernel/checks.h>
#include <kernel/mempool_persist.h>
#include <kernel/validation_cache_sizes.h>

#include <addrman.h>
#include <banman.h>
Expand Down Expand Up @@ -48,13 +49,15 @@
#include <node/blockstorage.h>
#include <node/caches.h>
#include <node/chainstate.h>
#include <node/chainstatemanager_args.h>
#include <node/context.h>
#include <node/interface_ui.h>
#include <node/mempool_args.h>
#include <node/mempool_persist_args.h>
#include <node/miner.h>
#include <node/sync_manager.h>
#include <node/txreconciliation.h>
#include <node/validation_cache_args.h>
#include <policy/feerate.h>
#include <policy/fees.h>
#include <policy/fees_args.h>
Expand Down Expand Up @@ -155,7 +158,9 @@

using kernel::CoinStatsHashType;
using kernel::DumpMempool;
using kernel::ValidationCacheSizes;

using node::ApplyArgsManOptions;
using node::CacheSizes;
using node::CalculateCacheSizes;
using node::DEFAULT_PERSIST_MEMPOOL;
Expand Down Expand Up @@ -805,8 +810,11 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-watchquorums=<n>", strprintf("Watch and validate quorum communication (default: %u)", llmq::DEFAULT_WATCH_QUORUMS), 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("-disablegovernance", strprintf("Disable governance validation (0-1, default: %u)", 0), ArgsManager::ALLOW_ANY, 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("-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("-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)",
Ticks<std::chrono::seconds>(DEFAULT_MAX_TIP_AGE)),
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("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-pushversion", "Protocol version to report to other nodes", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
Expand Down Expand Up @@ -1300,21 +1308,6 @@ bool AppInitParameterInteraction(const ArgsManager& args)
init::SetLoggingCategories(args);
init::SetLoggingLevel(args);

fCheckBlockIndex = args.GetBoolArg("-checkblockindex", chainparams.DefaultConsistencyChecks());
fCheckpointsEnabled = args.GetBoolArg("-checkpoints", DEFAULT_CHECKPOINTS_ENABLED);

hashAssumeValid = uint256S(args.GetArg("-assumevalid", chainparams.GetConsensus().defaultAssumeValid.GetHex()));

if (args.IsArgSet("-minimumchainwork")) {
const std::string minChainWorkStr = args.GetArg("-minimumchainwork", "");
if (!IsHexNumber(minChainWorkStr)) {
return InitError(strprintf(Untranslated("Invalid non-hex (%s) minimum chain work value specified"), minChainWorkStr));
}
nMinimumChainWork = UintToArith256(uint256S(minChainWorkStr));
} else {
nMinimumChainWork = UintToArith256(chainparams.GetConsensus().nMinimumChainWork);
}

// block pruning; get the amount of disk space (in MiB) to allot for block & undo files
int64_t nPruneArg = args.GetIntArg("-prune", 0);
if (nPruneArg < 0) {
Expand Down Expand Up @@ -1366,8 +1359,6 @@ bool AppInitParameterInteraction(const ArgsManager& args)
if (args.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS))
nLocalServices = ServiceFlags(nLocalServices | NODE_BLOOM);

nMaxTipAge = args.GetIntArg("-maxtipage", DEFAULT_MAX_TIP_AGE);

if (args.GetBoolArg("-reindex-chainstate", false)) {
// indexes that must be deactivated to prevent index corruption, see #24630
if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) {
Expand Down Expand Up @@ -1418,6 +1409,16 @@ bool AppInitParameterInteraction(const ArgsManager& args)
Untranslated("")));
}

// Also report errors from parsing before daemonization
{
ChainstateManager::Options chainman_opts_dummy{
.chainparams = chainparams,
};
if (const auto error{ApplyArgsManOptions(args, chainman_opts_dummy)}) {
return InitError(*error);
}
}

return true;
}

Expand Down Expand Up @@ -1497,8 +1498,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
args.GetArg("-datadir", ""), fs::PathToString(fs::current_path()));
}

InitSignatureCache();
InitScriptExecutionCache();
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);
if (script_threads <= 0) {
Expand All @@ -1515,7 +1521,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

LogPrintf("Script verification uses %d additional threads\n", script_threads);
if (script_threads >= 1) {
g_parallel_script_checks = true;
StartScriptCheckWorkerThreads(script_threads);
}

Expand Down Expand Up @@ -1915,6 +1920,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

fReindex = args.GetBoolArg("-reindex", false);
bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false);
ChainstateManager::Options chainman_opts{
.chainparams = chainparams,
};
Assert(!ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// cache size calculations
CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size());
Expand Down Expand Up @@ -1984,9 +1993,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
return {node::ChainstateLoadStatus::SUCCESS, {}};
});

const ChainstateManager::Options chainman_opts{
.chainparams = chainparams,
};
node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
ChainstateManager& chainman = *node.chainman;

Expand Down
16 changes: 16 additions & 0 deletions src/kernel/chainstatemanager_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@
#ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
#define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H

#include <arith_uint256.h>
#include <uint256.h>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Include the time utilities before using the chrono literal

This header now uses the 6h chrono literal and declares std::chrono::seconds, but it neither includes <chrono> nor imports std::chrono_literals. More importantly, validation.cpp includes validation.h first, and validation.h includes this header before its own <chrono> include, so the normal build fails at DEFAULT_MAX_TIP_AGE with no matching operator""h. Include util/time.h, as the upstream chainstate options header does; it supplies the chrono declarations and literal namespace used throughout this codebase.

Suggested change
#include <uint256.h>
#include <uint256.h>
#include <util/time.h>

source: ['codex']

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — Include the time utilities before using the chrono literal no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

#include <util/time.h>

#include <cstdint>
#include <optional>

class CChainParams;

static constexpr bool DEFAULT_CHECKPOINTS_ENABLED{true};
static constexpr auto DEFAULT_MAX_TIP_AGE{6h}; // ~144 blocks behind -> 2 x fork detection time, was 24h in bitcoin

namespace kernel {

/**
Expand All @@ -18,6 +26,14 @@ namespace kernel {
*/
struct ChainstateManagerOpts {
const CChainParams& chainparams;
std::optional<bool> check_block_index{};
bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED};
//! If set, it will override the minimum work we will assume exists on some valid chain.
std::optional<arith_uint256> minimum_chain_work{};
//! If set, it will override the block hash whose ancestors we will assume to have valid scripts without checking them.
std::optional<uint256> assumed_valid_block{};
//! If the tip is older than this, the node is considered to be in initial block download.
std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE};
};

} // namespace kernel
Expand Down
20 changes: 20 additions & 0 deletions src/kernel/validation_cache_sizes.h
Original file line number Diff line number Diff line change
@@ -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 <cstddef>
#include <limits>

namespace kernel {
struct ValidationCacheSizes {
size_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
size_t script_execution_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
};
}

#endif // BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H
Loading
Loading