diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 0781d1d1..e15a6c4c 100644 --- a/routing/pathfind_test.go +++ b/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, diff --git a/routing/router.go b/routing/router.go index b805c704..5e984b8a 100644 --- a/routing/router.go +++ b/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 -// 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 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 - // directional edge - chansToPrune = append(chansToPrune, info.ChannelPoint) + isZombieChan := e1Zombie && e2Zombie - // 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{}{} + // If AssumeChannelValid is present and 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. + 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 "+ - "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) } } @@ -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 diff --git a/routing/router_test.go b/routing/router_test.go index 5494d06d..ab53991b 100644 --- a/routing/router_test.go +++ b/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