Gossip queries fixes and improvements - #3345
Merged
Merged
Conversation
A `query_channel_range` is a few dozen bytes to send, but answering one forces us to scan our whole routing table, compute a checksum for every channel_update we know and send back megabytes of data. We introduce rate-limits for gossip queries, ack them only when they've been processed, and make processing them more efficient: - Add per-connection rate-limits for `query_channel_range` per connection. Queries that we drop are still acked, otherwise the transport would stop reading from the connection. The new`router.sync.max-queries-per-second` defaults to 10: peers only need to send a handful of those per connection, so this doesn't affect normal syncing. - Only acks a query once we've sent our replies: acking first releases the transport-level back-pressure while we're still working, which lets queries pile up in the router's mailbox. - Ignore gossip queries that are for another chain: we were answering them, which leaked our routing table to nodes that aren't even on our network. We already had that check for `gossip_timestamp_range`. - cache the timestamps and checksums of our channel updates in `PublicChannel`, instead of re-serializing every update we know on every incoming query. The instance is replaced whenever an update changes, so the cache is transparently invalidated. - remove the spent channels with `--` instead of rebuilding the whole channels map with `filterNot`. - drop the `keep` filter in `handleQueryChannelRange`: it built a copy of our whole routing table, and `split` already restricts ids to the requested range as it iterates them in block height order.
Ignore duplicate `short_channel_id`s: the spec doesn't forbid them, but a query is capped at 65kB while our reply isn't capped at all, so repeating the same scid was worth ~90x amplification to a peer. We already tracked the node announcements we had sent for that reason, we now track channel ids the same way. Reject queries whose `encoded_query_flags` don't decode to exactly one flag per `short_channel_id`, and queries that arrive before we've sent the `reply_short_channel_ids_end` for the previous one. BOLT 7 explicitly allows sending a `warning` for both, and forbids senders from doing either. The dead `numca`/`numcu` accumulators in `processChannelQuery` are removed: they were never incremented and the result was discarded. The counts that we log and report to Kamon come from the callbacks and are unchanged.
Our peer decides when a routing table sync ends, so we can't let it dictate how much we buffer for it. Cap the number of `query_short_channel_ids` we queue for a given peer during a sync with `router.sync.max-queries-per-sync` (default 2000, i.e. 200 000 channels at the default chunk size, several times the size of the network). Past that we ignore the extra channel ids and keep draining the queries we already have: the sync is then incomplete for that peer, but gossip and our other sync peers make up for it. Ignore `reply_short_channel_ids_end` messages that don't answer a query we have in flight. We were popping and sending the next query on any of them, so a peer could send one right after answering our `query_channel_range`, when we have nothing in flight: we would log "sync complete", drop our sync state, and ignore the rest of its replies as unsolicited. Note that `reply_short_channel_ids_end` carries nothing that identifies the query it answers, so we can't do better than tracking whether we're waiting for one: a peer that answers our queries can still send extra end messages to drain our pending queries faster than we intended. That is now bounded by the cap above, and the queries only go back to that peer. `Syncing.started` is replaced by the explicit `queryInFlight` flag it was being used as a proxy for.
`addToSync` built a fresh `Syncing` in the branch that sends a query right away, which discarded `current.remainingQueries`. That branch used to be reached only when we had no state at all for that peer, but since #1587 we pre-create a `Syncing(Nil, 0)` entry when we send our `query_channel_range`, so it now overwrites real state instead of creating the first one. This is not currently a bug as `remainingQueries` is always empty there today: it's an implicit invariant that nothing enforces. We pop from the queued list instead of overwriting it, so we can never silently drop queries. Also narrow `Syncing.remainingQueries` to `List[QueryShortChannelIds]`: that's the only thing we ever put in it, and `addToSync` already returns that type.
t-bast
reviewed
Aug 12, 2026
t-bast
left a comment
t-bast
left a comment
Member
There was a problem hiding this comment.
Great stuff! It is honestly a bit hard to review for correctness of the protocol (but it was already the case before this PR - gossip would benefit from a large refactoring at some point, but that's not for today), but it's quite clear what is being fixed and the fixes look good to me.
I think there are still a few DoS vectors that we can improve (see comments for details), but it shouldn't be too hard to add.
Same reasoning as for query_channel_range.
t-bast
approved these changes
Aug 13, 2026
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.
This PR limits, for a given peer, the number of gossip queries that we will queue and process, as well as the amount of work we will do. It also makes processing queries more efficient, with a few minor fixes.