diff options
-rwxr-xr-x | ci/test/06_script_b.sh | 3 | ||||
-rw-r--r-- | src/net_processing.cpp | 5 | ||||
-rw-r--r-- | src/node/blockstorage.cpp | 6 | ||||
-rw-r--r-- | src/test/fuzz/load_external_block_file.cpp | 11 | ||||
-rw-r--r-- | src/test/fuzz/txorphan.cpp | 4 | ||||
-rw-r--r-- | src/txorphanage.cpp | 4 | ||||
-rw-r--r-- | src/txorphanage.h | 2 | ||||
-rw-r--r-- | src/validation.cpp | 23 | ||||
-rw-r--r-- | src/validation.h | 32 |
9 files changed, 67 insertions, 23 deletions
diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 84b7ebe3fd..1220b15d5a 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -35,8 +35,9 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then fi if [ "${RUN_TIDY}" = "true" ]; then + set -eo pipefail export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/src/" - CI_EXEC run-clang-tidy "${MAKEJOBS}" + ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error" export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/" CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\ " src/compat"\ diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7507b8ebca..0e10fa5f9d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3631,10 +3631,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789) unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetIntArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); - unsigned int nEvicted = m_orphanage.LimitOrphans(nMaxOrphanTx); - if (nEvicted > 0) { - LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); - } + m_orphanage.LimitOrphans(nMaxOrphanTx); } else { LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); // We will continue to reject this tx since it has rejected diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index bac05f6be2..601d0bdf58 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -21,6 +21,7 @@ #include <util/system.h> #include <validation.h> +#include <map> #include <unordered_map> namespace node { @@ -834,6 +835,9 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile // -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<uint256, FlatFilePos> blocks_with_unknown_parent; while (true) { FlatFilePos pos(nFile, 0); if (!fs::exists(GetBlockPosFilename(pos))) { @@ -844,7 +848,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile 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 bfa977520b..f4b7dc08fd 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<uint256, FlatFilePos> 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/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 3fc6cde84e..7580651371 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -138,10 +138,8 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) [&] { // test mocktime and expiry SetMockTime(ConsumeTime(fuzzed_data_provider)); - auto size_before = orphanage.Size(); auto limit = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); - auto n_evicted = WITH_LOCK(g_cs_orphans, return orphanage.LimitOrphans(limit)); - Assert(size_before - n_evicted <= limit); + WITH_LOCK(g_cs_orphans, orphanage.LimitOrphans(limit)); Assert(orphanage.Size() <= limit); }); } diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index ed4783f1a5..69ae8ea582 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -102,7 +102,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer); } -unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans) +void TxOrphanage::LimitOrphans(unsigned int max_orphans) { AssertLockHeld(g_cs_orphans); @@ -135,7 +135,7 @@ unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans) EraseTx(m_orphan_list[randompos]->first); ++nEvicted; } - return nEvicted; + if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); } void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const diff --git a/src/txorphanage.h b/src/txorphanage.h index 24c8318f36..9363e6f733 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -41,7 +41,7 @@ public: void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans); /** Limit the orphanage to the given maximum */ - unsigned int LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** Add any orphans that list a particular tx as a parent into a peer's work set * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */ diff --git a/src/validation.cpp b/src/validation.cpp index d64ef4df0b..17211956f5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -57,6 +57,7 @@ #include <warnings.h> #include <algorithm> +#include <cassert> #include <chrono> #include <deque> #include <numeric> @@ -4256,11 +4257,16 @@ bool CChainState::LoadGenesisBlock() return true; } -void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) +void CChainState::LoadExternalBlockFile( + FILE* fileIn, + FlatFilePos* dbp, + std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent) { AssertLockNotHeld(m_chainstate_mutex); - // Map of disk positions for blocks with unknown parent (only used for reindex) - static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent; + + // Either both should be specified (-reindex), or neither (-loadblock). + assert(!dbp == !blocks_with_unknown_parent); + int64_t nStart = GetTimeMillis(); int nLoaded = 0; @@ -4310,8 +4316,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; } @@ -4340,13 +4347,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<uint256> queue; queue.push_back(hash); while (!queue.empty()) { uint256 head = queue.front(); queue.pop_front(); - std::pair<std::multimap<uint256, FlatFilePos>::iterator, std::multimap<uint256, FlatFilePos>::iterator> range = mapBlocksUnknownParent.equal_range(head); + auto range = blocks_with_unknown_parent->equal_range(head); while (range.first != range.second) { std::multimap<uint256, FlatFilePos>::iterator it = range.first; std::shared_ptr<CBlock> pblockrecursive = std::make_shared<CBlock>(); @@ -4361,7 +4370,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 a44dbd9c7a..9fef69799b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -575,8 +575,36 @@ public: 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<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr) EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex); /** |