routing: prune channels with disabled bit set on both edges

In this commit, we add an additional heuristic when running with
AssumeChannelValid. Since AssumeChannelValid being present assumes that
we're not able to quickly determine whether channels are valid, we also
assume that any channels with the disabled bit set on both sides are
considered zombie. This should be relatively safe to do, since the
disabled bits are usually set when the channel is closed on-chain. In
the case that they aren't, we'll have to wait until both edges haven't
had a new update within two weeks to prune them.

routing/pathfind_test.go

@@ -305,6 +305,7 @@ type testChannelPolicy struct {
 	FeeBaseMsat lnwire.MilliSatoshi
 	FeeRate     lnwire.MilliSatoshi
 	LastUpdate  time.Time
+	Disabled    bool
 }
 
 type testChannelEnd struct {
@@ -322,6 +323,7 @@ func defaultTestChannelEnd(alias string, capacity btcutil.Amount) *testChannelEn
 			FeeBaseMsat: lnwire.MilliSatoshi(1000),
 			FeeRate:     lnwire.MilliSatoshi(1),
 			LastUpdate:  testTime,
+			Disabled:    false,
 		},
 	}
 }
@@ -496,12 +498,16 @@ func createTestGraphFromChannels(testChannels []*testChannel) (*testGraphInstanc
 		if testChannel.Node1.testChannelPolicy != nil {
 			var msgFlags lnwire.ChanUpdateMsgFlags
 			if testChannel.Node1.MaxHTLC != 0 {
-				msgFlags = 1
+				msgFlags |= lnwire.ChanUpdateOptionMaxHtlc
+			}
+			var channelFlags lnwire.ChanUpdateChanFlags
+			if testChannel.Node1.Disabled {
+				channelFlags |= lnwire.ChanUpdateDisabled
 			}
 			edgePolicy := &channeldb.ChannelEdgePolicy{
 				SigBytes:                  testSig.Serialize(),
 				MessageFlags:              msgFlags,
-				ChannelFlags:              0,
+				ChannelFlags:              channelFlags,
 				ChannelID:                 channelID,
 				LastUpdate:                testChannel.Node1.LastUpdate,
 				TimeLockDelta:             testChannel.Node1.Expiry,
@@ -518,12 +524,16 @@ func createTestGraphFromChannels(testChannels []*testChannel) (*testGraphInstanc
 		if testChannel.Node2.testChannelPolicy != nil {
 			var msgFlags lnwire.ChanUpdateMsgFlags
 			if testChannel.Node2.MaxHTLC != 0 {
-				msgFlags = 1
+				msgFlags |= lnwire.ChanUpdateOptionMaxHtlc
+			}
+			channelFlags := lnwire.ChanUpdateDirection
+			if testChannel.Node2.Disabled {
+				channelFlags |= lnwire.ChanUpdateDisabled
 			}
 			edgePolicy := &channeldb.ChannelEdgePolicy{
 				SigBytes:                  testSig.Serialize(),
 				MessageFlags:              msgFlags,
-				ChannelFlags:              lnwire.ChanUpdateDirection,
+				ChannelFlags:              channelFlags,
 				ChannelID:                 channelID,
 				LastUpdate:                testChannel.Node2.LastUpdate,
 				TimeLockDelta:             testChannel.Node2.Expiry,

routing/router.go

@@ -613,13 +613,15 @@ func (r *ChannelRouter) syncGraphWithChain() error {
 
 // pruneZombieChans is a method that will be called periodically to prune out
 // any "zombie" channels. We consider channels zombies if *both* edges haven't
-// been updated since our zombie horizon. We do this periodically to keep a
+// been updated since our zombie horizon. If AssumeChannelValid is present,
-// health, lively routing table.
+// we'll also consider channels zombies if *both* edges are disabled. This
+// usually signals that a channel has been closed on-chain. We do this
+// periodically to keep a healthy, lively routing table.
 func (r *ChannelRouter) pruneZombieChans() error {
 	var chansToPrune []wire.OutPoint
 	chanExpiry := r.cfg.ChannelPruneExpiry
 
-	log.Infof("Examining Channel Graph for zombie channels")
+	log.Infof("Examining channel graph for zombie channels")
 
 	// First, we'll collect all the channels which are eligible for garbage
 	// collection due to being zombies.
@@ -655,20 +657,49 @@ func (r *ChannelRouter) pruneZombieChans() error {
 					info.ChannelPoint, e2.LastUpdate)
 			}
 		}
-		if e1Zombie && e2Zombie {
-			log.Debugf("ChannelPoint(%v) is a zombie, collecting "+
-				"to prune", info.ChannelPoint)
 
-			// TODO(roasbeef): add ability to delete single
+		isZombieChan := e1Zombie && e2Zombie
-			// directional edge
-			chansToPrune = append(chansToPrune, info.ChannelPoint)
 
-			// As we're detecting this as a zombie channel, we'll
+		// If AssumeChannelValid is present and we've determined the
-			// add this to the set of recently rejected items so we
+		// channel is not a zombie, we'll look at the disabled bit for
-			// don't re-accept it shortly after.
+		// both edges. If they're both disabled, then we can interpret
-			r.rejectCache[info.ChannelID] = struct{}{}
+		// this as the channel being closed and can prune it from our
+		// graph.
+		if r.cfg.AssumeChannelValid && !isZombieChan {
+			var e1Disabled, e2Disabled bool
+			if e1 != nil {
+				e1Disabled = e1.IsDisabled()
+				log.Tracef("Edge #1 of ChannelID(%v) "+
+					"disabled=%v", info.ChannelID,
+					e1Disabled)
+			}
+			if e2 != nil {
+				e2Disabled = e2.IsDisabled()
+				log.Tracef("Edge #2 of ChannelID(%v) "+
+					"disabled=%v", info.ChannelID,
+					e2Disabled)
+			}
+
+			isZombieChan = e1Disabled && e2Disabled
 		}
 
+		// If the channel is not considered zombie, we can move on to
+		// the next.
+		if !isZombieChan {
+			return nil
+		}
+
+		log.Debugf("ChannelPoint(%v) is a zombie, collecting to prune",
+			info.ChannelPoint)
+
+		// TODO(roasbeef): add ability to delete single directional edge
+		chansToPrune = append(chansToPrune, info.ChannelPoint)
+
+		// As we're detecting this as a zombie channel, we'll add this
+		// to the set of recently rejected items so we don't re-accept
+		// it shortly after.
+		r.rejectCache[info.ChannelID] = struct{}{}
+
 		return nil
 	}
 
@@ -677,21 +708,22 @@ func (r *ChannelRouter) pruneZombieChans() error {
 
 	err := r.cfg.Graph.ForEachChannel(filterPruneChans)
 	if err != nil {
-		return fmt.Errorf("Unable to filter local zombie "+
+		return fmt.Errorf("unable to filter local zombie channels: "+
-			"chans: %v", err)
+			"%v", err)
 	}
 
-	log.Infof("Pruning %v Zombie Channels", len(chansToPrune))
+	log.Infof("Pruning %v zombie channels", len(chansToPrune))
 
-	// With the set zombie-like channels obtained, we'll do another pass to
+	// With the set of zombie-like channels obtained, we'll do another pass
-	// delete al zombie channels from the channel graph.
+	// to delete them from the channel graph.
 	for _, chanToPrune := range chansToPrune {
-		log.Tracef("Pruning zombie chan ChannelPoint(%v)", chanToPrune)
+		log.Tracef("Pruning zombie channel with ChannelPoint(%v)",
+			chanToPrune)
 
 		err := r.cfg.Graph.DeleteChannelEdge(&chanToPrune)
 		if err != nil {
-			return fmt.Errorf("Unable to prune zombie "+
+			return fmt.Errorf("unable to prune zombie with "+
-				"chans: %v", err)
+				"ChannelPoint(%v): %v", chanToPrune, err)
 		}
 	}
 
@@ -906,7 +938,7 @@ func (r *ChannelRouter) networkHandler() {
 		// for pruning.
 		case <-graphPruneTicker.C:
 			if err := r.pruneZombieChans(); err != nil {
-				log.Errorf("unable to prune zombies: %v", err)
+				log.Errorf("Unable to prune zombies: %v", err)
 			}
 
 		// The router has been signalled to exit, to we exit our main

routing/router_test.go

@@ -2097,6 +2097,124 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) {
 	assertChannelsPruned(t, ctx.graph, testChannels, prunedChannel)
 }
 
+// TestPruneChannelGraphDoubleDisabled test that we can properly prune channels
+// with both edges disabled from our channel graph.
+func TestPruneChannelGraphDoubleDisabled(t *testing.T) {
+	t.Parallel()
+
+	// We'll create the following test graph so that only the last channel
+	// is pruned. We'll use a fresh timestamp to ensure they're not pruned
+	// according to that heuristic.
+	timestamp := time.Now()
+	testChannels := []*testChannel{
+		// No edges.
+		{
+			Node1:     &testChannelEnd{Alias: "a"},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 1,
+		},
+
+		// Only one edge disabled.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: timestamp,
+					Disabled:   true,
+				},
+			},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 2,
+		},
+
+		// Only one edge enabled.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: timestamp,
+					Disabled:   false,
+				},
+			},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 3,
+		},
+
+		// One edge disabled, one edge enabled.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: timestamp,
+					Disabled:   true,
+				},
+			},
+			Node2: &testChannelEnd{
+				Alias: "b",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: timestamp,
+					Disabled:   false,
+				},
+			},
+			Capacity:  100000,
+			ChannelID: 1,
+		},
+
+		// Both edges enabled.
+		symmetricTestChannel("c", "d", 100000, &testChannelPolicy{
+			LastUpdate: timestamp,
+			Disabled:   false,
+		}, 2),
+
+		// Both edges disabled, only one pruned.
+		symmetricTestChannel("e", "f", 100000, &testChannelPolicy{
+			LastUpdate: timestamp,
+			Disabled:   true,
+		}, 3),
+	}
+
+	// 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 the channels should exist within the graph before pruning them.
+	assertChannelsPruned(t, ctx.graph, testChannels)
+
+	// If we attempt to prune them without AssumeChannelValid being set,
+	// none should be pruned.
+	if err := ctx.router.pruneZombieChans(); err != nil {
+		t.Fatalf("unable to prune zombie channels: %v", err)
+	}
+
+	assertChannelsPruned(t, ctx.graph, testChannels)
+
+	// Now that AssumeChannelValid is set, we'll prune the graph again and
+	// the last channel should be the only one pruned.
+	ctx.router.cfg.AssumeChannelValid = true
+	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