From 92de9424ce378972dd23933619e356f64d9c8274 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 16:55:28 +0800 Subject: [PATCH 1/7] chanfitness: only process online events for peers with channels --- chanfitness/chaneventstore.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/chanfitness/chaneventstore.go b/chanfitness/chaneventstore.go index 29a1df917..fe7ca4028 100644 --- a/chanfitness/chaneventstore.go +++ b/chanfitness/chaneventstore.go @@ -313,12 +313,15 @@ func (c *ChannelEventStore) closeChannel(channelPoint wire.OutPoint, } } -// peerEvent creates a peer monitor for a peer if we do not currently have -// one, and adds an online event to it. +// peerEvent adds an online event to a peer's monitor. If the peer is not +// yet known to the event store, the event is ignored. A peer is only known to +// the event store if we have an open channel with them. func (c *ChannelEventStore) peerEvent(peer route.Vertex, online bool) { - peerMonitor, err := c.getPeerMonitor(peer) - if err != nil { - log.Error("could not create monitor: %v", err) + peerMonitor, ok := c.peers[peer] + if !ok { + log.Tracef("Ignore peer event (online=%v) from non-channel "+ + "peer: %v", online, peer) + return } From 455e4564eb500e966e9e8f00515204180724815a Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 17:01:03 +0800 Subject: [PATCH 2/7] chanfitness: send peer online event when we see the first channel We rename `getPeerMonitor` to `getOrCreatePeerMonitor` for clarity - it will now send an online event when the peer monitor is being created, which means we will only start counting the peer as online when it has opened a channel with us. --- chanfitness/chaneventstore.go | 26 ++++++++++++------- chanfitness/chaneventstore_test.go | 41 +++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/chanfitness/chaneventstore.go b/chanfitness/chaneventstore.go index fe7ca4028..881e9a35e 100644 --- a/chanfitness/chaneventstore.go +++ b/chanfitness/chaneventstore.go @@ -245,7 +245,7 @@ func (c *ChannelEventStore) Stop() error { func (c *ChannelEventStore) addChannel(channelPoint wire.OutPoint, peer route.Vertex) { - peerMonitor, err := c.getPeerMonitor(peer) + peerMonitor, err := c.getOrCreatePeerMonitor(peer) if err != nil { log.Error("could not create monitor: %v", err) return @@ -256,10 +256,12 @@ func (c *ChannelEventStore) addChannel(channelPoint wire.OutPoint, } } -// getPeerMonitor tries to get an existing peer monitor from our in memory list, -// and falls back to creating a new monitor if it is not currently known. -func (c *ChannelEventStore) getPeerMonitor(peer route.Vertex) (peerMonitor, - error) { +// getOrCreatePeerMonitor tries to get an existing peer monitor from our in +// memory list. If the peer is not yet known to us, it will create a new +// monitor and add it to the list. When a new monitor is created, we also send +// an initial online event for the peer. +func (c *ChannelEventStore) getOrCreatePeerMonitor( + peer route.Vertex) (peerMonitor, error) { peerMonitor, ok := c.peers[peer] if ok { @@ -289,6 +291,9 @@ func (c *ChannelEventStore) getPeerMonitor(peer route.Vertex) (peerMonitor, peerMonitor = newPeerLog(c.cfg.Clock, flapCount, lastFlap) c.peers[peer] = peerMonitor + // Send an online event given it's the first time we see this peer. + peerMonitor.onlineEvent(true) + return peerMonitor, nil } @@ -374,9 +379,12 @@ func (c *ChannelEventStore) consume(subscriptions *subscriptions) { "from: %v", compressed) } - c.addChannel( - event.Channel.FundingOutpoint, peerKey, - ) + op := event.Channel.FundingOutpoint + + log.Tracef("Received OpenChannelEvent(%v) "+ + "from %v", op, peerKey) + + c.addChannel(op, peerKey) // A channel has been closed, we must remove the channel // from the store and record a channel closed event. @@ -490,7 +498,7 @@ func (c *ChannelEventStore) getChanInfo(req channelInfoRequest) (*ChannelInfo, peerMonitor, ok := c.peers[req.peer] if !ok { - return nil, ErrPeerNotFound + return nil, fmt.Errorf("%w: %v", ErrPeerNotFound, req.peer) } lifetime, uptime, err := peerMonitor.channelUptime(req.channelPoint) diff --git a/chanfitness/chaneventstore_test.go b/chanfitness/chaneventstore_test.go index 0608889a9..ecec3ea47 100644 --- a/chanfitness/chaneventstore_test.go +++ b/chanfitness/chaneventstore_test.go @@ -189,11 +189,11 @@ func TestStoreFlapCount(t *testing.T) { // flush our tick count to disk. testCtx.tickFlapCount() - // Since we just tracked a offline event, we expect a single flap for - // our peer. + // Since we just tracked an offline event, we expect two flaps for our + // peer because the channel opening also records an online event. expectedUpdate := peerFlapCountMap{ pubkey: { - Count: 1, + Count: 2, LastFlap: testCtx.clock.Now(), }, } @@ -210,9 +210,9 @@ func TestStoreFlapCount(t *testing.T) { testCtx.tickFlapCount() // Since we have processed 3 more events for our peer, we update our - // expected online map to have a flap count of 4 for this peer. + // expected online map to have a flap count of 5 for this peer. expectedUpdate[pubkey] = &channeldb.FlapCount{ - Count: 4, + Count: 5, LastFlap: testCtx.clock.Now(), } testCtx.assertFlapCountUpdated() @@ -234,13 +234,13 @@ func TestGetChanInfo(t *testing.T) { peer, pk, channel := ctx.newChannel() // Send an online event for our peer, although we do not yet have an - // open channel. + // open channel - this event will be ignored. ctx.peerEvent(peer, true) // Try to get info for a channel that has not been opened yet, we // expect to get an error. _, err := ctx.store.GetChanInfo(channel, peer) - require.Equal(t, ErrChannelNotFound, err) + require.ErrorIs(t, err, ErrPeerNotFound) // Now we send our store a notification that a channel has been opened. ctx.sendChannelOpenedUpdate(pk, channel) @@ -310,9 +310,13 @@ func TestFlapCount(t *testing.T) { ctx.start() - // Create test variables for a peer and channel, but do not add it to - // our store yet. - peer1 := route.Vertex{1, 2, 3} + // Create a valid public key for our peer. + priv, err := btcec.NewPrivateKey() + require.NoError(t, err) + pubkeyBytes := priv.PubKey().SerializeCompressed() + + var peer1 route.Vertex + copy(peer1[:], pubkeyBytes) // First, query for a peer that we have no record of in memory or on // disk and confirm that we indicate that the peer was not found. @@ -320,10 +324,23 @@ func TestFlapCount(t *testing.T) { require.NoError(t, err) require.Nil(t, ts) - // Send an online event for our peer. + // Send an online event for our peer - given this peer doesn't have a + // channel with, this event will be ignored. ctx.peerEvent(peer1, true) - // Assert that we now find a record of the peer with flap count = 1. + // Assert the event is ignored. + _, ts, err = ctx.store.FlapCount(peer1) + require.NoError(t, err) + require.Nil(t, ts) + + // Create a channel for the peer so that it will be tracked by the + // store. + pk, err := btcec.ParsePubKey(peer1[:]) + require.NoError(t, err) + ctx.sendChannelOpenedUpdate(pk, wire.OutPoint{Index: 9}) + + // Assert that we now find a record of the peer with flap count = 1, + // because the channel open fires an online event. count, ts, err := ctx.store.FlapCount(peer1) require.NoError(t, err) require.Equal(t, lastFlap, *ts) From 6c56f33a7015ad633c1bff00bd6cbf7abf62e5ca Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 18 Sep 2025 21:48:30 +0800 Subject: [PATCH 3/7] lntest: fix error msg in `ReceiveChannelEvent` --- lntest/harness.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lntest/harness.go b/lntest/harness.go index ab02af7ab..21c32a531 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -2242,7 +2242,7 @@ func (h *HarnessTest) ReceiveChannelEvent( select { case <-time.After(DefaultTimeout): - require.Fail(h, "timeout", "timeout intercepting htlc") + require.Fail(h, "timeout", "timeout receiving channel events") case err := <-errChan: require.Failf(h, "err from stream", From 22fddd626bead7655ebabecf79f99124ad79c603 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 17:02:20 +0800 Subject: [PATCH 4/7] lntest+rpcserver: ignore `chanfitness.ErrPeerNotFound` in rpc Similar to how we handle `chanfitness.ErrChannelNotFound`, we now also ignore the `ErrPeerNotFound`. This is needed as previously we will always see the peer in the channel event store given it'd added when connected. This is no longer the case as we only add the peer to the map when the channel is added. --- rpcserver.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index 96dee318b..6fb050711 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -5080,14 +5080,19 @@ func createRPCOpenChannel(ctx context.Context, r *rpcServer, // being notified of it. outpoint := dbChannel.FundingOutpoint info, err := r.server.chanEventStore.GetChanInfo(outpoint, peer) - switch err { + switch { + // If the store does not know about the peer, we just log it. + case errors.Is(err, chanfitness.ErrPeerNotFound): + rpcsLog.Warnf("peer: %v not found by channel event store", + peer) + // If the store does not know about the channel, we just log it. - case chanfitness.ErrChannelNotFound: - rpcsLog.Infof("channel: %v not found by channel event store", + case errors.Is(err, chanfitness.ErrChannelNotFound): + rpcsLog.Warnf("channel: %v not found by channel event store", outpoint) // If we got our channel info, we further populate the channel. - case nil: + case err == nil: channel.Uptime = int64(info.Uptime.Seconds()) channel.Lifetime = int64(info.Lifetime.Seconds()) From e50ecac2f22c240230aa11761afc00f37a98eb28 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 17:35:48 +0800 Subject: [PATCH 5/7] lnrpc: update docs for `FlapCount` and `LastFlapNs` --- lnrpc/lightning.pb.go | 2 ++ lnrpc/lightning.proto | 2 ++ lnrpc/lightning.swagger.json | 4 ++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lnrpc/lightning.pb.go b/lnrpc/lightning.pb.go index 3cb556c47..d08cf3a6a 100644 --- a/lnrpc/lightning.pb.go +++ b/lnrpc/lightning.pb.go @@ -5816,11 +5816,13 @@ type Peer struct { // stored for peers that we have channels open with, to prevent peers from // spamming us with errors at no cost. Errors []*TimestampedError `protobuf:"bytes,12,rep,name=errors,proto3" json:"errors,omitempty"` + // This field is populated when the peer has at least one channel with us. // The number of times we have recorded this peer going offline or coming // online, recorded across restarts. Note that this value is decreased over // time if the peer has not recently flapped, so that we can forgive peers // with historically high flap counts. FlapCount int32 `protobuf:"varint,13,opt,name=flap_count,json=flapCount,proto3" json:"flap_count,omitempty"` + // This field is populated when the peer has at least one channel with us. // The timestamp of the last flap we observed for this peer. If this value is // zero, we have not observed any flaps for this peer. LastFlapNs int64 `protobuf:"varint,14,opt,name=last_flap_ns,json=lastFlapNs,proto3" json:"last_flap_ns,omitempty"` diff --git a/lnrpc/lightning.proto b/lnrpc/lightning.proto index 31e19ff0b..9c9abddfc 100644 --- a/lnrpc/lightning.proto +++ b/lnrpc/lightning.proto @@ -1919,6 +1919,7 @@ message Peer { repeated TimestampedError errors = 12; /* + This field is populated when the peer has at least one channel with us. The number of times we have recorded this peer going offline or coming online, recorded across restarts. Note that this value is decreased over time if the peer has not recently flapped, so that we can forgive peers @@ -1927,6 +1928,7 @@ message Peer { int32 flap_count = 13; /* + This field is populated when the peer has at least one channel with us. The timestamp of the last flap we observed for this peer. If this value is zero, we have not observed any flaps for this peer. */ diff --git a/lnrpc/lightning.swagger.json b/lnrpc/lightning.swagger.json index c27c675f5..2fd4580da 100644 --- a/lnrpc/lightning.swagger.json +++ b/lnrpc/lightning.swagger.json @@ -6926,12 +6926,12 @@ "flap_count": { "type": "integer", "format": "int32", - "description": "The number of times we have recorded this peer going offline or coming\nonline, recorded across restarts. Note that this value is decreased over\ntime if the peer has not recently flapped, so that we can forgive peers\nwith historically high flap counts." + "description": "This field is populated when the peer has at least one channel with us.\nThe number of times we have recorded this peer going offline or coming\nonline, recorded across restarts. Note that this value is decreased over\ntime if the peer has not recently flapped, so that we can forgive peers\nwith historically high flap counts." }, "last_flap_ns": { "type": "string", "format": "int64", - "description": "The timestamp of the last flap we observed for this peer. If this value is\nzero, we have not observed any flaps for this peer." + "description": "This field is populated when the peer has at least one channel with us.\nThe timestamp of the last flap we observed for this peer. If this value is\nzero, we have not observed any flaps for this peer." }, "last_ping_payload": { "type": "string", From 2d59976e169d9306eec39adb454279b5af4fc263 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 17:47:06 +0800 Subject: [PATCH 6/7] rpcserver+itest: skip err from `FlapCount` We need to make sure the `ListPeers` to be robust against errors from the `FlapCount` so this RPC won't fail due to no flap count info. Also updated the itest to check this field. --- itest/lnd_misc_test.go | 11 +++++++++++ rpcserver.go | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/itest/lnd_misc_test.go b/itest/lnd_misc_test.go index 138232706..f6252132f 100644 --- a/itest/lnd_misc_test.go +++ b/itest/lnd_misc_test.go @@ -257,6 +257,17 @@ func testListChannels(ht *lntest.HarnessTest) { assertChannelConstraintsEqual( ht, aliceChannel.RemoteConstraints, bobChannel.LocalConstraints, ) + + // Finally we assert that the flap count is updated as expected. + resp := alice.RPC.ListPeers() + + // Assert Alice only have one peer. + require.Len(ht, resp.Peers, 1) + for _, p := range resp.Peers { + // The channel open event resulted in an online event, so we + // expect the flap count to be 1. + require.EqualValues(ht, 1, p.FlapCount) + } } // testMaxPendingChannels checks that error is returned from remote peer if diff --git a/rpcserver.go b/rpcserver.go index 6fb050711..d3d3c5180 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -3561,8 +3561,12 @@ func (r *rpcServer) ListPeers(ctx context.Context, flap, ts, err := r.server.chanEventStore.FlapCount( vertex, ) + + // Log the error if we cannot get the flap count instead + // of failing this RPC call. if err != nil { - return nil, err + rpcsLog.Debugf("Failed to get flap count for "+ + "peer %v", vertex) } // If our timestamp is non-nil, we have values for our From 7b6589366d7130c3d10108ebabe689c075860bf0 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 15 Sep 2025 17:50:18 +0800 Subject: [PATCH 7/7] docs: update release notes --- docs/release-notes/release-notes-0.20.0.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 50653dcff..e77089ef0 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -186,6 +186,10 @@ circuit. The indices are only available for forwarding events saved after v0.20. for the case where the DB is kvdb backed and no invoices have yet been added to the database. +* The `FlapCount` and `LastFlapNs` have been + [changed](https://github.com/lightningnetwork/lnd/pull/10211) to track + exclusively for peers that have channels with us. + ## lncli Updates * Previously, users could only specify one `outgoing_chan_id` when calling the `lncli queryroutes` or the QueryRoutes RPC. With this change, multiple