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/router.go

@@ -628,13 +628,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
-// health, lively routing table.
+// been updated since our zombie horizon. If AssumeChannelValid is present,
+// 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 health, 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.
@@ -670,20 +672,48 @@ 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
-			// 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{}{}
+
+		isZombieChan := e1Zombie && e2Zombie
+
+		switch {
+		// If AssumeChannelValid is present and If we've determined the
+		// channel is not a zombie, we'll look at the disabled bit for
+		// both edges. If they're both disabled, then we can interpret
+		// this as the channel being closed and can prune it from our
+		// graph.
+		case r.cfg.AssumeChannelValid && !isZombieChan:
+			e1Disabled := e1.ChannelFlags&
+				lnwire.ChanUpdateDisabled == lnwire.ChanUpdateDisabled
+			e2Disabled := e2.ChannelFlags&
+				lnwire.ChanUpdateDisabled == lnwire.ChanUpdateDisabled
+
+			log.Tracef("Edge #1 of ChannelPoint(%v) disabled=%v",
+				e1Disabled)
+			log.Tracef("Edge #2 of ChannelPoint(%v) disabled=%v",
+				e2Disabled)
+
+			isZombieChan = e1Disabled && e2Disabled
+			if !isZombieChan {
+				return nil
+			}
+
+		// If the channel is not considered zombie, we can move on to
+		// the next.
+		case !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
 	}
 
@@ -692,21 +722,22 @@ func (r *ChannelRouter) pruneZombieChans() error {
 
 	err := r.cfg.Graph.ForEachChannel(filterPruneChans)
 	if err != nil {
-		return fmt.Errorf("Unable to filter local zombie "+
-			"chans: %v", err)
+		return fmt.Errorf("unable to filter local zombie channels: "+
+			"%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
-	// delete al zombie channels from the channel graph.
+	// With the set of zombie-like channels obtained, we'll do another pass
+	// 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 "+
-				"chans: %v", err)
+			return fmt.Errorf("unable to prune zombie with "+
+				"ChannelPoint(%v): %v", chanToPrune, err)
 		}
 	}
 
@@ -939,7 +970,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

@@ -1966,6 +1966,107 @@ func TestRouterChansClosedOfflinePruneGraph(t *testing.T) {
 	}
 }
 
+// TestPruneChannelGraphDoubleDisabled test that we can properly prune channels
+// with both edges disabled from our channel graph.
+func TestPruneChannelGraphDoubleDisabled(t *testing.T) {
+	t.Parallel()
+
+	// For this test, we'll be creating three channels:
+	//   * The first will have *both* edges disabled.
+	//   * The second will *not* have any edges disabled.
+	//   * The third will have *one* edge disabled.
+	//
+	// With this setup, only the first will be pruned from the graph.
+	testChannels := []*testChannel{
+		symmetricTestChannel("a", "b", 100000, &testChannelPolicy{
+			Disabled: true,
+		}, 1),
+		symmetricTestChannel("b", "c", 100000, &testChannelPolicy{
+			Disabled: false,
+		}, 2),
+		symmetricTestChannel("c", "d", 100000, &testChannelPolicy{
+			Disabled: false,
+		}, 3),
+	}
+	testChannels[len(testChannels)-1].Node1.Disabled = true
+
+	// 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()
+
+	// assertChannelExistence is a helper closure that ensures channels are
+	// properly pruned from the graph.
+	assertChannelExistence := func(channels ...uint64) {
+		t.Helper()
+
+		s := make(map[uint64]struct{}, len(channels))
+		for _, channel := range channels {
+			s[channel] = struct{}{}
+		}
+
+		for _, channel := range testChannels {
+			_, shouldExist := s[channel.ChannelID]
+
+			_, _, exist, _, err := ctx.router.cfg.Graph.HasChannelEdge(
+				channel.ChannelID,
+			)
+			if err != nil {
+				t.Fatalf("unable to determine existence of "+
+					"channel=%v in the graph: %v",
+					channel.ChannelID, err)
+			}
+			if shouldExist && !exist {
+				t.Fatalf("expected channel=%v to exist within "+
+					"the graph", channel.ChannelID)
+			}
+			if !shouldExist && exist {
+				t.Fatalf("expected channel=%v to not exist "+
+					"within the graph", channel.ChannelID)
+			}
+		}
+	}
+
+	// All of the channels we created above have a LastUpdate of `testTime`.
+	// To ensure these channels are not pruned because of the stale
+	// heuristic, we'll set the window to an hour before the existing
+	// LastUpdate.
+	ctx.router.cfg.ChannelPruneExpiry = time.Since(testTime.Add(-time.Hour))
+
+	// All the channels should exist within the graph before pruning them.
+	assertChannelExistence(1, 2, 3)
+
+	// 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)
+	}
+
+	assertChannelExistence(1, 2, 3)
+
+	// Now that AssumeChannelValid is set, we'll prune the graph again and
+	// the first 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)
+	}
+
+	assertChannelExistence(2, 3)
+}
+
 // 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