Inconsistent behavior around graph dump on gossip_timestamp_filter.

[TheBlueMatt]

In conversation with @t-bast at lightningdevkit/rust-lightning#1382 (comment), 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 :).

[vincenzopalazzo]

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!

[TheBlueMatt]

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 :)

[Roasbeef]

SHOULD -> MUST?

[rustyrussell]

So, there are two cases:

1. The very first gossip_timestamp_filter, which activates gossip (assuming gossip_queries feature).
2. 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.

[rustyrussell]

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.