From 4d00eb2aa43669b3367e6cb7a42bfb83ed8e0f18 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 19 Feb 2025 06:24:17 -0300 Subject: [PATCH] graph/db: move FilterKnownChanIDs zombie logic up one layer Here, we move the business logic in FilterKnownChanIDs from the CRUD layer to the ChannelGraph layer. We also add a test for the logic. --- graph/db/graph.go | 53 ++++++++++++++++++++++++++++++++ graph/db/graph_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ graph/db/kv_store.go | 61 ++++++++++-------------------------- 3 files changed, 140 insertions(+), 44 deletions(-) diff --git a/graph/db/graph.go b/graph/db/graph.go index 6185de410..1d53ad3c2 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -1,6 +1,7 @@ package graphdb import ( + "errors" "sync" "time" @@ -386,3 +387,55 @@ func (c *ChannelGraph) PruneGraphNodes() error { return nil } + +// FilterKnownChanIDs takes a set of channel IDs and return the subset of chan +// ID's that we don't know and are not known zombies of the passed set. In other +// words, we perform a set difference of our set of chan ID's and the ones +// passed in. This method can be used by callers to determine the set of +// channels another peer knows of that we don't. +func (c *ChannelGraph) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo, + isZombieChan func(time.Time, time.Time) bool) ([]uint64, error) { + + unknown, knownZombies, err := c.KVStore.FilterKnownChanIDs(chansInfo) + if err != nil { + return nil, err + } + + for _, info := range knownZombies { + // TODO(ziggie): Make sure that for the strict pruning case we + // compare the pubkeys and whether the right timestamp is not + // older than the `ChannelPruneExpiry`. + // + // NOTE: The timestamp data has no verification attached to it + // in the `ReplyChannelRange` msg so we are trusting this data + // at this point. However it is not critical because we are just + // removing the channel from the db when the timestamps are more + // recent. During the querying of the gossip msg verification + // happens as usual. However we should start punishing peers + // when they don't provide us honest data ? + isStillZombie := isZombieChan( + info.Node1UpdateTimestamp, info.Node2UpdateTimestamp, + ) + + if isStillZombie { + continue + } + + // If we have marked it as a zombie but the latest update + // timestamps could bring it back from the dead, then we mark it + // alive, and we let it be added to the set of IDs to query our + // peer for. + err := c.KVStore.MarkEdgeLive( + info.ShortChannelID.ToUint64(), + ) + // Since there is a chance that the edge could have been marked + // as "live" between the FilterKnownChanIDs call and the + // MarkEdgeLive call, we ignore the error if the edge is already + // marked as live. + if err != nil && !errors.Is(err, ErrZombieEdgeNotFound) { + return nil, err + } + } + + return unknown, nil +} diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index cf5452eb7..f336294a1 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -1919,6 +1919,76 @@ func TestNodeUpdatesInHorizon(t *testing.T) { } } +// TestFilterKnownChanIDsZombieRevival tests that if a ChannelUpdateInfo is +// passed to FilterKnownChanIDs that contains a channel that we have marked as +// a zombie, then we will mark it as live again if the new ChannelUpdate has +// timestamps that would make the channel be considered live again. +// +// NOTE: this tests focuses on zombie revival. The main logic of +// FilterKnownChanIDs is tested in TestFilterKnownChanIDs. +func TestFilterKnownChanIDsZombieRevival(t *testing.T) { + t.Parallel() + + graph, err := MakeTestGraph(t) + require.NoError(t, err) + + var ( + scid1 = lnwire.ShortChannelID{BlockHeight: 1} + scid2 = lnwire.ShortChannelID{BlockHeight: 2} + scid3 = lnwire.ShortChannelID{BlockHeight: 3} + ) + + isZombie := func(scid lnwire.ShortChannelID) bool { + zombie, _, _ := graph.IsZombieEdge(scid.ToUint64()) + return zombie + } + + // Mark channel 1 and 2 as zombies. + err = graph.MarkEdgeZombie(scid1.ToUint64(), [33]byte{}, [33]byte{}) + require.NoError(t, err) + err = graph.MarkEdgeZombie(scid2.ToUint64(), [33]byte{}, [33]byte{}) + require.NoError(t, err) + + require.True(t, isZombie(scid1)) + require.True(t, isZombie(scid2)) + require.False(t, isZombie(scid3)) + + // Call FilterKnownChanIDs with an isStillZombie call-back that would + // result in the current zombies still be considered as zombies. + _, err = graph.FilterKnownChanIDs([]ChannelUpdateInfo{ + {ShortChannelID: scid1}, + {ShortChannelID: scid2}, + {ShortChannelID: scid3}, + }, func(_ time.Time, _ time.Time) bool { + return true + }) + require.NoError(t, err) + + require.True(t, isZombie(scid1)) + require.True(t, isZombie(scid2)) + require.False(t, isZombie(scid3)) + + // Now call it again but this time with a isStillZombie call-back that + // would result in channel with SCID 2 no longer being considered a + // zombie. + _, err = graph.FilterKnownChanIDs([]ChannelUpdateInfo{ + {ShortChannelID: scid1}, + { + ShortChannelID: scid2, + Node1UpdateTimestamp: time.Unix(1000, 0), + }, + {ShortChannelID: scid3}, + }, func(t1 time.Time, _ time.Time) bool { + return !t1.Equal(time.Unix(1000, 0)) + }) + require.NoError(t, err) + + // Show that SCID 2 has been marked as live. + require.True(t, isZombie(scid1)) + require.False(t, isZombie(scid2)) + require.False(t, isZombie(scid3)) +} + // TestFilterKnownChanIDs tests that we're able to properly perform the set // differences of an incoming set of channel ID's, and those that we already // know of on disk. diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go index 486971e03..04e0d75ae 100644 --- a/graph/db/kv_store.go +++ b/graph/db/kv_store.go @@ -2155,16 +2155,20 @@ func (c *KVStore) NodeUpdatesInHorizon(startTime, // ID's that we don't know and are not known zombies of the passed set. In other // words, we perform a set difference of our set of chan ID's and the ones // passed in. This method can be used by callers to determine the set of -// channels another peer knows of that we don't. -func (c *KVStore) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo, - isZombieChan func(time.Time, time.Time) bool) ([]uint64, error) { +// channels another peer knows of that we don't. The ChannelUpdateInfos for the +// known zombies is also returned. +func (c *KVStore) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo) ([]uint64, + []ChannelUpdateInfo, error) { - var newChanIDs []uint64 + var ( + newChanIDs []uint64 + knownZombies []ChannelUpdateInfo + ) c.cacheMu.Lock() defer c.cacheMu.Unlock() - err := kvdb.Update(c.db, func(tx kvdb.RwTx) error { + err := kvdb.View(c.db, func(tx kvdb.RTx) error { edges := tx.ReadBucket(edgeBucket) if edges == nil { return ErrGraphNoEdgesFound @@ -2197,44 +2201,12 @@ func (c *KVStore) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo, zombieIndex, scid, ) - // TODO(ziggie): Make sure that for the strict - // pruning case we compare the pubkeys and - // whether the right timestamp is not older than - // the `ChannelPruneExpiry`. - // - // NOTE: The timestamp data has no verification - // attached to it in the `ReplyChannelRange` msg - // so we are trusting this data at this point. - // However it is not critical because we are - // just removing the channel from the db when - // the timestamps are more recent. During the - // querying of the gossip msg verification - // happens as usual. - // However we should start punishing peers when - // they don't provide us honest data ? - isStillZombie := isZombieChan( - info.Node1UpdateTimestamp, - info.Node2UpdateTimestamp, - ) + if isZombie { + knownZombies = append( + knownZombies, info, + ) - switch { - // If the edge is a known zombie and if we - // would still consider it a zombie given the - // latest update timestamps, then we skip this - // channel. - case isZombie && isStillZombie: continue - - // Otherwise, if we have marked it as a zombie - // but the latest update timestamps could bring - // it back from the dead, then we mark it alive, - // and we let it be added to the set of IDs to - // query our peer for. - case isZombie && !isStillZombie: - err := c.markEdgeLiveUnsafe(tx, scid) - if err != nil { - return err - } } } @@ -2244,6 +2216,7 @@ func (c *KVStore) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo, return nil }, func() { newChanIDs = nil + knownZombies = nil }) switch { // If we don't know of any edges yet, then we'll return the entire set @@ -2254,13 +2227,13 @@ func (c *KVStore) FilterKnownChanIDs(chansInfo []ChannelUpdateInfo, ogChanIDs[i] = info.ShortChannelID.ToUint64() } - return ogChanIDs, nil + return ogChanIDs, nil, nil case err != nil: - return nil, err + return nil, nil, err } - return newChanIDs, nil + return newChanIDs, knownZombies, nil } // ChannelUpdateInfo couples the SCID of a channel with the timestamps of the