routing: findRoute now returns a slice of selected hops in reverse order

This commit modifies the findRoute function to decouple the
validation+creation of a route, from the path finding algorithm itself.
When we say “route”, we mean the full payment route complete with
time-lock and fee information. When we say “path” we simple mean an
ordered set of channel edges from one node to another target node.

With this commit we can now perform path finding independent of route
creation which will be needed in the up coming refactor to implement a
new modified k-shortest paths algorithm.

routing/pathfind.go

@@ -54,25 +54,6 @@ type Route struct {
 	Hops []*Hop
 }
 
-// ChannelHop is an intermediate hop within the network with a greater
-// multi-hop payment route. This struct contains the relevant routing policy of
-// the particular edge, as well as the total capacity, and origin chain of the
-// channel itself.
-type ChannelHop struct {
-	// Capacity is the total capacity of the channel being traversed. This
-	// value is expressed for stability in satoshis.
-	Capacity btcutil.Amount
-
-	// Chain is a 32-byte has that denotes the base blockchain network of
-	// the channel. The 32-byte hash is the "genesis" block of the
-	// blockchain, or the very first block in the chain.
-	//
-	// TODO(roasbeef): store chain within edge info/policy in database.
-	Chain chainhash.Hash
-
-	*channeldb.ChannelEdgePolicy
-}
-
 // Hop represents the forwarding details at a particular position within the
 // final route. This struct houses the values necessary to create the HTLC
 // which will travel along this hop, and also encode the per-hop payload
@@ -97,6 +78,25 @@ type Hop struct {
 	Fee btcutil.Amount
 }
 
+// ChannelHop is an intermediate hop within the network with a greater
+// multi-hop payment route. This struct contains the relevant routing policy of
+// the particular edge, as well as the total capacity, and origin chain of the
+// channel itself.
+type ChannelHop struct {
+	// Capacity is the total capacity of the channel being traversed. This
+	// value is expressed for stability in satoshis.
+	Capacity btcutil.Amount
+
+	// Chain is a 32-byte has that denotes the base blockchain network of
+	// the channel. The 32-byte hash is the "genesis" block of the
+	// blockchain, or the very first block in the chain.
+	//
+	// TODO(roasbeef): store chain within edge info/policy in database.
+	Chain chainhash.Hash
+
+	*channeldb.ChannelEdgePolicy
+}
+
 // computeFee computes the fee to forward an HTLC of `amt` satoshis over the
 // passed active payment channel. This value is currently computed as specified
 // in BOLT07, but will likely change in the near future.
