ChannelAnnouncement rebroadcasting is broken

[TheBlueMatt]

#2382 noted that in ChannelManager::peer_connected the code that used to walk all our channels and send ChannelAnnouncement messages in response to peers connecting was made dead code when we moved channel storage into per-peer maps. This was the only place we ever rebroadcasted ChannelAnnouncement messages, which means we now are one-and-done, sending our announcement messages when we first exchange signatures and that's it. Luckily our peer should rebroadcast these for us, and if the announcement makes it into our local network graph on-disk we'll do so during normal gossip sync, so its not a huge deal, but we should make sure we rebroadcast them somewhere so that we don't have ldk <-> ldk channels that aren't visible to the network.

[TheBlueMatt]

we'll do so during normal gossip sync, so its not a huge deal

slipping to 118

[tnull]

Want to note that the same goes for node announcements, at least it would be very helpful if we'd take care of (re)broadcasting, especially after a channel has been opened. This would useful if our counterparties are mostly-offline mobile users that that are running of RGS data, i.e., would maybe never see a node announcements for their channel counterparty if they don't happen to be online at the right time (cf. lightningdevkit/ldk-node#234 (comment)).

I think generally it would be preferable if LDK could handle this internally, but an intermediate solution is #2866, which would allow us to trigger the rebroadcasting for a specific counterparty after the channel has been created.

[TheBlueMatt]

Hmm, node_announcements shouldnt ever be sent by mostly-offline nodes, however? They're only relevant for routing nodes, so rebroadcasting on a timer once an hour should more than suffice.

[tnull]

Hmm, node_announcements shouldnt ever be sent by mostly-offline nodes, however?
They're only relevant for routing nodes, so rebroadcasting on a timer once an hour should more than suffice.

I agree that only routing nodes should broadcast node announcements, the issue is that if our counterparty (see above) is a mobile node running on RGS they wouldn't get the node announcement via RGS and therefore would only get useful information (e.g. node alias, listening addresses allowing to reconnect) if they happen to be online at the time we decide to broadcast.

So I disagree that just rebroadcasting once an hour should suffice, that would only work if our counterparty is also reliably online at that point in time. We should make sure to broadcast our node announcement over freshly opened or reconnected channels.

[TheBlueMatt]

In that case the counterparty should get the node info when they do regular gossip sync from us, rather than via our hourly (or whatever) broadcast timer.

[tnull]

In that case the counterparty should get the node info when they do regular gossip sync from us, rather than via our hourly (or whatever) broadcast timer.

Yes I agree this is the intended behavior, but I'm not sure this works currently? Note that neither RapidGossipSync nor GossipSync::Rapid variant provide a RoutingMessageHandler so in practice you'll set MessageHandler's router_handler: IgnoringMessageHandler {} (e.g., in LDK Node: https://github.com/lightningdevkit/ldk-node/blob/6dcee911fbcec75621e578ecd2e90b9438d79198/src/builder.rs#L749-L767). So even if the routing node would provide the node announcements via gossip syncing (which I'm not sure of) any RGS-synced node would just ignore them.

[TheBlueMatt]

Wait, so we're talking about a setup where we have one public, large-ish routing node, with open channels and multiple peers that's online 24/7, and an LDK Node node that's hanging off of it. The LDK Node instance, if its using RGS, indeed should not receive info of the large routing node's channel_announcement because it explicitly opted to use RGS and wishes to receive the network graph that way rather than using the regular gossip sync mechanism. Exposing network information via RGS is a somewhat separate topic, but one we'd discussed possibly doing.

[tnull]

Wait, so we're talking about a setup where we have one public, large-ish routing node, with open channels and multiple peers that's online 24/7, and an LDK Node node that's hanging off of it. The LDK Node instance, if its using RGS, indeed should not receive info of the large routing node's channel_announcement because it explicitly opted to use RGS and wishes to receive the network graph that way rather than using the regular gossip sync mechanism. Exposing network information via RGS is a somewhat separate topic, but one we'd discussed possibly doing.

Mostly, yes, but the issues are around node announcements rather than channel announcements. I think there are two related issues here:

a) I'm not convinced the routing node would reliably send the node announcements to the 'client' node, if the latter doesn't happen to be online when the (hourly or daily) broadcast is triggered.
b) A client node running RGS would never get any node announcements since they are not included in RGS and it would ignore all P2P gossip. While channel announcements are likewise ignored, at least they will be retrieved at some point via RGS. But node announcements won't ever be available to RGS-based nodes, IIUC.

[TheBlueMatt]

a) I'm not convinced the routing node would reliably send the node announcements to the 'client' node, if the latter doesn't happen to be online when the (hourly or daily) broadcast is triggered.

I think we're on the same page here. In generally I think this is actually preferred - the client isn't doing gossip sync so we shouldn't be sending them gossip.

b) A client node running RGS would never get any node announcements since they are not included in RGS and it would ignore all P2P gossip. While channel announcements are likewise ignored, at least they will be retrieved at some point via RGS. But node announcements won't ever be available to RGS-based nodes, IIUC.

Right, for IPs specifically we're looking at including hosts in the RGS data.

[tnull]

Right, for IPs specifically we're looking at including hosts in the RGS data.

Aliases would also be a nice-to-have as Wallet will want to present human-readable names. Currently, they'd do a lookup to mempool.info, which of course unnecessarily leaks data.

[TheBlueMatt]

Hmm, I'm vaguely surprised a mobile wallet would care what a node's alias is? Should they really be displaying that vs picking a node they know?

[tnull]

Hmm, I'm vaguely surprised a mobile wallet would care what a node's alias is? Should they really be displaying that vs picking a node they know?

I think it has mostly to do with showing something prettier / more human readable than a 33-byte pubkey in the UI, i.e., even after the channel is already open.

[TheBlueMatt]

Right, but presumably a mobile wallet isn't opening channels to random nodes, so it can display the node alias via some out of band knowledge.