diff --git a/channeldb/graph.go b/channeldb/graph.go index b975bd92..fac89017 100644 --- a/channeldb/graph.go +++ b/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 -// and marks them as zombies. This ensures that we're unable to re-add it to our -// database once again. If an edge does not exist within the database, then -// ErrEdgeNotFound will be returned. -func (c *ChannelGraph) DeleteChannelEdges(chanIDs ...uint64) error { +// DeleteChannelEdges removes edges with the given channel IDs from the +// database and marks them as zombies. This ensures that we're unable to re-add +// it to our database once again. If an edge does not exist within the +// database, then ErrEdgeNotFound will be returned. If strictZombiePruning is +// 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, diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 331d1769..0708c3ad 100644 --- a/channeldb/graph_test.go +++ b/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) } diff --git a/routing/router.go b/routing/router.go index 25ce4994..a1f50215 100644 --- a/routing/router.go +++ b/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 !e1Zombie && !e2Zombie { + // If we're using strict zombie pruning, then a channel is only + // 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) } diff --git a/routing/router_test.go b/routing/router_test.go index ff990350..c530daec 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -69,16 +69,17 @@ func (c *testCtx) RestartRouter() error { return nil } -func createTestCtxFromGraphInstance(startingHeight uint32, graphInstance *testGraphInstance) ( - *testCtx, func(), error) { +func createTestCtxFromGraphInstance(startingHeight uint32, graphInstance *testGraphInstance, + 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, - Clock: clock.NewTestClock(time.Unix(1, 0)), - AssumeChannelValid: assumeValid, + PathFindingConfig: pathFindingConfig, + Clock: clock.NewTestClock(time.Unix(1, 0)), + 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, - testGraph) - + ctx, cleanUp, err := createTestCtxFromGraphInstance( + 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 - // channels we've created. - testGraph, err := createTestGraphFromChannels(testChannels, "a") - if err != nil { - t.Fatalf("unable to create test graph: %v", err) + for _, strictPruning := range []bool{true, false} { + // We'll create our test graph and router backed with these test + // channels we've created. + testGraph, err := createTestGraphFromChannels(testChannels, "a") + 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, - testGraph) - + ctx, cleanUp, err := createTestCtxFromGraphInstance( + 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) diff --git a/rpcserver.go b/rpcserver.go index 85a0ecef..dd7e42c9 100644 --- a/rpcserver.go +++ b/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