diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp index 1378a7b20a5..252cbb163b6 100644 --- a/src/bench/load_external.cpp +++ b/src/bench/load_external.cpp @@ -49,14 +49,13 @@ static void LoadExternalBlockFile(benchmark::Bench& bench) fclose(file); } - Chainstate& chainstate{testing_setup->m_node.chainman->ActiveChainstate()}; std::multimap blocks_with_unknown_parent; FlatFilePos pos; bench.run([&] { // "rb" is "binary, O_RDONLY", positioned to the start of the file. // The file will be closed by LoadExternalBlockFile(). FILE* file{fsbridge::fopen(blkfile, "rb")}; - chainstate.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); + testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); }); fs::remove(blkfile); } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 3ef12471a5f..0d25c798ce3 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -907,7 +907,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector vImportFile break; // This error is logged in OpenBlockFile } LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile); - chainman.ActiveChainstate().LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); + chainman.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); if (chainman.m_interrupt) { LogPrintf("Interrupt requested. Exit %s\n", __func__); return; @@ -926,7 +926,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector vImportFile FILE* file = fsbridge::fopen(path, "rb"); if (file) { LogPrintf("Importing blocks file %s...\n", fs::PathToString(path)); - chainman.ActiveChainstate().LoadExternalBlockFile(file); + chainman.LoadExternalBlockFile(file); if (chainman.m_interrupt) { LogPrintf("Interrupt requested. Exit %s\n", __func__); return; diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 74d6d7231a7..787a196a0ca 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -98,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) LOCK(cs_main); BlockValidationState state; BOOST_CHECK(CheckBlock(block, state, params.GetConsensus())); - BOOST_CHECK(chainstate.AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true)); + BOOST_CHECK(m_node.chainman->AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true)); CCoinsViewCache view(&chainstate.CoinsTip()); BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view)); } diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index f4b7dc08fdd..7f8c9c4e71c 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -35,9 +35,9 @@ FUZZ_TARGET_INIT(load_external_block_file, initialize_load_external_block_file) // Corresponds to the -reindex case (track orphan blocks across files). FlatFilePos flat_file_pos; std::multimap blocks_with_unknown_parent; - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent); + g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent); } else { // Corresponds to the -loadblock= case (orphan blocks aren't tracked across files). - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file); + g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file); } } diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 2078fcd8f8c..3ea87143b0c 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -120,10 +120,11 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) LOCK(::cs_main); bool checked = CheckBlock(*pblock, state, chainparams.GetConsensus()); BOOST_CHECK(checked); - bool accepted = background_cs.AcceptBlock( + bool accepted = chainman.AcceptBlock( pblock, state, &pindex, true, nullptr, &newblock, true); BOOST_CHECK(accepted); } + // UpdateTip is called here bool block_added = background_cs.ActivateBestChain(state, pblock); diff --git a/src/validation.cpp b/src/validation.cpp index ec29abd5217..16cec9198a8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3426,7 +3426,7 @@ void Chainstate::TryAddBlockIndexCandidate(CBlockIndex* pindex) } /** Mark a block as having its data received and checked (up to BLOCK_VALID_TRANSACTIONS). */ -void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) +void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) { AssertLockHeld(cs_main); pindexNew->nTx = block.vtx.size(); @@ -3435,7 +3435,7 @@ void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pin pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; pindexNew->nStatus |= BLOCK_HAVE_DATA; - if (DeploymentActiveAt(*pindexNew, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(*pindexNew, *this, Consensus::DEPLOYMENT_SEGWIT)) { pindexNew->nStatus |= BLOCK_OPT_WITNESS; } pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); @@ -3451,8 +3451,10 @@ void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pin CBlockIndex *pindex = queue.front(); queue.pop_front(); pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; - pindex->nSequenceId = m_chainman.nBlockSequenceId++; - TryAddBlockIndexCandidate(pindex); + pindex->nSequenceId = nBlockSequenceId++; + for (Chainstate *c : GetAll()) { + c->TryAddBlockIndexCandidate(pindex); + } std::pair::iterator, std::multimap::iterator> range = m_blockman.m_blocks_unlinked.equal_range(pindex); while (range.first != range.second) { std::multimap::iterator it = range.first; @@ -3912,7 +3914,7 @@ void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t } /** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */ -bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) +bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) { const CBlock& block = *pblock; @@ -3922,23 +3924,24 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV CBlockIndex *pindexDummy = nullptr; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; - bool accepted_header{m_chainman.AcceptBlockHeader(block, state, &pindex, min_pow_checked)}; - CheckBlockIndex(); + bool accepted_header{AcceptBlockHeader(block, state, &pindex, min_pow_checked)}; + ActiveChainstate().CheckBlockIndex(); if (!accepted_header) return false; - // Try to process all requested blocks that we don't have, but only - // process an unrequested block if it's new and has enough work to - // advance our tip, and isn't too many blocks ahead. + // Check all requested blocks that we do not already have for validity and + // save them to disk. Skip processing of unrequested blocks as an anti-DoS + // measure, unless the blocks have more work than the active chain tip, and + // aren't too far ahead of it, so are likely to be attached soon. bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA; - bool fHasMoreOrSameWork = (m_chain.Tip() ? pindex->nChainWork >= m_chain.Tip()->nChainWork : true); + bool fHasMoreOrSameWork = (ActiveTip() ? pindex->nChainWork >= ActiveTip()->nChainWork : true); // Blocks that are too out-of-order needlessly limit the effectiveness of // pruning, because pruning will not delete block files that contain any // blocks which are too close in height to the tip. Apply this test // regardless of whether pruning is enabled; it should generally be safe to // not process unrequested blocks. - bool fTooFarAhead{pindex->nHeight > m_chain.Height() + int(MIN_BLOCKS_TO_KEEP)}; + bool fTooFarAhead{pindex->nHeight > ActiveHeight() + int(MIN_BLOCKS_TO_KEEP)}; // TODO: Decouple this function from the block download logic by removing fRequested // This requires some new chain data structure to efficiently look up if a @@ -3958,13 +3961,13 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV // If our tip is behind, a peer could try to send us // low-work blocks on a fake chain that we would never // request; don't process these. - if (pindex->nChainWork < m_chainman.MinimumChainWork()) return true; + if (pindex->nChainWork < MinimumChainWork()) return true; } - const CChainParams& params{m_chainman.GetParams()}; + const CChainParams& params{GetParams()}; if (!CheckBlock(block, state, params.GetConsensus()) || - !ContextualCheckBlock(block, state, m_chainman, pindex->pprev)) { + !ContextualCheckBlock(block, state, *this, pindex->pprev)) { if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_blockman.m_dirty_blockindex.insert(pindex); @@ -3974,7 +3977,7 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV // Header is valid/has work, merkle tree and segwit merkle tree are good...RELAY NOW // (but if it does not build on our best tip, let the SendMessages loop relay it) - if (!IsInitialBlockDownload() && m_chain.Tip() == pindex->pprev) + if (!ActiveChainstate().IsInitialBlockDownload() && ActiveTip() == pindex->pprev) GetMainSignals().NewPoWValidBlock(pindex, pblock); // Write block to history file @@ -3987,12 +3990,19 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { - return FatalError(m_chainman.GetNotifications(), state, std::string("System error: ") + e.what()); + return FatalError(GetNotifications(), state, std::string("System error: ") + e.what()); } - FlushStateToDisk(state, FlushStateMode::NONE); + // TODO: FlushStateToDisk() handles flushing of both block and chainstate + // data, so we should move this to ChainstateManager so that we can be more + // intelligent about how we flush. + // For now, since FlushStateMode::NONE is used, all that can happen is that + // the block files may be pruned, so we can just call this on one + // chainstate (particularly if we haven't implemented pruning with + // background validation yet). + ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE); - CheckBlockIndex(); + ActiveChainstate().CheckBlockIndex(); return true; } @@ -4018,7 +4028,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& blo bool ret = CheckBlock(*block, state, GetConsensus()); if (ret) { // Store to disk - ret = ActiveChainstate().AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block, min_pow_checked); + ret = AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block, min_pow_checked); } if (!ret) { GetMainSignals().BlockChecked(*block, state); @@ -4507,7 +4517,7 @@ bool Chainstate::LoadGenesisBlock() return error("%s: writing genesis block to disk failed", __func__); } CBlockIndex* pindex = m_blockman.AddToBlockIndex(block, m_chainman.m_best_header); - ReceivedBlockTransactions(block, pindex, blockPos); + m_chainman.ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { return error("%s: failed to write genesis block: %s", __func__, e.what()); } @@ -4515,18 +4525,16 @@ bool Chainstate::LoadGenesisBlock() return true; } -void Chainstate::LoadExternalBlockFile( +void ChainstateManager::LoadExternalBlockFile( FILE* fileIn, FlatFilePos* dbp, std::multimap* blocks_with_unknown_parent) { - AssertLockNotHeld(m_chainstate_mutex); - // Either both should be specified (-reindex), or neither (-loadblock). assert(!dbp == !blocks_with_unknown_parent); const auto start{SteadyClock::now()}; - const CChainParams& params{m_chainman.GetParams()}; + const CChainParams& params{GetParams()}; int nLoaded = 0; try { @@ -4536,7 +4544,7 @@ void Chainstate::LoadExternalBlockFile( // such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); while (!blkdat.eof()) { - if (m_chainman.m_interrupt) return; + if (m_interrupt) return; blkdat.SetPos(nRewind); nRewind++; // start one byte further next time, in case of failure @@ -4611,8 +4619,15 @@ void Chainstate::LoadExternalBlockFile( // Activate the genesis block so normal node progress can continue if (hash == params.GetConsensus().hashGenesisBlock) { - BlockValidationState state; - if (!ActivateBestChain(state, nullptr)) { + bool genesis_activation_failure = false; + for (auto c : GetAll()) { + BlockValidationState state; + if (!c->ActivateBestChain(state, nullptr)) { + genesis_activation_failure = true; + break; + } + } + if (genesis_activation_failure) { break; } } @@ -4625,14 +4640,21 @@ void Chainstate::LoadExternalBlockFile( // until after all of the block files are loaded. ActivateBestChain can be // called by concurrent network message processing. but, that is not // reliable for the purpose of pruning while importing. - BlockValidationState state; - if (!ActivateBestChain(state, pblock)) { - LogPrint(BCLog::REINDEX, "failed to activate chain (%s)\n", state.ToString()); + bool activation_failure = false; + for (auto c : GetAll()) { + BlockValidationState state; + if (!c->ActivateBestChain(state, pblock)) { + LogPrint(BCLog::REINDEX, "failed to activate chain (%s)\n", state.ToString()); + activation_failure = true; + break; + } + } + if (activation_failure) { break; } } - NotifyHeaderTip(*this); + NotifyHeaderTip(ActiveChainstate()); if (!blocks_with_unknown_parent) continue; @@ -4658,7 +4680,7 @@ void Chainstate::LoadExternalBlockFile( } range.first++; blocks_with_unknown_parent->erase(it); - NotifyHeaderTip(*this); + NotifyHeaderTip(ActiveChainstate()); } } } catch (const std::exception& e) { @@ -4677,7 +4699,7 @@ void Chainstate::LoadExternalBlockFile( } } } catch (const std::runtime_error& e) { - m_chainman.GetNotifications().fatalError(std::string("System error: ") + e.what()); + GetNotifications().fatalError(std::string("System error: ") + e.what()); } LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks(SteadyClock::now() - start)); } diff --git a/src/validation.h b/src/validation.h index c38381aa363..d5544fe3387 100644 --- a/src/validation.h +++ b/src/validation.h @@ -609,37 +609,6 @@ public: bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** - * Import blocks from an external file - * - * During reindexing, this function is called for each block file (datadir/blocks/blk?????.dat). - * It reads all blocks contained in the given file and attempts to process them (add them to the - * block index). The blocks may be out of order within each file and across files. Often this - * function reads a block but finds that its parent hasn't been read yet, so the block can't be - * processed yet. The function will add an entry to the blocks_with_unknown_parent map (which is - * passed as an argument), so that when the block's parent is later read and processed, this - * function can re-read the child block from disk and process it. - * - * Because a block's parent may be in a later file, not just later in the same file, the - * blocks_with_unknown_parent map must be passed in and out with each call. It's a multimap, - * rather than just a map, because multiple blocks may have the same parent (when chain splits - * or stale blocks exist). It maps from parent-hash to child-disk-position. - * - * This function can also be used to read blocks from user-specified block files using the - * -loadblock= option. There's no unknown-parent tracking, so the last two arguments are omitted. - * - * - * @param[in] fileIn FILE handle to file containing blocks to read - * @param[in] dbp (optional) Disk block position (only for reindex) - * @param[in,out] blocks_with_unknown_parent (optional) Map of disk positions for blocks with - * unknown parent, key is parent block hash - * (only used for reindex) - * */ - void LoadExternalBlockFile( - FILE* fileIn, - FlatFilePos* dbp = nullptr, - std::multimap* blocks_with_unknown_parent = nullptr) - EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex); /** * Update the on-disk chain state. @@ -691,8 +660,6 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) LOCKS_EXCLUDED(::cs_main); - bool AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -775,7 +742,6 @@ private: void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1097,6 +1063,37 @@ public: return m_snapshot_chainstate && m_ibd_chainstate && m_ibd_chainstate->m_disabled; } + /** + * Import blocks from an external file + * + * During reindexing, this function is called for each block file (datadir/blocks/blk?????.dat). + * It reads all blocks contained in the given file and attempts to process them (add them to the + * block index). The blocks may be out of order within each file and across files. Often this + * function reads a block but finds that its parent hasn't been read yet, so the block can't be + * processed yet. The function will add an entry to the blocks_with_unknown_parent map (which is + * passed as an argument), so that when the block's parent is later read and processed, this + * function can re-read the child block from disk and process it. + * + * Because a block's parent may be in a later file, not just later in the same file, the + * blocks_with_unknown_parent map must be passed in and out with each call. It's a multimap, + * rather than just a map, because multiple blocks may have the same parent (when chain splits + * or stale blocks exist). It maps from parent-hash to child-disk-position. + * + * This function can also be used to read blocks from user-specified block files using the + * -loadblock= option. There's no unknown-parent tracking, so the last two arguments are omitted. + * + * + * @param[in] fileIn FILE handle to file containing blocks to read + * @param[in] dbp (optional) Disk block position (only for reindex) + * @param[in,out] blocks_with_unknown_parent (optional) Map of disk positions for blocks with + * unknown parent, key is parent block hash + * (only used for reindex) + * */ + void LoadExternalBlockFile( + FILE* fileIn, + FlatFilePos* dbp = nullptr, + std::multimap* blocks_with_unknown_parent = nullptr); + /** * Process an incoming block. This only returns after the best known valid * block is made active. Note that it does not, however, guarantee that the @@ -1136,6 +1133,29 @@ public: */ bool ProcessNewBlockHeaders(const std::vector& block, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); + /** + * Sufficiently validate a block for disk storage (and store on disk). + * + * @param[in] pblock The block we want to process. + * @param[in] fRequested Whether we requested this block from a + * peer. + * @param[in] dbp The location on disk, if we are importing + * this block from prior storage. + * @param[in] min_pow_checked True if proof-of-work anti-DoS checks have + * been done by caller for headers chain + * + * @param[out] state The state of the block validation. + * @param[out] ppindex Optional return parameter to get the + * CBlockIndex pointer for this block. + * @param[out] fNewBlock Optional return parameter to indicate if the + * block is new to our storage. + * + * @returns False if the block or header is invalid, or if saving to disk fails (likely a fatal error); true otherwise. + */ + bool AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** * Try to add a transaction to the memory pool. *