From 984bb193c1aa045c4660cbe425be74f20aaf25cf Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 26 Jul 2020 23:43:01 +0300 Subject: [PATCH] merge bitcoin#25571: Make mapBlocksUnknownParent local, and rename it --- src/node/blockstorage.cpp | 7 ++++- src/test/fuzz/load_external_block_file.cpp | 11 ++++++-- src/validation.cpp | 23 +++++++++++----- src/validation.h | 32 ++++++++++++++++++++-- 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 88e80fbba84b5c..b18fe927498f69 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -23,6 +23,8 @@ #include #include +#include + std::atomic_bool fImporting(false); std::atomic_bool fReindex(false); bool fPruneMode = false; @@ -817,6 +819,9 @@ void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, // -reindex if (fReindex) { int nFile = 0; + // Map of disk positions for blocks with unknown parent (only used for reindex); + // parent hash -> child disk position, multiple children can have the same parent. + std::multimap blocks_with_unknown_parent; while (true) { FlatFilePos pos(nFile, 0); if (!fs::exists(GetBlockPosFilename(pos))) { @@ -827,7 +832,7 @@ void ThreadImport(ChainstateManager& chainman, CDeterministicMNManager& dmnman, break; // This error is logged in OpenBlockFile } LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile); - chainman.ActiveChainstate().LoadExternalBlockFile(file, &pos); + chainman.ActiveChainstate().LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); if (ShutdownRequested()) { LogPrintf("Shutdown requested. Exit %s\n", __func__); return; diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index 7a2795695db2de..16b916dc700433 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -31,6 +31,13 @@ FUZZ_TARGET_INIT(load_external_block_file, initialize_load_external_block_file) if (fuzzed_block_file == nullptr) { return; } - FlatFilePos flat_file_pos; - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file, fuzzed_data_provider.ConsumeBool() ? &flat_file_pos : nullptr); + if (fuzzed_data_provider.ConsumeBool()) { + // 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); + } else { + // Corresponds to the -loadblock= case (orphan blocks aren't tracked across files). + g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file); + } } diff --git a/src/validation.cpp b/src/validation.cpp index 71a1795f70737d..1db803e4272040 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -68,6 +68,7 @@ #include #include +#include #include #include #include @@ -4519,11 +4520,16 @@ bool CChainState::LoadGenesisBlock() return true; } -void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) +void CChainState::LoadExternalBlockFile( + FILE* fileIn, + FlatFilePos* dbp, + std::multimap* blocks_with_unknown_parent) { AssertLockNotHeld(m_chainstate_mutex); - // Map of disk positions for blocks with unknown parent (only used for reindex) - static std::multimap mapBlocksUnknownParent; + + // Either both should be specified (-reindex), or neither (-loadblock). + assert(!dbp == !blocks_with_unknown_parent); + int64_t nStart = GetTimeMillis(); int nLoaded = 0; @@ -4574,8 +4580,9 @@ void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) if (hash != m_params.GetConsensus().hashGenesisBlock && !m_blockman.LookupBlockIndex(block.hashPrevBlock)) { LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), block.hashPrevBlock.ToString()); - if (dbp) - mapBlocksUnknownParent.insert(std::make_pair(block.hashPrevBlock, *dbp)); + if (dbp && blocks_with_unknown_parent) { + blocks_with_unknown_parent->emplace(block.hashPrevBlock, *dbp); + } continue; } @@ -4604,13 +4611,15 @@ void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) NotifyHeaderTip(*this); + if (!blocks_with_unknown_parent) continue; + // Recursively process earlier encountered successors of this block std::deque queue; queue.push_back(hash); while (!queue.empty()) { uint256 head = queue.front(); queue.pop_front(); - std::pair::iterator, std::multimap::iterator> range = mapBlocksUnknownParent.equal_range(head); + auto range = blocks_with_unknown_parent->equal_range(head); while (range.first != range.second) { std::multimap::iterator it = range.first; std::shared_ptr pblockrecursive = std::make_shared(); @@ -4625,7 +4634,7 @@ void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) } } range.first++; - mapBlocksUnknownParent.erase(it); + blocks_with_unknown_parent->erase(it); NotifyHeaderTip(*this); } } diff --git a/src/validation.h b/src/validation.h index 16d0e9b0f489a5..340081e2a55a30 100644 --- a/src/validation.h +++ b/src/validation.h @@ -606,8 +606,36 @@ class CChainState bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** Import blocks from an external file */ - void LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp = nullptr) + /** + * 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); /**