@@ -106,34 +106,12 @@ func computeFee(amt btcutil.Amount, edge *ChannelHop) btcutil.Amount {
 
 // newRoute returns a fully valid route between the source and target that's
 // capable of supporting a payment of `amtToSend` after fees are fully
-// computed. IF the route is too long, or the selected path cannot support the
+// computed. If the route is too long, or the selected path cannot support the
-// fully payment including fees, then a non-nil error is returned. prevHop maps
+// fully payment including fees, then a non-nil error is returned.
-// a vertex to the channel required to get to it.
+//
-func newRoute(amtToSend btcutil.Amount, source, target vertex,
+// NOTE: The passed slice of ChannelHops MUST be sorted in reverse order: from
-	prevHop map[vertex]edgeWithPrev) (*Route, error) {
+// the target to the source node of the path finding aattempt.
-
+func newRoute(amtToSend btcutil.Amount, pathEdges []*ChannelHop) (*Route, error) {
-	// If the potential route if below the max hop limit, then we'll use
-	// the prevHop map to unravel the path. We end up with a list of edges
-	// in the reverse direction which we'll use to properly calculate the
-	// timelock and fee values.
-	pathEdges := make([]*ChannelHop, 0, len(prevHop))
-	prev := target
-	for prev != source { // TODO(roasbeef): assumes no cycles
-		// Add the current hop to the limit of path edges then walk
-		// backwards from this hop via the prev pointer for this hop
-		// within the prevHop map.
-		pathEdges = append(pathEdges, prevHop[prev].edge)
-		prev = newVertex(prevHop[prev].prevNode)
-	}
-
-	// The route is invalid if it spans more than 20 hops. The current
-	// Sphinx (onion routing) implementation can only encode up to 20 hops
-	// as the entire packet is fixed size. If this route is more than 20 hops,
-	// then it's invalid.
-	if len(pathEdges) > HopLimit {
-		return nil, ErrMaxHopsExceeded
-	}
-
 	route := &Route{
 		Hops: make([]*Hop, len(pathEdges)),
 	}
@@ -229,20 +207,17 @@ func edgeWeight(e *channeldb.ChannelEdgePolicy) float64 {
 
 // findRoute attempts to find a path from the source node within the
 // ChannelGraph to the target node that's capable of supporting a payment of
-// `amt` value. The current approach is used a multiple pass path finding
+// `amt` value. The current approach implemented is modified version of
-// algorithm. First we employ a modified version of Dijkstra's algorithm to
+// Dijkstra's algorithm to find a single shortest path between the source node
-// find a potential set of shortest paths, the distance metric is related to
+// and the destination. The distance metric used for edges is related to the
-// the time-lock+fee along the route. Once we have a set of candidate routes,
+// time-lock+fee costs along a particular edge. If a path is found, this
-// we calculate the required fee and time lock values running backwards along
+// function returns a slice of ChannelHop structs which encoded the chosen path
-// the route. The route that's selected is the one with the lowest total fee.
+// (backwards) from the target to the source.
-//
-// TODO(roasbeef): make member, add caching
-//  * add k-path
 func findRoute(graph *channeldb.ChannelGraph, sourceNode *channeldb.LightningNode,
-	target *btcec.PublicKey, amt btcutil.Amount) (*Route, error) {
+	target *btcec.PublicKey, amt btcutil.Amount) ([]*ChannelHop, error) {
 
 
-	// First we'll initilaze an empty heap which'll help us to quickly
+	// First we'll initialize an empty heap which'll help us to quickly
 	// locate the next edge we should visit next during our graph
 	// traversal.
 	var nodeHeap distanceHeap
@@ -343,8 +318,28 @@ func findRoute(graph *channeldb.ChannelGraph, sourceNode *channeldb.LightningNod
 		return nil, ErrNoPathFound
 	}
 
-	// Otherwise, we construct a new route which calculate the relevant
+	// If the potential route if below the max hop limit, then we'll use
-	// total fees and proper time lock values for each hop.
+	// the prevHop map to unravel the path. We end up with a list of edges
-	targetVerex := newVertex(target)
+	// in the reverse direction which we'll use to properly calculate the
-	return newRoute(amt, sourceVertex, targetVerex, prev)
+	// timelock and fee values.
+	pathEdges := make([]*ChannelHop, 0, len(prev))
+	prevNode := newVertex(target)
+	for prevNode != sourceVertex { // TODO(roasbeef): assumes no cycles
+		// Add the current hop to the limit of path edges then walk
+		// backwards from this hop via the prev pointer for this hop
+		// within the prevHop map.
+		pathEdges = append(pathEdges, prev[prevNode].edge)
+		prevNode = newVertex(prev[prevNode].prevNode)
+	}
+
+	// The route is invalid if it spans more than 20 hops. The current
+	// Sphinx (onion routing) implementation can only encode up to 20 hops
+	// as the entire packet is fixed size. If this route is more than 20
+	// hops, then it's invalid.
+	if len(pathEdges) > HopLimit {
+		return nil, ErrMaxHopsExceeded
+	}
+
+	return pathEdges, nil
+}
 }

routing/pathfind_test.go

@@ -300,9 +300,13 @@ func TestBasicGraphPathFinding(t *testing.T) {
 
 	const paymentAmt = btcutil.Amount(100)
 	target := aliases["sophon"]
-	route, err := findRoute(graph, sourceNode, target, paymentAmt)
+	path, err := findRoute(graph, sourceNode, target, paymentAmt)
 	if err != nil {
-		t.Fatalf("unable to find route: %v", err)
+		t.Fatalf("unable to find path: %v", err)
+	}
+	route, err := newRoute(paymentAmt, path)
+	if err != nil {
+		t.Fatalf("unable to create path: %v", err)
 	}
 
 	// The length of the route selected should be of exactly length two.
@@ -334,15 +338,20 @@ func TestBasicGraphPathFinding(t *testing.T) {
 	// exist two possible paths in the graph, but the shorter (1 hop) path
 	// should be selected.
 	target = aliases["luoji"]
-	route, err = findRoute(graph, sourceNode, target, paymentAmt)
+	path, err = findRoute(graph, sourceNode, target, paymentAmt)
 	if err != nil {
 		t.Fatalf("unable to find route: %v", err)
 	}
+	route, err = newRoute(paymentAmt, path)
+	if err != nil {
+		t.Fatalf("unable to create path: %v", err)
+	}
 
 	// The length of the path should be exactly one hop as it's the
 	// "shortest" known path in the graph.
 	if len(route.Hops) != 1 {
-		t.Fatalf("shortest path not selected, should be of length 1, "+"is instead: %v", len(route.Hops))
+		t.Fatalf("shortest path not selected, should be of length 1, "+
+			"is instead: %v", len(route.Hops))
 	}
 
 	// As we have a direct path, the total time lock value should be
@@ -380,19 +389,18 @@ func TestNewRoutePathTooLong(t *testing.T) {
 	// We start by confirminig that routing a payment 20 hops away is possible.
 	// Alice should be able to find a valid route to ursula.
 	target := aliases["ursula"]
-	route, err := findRoute(graph, sourceNode, target, paymentAmt)
+	if _, err = findRoute(graph, sourceNode, target, paymentAmt); err != nil {
-	if err != nil {
 		t.Fatalf("path should have been found")
 	}
 
 	// Vincent is 21 hops away from Alice, and thus no valid route should be
 	// presented to Alice.
 	target = aliases["vincent"]
-	route, err = findRoute(graph, sourceNode, target, paymentAmt)
+	path, err := findRoute(graph, sourceNode, target, paymentAmt)
 	if err == nil {
 		t.Fatalf("should not have been able to find path, supposed to be "+
 			"greater than 20 hops, found route with %v hops",
-			len(route.Hops))
+			len(path))
 	}
 
 }

routing/router.go

@@ -1052,7 +1052,10 @@ func (r *ChannelRouter) ProcessRoutingMessage(msg lnwire.Message, src *btcec.Pub
 
 // FindRoute attempts to query the ChannelRouter for the "best" path to a
 // particular target destination which is able to send `amt` after factoring in
-// channel capacities and cumulative fees along the route.
+// channel capacities and cumulative fees along the route.  Once we have a set
+// of candidate routes, we calculate the required fee and time lock values
+// running backwards along the route. The route that will be ranked the highest
+// is the one with the lowest cumulative fee along the route.
 func (r *ChannelRouter) FindRoute(target *btcec.PublicKey, amt btcutil.Amount) (*Route, error) {
 	dest := target.SerializeCompressed()
 
@@ -1067,14 +1070,24 @@ func (r *ChannelRouter) FindRoute(target *btcec.PublicKey, amt btcutil.Amount) (
 		return nil, ErrTargetNotInNetwork
 	}
 
+	// First we'll find a single shortest path from the source (our
+	// selfNode) to the target destination that's capable of carrying amt
+	// satoshis along the path before fees are calculated.
+	//
 	// TODO(roasbeef): add k-shortest paths
-	route, err := findRoute(r.cfg.Graph, r.selfNode, target, amt)
+	routeHops, err := findRoute(r.cfg.Graph, r.selfNode, target, amt)
 	if err != nil {
 		log.Errorf("Unable to find path: %v", err)
 		return nil, err
 	}
 
-	// TODO(roabseef): also create the Sphinx packet and add in the route
+	// If we were able to find a path we construct a new route which
+	// calculate the relevant total fees and proper time lock values for
+	// each hop.
+	route, err := newRoute(amt, routeHops)
+	if err != nil {
+		return nil, err
+	}
 
 	log.Debugf("Obtained path sending %v to %x: %v", amt, dest,
 		newLogClosure(func() string {