diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -1855,9 +1855,6 @@ // Step 3: parameter-to-internal-flags init::SetLoggingCategories(args); - fCheckBlockIndex = args.GetBoolArg("-checkblockindex", - chainparams.DefaultConsistencyChecks()); - // Configure excessive block size. const int64_t nProposedExcessiveBlockSize = args.GetIntArg("-excessiveblocksize", DEFAULT_MAX_BLOCK_SIZE); diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -29,6 +29,7 @@ const Config &config; const std::function adjusted_time_callback{ nullptr}; + std::optional 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. diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -19,6 +19,10 @@ namespace node { std::optional ApplyArgsManOptions(const ArgsManager &args, ChainstateManager::Options &opts) { + if (auto value{args.GetBoolArg("-checkblockindex")}) { + opts.check_block_index = *value; + } + if (auto value{args.GetBoolArg("-checkpoints")}) { opts.checkpoints_enabled = *value; } diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -63,16 +63,6 @@ malleation(auto_infile, metadata); if (reset_chainstate) { - // Disable CheckBlockIndex until we are finished resetting the - // chainstate. It is enabled again and CheckBlockIndex is - // called manually after the snapshot is activated. - // This is necessary for Bitcoin ABC because, as of D4717, - // CheckBlockIndex is called more aggressively in ActivateBestChain and - // would not support resetting the chainstate while preserving the - // block index. - bool prev_check_block_index = fCheckBlockIndex; - fCheckBlockIndex = false; - { // What follows is code to selectively reset chainstate data without // disturbing the existing BlockManager instance, which is needed to @@ -100,20 +90,22 @@ node.chainman->MaybeRebalanceCaches(); } BlockValidationState state; - if (!node.chainman->ActiveChainstate().ActivateBestChain(state)) { + // Skip checking the block index when calling ActivateBestChain, because + // as of D4717 CheckBlockIndex is called more aggressively and would not + // support resetting the chainstate while preserving the block index. + // We call CheckBlockIndex() explicitly below, after ActivateSnapshot. + if (!node.chainman->ActiveChainstate().ActivateBestChain( + state, /*pblock=*/nullptr, /*skip_checkblockindex=*/true)) { throw std::runtime_error( strprintf("ActivateBestChain failed. (%s)", state.ToString())); } - fCheckBlockIndex = prev_check_block_index; Assert(0 == WITH_LOCK(node.chainman->GetMutex(), return node.chainman->ActiveHeight())); } bool ret = node.chainman->ActivateSnapshot(auto_infile, metadata, in_memory_chainstate); - if (fCheckBlockIndex) { - node.chainman->ActiveChainstate().CheckBlockIndex(); - } + node.chainman->ActiveChainstate().CheckBlockIndex(); return ret; } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -168,7 +168,6 @@ m_node.chain = interfaces::MakeChain(m_node, config.GetChainParams()); g_wallet_init_interface.Construct(m_node); - fCheckBlockIndex = true; static bool noui_connected = false; if (!noui_connected) { noui_connect(); @@ -214,6 +213,7 @@ ChainstateManager::Options chainman_opts{ .config = config, .adjusted_time_callback = GetAdjustedTime, + .check_block_index = true, }; ApplyArgsManOptions(*m_node.args, chainman_opts); m_node.chainman = std::make_unique(chainman_opts); diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -116,7 +116,6 @@ extern std::condition_variable g_best_block_cv; /** Used to notify getblocktemplate RPC of new tips. */ extern uint256 g_best_block; -extern bool fCheckBlockIndex; /** Documentation for argument 'checklevel'. */ extern const std::vector CHECKLEVEL_DOC; @@ -878,10 +877,16 @@ * the background UTXO set to verify the assumeutxo value the snapshot was * activated with. `cs_main` will be held during this time. * + * @param[in] skip_checkblockindex (optional) + * If true, skip calling CheckBlockIndex even if -checkblockindex is + * true. If false (default behavior), respect the -checkblockindex arg. + * This is used in tests when we need to skip the checks only + * temporarily, and resume normal behavior later. * @returns true unless a system error occurred */ bool ActivateBestChain(BlockValidationState &state, - std::shared_ptr pblock = nullptr) + std::shared_ptr pblock = nullptr, + bool skip_checkblockindex = false) EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex, !cs_avalancheFinalizedBlockIndex) LOCKS_EXCLUDED(cs_main); @@ -991,7 +996,7 @@ * Make various assertions about the state of the block index. * * By default this only executes fully when using the Regtest chain; see: - * fCheckBlockIndex. + * m_options.check_block_index. */ void CheckBlockIndex(); @@ -1232,6 +1237,9 @@ const Consensus::Params &GetConsensus() const { return m_options.config.GetChainParams().GetConsensus(); } + bool ShouldCheckBlockIndex() const { + return *Assert(m_options.check_block_index); + } const arith_uint256 &MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -109,7 +109,6 @@ GlobalMutex g_best_block_mutex; std::condition_variable g_best_block_cv; uint256 g_best_block; -bool fCheckBlockIndex = false; BlockValidationOptions::BlockValidationOptions(const Config &config) : excessiveBlockSize(config.GetMaxBlockSize()), checkPoW(true), @@ -3074,7 +3073,8 @@ } bool Chainstate::ActivateBestChain(BlockValidationState &state, - std::shared_ptr pblock) { + std::shared_ptr pblock, + bool skip_checkblockindex) { AssertLockNotHeld(m_chainstate_mutex); // Note that while we're often called here from ProcessNewBlock, this is @@ -3184,7 +3184,9 @@ // we're going to release cs_main soon. If the index is in a bad // state now, then it's better to know immediately rather than // randomly have it cause a problem in a race. - CheckBlockIndex(); + if (!skip_checkblockindex) { + CheckBlockIndex(); + } if (blocks_connected) { const CBlockIndex *pindexFork = m_chain.FindFork(starting_tip); @@ -5315,7 +5317,7 @@ } void Chainstate::CheckBlockIndex() { - if (!fCheckBlockIndex) { + if (!m_chainman.ShouldCheckBlockIndex()) { return; } @@ -6367,6 +6369,11 @@ * operators that accept nullopt, thus ignoring the intended default value. */ static ChainstateManager::Options &&Flatten(ChainstateManager::Options &&opts) { + if (!opts.check_block_index.has_value()) { + opts.check_block_index = + opts.config.GetChainParams().DefaultConsistencyChecks(); + } + if (!opts.minimum_chain_work.has_value()) { opts.minimum_chain_work = UintToArith256( opts.config.GetChainParams().GetConsensus().nMinimumChainWork);