aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/Makefile.bench.include1
-rw-r--r--src/bench/checkblockindex.cpp20
-rw-r--r--src/init.cpp2
-rw-r--r--src/kernel/chainstatemanager_opts.h2
-rw-r--r--src/node/chainstatemanager_args.cpp5
-rw-r--r--src/test/util/setup_common.cpp2
-rw-r--r--src/validation.cpp58
-rw-r--r--src/validation.h2
8 files changed, 73 insertions, 19 deletions
diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 7ba0111fa6..2ba72c9e76 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -23,6 +23,7 @@ bench_bench_bitcoin_SOURCES = \
bench/ccoins_caching.cpp \
bench/chacha20.cpp \
bench/checkblock.cpp \
+ bench/checkblockindex.cpp \
bench/checkqueue.cpp \
bench/crypto_hash.cpp \
bench/data.cpp \
diff --git a/src/bench/checkblockindex.cpp b/src/bench/checkblockindex.cpp
new file mode 100644
index 0000000000..e8a848dbd4
--- /dev/null
+++ b/src/bench/checkblockindex.cpp
@@ -0,0 +1,20 @@
+// Copyright (c) 2023-present 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 <bench/bench.h>
+#include <test/util/setup_common.h>
+#include <validation.h>
+
+static void CheckBlockIndex(benchmark::Bench& bench)
+{
+ auto testing_setup{MakeNoLogFileContext<TestChain100Setup>()};
+ // Mine some more blocks
+ testing_setup->mineBlocks(1000);
+ bench.run([&] {
+ testing_setup->m_node.chainman->CheckBlockIndex();
+ });
+}
+
+
+BENCHMARK(CheckBlockIndex, benchmark::PriorityLevel::HIGH);
diff --git a/src/init.cpp b/src/init.cpp
index 91de9551ce..75202c85d3 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -598,7 +598,7 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-checkblocks=<n>", strprintf("How many blocks to check at startup (default: %u, 0 = all)", DEFAULT_CHECKBLOCKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checklevel=<n>", strprintf("How thorough the block verification of -checkblocks is: %s (0-4, default: %u)", Join(CHECKLEVEL_DOC, ", "), DEFAULT_CHECKLEVEL), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
- argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
+ argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures every <n> operations. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkaddrman=<n>", strprintf("Run addrman consistency checks every <n> operations. Use 0 to disable. (default: %u)", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkmempool=<n>", strprintf("Run mempool consistency checks every <n> transactions. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkpoints", strprintf("Enable rejection of any forks from the known historical chain until block %s (default: %u)", defaultChainParams->Checkpoints().GetHeight(), DEFAULT_CHECKPOINTS_ENABLED), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h
index de5f78494a..076841c3c9 100644
--- a/src/kernel/chainstatemanager_opts.h
+++ b/src/kernel/chainstatemanager_opts.h
@@ -33,7 +33,7 @@ namespace kernel {
struct ChainstateManagerOpts {
const CChainParams& chainparams;
fs::path datadir;
- std::optional<bool> check_block_index{};
+ std::optional<int32_t> 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{};
diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp
index 1cc126cb05..bc4a815a3e 100644
--- a/src/node/chainstatemanager_args.cpp
+++ b/src/node/chainstatemanager_args.cpp
@@ -24,7 +24,10 @@
namespace node {
util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
{
- if (auto value{args.GetBoolArg("-checkblockindex")}) opts.check_block_index = *value;
+ if (auto value{args.GetIntArg("-checkblockindex")}) {
+ // Interpret bare -checkblockindex argument as 1 instead of 0.
+ opts.check_block_index = args.GetArg("-checkblockindex")->empty() ? 1 : *value;
+ }
if (auto value{args.GetBoolArg("-checkpoints")}) opts.checkpoints_enabled = *value;
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 11a76a9c76..9e9db32351 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -238,7 +238,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
const ChainstateManager::Options chainman_opts{
.chainparams = chainparams,
.datadir = m_args.GetDataDirNet(),
- .check_block_index = true,
+ .check_block_index = 1,
.notifications = *m_node.notifications,
.signals = m_node.validation_signals.get(),
.worker_threads_num = 2,
diff --git a/src/validation.cpp b/src/validation.cpp
index aafb5629f4..b5f1d1a426 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5092,6 +5092,14 @@ void ChainstateManager::LoadExternalBlockFile(
LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
}
+bool ChainstateManager::ShouldCheckBlockIndex() const
+{
+ // Assert to verify Flatten() has been called.
+ if (!*Assert(m_options.check_block_index)) return false;
+ if (GetRand(*m_options.check_block_index) >= 1) return false;
+ return true;
+}
+
void ChainstateManager::CheckBlockIndex()
{
if (!ShouldCheckBlockIndex()) {
@@ -5108,19 +5116,28 @@ void ChainstateManager::CheckBlockIndex()
return;
}
- // Build forward-pointing map of the entire block tree.
+ // Build forward-pointing data structure for the entire block tree.
+ // For performance reasons, indexes of the best header chain are stored in a vector (within CChain).
+ // All remaining blocks are stored in a multimap.
+ // The best header chain can differ from the active chain: E.g. its entries may belong to blocks that
+ // are not yet validated.
+ CChain best_hdr_chain;
+ assert(m_best_header);
+ best_hdr_chain.SetTip(*m_best_header);
+
std::multimap<CBlockIndex*,CBlockIndex*> forward;
for (auto& [_, block_index] : m_blockman.m_block_index) {
- forward.emplace(block_index.pprev, &block_index);
+ // Only save indexes in forward that are not part of the best header chain.
+ if (!best_hdr_chain.Contains(&block_index)) {
+ // Only genesis, which must be part of the best header chain, can have a nullptr parent.
+ assert(block_index.pprev);
+ forward.emplace(block_index.pprev, &block_index);
+ }
}
+ assert(forward.size() + best_hdr_chain.Height() + 1 == m_blockman.m_block_index.size());
- assert(forward.size() == m_blockman.m_block_index.size());
-
- std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeGenesis = forward.equal_range(nullptr);
- CBlockIndex *pindex = rangeGenesis.first->second;
- rangeGenesis.first++;
- assert(rangeGenesis.first == rangeGenesis.second); // There is only one index entry with parent nullptr.
-
+ CBlockIndex* pindex = best_hdr_chain[0];
+ assert(pindex);
// Iterate over the entire block tree, using depth-first search.
// Along the way, remember whether there are blocks on the path from genesis
// block being explored which are the first to have certain properties.
@@ -5332,14 +5349,21 @@ void ChainstateManager::CheckBlockIndex()
// assert(pindex->GetBlockHash() == pindex->GetBlockHeader().GetHash()); // Perhaps too slow
// End: actual consistency checks.
- // Try descending into the first subnode.
+
+ // Try descending into the first subnode. Always process forks first and the best header chain after.
snap_update_firsts();
std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> range = forward.equal_range(pindex);
if (range.first != range.second) {
- // A subnode was found.
+ // A subnode not part of the best header chain was found.
pindex = range.first->second;
nHeight++;
continue;
+ } else if (best_hdr_chain.Contains(pindex)) {
+ // Descend further into best header chain.
+ nHeight++;
+ pindex = best_hdr_chain[nHeight];
+ if (!pindex) break; // we are finished, since the best header chain is always processed last
+ continue;
}
// This is a leaf node.
// Move upwards until we reach a node of which we have not yet visited the last child.
@@ -5365,9 +5389,15 @@ void ChainstateManager::CheckBlockIndex()
// Proceed to the next one.
rangePar.first++;
if (rangePar.first != rangePar.second) {
- // Move to the sibling.
+ // Move to a sibling not part of the best header chain.
pindex = rangePar.first->second;
break;
+ } else if (pindexPar == best_hdr_chain[nHeight - 1]) {
+ // Move to pindex's sibling on the best-chain, if it has one.
+ pindex = best_hdr_chain[nHeight];
+ // There will not be a next block if (and only if) parent block is the best header.
+ assert((pindex == nullptr) == (pindexPar == best_hdr_chain.Tip()));
+ break;
} else {
// Move up further.
pindex = pindexPar;
@@ -5377,8 +5407,8 @@ void ChainstateManager::CheckBlockIndex()
}
}
- // Check that we actually traversed the entire map.
- assert(nNodes == forward.size());
+ // Check that we actually traversed the entire block index.
+ assert(nNodes == forward.size() + best_hdr_chain.Height() + 1);
}
std::string Chainstate::ToString()
diff --git a/src/validation.h b/src/validation.h
index ea6b6cad7e..ab7891539a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -938,7 +938,7 @@ public:
const CChainParams& GetParams() const { return m_options.chainparams; }
const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); }
- bool ShouldCheckBlockIndex() const { return *Assert(m_options.check_block_index); }
+ bool ShouldCheckBlockIndex() const;
const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); }
const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
kernel::Notifications& GetNotifications() const { return m_options.notifications; };
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-16 09:44:33 +0000