From d70e4bb0a0c7200a0458b6b92e8cb46e3569aa86 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 10 Jan 2018 15:15:49 -0800 Subject: [PATCH] routing: account for case where final destination send TemporaryChannelFailure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we fix an existing bug that could cause lnd to crash if we sent a payment, and the *destination* sent a temp channel failure error message. When handling such a message, we’ll look in the nextHop map to see which channel was *after* the node that sent the payment. However, if the destination sends this error, then there’ll be no entry in this map. To address this case, we now add a prevHop map. If we attempt to lookup a node in the nextHop map, and they don’t have an entry, then we’ll consult the prevHop map. We also update the set of tests to ensure that we’re properly setting both the prevHop map and the nextHop map. --- routing/missioncontrol.go | 4 ++++ routing/pathfind.go | 27 ++++++++++++++++++++++++--- routing/pathfind_test.go | 35 +++++++++++++++++++++++++++++++++++ routing/router.go | 17 +++++++++++++++-- 4 files changed, 78 insertions(+), 5 deletions(-) diff --git a/routing/missioncontrol.go b/routing/missioncontrol.go index 059d3efe7..5fe44b5cf 100644 --- a/routing/missioncontrol.go +++ b/routing/missioncontrol.go @@ -222,6 +222,10 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, // shrinking. pruneView := p.pruneViewSnapshot + log.Debugf("Mission Control session using prune view of %v "+ + "edges, %v vertexes", len(pruneView.edges), + len(pruneView.vertexes)) + // TODO(roasbeef): sync logic amongst dist sys // Taking into account this prune view, we'll attempt to locate a path diff --git a/routing/pathfind.go b/routing/pathfind.go index 06986f129..fce5ceeef 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -137,6 +137,11 @@ type Route struct { // off to. With this map, we can easily look up the next outgoing // channel or node for pruning purposes. nextHopMap map[Vertex]*ChannelHop + + // prevHop maps a node, to the channel that was directly before it + // within the route. With this map, we can easily look up the previous + // channel or node for pruning purposes. + prevHopMap map[Vertex]*ChannelHop } // nextHopVertex returns the next hop (by Vertex) after the target node. If the @@ -147,11 +152,19 @@ func (r *Route) nextHopVertex(n *btcec.PublicKey) (Vertex, bool) { } // nextHopChannel returns the uint64 channel ID of the next hop after the -// target node. If the target node is not foud in the route, then false is +// target node. If the target node is not found in the route, then false is // returned. -func (r *Route) nextHopChannel(n *btcec.PublicKey) (uint64, bool) { +func (r *Route) nextHopChannel(n *btcec.PublicKey) (*ChannelHop, bool) { hop, ok := r.nextHopMap[NewVertex(n)] - return hop.ChannelID, ok + return hop, ok +} + +// prevHopChannel returns the uint64 channel ID of the before hop after the +// target node. If the target node is not found in the route, then false is +// returned. +func (r *Route) prevHopChannel(n *btcec.PublicKey) (*ChannelHop, bool) { + hop, ok := r.prevHopMap[NewVertex(n)] + return hop, ok } // containsNode returns true if a node is present in the target route, and @@ -224,6 +237,7 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex Vertex, nodeIndex: make(map[Vertex]struct{}), chanIndex: make(map[uint64]struct{}), nextHopMap: make(map[Vertex]*ChannelHop), + prevHopMap: make(map[Vertex]*ChannelHop), } // TODO(roasbeef): need to do sanity check to ensure we don't make a @@ -344,6 +358,13 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex Vertex, route.Hops[i] = nextHop } + // We'll then make a second run through our route in order to set up + // our prev hop mapping. + for _, hop := range route.Hops { + vertex := NewVertex(hop.Channel.Node.PubKey) + route.prevHopMap[vertex] = hop.Channel + } + // The total amount required for this route will be the value the // source extends to the first hop in the route. route.TotalAmount = runningAmt diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 31feef12b..b67a85c80 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -411,6 +411,41 @@ func TestBasicGraphPathFinding(t *testing.T) { paymentAmt+firstHopFee, route.Hops[1].AmtToForward) } + // Finally, the next and prev hop maps should be properly set. + // + // The previous hop from goku should be the channel from roasbeef, and + // the next hop should be the channel to sophon. + gokuPrevChan, ok := route.prevHopChannel(aliases["songoku"]) + if !ok { + t.Fatalf("goku didn't have next chan but should have") + } + if gokuPrevChan.ChannelID != route.Hops[0].Channel.ChannelID { + t.Fatalf("incorrect prev chan: expected %v, got %v", + gokuPrevChan.ChannelID, route.Hops[0].Channel.ChannelID) + } + gokuNextChan, ok := route.nextHopChannel(aliases["songoku"]) + if !ok { + t.Fatalf("goku didn't have prev chan but should have") + } + if gokuNextChan.ChannelID != route.Hops[1].Channel.ChannelID { + t.Fatalf("incorrect prev chan: expected %v, got %v", + gokuNextChan.ChannelID, route.Hops[1].Channel.ChannelID) + } + + // Sophon shouldn't have a next chan, but she should have a prev chan. + if _, ok := route.nextHopChannel(aliases["sophon"]); ok { + t.Fatalf("incorrect next hop map, no vertexes should " + + "be after sophon") + } + sophonPrevEdge, ok := route.prevHopChannel(aliases["sophon"]) + if !ok { + t.Fatalf("sophon didn't have prev chan but should have") + } + if sophonPrevEdge.ChannelID != route.Hops[1].Channel.ChannelID { + t.Fatalf("incorrect prev chan: expected %v, got %v", + sophonPrevEdge.ChannelID, route.Hops[1].Channel.ChannelID) + } + // Next, attempt to query for a path to Luo Ji for 100 satoshis, there // exist two possible paths in the graph, but the shorter (1 hop) path // should be selected. diff --git a/routing/router.go b/routing/router.go index 07041845d..5fae3c7fc 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1549,6 +1549,10 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route errSource := fErr.ErrorSource + log.Tracef("node=%x reported failure when sending "+ + "htlc=%x", errSource.SerializeCompressed(), + payment.PaymentHash[:]) + switch onionErr := fErr.FailureMessage.(type) { // If the end destination didn't know they payment // hash, then we'll terminate immediately. @@ -1650,13 +1654,22 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route // error was meant to pass the HTLC along to. badChan, ok := route.nextHopChannel(errSource) if !ok { - continue + // If we weren't able to find the hop + // *after* this node, then we'll + // attempt to disable the previous + // channel. + badChan, ok = route.prevHopChannel( + errSource, + ) + if !ok { + continue + } } // If the channel was found, then we'll inform // mission control of this failure so future // attempts avoid this link temporarily. - paySession.ReportChannelFailure(badChan) + paySession.ReportChannelFailure(badChan.ChannelID) continue // If the send fail due to a node not having the