Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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