p2p: During block download, adjust pindexLastCommonBlock better
What the developer wrote
Authored by Martin Zumsande
p2p: During block download, adjust pindexLastCommonBlock better
Simplify and improve the logic for calculating pindexLastCommonBlock, in order to calculate
nWindowEnd better.
The previous logic would not take into account when the chain tip had moved forward, so that
FindNextBlocks could iterate over many blocks already downloaded and
connected, which could result in blocks not being requested for download that should have been
requested, and peers being wrongly marked as staller.
It also removes extra logic from commit 49d569cb1fdd62a9da8dff51dccaf4680fe3d0eb
for the situation right after a snapshot was loaded:
After snapshot loading, our tip becomes the snapshot block.
For peers that have the most-work chain, which inlcludes the snapshot,
our tip is an ancestor of the peer's best block, hence the general
advancement logic will move pindexLastCommonBlock
from any pre-snapshot position to the snapshot height automatically.
Co-authored-by: stringintech <stringintech@gmail.com>
Co-authored-by: Pieter Wuille <pieter@wuille.net>
What changed, and why it matters
This change fixes how Bitcoin nodes decide which blocks to ask peers for during initial sync or catch-up. Previously, the node could get stuck reviewing blocks it already had, fail to request new blocks it actually needed, and incorrectly label peers as slow (stallers). The patch makes the 'last common block' calculation follow the current chain tip and the peer's known chain more accurately. It is a robustness improvement to block download scheduling, not a direct theft or remote-code-execution bug.
Treat as a normal bug-fix merge. Nodes should upgrade in due course, especially if they rely on fast sync or operate with many peers. No emergency response is indicated by the diff alone. Monitor release notes for any additional context on stalling scenarios.
Security signals we found
Denial-of-service vector: incorrect staller marking could slow or stall block download for a node
Logic bug in P2P block download scheduling
No input validation bypass, memory corruption, or cryptographic weakness visible in diff
Patch is small and targeted (1 file, ~9 lines changed)
Evidence from the diff
In PeerManagerImpl::FindNextBlocksToDownload(), pindexLastCommonBlock is used as the starting scan point for blocks to request. The old code guessed the fork point by height and then ran LastCommonAncestor against the peer’s best known block, but did not account for our own chain tip having moved forward. That could leave pindexLastCommonBlock far behind the current tip, causing the download window to include already-downloaded/connected blocks, skip needed blocks, and trigger false staller detection. The new code computes the fork point between the peer’s best known block and our active tip, and only moves pindexLastCommonBlock forward when the fork point has more chain work or when the saved common block is no longer an ancestor of the peer’s best known block. It also removes a special-case snapshot-load adjustment because the general logic now covers it.
Changed components
src/net_processing.cppPeerManagerImpl::FindNextBlocksToDownload()P2P block download / stalling logicInspect captured patch +8 / −9
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 10fbae5e..3ac8b132 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1397,17 +1397,16 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
return;
}
- // Bootstrap quickly by guessing a parent of our best tip is the forking point.
- // Guessing wrong in either direction is not a problem.
- // Also reset pindexLastCommonBlock after a snapshot was loaded, so that blocks after the snapshot will be prioritised for download.
+ // Determine the forking point between the peer's chain and our chain:
+ // pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock, and will be used as a starting point.
+ // It is being set to the fork point between the peer's best known block and the current tip, unless it is already set to
+ // an ancestor with more work than the fork point.
+ auto fork_point = LastCommonAncestor(state->pindexBestKnownBlock, m_chainman.ActiveTip());
if (state->pindexLastCommonBlock == nullptr ||
- (snap_base && state->pindexLastCommonBlock->nHeight < snap_base->nHeight)) {
- state->pindexLastCommonBlock = m_chainman.ActiveChain()[std::min(state->pindexBestKnownBlock->nHeight, m_chainman.ActiveChain().Height())];
+ fork_point->nChainWork > state->pindexLastCommonBlock->nChainWork ||
+ state->pindexBestKnownBlock->GetAncestor(state->pindexLastCommonBlock->nHeight) != state->pindexLastCommonBlock) {
+ state->pindexLastCommonBlock = fork_point;
}
-
- // If the peer reorganized, our previous pindexLastCommonBlock may not be an ancestor
- // of its current tip anymore. Go back enough to fix that.
- state->pindexLastCommonBlock = LastCommonAncestor(state->pindexLastCommonBlock, state->pindexBestKnownBlock);
if (state->pindexLastCommonBlock == state->pindexBestKnownBlock)
return;
Why this scored 36/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.