From 52575e96e72a0402c448f86728b2e84836b1e817 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 28 Aug 2024 15:06:16 +0200 Subject: [PATCH 1/3] validation: Use span for ProcessNewBlockHeaders Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory. --- src/net_processing.cpp | 2 +- src/rpc/mining.cpp | 2 +- src/test/blockfilter_index_tests.cpp | 2 +- src/test/fuzz/utxo_snapshot.cpp | 4 ++-- src/test/validation_block_tests.cpp | 2 +- src/validation.cpp | 3 ++- src/validation.h | 5 +++-- 7 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 13ea3a29be804..1d3a0095255b5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4764,7 +4764,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const CBlockIndex *pindex = nullptr; BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, /*min_pow_checked=*/true, state, &pindex)) { + if (!m_chainman.ProcessNewBlockHeaders({{cmpctblock.header}}, /*min_pow_checked=*/true, state, &pindex)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block=*/true, "invalid header via cmpctblock"); return; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3c41e136ecafd..1111c9aa132e0 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1095,7 +1095,7 @@ static RPCHelpMan submitheader() } BlockValidationState state; - chainman.ProcessNewBlockHeaders({h}, /*min_pow_checked=*/true, state); + chainman.ProcessNewBlockHeaders({{h}}, /*min_pow_checked=*/true, state); if (state.IsValid()) return UniValue::VNULL; if (state.IsError()) { throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 067a32d6a40e4..f6024f1ef3577 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -103,7 +103,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, CBlockHeader header = block->GetBlockHeader(); BlockValidationState state; - if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, true, state, &pindex)) { + if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({{header}}, true, state, &pindex)) { return false; } } diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 21c305e222108..1241bba8be382 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -58,7 +58,7 @@ void initialize_chain() auto& chainman{*setup->m_node.chainman}; for (const auto& block : chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)}; + bool processed{chainman.ProcessNewBlockHeaders({{block->GetBlockHeader()}}, true, dummy)}; Assert(processed); const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))}; Assert(index); @@ -137,7 +137,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) if constexpr (!INVALID) { for (const auto& block : *g_chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)}; + bool processed{chainman.ProcessNewBlockHeaders({{block->GetBlockHeader()}}, true, dummy)}; Assert(processed); const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))}; Assert(index); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 588ac60498c1f..d9ce059a04494 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -101,7 +101,7 @@ std::shared_ptr MinerTestingSetup::FinalizeBlock(std::shared_ptr // submit block header, so that miner can get the block height from the // global state and the node has the topology of the chain BlockValidationState ignored; - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, true, ignored)); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({{pblock->GetBlockHeader()}}, true, ignored)); return pblock; } diff --git a/src/validation.cpp b/src/validation.cpp index 8f75b2e30a0cb..4868a776e44ce 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -70,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -4384,7 +4385,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida } // Exposed wrapper for AcceptBlockHeader -bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& headers, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex) +bool ChainstateManager::ProcessNewBlockHeaders(std::span headers, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex) { AssertLockNotHeld(cs_main); { diff --git a/src/validation.h b/src/validation.h index f905d6e624037..9ec592b03dd20 100644 --- a/src/validation.h +++ b/src/validation.h @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -1217,12 +1218,12 @@ class ChainstateManager * May not be called in a * validationinterface callback. * - * @param[in] block The block headers themselves + * @param[in] headers The block headers themselves * @param[in] min_pow_checked True if proof-of-work anti-DoS checks have been done by caller for headers chain * @param[out] state This may be set to an Error state if any error occurred processing them * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers */ - bool ProcessNewBlockHeaders(const std::vector& block, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlockHeaders(std::span headers, 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). From 20515ea3f5bd426f6e3746cf5cddd2324dacae31 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 28 Aug 2024 15:10:15 +0200 Subject: [PATCH 2/3] validation: Use span for CalculateClaimedHeadersWork Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory. --- src/net_processing.cpp | 4 ++-- src/validation.cpp | 2 +- src/validation.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1d3a0095255b5..7a4f4de23df2e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4751,7 +4751,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), *peer); } return; - } else if (prev_block->nChainWork + CalculateClaimedHeadersWork({cmpctblock.header}) < GetAntiDoSWorkThreshold()) { + } else if (prev_block->nChainWork + CalculateClaimedHeadersWork({{cmpctblock.header}}) < GetAntiDoSWorkThreshold()) { // If we get a low-work header in a compact block, we can ignore it. LogPrint(BCLog::NET, "Ignoring low-work compact block from peer %d\n", pfrom.GetId()); return; @@ -5068,7 +5068,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true)); // Check claimed work on this block against our anti-dos thresholds. - if (prev_block && prev_block->nChainWork + CalculateClaimedHeadersWork({pblock->GetBlockHeader()}) >= GetAntiDoSWorkThreshold()) { + if (prev_block && prev_block->nChainWork + CalculateClaimedHeadersWork({{pblock->GetBlockHeader()}}) >= GetAntiDoSWorkThreshold()) { min_pow_checked = true; } } diff --git a/src/validation.cpp b/src/validation.cpp index 4868a776e44ce..04c04f5e3c320 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4137,7 +4137,7 @@ bool IsBlockMutated(const CBlock& block, bool check_witness_root) return false; } -arith_uint256 CalculateClaimedHeadersWork(const std::vector& headers) +arith_uint256 CalculateClaimedHeadersWork(std::span headers) { arith_uint256 total_work{0}; for (const CBlockHeader& header : headers) { diff --git a/src/validation.h b/src/validation.h index 9ec592b03dd20..35a8c971bae5e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -408,7 +408,7 @@ bool HasValidProofOfWork(const std::vector& headers, const Consens bool IsBlockMutated(const CBlock& block, bool check_witness_root); /** Return the sum of the claimed work on a given set of headers. No verification of PoW is done. */ -arith_uint256 CalculateClaimedHeadersWork(const std::vector& headers); +arith_uint256 CalculateClaimedHeadersWork(std::span headers); enum class VerifyDBResult { SUCCESS, From a2955f09792b6232f3a45aa44a498b466279a8b7 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 30 Aug 2024 10:16:29 +0200 Subject: [PATCH 3/3] validation: Use span for ImportBlocks paths Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory. --- src/node/blockstorage.cpp | 4 ++-- src/node/blockstorage.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 96cf69927c355..1fa657545a598 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -1210,7 +1210,7 @@ class ImportingNow } }; -void ImportBlocks(ChainstateManager& chainman, std::vector vImportFiles) +void ImportBlocks(ChainstateManager& chainman, std::span import_paths) { ImportingNow imp{chainman.m_blockman.m_importing}; @@ -1245,7 +1245,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector vImportFile } // -loadblock= - for (const fs::path& path : vImportFiles) { + for (const fs::path& path : import_paths) { AutoFile file{fsbridge::fopen(path, "rb")}; if (!file.IsNull()) { LogPrintf("Importing blocks file %s...\n", fs::PathToString(path)); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 821bbf5109b2d..03bc5f4600720 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -429,7 +430,7 @@ class BlockManager void CleanupBlockRevFiles() const; }; -void ImportBlocks(ChainstateManager& chainman, std::vector vImportFiles); +void ImportBlocks(ChainstateManager& chainman, std::span import_paths); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H