From 8b8e82a1717866394ebd78e44eb1ff305b79375b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 29 Nov 2018 16:36:37 +0100 Subject: [PATCH] routing: remove route hop maps Hop maps were used in a test to verify the population of the hop map itself and further only in a single function (getFailedChannelID). Rewrote that function and removed the hop maps completely. --- routing/pathfind.go | 51 +++++----------------------------------- routing/pathfind_test.go | 29 ----------------------- routing/router.go | 48 +++++++++++++++++++++++-------------- 3 files changed, 37 insertions(+), 91 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index a1910292..8bedfe3c 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -146,6 +146,10 @@ type Route struct { // amount of fees. TotalAmount lnwire.MilliSatoshi + // SourcePubKey is the pubkey of the node where this route originates + // from. + SourcePubKey Vertex + // Hops contains details concerning the specific forwarding details at // each hop. Hops []*Hop @@ -158,16 +162,6 @@ type Route struct { // is present in this route or not. Channels are identified by the // uint64 version of the short channel ID. chanIndex map[uint64]struct{} - - // nextHop maps a node, to the next channel that it will pass the HTLC - // off to. With this map, we can easily look up the next outgoing - // channel or node for pruning purposes. - nextHopMap map[Vertex]*Hop - - // 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]*Hop } // HopFee returns the fee charged by the route hop indicated by hopIndex. @@ -183,29 +177,6 @@ func (r *Route) HopFee(hopIndex int) lnwire.MilliSatoshi { return incomingAmt - r.Hops[hopIndex].AmtToForward } -// nextHopVertex returns the next hop (by Vertex) after the target node. If the -// target node is not found in the route, then false is returned. -func (r *Route) nextHopVertex(n *btcec.PublicKey) (Vertex, bool) { - hop, ok := r.nextHopMap[NewVertex(n)] - return Vertex(hop.PubKeyBytes), ok -} - -// nextHopChannel returns the uint64 channel ID of the next hop after the -// target node. If the target node is not found in the route, then false is -// returned. -func (r *Route) nextHopChannel(n *btcec.PublicKey) (*Hop, bool) { - hop, ok := r.nextHopMap[NewVertex(n)] - 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) (*Hop, bool) { - hop, ok := r.prevHopMap[NewVertex(n)] - return hop, ok -} - // containsNode returns true if a node is present in the target route, and // false otherwise. func (r *Route) containsNode(v Vertex) bool { @@ -381,31 +352,21 @@ func NewRouteFromHops(amtToSend lnwire.MilliSatoshi, timeLock uint32, // that is send from the source and the final amount that is received // by the destination. route := &Route{ + SourcePubKey: sourceVertex, Hops: hops, TotalTimeLock: timeLock, TotalAmount: amtToSend, TotalFees: amtToSend - hops[len(hops)-1].AmtToForward, nodeIndex: make(map[Vertex]struct{}), chanIndex: make(map[uint64]struct{}), - nextHopMap: make(map[Vertex]*Hop), - prevHopMap: make(map[Vertex]*Hop), } // Then we'll update the node and channel index, to indicate that this // Vertex and incoming channel link are present within this route. - // Also, the prev and next hop maps will be populated. - prevNode := sourceVertex - for i := 0; i < len(hops); i++ { - hop := hops[i] - + for _, hop := range hops { v := Vertex(hop.PubKeyBytes) - route.nodeIndex[v] = struct{}{} route.chanIndex[hop.ChannelID] = struct{}{} - route.prevHopMap[v] = hop - route.nextHopMap[prevNode] = hop - - prevNode = v } return route diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 4bc0e528..df41825e 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -845,35 +845,6 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc t.Fatalf("expected time lock of %v, instead have %v", 2, route.TotalTimeLock) } - - // The next and prev hop maps should be properly set. - for i := 0; i < expectedHopCount; i++ { - prevChan, ok := route.prevHopChannel(aliases[expectedHops[i].alias]) - if !ok { - t.Fatalf("hop didn't have prev chan but should have") - } - if prevChan.ChannelID != route.Hops[i].ChannelID { - t.Fatalf("incorrect prev chan: expected %v, got %v", - prevChan.ChannelID, route.Hops[i].ChannelID) - } - } - - for i := 0; i < expectedHopCount-1; i++ { - nextChan, ok := route.nextHopChannel(aliases[expectedHops[i].alias]) - if !ok { - t.Fatalf("hop didn't have prev chan but should have") - } - if nextChan.ChannelID != route.Hops[i+1].ChannelID { - t.Fatalf("incorrect prev chan: expected %v, got %v", - nextChan.ChannelID, route.Hops[i+1].ChannelID) - } - } - - // Final hop shouldn't have a next chan - if _, ok := route.nextHopChannel(aliases[expectedHops[lastHopIndex].alias]); ok { - t.Fatalf("incorrect next hop map, no vertexes should " + - "be after sophon") - } } func TestPathFindingWithAdditionalEdges(t *testing.T) { diff --git a/routing/router.go b/routing/router.go index f5590234..eb0ec879 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1760,7 +1760,9 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment, // Always determine chan id ourselves, because a channel // update with id may not be available. - failedChanID, err := getFailedChannelID(route, errSource) + failedChanID, err := getFailedChannelID( + route, errVertex, + ) if err != nil { return preImage, nil, err } @@ -1966,28 +1968,40 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment, // getFailedChannelID tries to locate the failing channel given a route and the // pubkey of the node that sent the error. It will assume that the error is // associated with the outgoing channel of the error node. -func getFailedChannelID(route *Route, errSource *btcec.PublicKey) ( +func getFailedChannelID(route *Route, errSource Vertex) ( uint64, error) { - // As this error indicates that the target channel was unable to carry - // this HTLC (for w/e reason), we'll query the index to find the - // _outgoing_ channel the source of the error was meant to pass the - // HTLC along to. - if badChan, ok := route.nextHopChannel(errSource); ok { - return badChan.ChannelID, nil + // If the error originates from ourselves, report our outgoing channel + // as failing. + if errSource == route.SourcePubKey { + return route.Hops[0].ChannelID, nil } - // If we weren't able to find the hop *after* this node, then we'll - // attempt to disable the previous channel. - // - // TODO(joostjager): errSource must be the final hop then? In that case, - // certain types of errors are not expected. For example - // FailUnknownNextPeer. This could be a reason to prune the node? - if prevChan, ok := route.prevHopChannel(errSource); ok { - return prevChan.ChannelID, nil + hopCount := len(route.Hops) + for i, hop := range route.Hops { + if errSource != hop.PubKeyBytes { + continue + } + + // If the errSource is the final hop, we assume that the + // failing channel is the incoming channel. + // + // TODO(joostjager): In this case, certain types of + // errors are not expected. For example + // FailUnknownNextPeer. This could be a reason to prune + // the node? + if i == hopCount-1 { + return route.Hops[i].ChannelID, nil + } + + // As this error indicates that the target channel was + // unable to carry this HTLC (for w/e reason), we'll + // query return the _outgoing_ channel that the source + // of the error was meant to pass the HTLC along to. + return route.Hops[i+1].ChannelID, nil } - return 0, fmt.Errorf("cannot find channel in route") + return 0, fmt.Errorf("cannot find error source node in route") } // applyChannelUpdate validates a channel update and if valid, applies it to the