Catch bad gossip peer and fix `UpdatesInHorizon` by yyforyongyu · Pull Request #10102 · lightningnetwork/lnd

yyforyongyu · 2025-07-22T10:33:30Z

This PR adds two changes,

update UpdatesInHorizon to be more robust about invalid announcements.
record bad peers when processing invalid channel_update.

gemini-code-assist

Summary of Changes

Hello @yyforyongyu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the robustness of the network gossip mechanism. I've made the UpdatesInHorizon function more resilient to invalid channel announcements and improved the system's ability to identify and manage misbehaving peers by centralizing and applying a consistent banning strategy for invalid gossip messages.

Highlights

Improved UpdatesInHorizon Robustness: I've updated the UpdatesInHorizon function in discovery/chan_series.go to be more robust. It now filters out channel_announcement messages that do not have corresponding channel_update messages, aligning with BOLT7 specifications. Additionally, NodeAnnouncement messages are now only included if the node is associated with an active channel within the current horizon, preventing the broadcast of node announcements for nodes without relevant channels.
Centralized Bad Peer Handling: I've introduced a new private helper function, handleBadPeer, in discovery/gossiper.go. This function centralizes the logic for incrementing a peer's ban score and disconnecting them if their score exceeds the threshold and they are not a channel peer. This reduces code duplication and improves maintainability.
Enhanced Gossip Peer Management: The handleBadPeer function is now utilized in the AuthenticatedGossiper to manage peers sending invalid gossip messages. Specifically, peers sending channel_announcement messages for closed channels or channel_update messages with skewed (future) timestamps will now have their ban score increased, potentially leading to disconnection. This applies only to remote peers, ensuring internal messages don't trigger bans.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces two main changes: it makes UpdatesInHorizon more robust by filtering out invalid announcements, and it adds logic to penalize peers for sending invalid channel_update messages. The refactoring of peer banning logic into a new handleBadPeer function is a great improvement for code clarity and reusability. I've identified a few minor style issues related to line length and a misleading comment, which should be addressed to maintain code quality. Overall, these are solid improvements.

discovery/gossiper.go

Roasbeef · 2025-07-22T15:45:38Z

discovery/chan_series.go

-		nodeAnn := nodeAnn
+		// If this node has not been seen in the above channels, we can
+		// skip sending its NodeAnnouncement.
+		if _, seen := nodesFromChan[nodeAnn.PubKeyBytes]; !seen {


Have you seen us send these node ann's in the wild? Each block we should be pruning node anns that don't have any active channels.

Not sure if it's in the wild, here I just wanna make sure we only send node anns when they are the channel participants in the channel_announcements we are about to send, given this log I think it doesn't make sense to generate 12k gossip msgs for a given week, so I suspect some invalid msgs were sent, unless there indeed had many channel activities.

2025-07-18 15:38:24.228 [INF] DISC: GossipSyncer(0346335333ff9da5ed6e7c32b9079459668ddf9b2cc86cc86ce12a351acefab4c9): applying new remote update horizon: start=2025-07-15 09:53:30 +0000 UTC, end=2161-08-21 16:21:45 +0000 UTC, backlog_size=125172

ellemouton

looks good! one suggestion re determining if we should broadcast a node announcement

ellemouton · 2025-07-23T06:23:43Z

discovery/chan_series.go

+		// Based on BOLT7, if a channel_announcement has no
+		// corresponding channel_updates, we must not send the
+		// channel_announcement.
+		if edge1 == nil && edge2 == nil {
+			log.Warnf("No edge found for channel=%v",
+				channel.Info.ChannelID)
+
+			continue
+		}


could do this earlier (above CreateChanAnn) and just use channel.Policy1 & channel.Policy2 instead

hmm I think it's different as channelPolicy1 doesn't guarantee a non-nil edge1, or that we will send a channel_update, which reminds me I should do a different check - basically we should only send it if we are also sending its corresponding channel_update.

discovery/chan_series.go

Roasbeef · 2025-07-25T00:47:24Z

discovery/chan_series.go

 	}
+
+	// nodesFromChan records the nodes seen from the channels.
+	nodesFromChan := make(map[[33]byte]struct{}, len(chansInHorizon)*2)


I swore we already had a filtration step like this....

yeah let me add some debug log below so we can see if we are sending unnecessary anns.

Roasbeef · 2025-07-25T00:48:56Z

discovery/gossiper.go

+
+		// Increment the peer's ban score if they are skewed channel
+		// updates.
+		dcErr := d.handleBadPeer(nMsg.peer)


Perhaps we should gate this behind a feature flag first? Just to mitigate any unforeseen circumstances.

you mean adding a config to allow turning this off?

Yeah or rather, "strict update validation" or something like that.

@djkazic has seen some instances in the wild where we're overzealous re banning, which ends up banning some neutrino nodes that may be incorrectly sending out zombie channels or the like.

ok added a config to turn it off

docs/release-notes/release-notes-0.20.0.md

Base on BOLT07: > If a channel_announcement has no corresponding channel_updates: > - MUST NOT send the channel_announcement.

If a node doesn't have any channels, there's little point to send its node_announcement as it cannot be used for routing.

So we can use the same piece of code elsewhere.

So we can configure it via gossip's config in a following commit.

When users set `gossip.ban-threshold` to 0, it's now treated as setting the ban score to max uint64, which effectively disables the banning. We still want to record the peer's ban score in case we need it for future debugging.

