Don't serve channels in the range reply without a valid ChanUpdate. #9041
Conversation
Only serve channels in the query channel range msg we have at least seen one ChanUpdate msg for.
|
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
| @@ -2376,6 +2367,15 @@ func (c *ChannelGraph) FilterChannelRange(startHeight, | |||
| chanInfo.Node2UpdateTimestamp = edge.LastUpdate | |||
| } | |||
|
|
|||
| //nolint:lll | |||
| // We don't serve Channels we havn't seen any ChanUpdate | |||
| // msg from. | |||
There was a problem hiding this comment.
drive-by style nit: remove nolint and...
t1 := chanInfo.Node1UpdateTimestamp
t2 := chanInfo.Node2UpdateTimestamp
if t1.Equal(time.Unix(0, 0)) && t2.Equal(time.Unix(0, 0)) {
continue
}
| @@ -2335,15 +2335,6 @@ func (c *ChannelGraph) FilterChannelRange(startHeight, | |||
| cid, time.Time{}, time.Time{}, | |||
| ) | |||
|
|
|||
| if !withTimestamps { | |||
There was a problem hiding this comment.
don't think we should encode timestamps if we've decided to not send them
There was a problem hiding this comment.
we are not encoding them here, we will not include them in the msg if timestamp is not selected, the above is just the filtering logic.
This is where we decide whether we include the timestamp:
Lines 1137 to 1157 in 5c3a8e9
|
The change here makes sense, I don't think it will negatively impact gossip. Need to think about it a little more. |
Related to #8889
The problem:
Currently nodes which were infected with Channel Announcements without any valid ChanUpdate would never delete those old
EdgeInfoentries in their db. When receivingquery_channel_rangerequests, they would serve those channels. LND Nodes depending on whether they send the range request with timestamps or not, would in the latter case query those channels and receive "invalid channel announments", this would lead the lnd node to ban this peer after hitting the banning limit. Although direct channel peers are not disconnected, node runners would probably notice this spam of the node and close the channel.Not sending those bad channel ids in general might be a good mitigation for a infected node, which is probably not aware that he is infected in the first place.
The only other solution the infected node runner has, to get rid of its old channels is to drop its channel graph via chantools.