Description
(Created on Aug, 16 2023)
-
V2 should only send (and allow receiving) "connected" block header lists, i.e. the ones that are connected to the chainstate of the receiving side (i.e. the parent of the first header in the list should exist in the chainstate).
Note: the old V1 implementation used to perform block announcements by sending a single header, which was generally "unconnected", so it had to allow receiving single unconnected headers as well. Because of that, the newer V1 implementation has to allow single unconnected headers too, though it itself never sends those. -
When sending new headers, V2 should take into account the best header that it has sent so far.Though this seemed like a useful improvement initially, it turned out that allowing received headers to connect only to "peer's known headers" would make it possible for a malicious peer to flood the node with headers, potentially exhausting its memory. So, it was reverted in this PR - Fix some p2p issues #1306
Note: the newer V1 implementation already tracks it in its incoming.best_sent_block_header field. However, it cannot use it to determine which headers to send without the risk of breaking the V1 protocol, because the old implementation is known to have issues with correctly updating its "peer's known headers" list. So, if the sent header list is connected to best_sent_block_header, an old peer may see it as disconnected in some cases. And since even old implementations only allow single unconnected headers, this may lead to a ban. -
(do we need this?) V2 should better not rely on the number of headers in the header list response to determine whether the peer has more headers. Instead, this fact should be specified in the response explicitly.
Note: the current implementation uses msg_header_count_limit as the header count limit; if the number of received headers equals the limit, the sending side is expected to have more headers. So, if the sending side has exactly msg_header_count_limit headers, it'll result in an unneeded extra headers request and an empty response.
(This is being removed because it doesn't help, and it's normal to send an empty message to indicate that "there's nothing more to send) -
The constants that have to be a part of the protocol (such as msg_header_count_limit) should be encapsulated properly...There is a separate issue for this - The P2pConfig struct needs a revamp #1201 -
In V2 we might want to allow peers to ask for headers even if the node is in initial block download state (IBD), e.g. based on some kind of system of permissions. At this moment it's not clear whether it's a good idea, so it makes sense to first check whether bitcoin does something like that.We decided that the node should just respond with an empty header list when in IBD. -
In V2 we probably should not silently ignore header requests; instead, we should communicate our best block (id/height/header?) from the start, so that the peer just knows that we don't have better blocks and doesn't ask us in the first place. Or, perhaps, the fact that the node is in IBD can be communicated as well.We decided that the node should just respond with an empty header list when in IBD.
Note: In V1, header requests can be ignored if the node is in IBD; this makes the handling of stalling a bit more convoluted than necessary.