lightninglabs-deploy · 2025-08-14T04:31:54Z

@yyforyongyu, remember to re-request review from reviewers when ready

ellemouton

very nice!! 🔥

ellemouton · 2025-08-15T12:05:09Z

discovery/chan_series.go

 		}

-		updates = append(updates, chanAnn)
+		// Create a slice to hold the `channel_announcement` and


should we expand one of the ApplyGossipFilter unit tests to cover this?

This method is mocked in the ApplyGossipFilter tests, I also find it weird to test it there. I will build the tests in a separate PR given it's a bit involved.

ellemouton · 2025-08-15T12:08:32Z

discovery/gossiper.go

 					key, &cachedReject{},
 				)

-				// Increment the peer's ban score. We check


Roasbeef

LGTM 🎉

Roasbeef · 2025-08-20T01:22:23Z

discovery/chan_series.go

 		}

-		updates = append(updates, chanAnn)
+		// Create a slice to hold the `channel_announcement` and


yyforyongyu self-assigned this Jul 22, 2025

yyforyongyu added enhancement Improvements to existing features / behaviour p2p Code related to the peer-to-peer behaviour bug fix gossip size/micro small bug fix or feature, less than 15 mins of review, less than 250 labels Jul 22, 2025

gemini-code-assist bot reviewed Jul 22, 2025

View reviewed changes

discovery/gossiper.go Show resolved Hide resolved

discovery/gossiper.go Show resolved Hide resolved

discovery/gossiper.go Outdated Show resolved Hide resolved

discovery/gossiper.go Show resolved Hide resolved

ellemouton self-requested a review July 22, 2025 10:37

yyforyongyu force-pushed the fix-UpdatesInHorizon branch from 6b26a42 to 26f9009 Compare July 22, 2025 11:00

Roasbeef reviewed Jul 22, 2025

View reviewed changes

ellemouton reviewed Jul 23, 2025

View reviewed changes

yyforyongyu force-pushed the fix-UpdatesInHorizon branch 3 times, most recently from e46feb8 to f9a2b02 Compare July 23, 2025 09:56

Roasbeef requested changes Jul 25, 2025

View reviewed changes

yyforyongyu force-pushed the fix-UpdatesInHorizon branch from f9a2b02 to bd4348e Compare July 25, 2025 09:37

alexbosworth reviewed Jul 27, 2025

View reviewed changes

docs/release-notes/release-notes-0.20.0.md Outdated Show resolved Hide resolved

yyforyongyu force-pushed the fix-UpdatesInHorizon branch 2 times, most recently from 57ba206 to fadaf0c Compare July 31, 2025 03:47

yyforyongyu mentioned this pull request Aug 4, 2025

multi: make gossip filter sends non-blocking, only allow a single backlog catch up goroutine per peer #10097

Merged

yyforyongyu added 9 commits August 4, 2025 15:44

discovery: skip sending channel_announcement with no channel_updates

f00073e

Base on BOLT07: > If a channel_announcement has no corresponding channel_updates: > - MUST NOT send the channel_announcement.

discovery: only send node_announcement associated with channels

9cae62d

If a node doesn't have any channels, there's little point to send its node_announcement as it cannot be used for routing.

discovery: add new method handleBadPeer

f1b2a47

So we can use the same piece of code elsewhere.

discovery: use handleBadPeer to increase peer's ban score

c3780a2

discovery: increase peer's ban score when received skewed channel_update

11c53b7

docs: update release notes

343418c

discovery: pass banThreshold to banman

3109a8f

So we can configure it via gossip's config in a following commit.

multi: add new config ban-threshold

a6f8617

multi: allow disable banning peers

60603f0

When users set `gossip.ban-threshold` to 0, it's now treated as setting the ban score to max uint64, which effectively disables the banning. We still want to record the peer's ban score in case we need it for future debugging.

yyforyongyu added 3 commits August 4, 2025 15:46

docs: add release notes for ban-threshold

84532e2

discovery: fix make lint

fc11e48

docs: update gossip rate limiting doc

7c46ba1

yyforyongyu force-pushed the fix-UpdatesInHorizon branch from fadaf0c to 7c46ba1 Compare August 4, 2025 07:55

yyforyongyu added this to the v0.20.0 milestone Aug 6, 2025

saubyk added this to lnd v0.20 Aug 7, 2025

saubyk moved this to In progress in lnd v0.20 Aug 7, 2025

yyforyongyu requested review from Roasbeef and ellemouton August 15, 2025 04:16

ellemouton approved these changes Aug 15, 2025

View reviewed changes

Roasbeef approved these changes Aug 20, 2025

View reviewed changes

discovery/chan_series.go

}

updates = append(updates, chanAnn)

// Create a slice to hold the `channel_announcement` and

Copy link

Member

Roasbeef Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

saubyk moved this from In progress to In review in lnd v0.20 Aug 20, 2025

yyforyongyu merged commit 0c2f045 into lightningnetwork:master Aug 20, 2025
38 checks passed

github-project-automation bot moved this from In review to Done in lnd v0.20 Aug 20, 2025

yyforyongyu deleted the fix-UpdatesInHorizon branch August 20, 2025 14:35

Conversation

yyforyongyu commented Jul 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yyforyongyu Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ellemouton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lightninglabs-deploy commented Aug 14, 2025

Uh oh!

ellemouton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Roasbeef left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yyforyongyu Jul 23, 2025 •

edited

Loading