From 0bf6139e194f355d121bb2aea74715d1c4099598 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 23 Aug 2022 18:19:51 -0400 Subject: [PATCH 1/8] p2p: Avoid an IsAncestorOfBestHeaderOrTip call Just don't call this function when it won't have any effect. Note that we can't remove the LookupBlockIndex call, since `last_received_header` is needed to check if new headers were received (`received_new_header`). --- src/net_processing.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 26107b0a9c4..5c8d92f0922 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2919,9 +2919,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, { LOCK(cs_main); last_received_header = m_chainman.m_blockman.LookupBlockIndex(headers.back().GetHash()); - if (IsAncestorOfBestHeaderOrTip(last_received_header)) { - already_validated_work = true; - } + already_validated_work = already_validated_work || IsAncestorOfBestHeaderOrTip(last_received_header); } // If our peer has NetPermissionFlags::NoBan privileges, then bypass our From 4066bfe561a45f61a3c9bf24bec7f600ddcc7467 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Mon, 18 Aug 2025 11:46:04 +0200 Subject: [PATCH 2/8] refactor: Compute work from headers without CBlockIndex Avoid the need to construct a CBlockIndex object just to compute work for a header, when its nBits value suffices for that. Co-Authored-By: Pieter Wuille --- src/chain.cpp | 4 ++-- src/chain.h | 10 +++++++++- src/headerssync.cpp | 4 ++-- src/net_processing.cpp | 4 ++-- src/validation.cpp | 3 +-- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/chain.cpp b/src/chain.cpp index 3dd22634110..622e018d34d 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -124,12 +124,12 @@ void CBlockIndex::BuildSkip() pskip = pprev->GetAncestor(GetSkipHeight(nHeight)); } -arith_uint256 GetBlockProof(const CBlockIndex& block) +arith_uint256 GetBitsProof(uint32_t bits) { arith_uint256 bnTarget; bool fNegative; bool fOverflow; - bnTarget.SetCompact(block.nBits, &fNegative, &fOverflow); + bnTarget.SetCompact(bits, &fNegative, &fOverflow); if (fNegative || fOverflow || bnTarget == 0) return 0; // We need to compute 2**256 / (bnTarget+1), but we can't represent 2**256 diff --git a/src/chain.h b/src/chain.h index 2c86526520b..3b8a672db18 100644 --- a/src/chain.h +++ b/src/chain.h @@ -348,7 +348,15 @@ protected: CBlockIndex& operator=(CBlockIndex&&) = delete; }; -arith_uint256 GetBlockProof(const CBlockIndex& block); +/** Compute how much work an nBits value corresponds to. */ +arith_uint256 GetBitsProof(uint32_t bits); + +/** Compute how much work a block index entry corresponds to. */ +inline arith_uint256 GetBlockProof(const CBlockIndex& block) { return GetBitsProof(block.nBits); } + +/** Compute how much work a block header corresponds to. */ +inline arith_uint256 GetBlockProof(const CBlockHeader& header) { return GetBitsProof(header.nBits); } + /** Return the time it would take to redo the work difference between from and to, assuming the current hashrate corresponds to the difficulty at tip, in seconds. */ int64_t GetBlockProofEquivalentTime(const CBlockIndex& to, const CBlockIndex& from, const CBlockIndex& tip, const Consensus::Params&); /** Find the forking point between two chain tips. */ diff --git a/src/headerssync.cpp b/src/headerssync.cpp index e52a8cdc486..fc73d4031de 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -202,7 +202,7 @@ bool HeadersSyncState::ValidateAndProcessSingleHeader(const CBlockHeader& curren } } - m_current_chain_work += GetBlockProof(CBlockIndex(current)); + m_current_chain_work += GetBlockProof(current); m_last_header_received = current; m_current_height = next_height; @@ -238,7 +238,7 @@ bool HeadersSyncState::ValidateAndStoreRedownloadedHeader(const CBlockHeader& he } // Track work on the redownloaded chain - m_redownload_chain_work += GetBlockProof(CBlockIndex(header)); + m_redownload_chain_work += GetBlockProof(header); if (m_redownload_chain_work >= m_minimum_required_work) { m_process_all_remaining_headers = true; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5c8d92f0922..5fd11d3c370 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4348,7 +4348,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 + GetBlockProof(cmpctblock.header) < GetAntiDoSWorkThreshold()) { // If we get a low-work header in a compact block, we can ignore it. LogDebug(BCLog::NET, "Ignoring low-work compact block from peer %d\n", pfrom.GetId()); return; @@ -4666,7 +4666,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 + GetBlockProof(pblock->GetBlockHeader()) >= GetAntiDoSWorkThreshold()) { min_pow_checked = true; } } diff --git a/src/validation.cpp b/src/validation.cpp index af523b06d74..ecc517451bc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4171,8 +4171,7 @@ arith_uint256 CalculateClaimedHeadersWork(std::span headers) { arith_uint256 total_work{0}; for (const CBlockHeader& header : headers) { - CBlockIndex dummy(header); - total_work += GetBlockProof(dummy); + total_work += GetBlockProof(header); } return total_work; } From 45686522224598bed9923e60daad109094d7bc29 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Fri, 8 Aug 2025 12:01:45 +0200 Subject: [PATCH 3/8] refactor: Use std::span in HasValidProofOfWork Co-Authored-By: Pieter Wuille --- src/validation.cpp | 6 +++--- src/validation.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index ecc517451bc..99f516ccf96 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4130,10 +4130,10 @@ std::vector ChainstateManager::GenerateCoinbaseCommitment(CBlock& return commitment; } -bool HasValidProofOfWork(const std::vector& headers, const Consensus::Params& consensusParams) +bool HasValidProofOfWork(std::span headers, const Consensus::Params& consensusParams) { - return std::all_of(headers.cbegin(), headers.cend(), - [&](const auto& header) { return CheckProofOfWork(header.GetHash(), header.nBits, consensusParams);}); + return std::ranges::all_of(headers, + [&](const auto& header) { return CheckProofOfWork(header.GetHash(), header.nBits, consensusParams); }); } bool IsBlockMutated(const CBlock& block, bool check_witness_root) diff --git a/src/validation.h b/src/validation.h index 291c1021dc9..daf954c820e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -410,8 +410,8 @@ BlockValidationState TestBlockValidity( bool check_pow, bool check_merkle_root) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Check with the proof of work on each blockheader matches the value in nBits */ -bool HasValidProofOfWork(const std::vector& headers, const Consensus::Params& consensusParams); +/** Check that the proof of work on each blockheader matches the value in nBits */ +bool HasValidProofOfWork(std::span headers, const Consensus::Params& consensusParams); /** Check if a block has been mutated (with respect to its merkle root and witness commitments). */ bool IsBlockMutated(const CBlock& block, bool check_witness_root); From ca0243e3a6d77d2b218749f1ba113b81444e3f4a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 31 Aug 2022 10:47:16 -0400 Subject: [PATCH 4/8] refactor: Remove useless CBlock::GetBlockHeader There is no need for a function to convert a CBlock to a CBlockHeader, as it's a child class of it. --- src/merkleblock.cpp | 2 +- src/net_processing.cpp | 2 +- src/primitives/block.h | 12 ------------ src/test/blockfilter_index_tests.cpp | 3 +-- src/test/fuzz/block_header.cpp | 4 ++-- src/test/fuzz/utxo_snapshot.cpp | 4 ++-- src/test/validation_block_tests.cpp | 2 +- 7 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index 34ff06e53a7..f5aaddbae22 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -29,7 +29,7 @@ std::vector BytesToBits(const std::vector& bytes) CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set* txids) { - header = block.GetBlockHeader(); + header = static_cast(block); std::vector vMatch; std::vector vHashes; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5fd11d3c370..3a712f2f800 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4666,7 +4666,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 + GetBlockProof(pblock->GetBlockHeader()) >= GetAntiDoSWorkThreshold()) { + if (prev_block && prev_block->nChainWork + GetBlockProof(*pblock) >= GetAntiDoSWorkThreshold()) { min_pow_checked = true; } } diff --git a/src/primitives/block.h b/src/primitives/block.h index 207d2b2980c..afaeba0dc5a 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -101,18 +101,6 @@ public: m_checked_merkle_root = false; } - CBlockHeader GetBlockHeader() const - { - CBlockHeader block; - block.nVersion = nVersion; - block.hashPrevBlock = hashPrevBlock; - block.hashMerkleRoot = hashMerkleRoot; - block.nTime = nTime; - block.nBits = nBits; - block.nNonce = nNonce; - return block; - } - std::string ToString() const; }; diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 224acb8b079..4a3f4d428f2 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -102,10 +102,9 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, chain.resize(length); for (auto& block : chain) { block = std::make_shared(CreateBlock(pindex, no_txns, coinbase_script_pub_key)); - CBlockHeader header = block->GetBlockHeader(); BlockValidationState state; - if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({{header}}, true, state, &pindex)) { + if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({{*block}}, true, state, &pindex)) { return false; } } diff --git a/src/test/fuzz/block_header.cpp b/src/test/fuzz/block_header.cpp index 2e446b16ebe..12a9e1a2d7a 100644 --- a/src/test/fuzz/block_header.cpp +++ b/src/test/fuzz/block_header.cpp @@ -33,10 +33,10 @@ FUZZ_TARGET(block_header) mut_block_header.SetNull(); assert(mut_block_header.IsNull()); CBlock block{*block_header}; - assert(block.GetBlockHeader().GetHash() == block_header->GetHash()); + assert(block.GetHash() == block_header->GetHash()); (void)block.ToString(); block.SetNull(); - assert(block.GetBlockHeader().GetHash() == mut_block_header.GetHash()); + assert(block.GetHash() == mut_block_header.GetHash()); } { std::optional block_locator = ConsumeDeserializable(fuzzed_data_provider); diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 56d3615570f..35faa73a5ae 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -89,7 +89,7 @@ void initialize_chain() auto& chainman{*setup->m_node.chainman}; for (const auto& block : chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({{block->GetBlockHeader()}}, true, dummy)}; + bool processed{chainman.ProcessNewBlockHeaders({{*block}}, true, dummy)}; Assert(processed); const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))}; Assert(index); @@ -171,7 +171,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) if constexpr (!INVALID) { for (const auto& block : *g_chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({{block->GetBlockHeader()}}, true, dummy)}; + bool processed{chainman.ProcessNewBlockHeaders({{*block}}, 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 a0b23f5d3b7..6497a243b70 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -104,7 +104,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}}, true, ignored)); return pblock; } From 256246a9fa5b05141c93aeeb359394b9c7a80e49 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Thu, 5 Jun 2025 13:59:14 +0200 Subject: [PATCH 5/8] refactor: Remove redundant parameter from CheckHeadersPoW No need to pass consensusParams, as CheckHeadersPoW already has access to m_chainparams.GetConsensus() Co-Authored-By: maflcko <6399679+maflcko@users.noreply.github.com> --- src/net_processing.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3a712f2f800..7ab70cd63a3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -644,7 +644,7 @@ private: EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Various helpers for headers processing, invoked by ProcessHeadersMessage() */ /** Return true if headers are continuous and have valid proof-of-work (DoS points assigned on failure) */ - bool CheckHeadersPoW(const std::vector& headers, const Consensus::Params& consensusParams, Peer& peer); + bool CheckHeadersPoW(const std::vector& headers, Peer& peer); /** Calculate an anti-DoS work threshold for headers chains */ arith_uint256 GetAntiDoSWorkThreshold(); /** Deal with state tracking and headers sync for peers that send @@ -2485,10 +2485,10 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo MakeAndPushMessage(pfrom, NetMsgType::BLOCKTXN, resp); } -bool PeerManagerImpl::CheckHeadersPoW(const std::vector& headers, const Consensus::Params& consensusParams, Peer& peer) +bool PeerManagerImpl::CheckHeadersPoW(const std::vector& headers, Peer& peer) { // Do these headers have proof-of-work matching what's claimed? - if (!HasValidProofOfWork(headers, consensusParams)) { + if (!HasValidProofOfWork(headers, m_chainparams.GetConsensus())) { Misbehaving(peer, "header with invalid proof of work"); return false; } @@ -2853,7 +2853,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // We'll rely on headers having valid proof-of-work further down, as an // anti-DoS criteria (note: this check is required before passing any // headers into HeadersSyncState). - if (!CheckHeadersPoW(headers, m_chainparams.GetConsensus(), peer)) { + if (!CheckHeadersPoW(headers, peer)) { // Misbehaving() calls are handled within CheckHeadersPoW(), so we can // just return. (Note that even if a header is announced via compact // block, the header itself should be valid, so this type of error can From 0488bdfefe92b2c9a924be9244c91fe472462aab Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Wed, 11 Jun 2025 18:14:12 +0200 Subject: [PATCH 6/8] refactor: Remove unused parameter in ReportHeadersPresync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Aurèle Oulès --- src/net_processing.cpp | 2 +- src/validation.cpp | 2 +- src/validation.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7ab70cd63a3..578a7d715c9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4620,7 +4620,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (it != m_headers_presync_stats.end()) stats = it->second; } if (stats.second) { - m_chainman.ReportHeadersPresync(stats.first, stats.second->first, stats.second->second); + m_chainman.ReportHeadersPresync(stats.second->first, stats.second->second); } } diff --git a/src/validation.cpp b/src/validation.cpp index 99f516ccf96..8846011032d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4380,7 +4380,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(std::span hea return true; } -void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t height, int64_t timestamp) +void ChainstateManager::ReportHeadersPresync(int64_t height, int64_t timestamp) { AssertLockNotHeld(GetMutex()); { diff --git a/src/validation.h b/src/validation.h index daf954c820e..57baa05f95f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1286,7 +1286,7 @@ public: * headers are not yet fed to validation during that time, but validation is (for now) * responsible for logging and signalling through NotifyHeaderTip, so it needs this * information. */ - void ReportHeadersPresync(const arith_uint256& work, int64_t height, int64_t timestamp); + void ReportHeadersPresync(int64_t height, int64_t timestamp); //! When starting up, search the datadir for a chainstate based on a UTXO //! snapshot that is in the process of being validated. From e37555e5401f9fca39ada0bd153e46b2c7ebd095 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Wed, 11 Jun 2025 18:14:49 +0200 Subject: [PATCH 7/8] refactor: Use initializer list in CompressedHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also mark CompressedHeader as explicit, and GetFullHeader as const Co-Authored-By: Aurèle Oulès --- src/headerssync.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/headerssync.h b/src/headerssync.h index 9e3af58d60a..a29726fb345 100644 --- a/src/headerssync.h +++ b/src/headerssync.h @@ -31,16 +31,17 @@ struct CompressedHeader { hashMerkleRoot.SetNull(); } - CompressedHeader(const CBlockHeader& header) + explicit CompressedHeader(const CBlockHeader& header) + : nVersion{header.nVersion}, + hashMerkleRoot{header.hashMerkleRoot}, + nTime{header.nTime}, + nBits{header.nBits}, + nNonce{header.nNonce} { - nVersion = header.nVersion; - hashMerkleRoot = header.hashMerkleRoot; - nTime = header.nTime; - nBits = header.nBits; - nNonce = header.nNonce; } - CBlockHeader GetFullHeader(const uint256& hash_prev_block) { + CBlockHeader GetFullHeader(const uint256& hash_prev_block) const + { CBlockHeader ret; ret.nVersion = nVersion; ret.hashPrevBlock = hash_prev_block; From de4242f47476769d0a7f3e79e8297ed2dd60d9a4 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Wed, 11 Jun 2025 18:33:30 +0200 Subject: [PATCH 8/8] refactor: Use reference for chain_start in HeadersSyncState chain_start can never be null, so it's better to pass it as a reference rather than a raw pointer Also slightly reformat HeaderSyncState constructor to make clang-format happy Lastly, remove `const` from `chain_start` declaration in headers_sync_chainwork_tests, to work aroud a false-positive dangling-reference warning in gcc 13.0 Co-Authored-By: maflcko <6399679+maflcko@users.noreply.github.com> --- src/headerssync.cpp | 41 ++++++++++++----------- src/headerssync.h | 6 ++-- src/net_processing.cpp | 12 +++---- src/test/fuzz/headerssync.cpp | 4 +-- src/test/headers_sync_chainwork_tests.cpp | 2 +- 5 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/headerssync.cpp b/src/headerssync.cpp index fc73d4031de..039e7f68e8a 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -14,18 +14,21 @@ // CompressedHeader (we should re-calculate parameters if we compress further). static_assert(sizeof(CompressedHeader) == 48); -HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus_params, - const HeadersSyncParams& params, const CBlockIndex* chain_start, - const arith_uint256& minimum_required_work) : - m_commit_offset((assert(params.commitment_period > 0), // HeadersSyncParams field must be initialized to non-zero. - FastRandomContext().randrange(params.commitment_period))), - m_id(id), m_consensus_params(consensus_params), - m_params(params), - m_chain_start(chain_start), - m_minimum_required_work(minimum_required_work), - m_current_chain_work(chain_start->nChainWork), - m_last_header_received(m_chain_start->GetBlockHeader()), - m_current_height(chain_start->nHeight) +HeadersSyncState::HeadersSyncState(NodeId id, + const Consensus::Params& consensus_params, + const HeadersSyncParams& params, + const CBlockIndex& chain_start, + const arith_uint256& minimum_required_work) + : m_commit_offset((assert(params.commitment_period > 0), // HeadersSyncParams field must be initialized to non-zero. + FastRandomContext().randrange(params.commitment_period))), + m_id(id), + m_consensus_params(consensus_params), + m_params(params), + m_chain_start(chain_start), + m_minimum_required_work(minimum_required_work), + m_current_chain_work(chain_start.nChainWork), + m_last_header_received(m_chain_start.GetBlockHeader()), + m_current_height(chain_start.nHeight) { // Estimate the number of blocks that could possibly exist on the peer's // chain *right now* using 6 blocks/second (fastest blockrate given the MTP @@ -35,7 +38,7 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus // exceeds this bound, because it's not possible for a consensus-valid // chain to be longer than this (at the current time -- in the future we // could try again, if necessary, to sync a longer chain). - const auto max_seconds_since_start{(Ticks(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}})) + const auto max_seconds_since_start{(Ticks(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start.GetMedianTimePast()}})) + MAX_FUTURE_BLOCK_TIME}; m_max_commitments = 6 * max_seconds_since_start / m_params.commitment_period; @@ -161,10 +164,10 @@ bool HeadersSyncState::ValidateAndStoreHeadersCommitments(std::span= m_minimum_required_work) { m_redownloaded_headers.clear(); - m_redownload_buffer_last_height = m_chain_start->nHeight; - m_redownload_buffer_first_prev_hash = m_chain_start->GetBlockHash(); - m_redownload_buffer_last_hash = m_chain_start->GetBlockHash(); - m_redownload_chain_work = m_chain_start->nChainWork; + m_redownload_buffer_last_height = m_chain_start.nHeight; + m_redownload_buffer_first_prev_hash = m_chain_start.GetBlockHash(); + m_redownload_buffer_last_hash = m_chain_start.GetBlockHash(); + m_redownload_chain_work = m_chain_start.nChainWork; m_download_state = State::REDOWNLOAD; LogDebug(BCLog::NET, "Initial headers sync transition with peer=%d: reached sufficient work at height=%i, redownloading from height=%i\n", m_id, m_current_height, m_redownload_buffer_last_height); } @@ -228,7 +231,7 @@ bool HeadersSyncState::ValidateAndStoreRedownloadedHeader(const CBlockHeader& he if (!m_redownloaded_headers.empty()) { previous_nBits = m_redownloaded_headers.back().nBits; } else { - previous_nBits = m_chain_start->nBits; + previous_nBits = m_chain_start.nBits; } if (!PermittedDifficultyTransition(m_consensus_params, next_height, @@ -295,7 +298,7 @@ CBlockLocator HeadersSyncState::NextHeadersRequestLocator() const Assume(m_download_state != State::FINAL); if (m_download_state == State::FINAL) return {}; - auto chain_start_locator = LocatorEntries(m_chain_start); + auto chain_start_locator = LocatorEntries(&m_chain_start); std::vector locator; if (m_download_state == State::PRESYNC) { diff --git a/src/headerssync.h b/src/headerssync.h index a29726fb345..6d720874411 100644 --- a/src/headerssync.h +++ b/src/headerssync.h @@ -137,8 +137,8 @@ public: * minimum_required_work: amount of chain work required to accept the chain */ HeadersSyncState(NodeId id, const Consensus::Params& consensus_params, - const HeadersSyncParams& params, const CBlockIndex* chain_start, - const arith_uint256& minimum_required_work); + const HeadersSyncParams& params, const CBlockIndex& chain_start, + const arith_uint256& minimum_required_work); /** Result data structure for ProcessNextHeaders. */ struct ProcessingResult { @@ -220,7 +220,7 @@ private: const HeadersSyncParams m_params; /** Store the last block in our block index that the peer's chain builds from */ - const CBlockIndex* m_chain_start{nullptr}; + const CBlockIndex& m_chain_start; /** Minimum work that we're looking for on this chain. */ const arith_uint256 m_minimum_required_work; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 578a7d715c9..5d97a464f7c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -686,8 +686,8 @@ private: * calling); false otherwise. */ bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, - const CBlockIndex* chain_start_header, - std::vector& headers) + const CBlockIndex& chain_start_header, + std::vector& headers) EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Return true if the given header is an ancestor of @@ -2633,10 +2633,10 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro return false; } -bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex* chain_start_header, std::vector& headers) +bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex& chain_start_header, std::vector& headers) { // Calculate the claimed total work on this chain. - arith_uint256 total_work = chain_start_header->nChainWork + CalculateClaimedHeadersWork(headers); + arith_uint256 total_work = chain_start_header.nChainWork + CalculateClaimedHeadersWork(headers); // Our dynamic anti-DoS threshold (minimum work required on a headers chain // before we'll store it) @@ -2667,7 +2667,7 @@ bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlo // handled inside of IsContinuationOfLowWorkHeadersSync. (void)IsContinuationOfLowWorkHeadersSync(peer, pfrom, headers); } else { - LogDebug(BCLog::NET, "Ignoring low-work chain (height=%u) from peer=%d\n", chain_start_header->nHeight + headers.size(), pfrom.GetId()); + LogDebug(BCLog::NET, "Ignoring low-work chain (height=%u) from peer=%d\n", chain_start_header.nHeight + headers.size(), pfrom.GetId()); } // The peer has not yet given us a chain that meets our work threshold, @@ -2933,7 +2933,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // Do anti-DoS checks to determine if we should process or store for later // processing. if (!already_validated_work && TryLowWorkHeadersSync(peer, pfrom, - chain_start_header, headers)) { + *chain_start_header, headers)) { // If we successfully started a low-work headers sync, then there // should be no headers to process any further. Assume(headers.empty()); diff --git a/src/test/fuzz/headerssync.cpp b/src/test/fuzz/headerssync.cpp index 5bfbed85375..b33f4dc728f 100644 --- a/src/test/fuzz/headerssync.cpp +++ b/src/test/fuzz/headerssync.cpp @@ -44,7 +44,7 @@ class FuzzedHeadersSyncState : public HeadersSyncState { public: FuzzedHeadersSyncState(const HeadersSyncParams& sync_params, const size_t commit_offset, - const CBlockIndex* chain_start, const arith_uint256& minimum_required_work) + const CBlockIndex& chain_start, const arith_uint256& minimum_required_work) : HeadersSyncState(/*id=*/0, Params().GetConsensus(), sync_params, chain_start, minimum_required_work) { const_cast(m_commit_offset) = commit_offset; @@ -74,7 +74,7 @@ FUZZ_TARGET(headers_sync_state, .init = initialize_headers_sync_state_fuzz) FuzzedHeadersSyncState headers_sync( params, /*commit_offset=*/fuzzed_data_provider.ConsumeIntegralInRange(0, params.commitment_period - 1), - /*chain_start=*/&start_index, + /*chain_start=*/start_index, /*minimum_required_work=*/min_work); // Store headers for potential redownload phase. diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp index c91420978ff..b6a5e153e51 100644 --- a/src/test/headers_sync_chainwork_tests.cpp +++ b/src/test/headers_sync_chainwork_tests.cpp @@ -52,7 +52,7 @@ constexpr size_t COMMITMENT_PERIOD{600}; // Somewhat close to mainnet. struct HeadersGeneratorSetup : public RegTestingSetup { const CBlock& genesis{Params().GenesisBlock()}; - const CBlockIndex* chain_start{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash()))}; + CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))}; // Generate headers for two different chains (using differing merkle roots // to ensure the headers are different).