Merge bitcoin/bitcoin#32740: refactor: Header sync optimisations & simplifications

de4242f474 refactor: Use reference for chain_start in HeadersSyncState (Daniela Brozzoni)
e37555e540 refactor: Use initializer list in CompressedHeader (Daniela Brozzoni)
0488bdfefe refactor: Remove unused parameter in ReportHeadersPresync (Daniela Brozzoni)
256246a9fa refactor: Remove redundant parameter from CheckHeadersPoW (Daniela Brozzoni)
ca0243e3a6 refactor: Remove useless CBlock::GetBlockHeader (Pieter Wuille)
4568652222 refactor: Use std::span in HasValidProofOfWork (Daniela Brozzoni)
4066bfe561 refactor: Compute work from headers without CBlockIndex (Daniela Brozzoni)
0bf6139e19 p2p: Avoid an IsAncestorOfBestHeaderOrTip call (Pieter Wuille)

Pull request description:

  This is a partial* revival of #25968

  It contains a list of most-unrelated simplifications and optimizations to the code merged in #25717:

  - Avoid an IsAncestorOfBestHeaderOrTip call: Just don't call this function when it won't have any effect.
  - 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. Also use some Spans where possible.
  - 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.

  It also contains the following code cleanups, which were suggested by reviewers in #25968:
  - Remove redundant parameter from CheckHeadersPoW: No need to pass consensusParams, as CheckHeadersPow already has access to m_chainparams.GetConsensus()
  - Remove unused parameter in ReportHeadersPresync
  - Use initializer list in CompressedHeader, also make GetFullHeader const
  - 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

  *I decided to leave out three commits that were in #25968 (4e7ac7b94d, ab52fb4e95, 7f1cf440ca), since they're a bit more involved, and I'm a new contributor. If this PR gets merged, I'll comment under #25968 to note that these three commits are still up for grabs :)

ACKs for top commit:
  l0rinc:
    ACK de4242f474
  polespinasa:
    re-ACK de4242f474
  sipa:
    ACK de4242f474
  achow101:
    ACK de4242f474
  hodlinator:
    re-ACK de4242f474

Tree-SHA512: 1de4f3ce0854a196712505f2b52ccb985856f5133769552bf37375225ea8664a3a7a6a9578c4fd461e935cd94a7cbbb08f15751a1da7651f8962c866146d9d4b
This commit is contained in:
Ava Chow
2026-01-14 11:38:07 -08:00
15 changed files with 78 additions and 82 deletions

View File

@@ -655,7 +655,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<CBlockHeader>& headers, const Consensus::Params& consensusParams, Peer& peer);
bool CheckHeadersPoW(const std::vector<CBlockHeader>& 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
@@ -697,8 +697,8 @@ private:
* calling); false otherwise.
*/
bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom,
const CBlockIndex* chain_start_header,
std::vector<CBlockHeader>& headers)
const CBlockIndex& chain_start_header,
std::vector<CBlockHeader>& 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
@@ -2584,10 +2584,10 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo
MakeAndPushMessage(pfrom, NetMsgType::BLOCKTXN, resp);
}
bool PeerManagerImpl::CheckHeadersPoW(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams, Peer& peer)
bool PeerManagerImpl::CheckHeadersPoW(const std::vector<CBlockHeader>& 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;
}
@@ -2732,10 +2732,10 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro
return false;
}
bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex* chain_start_header, std::vector<CBlockHeader>& headers)
bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex& chain_start_header, std::vector<CBlockHeader>& 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)
@@ -2766,7 +2766,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,
@@ -2952,7 +2952,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
@@ -3018,9 +3018,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
@@ -3034,7 +3032,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());
@@ -4548,7 +4546,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;
@@ -4820,7 +4818,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);
}
}
@@ -4866,7 +4864,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) >= GetAntiDoSWorkThreshold()) {
min_pow_checked = true;
}
}