routing: add strict zombie pruning as a config level param

In this commit, we add strict zombie pruning as a config level param.
This allow us to add the option for those that want a tighter graph, and
not change the default composition of the channel graph for most users
over night.

In addition, we expand the test case slightly by testing that the self
node won't be pruned, but also that if there's a node with only a single
known stale edge, then both variants will prune that edge.

channeldb/graph.go

@@ -188,6 +188,7 @@ type ChannelGraph struct {
 // returned instance has its own unique reject cache and channel cache.
 func newChannelGraph(db *DB, rejectCacheSize, chanCacheSize int,
 	batchCommitInterval time.Duration) *ChannelGraph {
+
 	g := &ChannelGraph{
 		db:          db,
 		rejectCache: newRejectCache(rejectCacheSize),
@@ -199,6 +200,7 @@ func newChannelGraph(db *DB, rejectCacheSize, chanCacheSize int,
 	g.nodeScheduler = batch.NewTimeScheduler(
 		db.Backend, nil, batchCommitInterval,
 	)
+
 	return g
 }
 
@@ -953,7 +955,7 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint,
 			// was successfully pruned.
 			err = delChannelEdge(
 				edges, edgeIndex, chanIndex, zombieIndex, nodes,
-				chanID, false,
+				chanID, false, false,
 			)
 			if err != nil && err != ErrEdgeNotFound {
 				return err
@@ -1202,7 +1204,7 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf
 		for _, k := range keys {
 			err = delChannelEdge(
 				edges, edgeIndex, chanIndex, zombieIndex, nodes,
-				k, false,
+				k, false, false,
 			)
 			if err != nil && err != ErrEdgeNotFound {
 				return err
@@ -1301,11 +1303,14 @@ func (c *ChannelGraph) PruneTip() (*chainhash.Hash, uint32, error) {
 	return &tipHash, tipHeight, nil
 }
 
-// DeleteChannelEdges removes edges with the given channel IDs from the database
+// DeleteChannelEdges removes edges with the given channel IDs from the
-// and marks them as zombies. This ensures that we're unable to re-add it to our
+// database and marks them as zombies. This ensures that we're unable to re-add
-// database once again. If an edge does not exist within the database, then
+// it to our database once again. If an edge does not exist within the
-// ErrEdgeNotFound will be returned.
+// database, then ErrEdgeNotFound will be returned. If strictZombiePruning is
-func (c *ChannelGraph) DeleteChannelEdges(chanIDs ...uint64) error {
+// true, then when we mark these edges as zombies, we'll set up the keys such
+// that we require the node that failed to send the fresh update to be the one
+// that resurrects the channel from its zombie state.
+func (c *ChannelGraph) DeleteChannelEdges(strictZombiePruning bool, chanIDs ...uint64) error {
 	// TODO(roasbeef): possibly delete from node bucket if node has no more
 	// channels
 	// TODO(roasbeef): don't delete both edges?
@@ -1340,7 +1345,7 @@ func (c *ChannelGraph) DeleteChannelEdges(chanIDs ...uint64) error {
 			byteOrder.PutUint64(rawChanID[:], chanID)
 			err := delChannelEdge(
 				edges, edgeIndex, chanIndex, zombieIndex, nodes,
-				rawChanID[:], true,
+				rawChanID[:], true, strictZombiePruning,
 			)
 			if err != nil {
 				return err
@@ -1929,7 +1934,7 @@ func delEdgeUpdateIndexEntry(edgesBucket kvdb.RwBucket, chanID uint64,
 }
 
 func delChannelEdge(edges, edgeIndex, chanIndex, zombieIndex,
-	nodes kvdb.RwBucket, chanID []byte, isZombie bool) error {
+	nodes kvdb.RwBucket, chanID []byte, isZombie, strictZombie bool) error {
 
 	edgeInfo, err := fetchChanEdgeInfo(edgeIndex, chanID)
 	if err != nil {
@@ -1997,7 +2002,10 @@ func delChannelEdge(edges, edgeIndex, chanIndex, zombieIndex,
 		return nil
 	}
 
-	nodeKey1, nodeKey2 := makeZombiePubkeys(&edgeInfo, edge1, edge2)
+	nodeKey1, nodeKey2 := edgeInfo.NodeKey1Bytes, edgeInfo.NodeKey2Bytes
+	if strictZombie {
+		nodeKey1, nodeKey2 = makeZombiePubkeys(&edgeInfo, edge1, edge2)
+	}
 
 	return markEdgeZombie(
 		zombieIndex, byteOrder.Uint64(chanID), nodeKey1, nodeKey2,

channeldb/graph_test.go

@@ -368,7 +368,7 @@ func TestEdgeInsertionDeletion(t *testing.T) {
 
 	// Next, attempt to delete the edge from the database, again this
 	// should proceed without any issues.
-	if err := graph.DeleteChannelEdges(chanID); err != nil {
+	if err := graph.DeleteChannelEdges(false, chanID); err != nil {
 		t.Fatalf("unable to delete edge: %v", err)
 	}
 
@@ -387,7 +387,7 @@ func TestEdgeInsertionDeletion(t *testing.T) {
 
 	// Finally, attempt to delete a (now) non-existent edge within the
 	// database, this should result in an error.
-	err = graph.DeleteChannelEdges(chanID)
+	err = graph.DeleteChannelEdges(false, chanID)
 	if err != ErrEdgeNotFound {
 		t.Fatalf("deleting a non-existent edge should fail!")
 	}
@@ -1756,7 +1756,7 @@ func TestFilterKnownChanIDs(t *testing.T) {
 		if err := graph.AddChannelEdge(&channel); err != nil {
 			t.Fatalf("unable to create channel edge: %v", err)
 		}
-		err := graph.DeleteChannelEdges(channel.ChannelID)
+		err := graph.DeleteChannelEdges(false, channel.ChannelID)
 		if err != nil {
 			t.Fatalf("unable to mark edge zombie: %v", err)
 		}
@@ -2038,7 +2038,7 @@ func TestFetchChanInfos(t *testing.T) {
 	if err := graph.AddChannelEdge(&zombieChan); err != nil {
 		t.Fatalf("unable to create channel edge: %v", err)
 	}
-	err = graph.DeleteChannelEdges(zombieChan.ChannelID)
+	err = graph.DeleteChannelEdges(false, zombieChan.ChannelID)
 	if err != nil {
 		t.Fatalf("unable to delete and mark edge zombie: %v", err)
 	}
@@ -2654,7 +2654,7 @@ func TestNodeIsPublic(t *testing.T) {
 	// graph. This will make Alice be seen as a private node as it no longer
 	// has any advertised edges.
 	for _, graph := range graphs {
-		err := graph.DeleteChannelEdges(aliceBobEdge.ChannelID)
+		err := graph.DeleteChannelEdges(false, aliceBobEdge.ChannelID)
 		if err != nil {
 			t.Fatalf("unable to remove edge: %v", err)
 		}
@@ -2671,7 +2671,7 @@ func TestNodeIsPublic(t *testing.T) {
 	// completely remove the edge as it is not possible for her to know of
 	// it without it being advertised.
 	for i, graph := range graphs {
-		err := graph.DeleteChannelEdges(bobCarolEdge.ChannelID)
+		err := graph.DeleteChannelEdges(false, bobCarolEdge.ChannelID)
 		if err != nil {
 			t.Fatalf("unable to remove edge: %v", err)
 		}
@@ -2779,7 +2779,7 @@ func TestDisabledChannelIDs(t *testing.T) {
 	}
 
 	// Delete the channel edge and ensure it is removed from the disabled list.
-	if err = graph.DeleteChannelEdges(edgeInfo.ChannelID); err != nil {
+	if err = graph.DeleteChannelEdges(false, edgeInfo.ChannelID); err != nil {
 		t.Fatalf("unable to delete channel edge: %v", err)
 	}
 	disabledChanIds, err = graph.DisabledChannelIDs()
@@ -3017,7 +3017,7 @@ func TestGraphZombieIndex(t *testing.T) {
 
 	// If we delete the edge and mark it as a zombie, then we should expect
 	// to see it within the index.
-	err = graph.DeleteChannelEdges(edge.ChannelID)
+	err = graph.DeleteChannelEdges(false, edge.ChannelID)
 	if err != nil {
 		t.Fatalf("unable to mark edge as zombie: %v", err)
 	}

routing/router.go

@@ -342,6 +342,13 @@ type Config struct {
 
 	// Clock is mockable time provider.
 	Clock clock.Clock
+
+	// StrictZombiePruning determines if we attempt to prune zombie
+	// channels according to a stricter criteria. If true, then we'll prune
+	// a channel if only *one* of the edges is considered a zombie.
+	// Otherwise, we'll only prune the channel when both edges have a very
+	// dated last update.
+	StrictZombiePruning bool
 }
 
 // EdgeLocator is a struct used to identify a specific edge.
@@ -839,8 +846,24 @@ func (r *ChannelRouter) pruneZombieChans() error {
 				info.NodeKey2Bytes, info.ChannelID)
 		}
 
-		// Return early if both sides have a recent update.
+		// If we're using strict zombie pruning, then a channel is only
-		if !e1Zombie && !e2Zombie {
+		// considered live if both edges have a recent update we know
+		// of.
+		var channelIsLive bool
+		switch {
+		case r.cfg.StrictZombiePruning:
+			channelIsLive = !e1Zombie && !e2Zombie
+
+		// Otherwise, if we're using the less strict variant, then a
+		// channel is considered live if either of the edges have a
+		// recent update.
+		default:
+			channelIsLive = !e1Zombie || !e2Zombie
+		}
+
+		// Return early if the channel is still considered to be live
+		// with the current set of configuration parameters.
+		if channelIsLive {
 			return nil
 		}
 
@@ -901,7 +924,8 @@ func (r *ChannelRouter) pruneZombieChans() error {
 		toPrune = append(toPrune, chanID)
 		log.Tracef("Pruning zombie channel with ChannelID(%v)", chanID)
 	}
-	if err := r.cfg.Graph.DeleteChannelEdges(toPrune...); err != nil {
+	err = r.cfg.Graph.DeleteChannelEdges(r.cfg.StrictZombiePruning, toPrune...)
+	if err != nil {
 		return fmt.Errorf("unable to delete zombie channels: %v", err)
 	}
 

routing/router_test.go

@@ -69,16 +69,17 @@ func (c *testCtx) RestartRouter() error {
 	return nil
 }
 
-func createTestCtxFromGraphInstance(startingHeight uint32, graphInstance *testGraphInstance) (
+func createTestCtxFromGraphInstance(startingHeight uint32, graphInstance *testGraphInstance,
-	*testCtx, func(), error) {
+	strictPruning bool) (*testCtx, func(), error) {
 
 	return createTestCtxFromGraphInstanceAssumeValid(
-		startingHeight, graphInstance, false,
+		startingHeight, graphInstance, false, strictPruning,
 	)
 }
 
 func createTestCtxFromGraphInstanceAssumeValid(startingHeight uint32,
-	graphInstance *testGraphInstance, assumeValid bool) (*testCtx, func(), error) {
+	graphInstance *testGraphInstance, assumeValid bool,
+	strictPruning bool) (*testCtx, func(), error) {
 
 	// We'll initialize an instance of the channel router with mock
 	// versions of the chain and channel notifier. As we don't need to test
@@ -134,9 +135,10 @@ func createTestCtxFromGraphInstanceAssumeValid(startingHeight uint32,
 			next := atomic.AddUint64(&uniquePaymentID, 1)
 			return next, nil
 		},
-		PathFindingConfig:  pathFindingConfig,
+		PathFindingConfig:   pathFindingConfig,
-		Clock:              clock.NewTestClock(time.Unix(1, 0)),
+		Clock:               clock.NewTestClock(time.Unix(1, 0)),
-		AssumeChannelValid: assumeValid,
+		AssumeChannelValid:  assumeValid,
+		StrictZombiePruning: strictPruning,
 	})
 	if err != nil {
 		return nil, nil, fmt.Errorf("unable to create router %v", err)
@@ -187,7 +189,7 @@ func createTestCtxSingleNode(startingHeight uint32) (*testCtx, func(), error) {
 		cleanUp: cleanup,
 	}
 
-	return createTestCtxFromGraphInstance(startingHeight, graphInstance)
+	return createTestCtxFromGraphInstance(startingHeight, graphInstance, false)
 }
 
 func createTestCtxFromFile(startingHeight uint32, testGraph string) (*testCtx, func(), error) {
@@ -198,7 +200,7 @@ func createTestCtxFromFile(startingHeight uint32, testGraph string) (*testCtx, f
 		return nil, nil, fmt.Errorf("unable to create test graph: %v", err)
 	}
 
-	return createTestCtxFromGraphInstance(startingHeight, graphInstance)
+	return createTestCtxFromGraphInstance(startingHeight, graphInstance, false)
 }
 
 // TestFindRoutesWithFeeLimit asserts that routes found by the FindRoutes method
@@ -365,9 +367,9 @@ func TestChannelUpdateValidation(t *testing.T) {
 
 	const startingBlockHeight = 101
 
-	ctx, cleanUp, err := createTestCtxFromGraphInstance(startingBlockHeight,
+	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		testGraph)
+		startingBlockHeight, testGraph, true,
-
+	)
 	defer cleanUp()
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)
@@ -1028,7 +1030,7 @@ func TestIgnoreChannelEdgePolicyForUnknownChannel(t *testing.T) {
 	defer testGraph.cleanUp()
 
 	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		startingBlockHeight, testGraph,
+		startingBlockHeight, testGraph, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)
@@ -1960,7 +1962,7 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) {
 		// Only one edge with a stale timestamp.
 		{
 			Node1: &testChannelEnd{
-				Alias: "a",
+				Alias: "d",
 				testChannelPolicy: &testChannelPolicy{
 					LastUpdate: staleTimestamp,
 				},
@@ -1970,6 +1972,20 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) {
 			ChannelID: 2,
 		},
 
+		// Only one edge with a stale timestamp, but it's the source
+		// node so it won't get pruned.
+		{
+			Node1: &testChannelEnd{
+				Alias: "a",
+				testChannelPolicy: &testChannelPolicy{
+					LastUpdate: staleTimestamp,
+				},
+			},
+			Node2:     &testChannelEnd{Alias: "b"},
+			Capacity:  100000,
+			ChannelID: 3,
+		},
+
 		// Only one edge with a fresh timestamp.
 		{
 			Node1: &testChannelEnd{
@@ -1980,10 +1996,11 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) {
 			},
 			Node2:     &testChannelEnd{Alias: "b"},
 			Capacity:  100000,
-			ChannelID: 3,
+			ChannelID: 4,
 		},
 
-		// One edge fresh, one edge stale.
+		// One edge fresh, one edge stale. This will be pruned with
+		// strict pruning activated.
 		{
 			Node1: &testChannelEnd{
 				Alias: "c",
@@ -1998,49 +2015,57 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) {
 				},
 			},
 			Capacity:  100000,
-			ChannelID: 4,
+			ChannelID: 5,
 		},
 
 		// Both edges fresh.
 		symmetricTestChannel("g", "h", 100000, &testChannelPolicy{
 			LastUpdate: freshTimestamp,
-		}, 5),
+		}, 6),
 
-		// Both edges stale, only one pruned.
+		// Both edges stale, only one pruned. This should be pruned for
+		// both normal and strict pruning.
 		symmetricTestChannel("e", "f", 100000, &testChannelPolicy{
 			LastUpdate: staleTimestamp,
-		}, 6),
+		}, 7),
 	}
 
-	// We'll create our test graph and router backed with these test
+	for _, strictPruning := range []bool{true, false} {
-	// channels we've created.
+		// We'll create our test graph and router backed with these test
-	testGraph, err := createTestGraphFromChannels(testChannels, "a")
+		// channels we've created.
-	if err != nil {
+		testGraph, err := createTestGraphFromChannels(testChannels, "a")
-		t.Fatalf("unable to create test graph: %v", err)
+		if err != nil {
+			t.Fatalf("unable to create test graph: %v", err)
+		}
+		defer testGraph.cleanUp()
+
+		const startingHeight = 100
+		ctx, cleanUp, err := createTestCtxFromGraphInstance(
+			startingHeight, testGraph, strictPruning,
+		)
+		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)
+		}
+
+		// We expect channels that have either both edges stale, or one edge
+		// stale with both known.
+		var prunedChannels []uint64
+		if strictPruning {
+			prunedChannels = []uint64{2, 5, 7}
+		} else {
+			prunedChannels = []uint64{2, 7}
+		}
+		assertChannelsPruned(t, ctx.graph, testChannels, prunedChannels...)
 	}
-	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)
-	}
-
-	// We expect channels that have either both edges stale, or one edge
-	// stale with both known.
-	prunedChannels := []uint64{4, 6}
-	assertChannelsPruned(t, ctx.graph, testChannels, prunedChannels...)
 }
 
 // TestPruneChannelGraphDoubleDisabled test that we can properly prune channels
@@ -2149,7 +2174,7 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) {
 
 	const startingHeight = 100
 	ctx, cleanUp, err := createTestCtxFromGraphInstanceAssumeValid(
-		startingHeight, testGraph, assumeValid,
+		startingHeight, testGraph, assumeValid, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to create test context: %v", err)
@@ -2531,9 +2556,9 @@ func TestUnknownErrorSource(t *testing.T) {
 
 	const startingBlockHeight = 101
 
-	ctx, cleanUp, err := createTestCtxFromGraphInstance(startingBlockHeight,
+	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		testGraph)
+		startingBlockHeight, testGraph, false,
-
+	)
 	defer cleanUp()
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)
@@ -2670,7 +2695,7 @@ func TestSendToRouteStructuredError(t *testing.T) {
 	const startingBlockHeight = 101
 
 	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		startingBlockHeight, testGraph,
+		startingBlockHeight, testGraph, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)
@@ -2906,7 +2931,7 @@ func TestSendToRouteMaxHops(t *testing.T) {
 	const startingBlockHeight = 101
 
 	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		startingBlockHeight, testGraph,
+		startingBlockHeight, testGraph, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)
@@ -3020,7 +3045,7 @@ func TestBuildRoute(t *testing.T) {
 	const startingBlockHeight = 101
 
 	ctx, cleanUp, err := createTestCtxFromGraphInstance(
-		startingBlockHeight, testGraph,
+		startingBlockHeight, testGraph, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to create router: %v", err)

rpcserver.go

@@ -2344,7 +2344,7 @@ func abandonChanFromGraph(chanGraph *channeldb.ChannelGraph,
 
 	// If the channel ID is still in the graph, then that means the channel
 	// is still open, so we'll now move to purge it from the graph.
-	return chanGraph.DeleteChannelEdges(chanID)
+	return chanGraph.DeleteChannelEdges(false, chanID)
 }
 
 // AbandonChannel removes all channel state from the database except for a