diff --git a/chanfitness/chaneventstore.go b/chanfitness/chaneventstore.go index 881e9a35e..a8014a8f9 100644 --- a/chanfitness/chaneventstore.go +++ b/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 } diff --git a/chanfitness/chaneventstore_test.go b/chanfitness/chaneventstore_test.go index f14eba1d3..f2e578157 100644 --- a/chanfitness/chaneventstore_test.go +++ b/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. diff --git a/chanfitness/chaneventstore_testctx_test.go b/chanfitness/chaneventstore_testctx_test.go index aff4c5fca..6906ebbb6 100644 --- a/chanfitness/chaneventstore_testctx_test.go +++ b/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 }, diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 45e760519..22ec010bb 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/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 diff --git a/server.go b/server.go index 5aad97198..9eb2b2a08 100644 --- a/server.go +++ b/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,