Merge pull request #10890 from erickcestari/fix-offline-peer-uptime

chanfitness: seed peer uptime from actual online state

chanfitness/chaneventstore.go

@@ -86,6 +86,13 @@ type Config struct {
 	// startup.
 	GetOpenChannels func() ([]*channeldb.OpenChannel, error)
 
+	// IsPeerOnline returns whether the peer with the given pubkey is
+	// currently connected. It is used to seed the initial online state of a
+	// peer when we first start tracking it, so that uptime is calculated
+	// from the peer's actual connectivity rather than assuming it is
+	// online.
+	IsPeerOnline func(route.Vertex) bool
+
 	// Clock is the time source that the subsystem uses, provided here
 	// for ease of testing.
 	Clock clock.Clock
@@ -291,8 +298,8 @@ func (c *ChannelEventStore) getOrCreatePeerMonitor(
 	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)
+	// Send an liveness event given it's the first time we see this peer.
+	peerMonitor.onlineEvent(c.cfg.IsPeerOnline(peer))
 
 	return peerMonitor, nil
 }

chanfitness/chaneventstore_test.go

@@ -287,6 +287,59 @@ func TestGetChanInfo(t *testing.T) {
 	ctx.stop()
 }
 
+// TestGetChanInfoOfflinePeer tests that a channel whose peer is offline when we
+// start tracking it reports zero uptime, rather than assuming the peer is
+// online (which would incorrectly report 100% uptime).
+func TestGetChanInfoOfflinePeer(t *testing.T) {
+	ctx := newChanEventStoreTestCtx(t)
+
+	// Report the peer as offline so that the channel open seeds an offline
+	// event instead of assuming the peer is connected.
+	ctx.peerOnline = func(route.Vertex) bool { return false }
+
+	ctx.start()
+
+	now := ctx.clock.Now()
+
+	peer, pk, channel := ctx.newChannel()
+	ctx.sendChannelOpenedUpdate(pk, channel)
+
+	// Wait for our channel to be recognized by our store.
+	require.Eventually(t, func() bool {
+		_, err := ctx.store.GetChanInfo(channel, peer)
+		return err == nil
+	}, timeout, time.Millisecond*20)
+
+	// Advance our clock by an hour. Since the peer has been offline the
+	// whole time, we expect the channel to have a full hour of lifetime but
+	// zero uptime.
+	now = now.Add(time.Hour)
+	ctx.clock.SetTime(now)
+
+	info, err := ctx.store.GetChanInfo(channel, peer)
+	require.NoError(t, err)
+	require.Equal(t, time.Hour, info.Lifetime)
+	require.Equal(t, time.Duration(0), info.Uptime)
+
+	// Once the peer comes online, uptime should start accruing from that
+	// point. We issue a blocking GetChanInfo afterwards to ensure the
+	// online event has been fully processed (and timestamped at the current
+	// time) by the store's main loop before we advance the clock.
+	ctx.peerEvent(peer, true)
+	_, err = ctx.store.GetChanInfo(channel, peer)
+	require.NoError(t, err)
+
+	now = now.Add(time.Hour)
+	ctx.clock.SetTime(now)
+
+	info, err = ctx.store.GetChanInfo(channel, peer)
+	require.NoError(t, err)
+	require.Equal(t, time.Hour*2, info.Lifetime)
+	require.Equal(t, time.Hour, info.Uptime)
+
+	ctx.stop()
+}
+
 // TestFlapCount tests querying the store for peer flap counts, covering the
 // case where the peer is tracked in memory, and the case where we need to
 // lookup the peer on disk.

chanfitness/chaneventstore_testctx_test.go

@@ -49,6 +49,13 @@ type chanEventStoreTestCtx struct {
 	// used to prevent calling of functions which can only be called after
 	// shutdown.
 	stopped chan struct{}
+
+	// peerOnline determines what the store's IsPeerOnline config returns
+	// for a peer. It defaults to reporting peers as online so that the
+	// channel open seeds an online event, matching the historical test
+	// assumption. Tests that exercise offline peers may override it before
+	// starting the store.
+	peerOnline func(route.Vertex) bool
 }
 
 // newChanEventStoreTestCtx creates a test context which can be used to test
@@ -62,10 +69,14 @@ func newChanEventStoreTestCtx(t *testing.T) *chanEventStoreTestCtx {
 		flapUpdates:         make(peerFlapCountMap),
 		flapCountUpdates:    make(chan peerFlapCountMap),
 		stopped:             make(chan struct{}),
+		peerOnline:          func(route.Vertex) bool { return true },
 	}
 
 	cfg := &Config{
 		Clock: testCtx.clock,
+		IsPeerOnline: func(peer route.Vertex) bool {
+			return testCtx.peerOnline(peer)
+		},
 		SubscribeChannelEvents: func() (subscribe.Subscription, error) {
 			return testCtx.channelSubscription, nil
 		},

docs/release-notes/release-notes-0.22.0.md

@@ -34,6 +34,13 @@
   the close transaction is actually broadcast, and
   `WaitingCloseChannel.ClosingTx` is never empty.
 
+* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/10890)
+  where `ListChannels` reported 100% `uptime` for channels whose peer
+  was offline. The channel fitness store assumed a peer was online when
+  it first started tracking it, but channels are loaded on startup
+  regardless of peer connectivity. Uptime is now seeded from the peer's
+  actual connection state.
+
 # New Features
 
 ## Functional Enhancements

server.go

@@ -1772,6 +1772,10 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr,
 			return s.peerNotifier.SubscribePeerEvents()
 		},
 		GetOpenChannels: s.chanStateDB.FetchAllOpenChannels,
+		IsPeerOnline: func(peer route.Vertex) bool {
+			_, err := s.FindPeerByPubStr(string(peer[:]))
+			return err == nil
+		},
 		Clock:           clock.NewDefaultClock(),
 		ReadFlapCount:   s.miscDB.ReadFlapCount,
 		WriteFlapCount:  s.miscDB.WriteFlapCounts,