lightning
Inconsistent behavior around graph dump on `gossip_timestamp_filter`.
In conversation with @t-bast at https://github.com/lightningdevkit/rust-lightning/pull/1382#discussion_r835038791, we recently discovered that (at least) CLN and eclair treat gossip_timestamp_filter messages differently.
Ultimately it comes down to the spec having this to say:
The receiver: SHOULD send all gossip messages whose timestamp is greater or equal to first_timestamp, and less than first_timestamp plus timestamp_range. MAY wait for the next outgoing gossip flush to send these.
which seems to say that when you get a gossip_timestamp_filter you've gotta go do a graph dump at the peer, ala initial-graph-sync. But this is somewhat confusing in that...what's the point of all the gossip queries if we just end up receiving a dump of gossip from the peer anyway (assuming you set the filter to two-weeks-ago to ensure you don't miss updates)?
It seems at least CLN does give you a graph dump, t-bast indicated eclair does not, and I'm not sure about lnd.
Of course at this point I figure we probably all want to just deprecate the gossip sync stuff in favor of something minisketch-based, but it'd be nice to get consistency here in the short-term. At least personally, I'd suggest we keep it as-is and do a graph dump when receiving a timestamp filter message, allowing us to "implement" gossip queries without bothering with the actual queries at all :).
At least personally, I'd suggest we keep it as-is and do a graph dump when receiving a timestamp filter message, allowing us to "implement" gossip queries without bothering with the actual queries at all :).
Concept ACK, If we are going to propose a solution with minisketch I think the solution is to keep as-is with graph dump to avoid some implementation to do the double work!
Concept ACK, If we are going to propose a solution with minisketch I think the solution is to keep as-is with graph dump to avoid some implementation to do the double work!
For context, in LDK we had the whole query stuff implemented, but when we noticed nodes doing a dump when we sent a gossip_timestamp_filter we yanked the query logic from the codebase entirely and replaced it with a three paragraph comment and a single message....much cleaner :)
So, there are two cases:
- The very first
gossip_timestamp_filter, which activates gossip (assuminggossip_queriesfeature). - Successive
gossip_timestamp_filter.
On the former, you really want to send all gossip that matches. On the latter, it could well apply only to future gossip, which would save load on the server; we currently seek back to the start to find any gossip which might match your updated timestamp.
In practice, we only send this in a binary fashion: start = 10 minutes ago, when we want to get gossip, and start = 0xFFFFFFFF, when we want to suppress it. We choose 3 peers to gossip with, generally.
Suggest we turn the timestamp filter into a trinary:
- 0: send everything.
- 0xFFFFFFFF: send nothing
- Any other value: send gossip as it comes in.
This matches current use in practice and is trivial to implement.