Repository: bitcoin/bitcoin
File: src/net_processing.cpp (C++)

Code before:
        // An actually better block was announced.
        if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) {
            state->pindexBestKnownBlock = pindex;
        }
    } else {
        // An unknown block was announced; just assume that the latest one is the best one.
        state->hashLastUnknownBlock = hash;
    }
}

// Logic for calculating which blocks to download from a given peer, given our current tip.
void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller)
{
    if (count == 0)
        return;

    vBlocks.reserve(vBlocks.size() + count);
    CNodeState *state = State(peer.m_id);
    assert(state != nullptr);

    // Make sure pindexBestKnownBlock is up to date, we'll need it.
    ProcessBlockAvailability(peer.m_id);

    if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < m_chainman.MinimumChainWork()) {
        // This peer has nothing interesting.
        return;
    }

    // When we sync with AssumeUtxo and discover the snapshot is not in the peer's best chain, abort:
    // We can't reorg to this chain due to missing undo data until the background sync has finished,
    // so downloading blocks from it would be futile.
    const CBlockIndex* snap_base{m_chainman.GetSnapshotBaseBlock()};
    if (snap_base && state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
        LogDebug(BCLog::NET, "Not downloading blocks from peer=%d, which doesn't have the snapshot block in its best chain.\n", peer.m_id);
        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.
    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())];
    }

    // 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;


Review comment (suggestion):
This change assumes their tip is higher than our tip. What if their tip is lower? Maybe worth advancing `pindexLastCommonBlock` in that case as well. Something like this:

```cpp
// If our tip has advanced beyond pindexLastCommonBlock, move it ahead to the tip. We don't need to
// download any blocks in between, and skipping ahead here allows us to determine nWindowEnd better.
const auto our_tip{m_chainman.ActiveTip()};
const auto peer_best_known{state->pindexBestKnownBlock};
if (our_tip->nHeight > state->pindexLastCommonBlock->nHeight) {
    if (peer_best_known->nHeight >= our_tip->nHeight) { 
        if (peer_best_known->GetAncestor(our_tip->nHeight) == our_tip) {
            state->pindexLastCommonBlock = our_tip;
        }     
    } else {                        
        if (our_tip->GetAncestor(peer_best_known->nHeight) == peer_best_known) {
            state->pindexLastCommonBlock = peer_best_known;
        }
        // FindNextBlocks() would be a noop below, so return here.
        return;
    }     
}
```

Code after:
    ProcessBlockAvailability(nodeid);

    const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(hash);
    if (pindex && pindex->nChainWork > 0) {
        // An actually better block was announced.
        if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) {
            state->pindexBestKnownBlock = pindex;
        }
    } else {
        // An unknown block was announced; just assume that the latest one is the best one.
        state->hashLastUnknownBlock = hash;
    }
}

// Logic for calculating which blocks to download from a given peer, given our current tip.
void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller)
{
    if (count == 0)
        return;

    vBlocks.reserve(vBlocks.size() + count);
    CNodeState *state = State(peer.m_id);
    assert(state != nullptr);

    // Make sure pindexBestKnownBlock is up to date, we'll need it.
    ProcessBlockAvailability(peer.m_id);

    if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < m_chainman.MinimumChainWork()) {
        // This peer has nothing interesting.
        return;
    }

    // When we sync with AssumeUtxo and discover the snapshot is not in the peer's best chain, abort:
    // We can't reorg to this chain due to missing undo data until the background sync has finished,
    // so downloading blocks from it would be futile.
    const CBlockIndex* snap_base{m_chainman.GetSnapshotBaseBlock()};
    if (snap_base && state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
        LogDebug(BCLog::NET, "Not downloading blocks from peer=%d, which doesn't have the snapshot block in its best chain.\n", peer.m_id);
        return;
    }

    // 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 ||
        fork_point->nChainWork > state->pindexLastCommonBlock->nChainWork ||
        state->pindexBestKnownBlock->GetAncestor(state->pindexLastCommonBlock->nHeight) != state->pindexLastCommonBlock) {
        state->pindexLastCommonBlock = fork_point;
    }