fix(net): request headers when a peer announces blocks via inv#587
Draft
0xsiddharthks wants to merge 2 commits into
Draft
fix(net): request headers when a peer announces blocks via inv#5870xsiddharthks wants to merge 2 commits into
0xsiddharthks wants to merge 2 commits into
Conversation
Kyoto negotiates BIP-130 sendheaders, but Bitcoin Core still reverts to announcing only the tip via inv when more than MAX_BLOCKS_TO_ANNOUNCE (8) blocks connect in one announcement round, or when a block queued for announcement was reorganized off the active chain. Those announcements were dropped by the reader, leaving the node blind to new blocks until the thirty minute stale tip check disconnected every peer. Surface block hashes from inv messages and probe the announcing peer with a single getheaders, letting the response drive all state changes through the usual handle_headers path. Unlike the handler removed with the move to sendheaders, this does not mutate NodeState, does not touch filter request state, does not assume a new tip, and does not reset the stale tip monitor, so spoofed invs cannot mask a stale tip and the known ban vectors do not reappear.
Burst-mine fifteen blocks in one RPC and re-mine twelve over a two block reorganization. Whenever Core batches more than eight announcements in a round it falls back to inv-of-tip, which only the inv handler picks up; the reader unit test pins the inv mapping deterministically.
Member
|
Why is this in draft? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Kyoto drops every
invmessage. With BIP-130 that's usually fine, but Core still falls back to announcing only the tip viainvwhen more than 8 blocks (MAX_BLOCKS_TO_ANNOUNCE) connect in one announcement round, or when a queued announcement gets reorged away. When that happens kyoto hears nothing until the 30-minute stale-tip check tears down every connection. On signet, which mines in bursts (we've seen 12-block runs), we counted 41 of these deaf episodes in 48 hours with no real 30-minute gap on chain.This restores a minimal version of the inv handling removed in #570: an unknown block hash in an inv triggers a single
getheadersto the announcing peer, and the response drives everything through the normalhandle_headerspath. Unlike the old handler, this one does not:NodeStateheadersresponse does)Invs are ignored while a header sync is in flight or when every hash is already known, the probe only goes to the announcing peer, and the existing response timeout disconnects a peer that announces but can't deliver. A peer can already cause strictly more work today with unsolicited
headers.Tests: a reader unit test pins the inv mapping deterministically; the integration test burst-mines past the 8-block limit as a liveness check (a fast regtest box often stays under 8 per round, so the unit test is the regression anchor).