routing: prune channels only if both edges are present

We do this to ensure we don't prune too aggressively, as it's possible
that we've only received the channel announcement for a channel, but not
its accompanying channel updates.

routing/pathfind_test.go

@@ -15,6 +15,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -303,6 +304,7 @@ type testChannelPolicy struct {
 	MaxHTLC     lnwire.MilliSatoshi
 	FeeBaseMsat lnwire.MilliSatoshi
 	FeeRate     lnwire.MilliSatoshi
+	LastUpdate  time.Time
 }
 
 type testChannelEnd struct {
@@ -319,6 +321,7 @@ func defaultTestChannelEnd(alias string, capacity btcutil.Amount) *testChannelEn
 			MaxHTLC:     lnwire.NewMSatFromSatoshis(capacity),
 			FeeBaseMsat: lnwire.MilliSatoshi(1000),
 			FeeRate:     lnwire.MilliSatoshi(1),
+			LastUpdate:  testTime,
 		},
 	}
 }
@@ -500,7 +503,7 @@ func createTestGraphFromChannels(testChannels []*testChannel) (*testGraphInstanc
 				MessageFlags:              msgFlags,
 				ChannelFlags:              0,
 				ChannelID:                 channelID,
-				LastUpdate:                testTime,
+				LastUpdate:                testChannel.Node1.LastUpdate,
 				TimeLockDelta:             testChannel.Node1.Expiry,
 				MinHTLC:                   testChannel.Node1.MinHTLC,
 				MaxHTLC:                   testChannel.Node1.MaxHTLC,
@@ -522,7 +525,7 @@ func createTestGraphFromChannels(testChannels []*testChannel) (*testGraphInstanc
 				MessageFlags:              msgFlags,
 				ChannelFlags:              lnwire.ChanUpdateDirection,
 				ChannelID:                 channelID,
-				LastUpdate:                testTime,
+				LastUpdate:                testChannel.Node2.LastUpdate,
 				TimeLockDelta:             testChannel.Node2.Expiry,
 				MinHTLC:                   testChannel.Node2.MinHTLC,
 				MaxHTLC:                   testChannel.Node2.MaxHTLC,

routing/router.go

@@ -638,7 +638,7 @@ func (r *ChannelRouter) pruneZombieChans() error {
 		// If *both* edges haven't been updated for a period of
 		// chanExpiry, then we'll mark the channel itself as eligible
 		// for graph pruning.
-		e1Zombie, e2Zombie := true, true
+		var e1Zombie, e2Zombie bool
 		if e1 != nil {
 			e1Zombie = time.Since(e1.LastUpdate) >= chanExpiry
 			if e1Zombie {

routing/router_test.go

@@ -1994,6 +1994,109 @@ func TestRouterChansClosedOfflinePruneGraph(t *testing.T) {
 	}
 }
 
+// TestPruneChannelGraphStaleEdges ensures that we properly prune stale edges
+// from the channel graph.
+func TestPruneChannelGraphStaleEdges(t *testing.T) {
+	t.Parallel()
+
+	freshTimestamp := time.Now()
+	staleTimestamp := time.Time{}
+
+	// We'll create the following test graph so that only the last channel
+	// is pruned.
+	testChannels := []*testChannel{
+		// No edges.
+		{
+			Node1:     &testChannelEnd{Alias: "a"},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 1,
+		},
+
+		// Only one edge with a stale timestamp.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: staleTimestamp,
+				},
+			},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 2,
+		},
+
+		// Only one edge with a fresh timestamp.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: freshTimestamp,
+				},
+			},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 3,
+		},
+
+		// One edge fresh, one edge stale.
+		{
+			Node1: &testChannelEnd{
+				Alias: "c",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: freshTimestamp,
+				},
+			},
+			Node2: &testChannelEnd{
+				Alias: "d",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: staleTimestamp,
+				},
+			},
+			Capacity:  100000,
+			ChannelID: 4,
+		},
+
+		// Both edges fresh.
+		symmetricTestChannel("g", "h", 100000, &testChannelPolicy{
+			LastUpdate: freshTimestamp,
+		}, 5),
+
+		// Both edges stale, only one pruned.
+		symmetricTestChannel("e", "f", 100000, &testChannelPolicy{
+			LastUpdate: staleTimestamp,
+		}, 6),
+	}
+
+	// We'll create our test graph and router backed with these test
+	// channels we've created.
+	testGraph, err := createTestGraphFromChannels(testChannels)
+	if err != nil {
+		t.Fatalf("unable to create test graph: %v", err)
+	}
+	defer testGraph.cleanUp()
+
+	const startingHeight = 100
+	ctx, cleanUp, err := createTestCtxFromGraphInstance(
+		startingHeight, testGraph,
+	)
+	if err != nil {
+		t.Fatalf("unable to create test context: %v", err)
+	}
+	defer cleanUp()
+
+	// All of the channels should exist before pruning them.
+	assertChannelsPruned(t, ctx.graph, testChannels)
+
+	// Proceed to prune the channels - only the last one should be pruned.
+	if err := ctx.router.pruneZombieChans(); err != nil {
+		t.Fatalf("unable to prune zombie channels: %v", err)
+	}
+
+	prunedChannel := testChannels[len(testChannels)-1].ChannelID
+	assertChannelsPruned(t, ctx.graph, testChannels, prunedChannel)
+}
+
 // TestFindPathFeeWeighting tests that the findPath method will properly prefer
 // routes with lower fees over routes with lower time lock values. This is
 // meant to exercise the fact that the internal findPath method ranks edges
@@ -2297,3 +2400,44 @@ func TestEmptyRoutesGenerateSphinxPacket(t *testing.T) {
 		t.Fatalf("expected empty hops error: instead got: %v", err)
 	}
 }
+
+// assertChannelsPruned ensures that only the given channels are pruned from the
+// graph out of the set of all channels.
+func assertChannelsPruned(t *testing.T, graph *channeldb.ChannelGraph,
+	channels []*testChannel, prunedChanIDs ...uint64) {
+
+	t.Helper()
+
+	pruned := make(map[uint64]struct{}, len(channels))
+	for _, chanID := range prunedChanIDs {
+		pruned[chanID] = struct{}{}
+	}
+
+	for _, channel := range channels {
+		_, shouldPrune := pruned[channel.ChannelID]
+		_, _, exists, isZombie, err := graph.HasChannelEdge(
+			channel.ChannelID,
+		)
+		if err != nil {
+			t.Fatalf("unable to determine existence of "+
+				"channel=%v in the graph: %v",
+				channel.ChannelID, err)
+		}
+		if !shouldPrune && !exists {
+			t.Fatalf("expected channel=%v to exist within "+
+				"the graph", channel.ChannelID)
+		}
+		if shouldPrune && exists {
+			t.Fatalf("expected channel=%v to not exist "+
+				"within the graph", channel.ChannelID)
+		}
+		if !shouldPrune && isZombie {
+			t.Fatalf("expected channel=%v to not be marked "+
+				"as zombie", channel.ChannelID)
+		}
+		if shouldPrune && !isZombie {
+			t.Fatalf("expected channel=%v to be marked as "+
+				"zombie", channel.ChannelID)
+		}
+	}
+}