routing: account for case where final destination send TemporaryChannelFailure

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.
This commit is contained in:
Olaoluwa Osuntokun 2018-01-10 15:15:49 -08:00
parent beeb75cb5f
commit d70e4bb0a0
No known key found for this signature in database
GPG Key ID: 964EA263DD637C21
4 changed files with 78 additions and 5 deletions

@ -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

@ -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

@ -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.

@ -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