A PR by Suhas Daftuar to reduce bandwidth during initial headers sync when a block is found, by changing the logic of initiating additional headers sync with all announcing peers of a new block, to just one sync for each new block announcement. Participants discussed the origins of the current logic, how we are protected against peers stalling headers sync, what changes and the benefits of the new logic.
ℹ️ This was made somewhat more bandwidth-wasteful after PR#25717.
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)? 🔗
- A node only announces with a
headers
message if it is sure that the peer has already seen the parent block. Meaning the peer has previously sent a header to which the new header connects to (see here). - This node is in IBD so it doesn't know (= has not downloaded) the parent block of the new block, therefore will not get a
headers
announcement.
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? 🔗
- We get a lot of announcements which translates to header syncing with a lot of peers resulting to receiving duplicate headers across multiple peers.
- This duplicate headers during IBD (across multiple peers) was attempted to be fixed back in 2016 with PR#8054, but the fix had to be reverted.
- A related issue with multiple
getheaders
message to the same peer was fixed with PR#25454
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) 🔗
- The amount of bandwidth wasted (duplicate data) depends on the progress made (in the initial sync) until the block announcement is received.
- The estimate is something like
size_of_header * remaining_number_of_blocks * (number_of_peers - 1)
. The estimate could be improved by accounting for thegetheaders
messages but they are probably small in comparison.
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
? 🔗
fSyncStarted
tells us whether we've started headers synchronization with this peer.nSyncStarted
corresponds to the number of peers withfSyncStarted = true
.m_headers_sync_timeout
is only used for our initially chosen headers-sync peer (ifnSyncStarted == 1
) and it tells us when to potentially disconnect peer for stalling headers download. Its value depends on how much time we have between tip and today.- (src: sipa) "Historically, I think the case is just that it was never intended that we'd be syncing headers from non-
nSyncStarted
peers. But it turned outs that (a) we actually do and (b) this is actually somewhat advantageous because it prevents a situation that the singular chosennSyncStarted
peer is malicious/broken/infinitely slow, and stalls your headers sync forever. (I can say this as the person who first introduced the headers syncing andnSyncStarted
concept, until not too recently I wasn't actually aware that we'd start fetching headers from other peers as well if they'd announce a new block to us)."
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? 🔗
- Peers that announce an
inv
to us have a higher probability of being responsive. The peer that manages to send us the blockinv
first is often also a very fast peer. So we'd not pick another slow peer if for some reason our initial peer is slow.
- Headers are actually 81 bytes on the wire, not 80. The notion of a block header as a protocol concept didn't exist originally. Headers were just blocks without transactions. That's why they are serialized as empty
CBlock
s, therefore having 1 extra byte for the empty transaction vector.