Reduce bandwidth during initial headers sync when a block is found (p2p)

https://github.com/bitcoin/bitcoin/pull/25720

Host: dergoegge  -  PR author: sdaftuar

Notes

  • On startup, if a node’s headers chain is more than a day behind the current time, it will pick one peer to sync headers with until its best headers chain is caught up. (See here) If a node’s headers chain is not caught up within 15 min + ~num headers missing * 1ms (either because the chosen peer was not able to or because they purposefully stalled the download), it will disconnect the chosen peer for stalling and pick a new peer to sync headers with. Blocks are only downloaded and validated after the node’s headers chain has sufficient work (nMinimumChainWork) to protect against DoS attacks.

  • When a node receives a block announcement via an inv during headers sync, it will add all announcing peers as additional headers sync peers. PR #25720 changes this logic such that only one of the announcing peers is added for headers sync.

  • BIP 130 (implemented in PR #7129) introduced the sendheaders message, which indicates that a node prefers receiving block announcements via a headers message rather than an inv.

Questions

  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?

  2. Why do nodes (mostly) receive inv block announcements while they are doing initial headers sync, even though they indicated preference for headers announcements (BIP 130)?

  3. Why is bandwidth wasted (during initial headers sync) by adding all peers that announce a block to us via an inv as headers sync peers?

  4. What would be your estimate (lower/upper bound) of how much bandwidth is wasted? (There is no one true answer here, the estimate depends on a couple of variables)

  5. What’s the purpose of CNodeState’s members fSyncStarted and m_headers_sync_timeout, and PeerManagerImpl::nSyncStarted? If we start syncing headers with peers that announce a block to us via an inv, why do we not increase nSyncStarted and set fSyncStarted = true and update m_headers_sync_timeout?

  6. An alternative to the approach taken in the PR would be to add an additional headers sync peer after a timeout (fixed or random). What is the benefit of the approach taken in the PR over this alternative?

  7. Can you think of any other alternatives?