routing: node-pair search

Instead of exploring the node-by-node, explore it pair-by-pair.
That way an potential side-effects caused by a negative inbound
fee are prevented, because the unit of traversal is the combination
of in and out fee of a candidate node. This combination can never
be negative.

routing/heap.go

@@ -18,12 +18,10 @@ type nodeWithDist struct {
 	// outgoing edges (channels) emanating from a node.
 	node route.Vertex
 
-	// netAmountReceived is the amount that should be received by this node.
+	// amtToSend is the amount that should be sent out by this node.
 	// Either as final payment to the final node or as an intermediate
-	// amount that includes also the fees for subsequent hops. This node's
-	// inbound fee is already subtracted from the htlc amount - if
-	// applicable.
-	netAmountReceived lnwire.MilliSatoshi
+	// amount that includes also the fees for subsequent hops.
+	amtToSend lnwire.MilliSatoshi
 
 	// outboundFee is the fee that this node charges on the outgoing
 	// channel.
@@ -49,6 +47,26 @@ type nodeWithDist struct {
 	// routingInfoSize is the total size requirement for the payloads field
 	// in the onion packet from this hop towards the final destination.
 	routingInfoSize uint64
+
+	// next is the next node in the path.
+	next *nodeWithDist
+}
+
+type heapKey struct {
+	from, to route.Vertex
+}
+
+func (n *nodeWithDist) key() heapKey {
+	var to route.Vertex
+
+	if n.next != nil {
+		to = n.next.node
+	}
+
+	return heapKey{
+		from: n.node,
+		to:   to,
+	}
 }
 
 // distanceHeap is a min-distance heap that's used within our path finding
@@ -59,14 +77,14 @@ type distanceHeap struct {
 	// pubkeyIndices maps public keys of nodes to their respective index in
 	// the heap. This is used as a way to avoid db lookups by using heap.Fix
 	// instead of having duplicate entries on the heap.
-	pubkeyIndices map[route.Vertex]int
+	pubkeyIndices map[heapKey]int
 }
 
 // newDistanceHeap initializes a new distance heap. This is required because
 // we must initialize the pubkeyIndices map for path-finding optimizations.
 func newDistanceHeap(numNodes int) distanceHeap {
 	distHeap := distanceHeap{
-		pubkeyIndices: make(map[route.Vertex]int, numNodes),
+		pubkeyIndices: make(map[heapKey]int, numNodes),
 		nodes:         make([]*nodeWithDist, 0, numNodes),
 	}
 
@@ -96,8 +114,8 @@ func (d *distanceHeap) Less(i, j int) bool {
 // NOTE: This is part of the heap.Interface implementation.
 func (d *distanceHeap) Swap(i, j int) {
 	d.nodes[i], d.nodes[j] = d.nodes[j], d.nodes[i]
-	d.pubkeyIndices[d.nodes[i].node] = i
-	d.pubkeyIndices[d.nodes[j].node] = j
+	d.pubkeyIndices[d.nodes[i].key()] = i
+	d.pubkeyIndices[d.nodes[j].key()] = j
 }
 
 // Push pushes the passed item onto the priority queue.
@@ -106,7 +124,7 @@ func (d *distanceHeap) Swap(i, j int) {
 func (d *distanceHeap) Push(x interface{}) {
 	n := x.(*nodeWithDist)
 	d.nodes = append(d.nodes, n)
-	d.pubkeyIndices[n.node] = len(d.nodes) - 1
+	d.pubkeyIndices[n.key()] = len(d.nodes) - 1
 }
 
 // Pop removes the highest priority item (according to Less) from the priority
@@ -118,7 +136,7 @@ func (d *distanceHeap) Pop() interface{} {
 	x := d.nodes[n-1]
 	d.nodes[n-1] = nil
 	d.nodes = d.nodes[0 : n-1]
-	delete(d.pubkeyIndices, x.node)
+	delete(d.pubkeyIndices, x.key())
 	return x
 }
 
@@ -128,7 +146,7 @@ func (d *distanceHeap) Pop() interface{} {
 // exist in the heap, then it is pushed onto the heap. Otherwise, we will end
 // up performing more db lookups on the same node in the pathfinding algorithm.
 func (d *distanceHeap) PushOrFix(dist *nodeWithDist) {
-	index, ok := d.pubkeyIndices[dist.node]
+	index, ok := d.pubkeyIndices[dist.key()]
 	if !ok {
 		heap.Push(d, dist)
 		return

routing/pathfind.go

@@ -614,8 +614,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	// traversal.
 	nodeHeap := newDistanceHeap(estimatedNodeCount)
 
-	// Holds the current best distance for a given node.
-	distance := make(map[route.Vertex]*nodeWithDist, estimatedNodeCount)
+	// Holds the current best distance for a given node pair.
+	distance := make(map[heapKey]*nodeWithDist, estimatedNodeCount)
 
 	additionalEdgesWithSrc := make(map[route.Vertex][]*edgePolicyWithSource)
 	for vertex, outgoingEdgePolicies := range g.additionalEdges {
@@ -670,13 +670,13 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	// Don't record the initial partial path in the distance map and reserve
 	// that key for the source key in the case we route to ourselves.
 	partialPath := &nodeWithDist{
-		dist:              0,
-		weight:            0,
-		node:              target,
-		netAmountReceived: amt,
-		incomingCltv:      finalHtlcExpiry,
-		probability:       1,
-		routingInfoSize:   finalHop.PayloadSize(0),
+		dist:            0,
+		weight:          0,
+		node:            target,
+		amtToSend:       amt,
+		incomingCltv:    finalHtlcExpiry,
+		probability:     1,
+		routingInfoSize: finalHop.PayloadSize(0),
 	}
 
 	// Calculate the absolute cltv limit. Use uint64 to prevent an overflow
@@ -713,13 +713,15 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 		edgesExpanded++
 
+		netAmtReceived := toNodeDist.amtToSend + toNodeDist.outboundFee
+
 		// Calculate inbound fee charged by "to" node, if it's not the
 		// exit hop.
 		var inboundFee int64
 		isExitHop := toNodeDist.nextHop == nil
 		if !isExitHop {
 			inboundFee = edge.inboundFees.CalcFee(
-				toNodeDist.netAmountReceived,
+				netAmtReceived,
 			)
 
 			// Make sure that the node total fee is never negative.
@@ -734,8 +736,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 		// Calculate amount that the candidate node would have to send
 		// out.
-		amountToSend := toNodeDist.netAmountReceived +
-			lnwire.MilliSatoshi(inboundFee)
+		amountToSend := netAmtReceived + lnwire.MilliSatoshi(inboundFee)
 
 		// Request the success probability for this edge.
 		edgeProbability := r.ProbabilitySource(
@@ -784,19 +785,12 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 			return
 		}
 
-		// netAmountToReceive is the amount that the node that is added
-		// to the distance map needs to receive from a (to be found)
-		// previous node in the route. The inbound fee of the receiving
-		// node is already subtracted from this value. The previous node
-		// will need to pay the amount that this node forwards plus the
-		// fee it charges plus this node's inbound fee.
-		netAmountToReceive := amountToSend +
-			lnwire.MilliSatoshi(outboundFee)
-
 		// Check if accumulated fees would exceed fee limit when this
 		// node would be added to the path.
-		totalFee := int64(netAmountToReceive) - int64(amt)
-		if totalFee > 0 && lnwire.MilliSatoshi(totalFee) > r.FeeLimit {
+		totalFee := toNodeDist.outboundFee +
+			lnwire.MilliSatoshi(inboundFee)
+
+		if lnwire.MilliSatoshi(totalFee) > r.FeeLimit {
 			return
 		}
 
@@ -812,21 +806,11 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 			return
 		}
 
-		// Calculate the combined fee for this edge. Dijkstra does not
-		// support negative edge weights. Because this fee feeds into
-		// the edge weight calculation, we don't allow it to be
-		// negative.
-		signedFee := inboundFee + outboundFee
-		fee := lnwire.MilliSatoshi(0)
-		if signedFee > 0 {
-			fee = lnwire.MilliSatoshi(signedFee)
-		}
-
 		// By adding fromVertex in the route, there will be an extra
 		// weight composed of the fee that this node will charge and
 		// the amount that will be locked for timeLockDelta blocks in
 		// the HTLC that is handed out to fromVertex.
-		weight := edgeWeight(netAmountToReceive, fee, timeLockDelta)
+		weight := edgeWeight(amountToSend, totalFee, timeLockDelta)
 
 		// Compute the tentative weight to this new channel/edge
 		// which is the weight from our toNode to the target node
@@ -845,7 +829,12 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 		// If there is already a best route stored, compare this
 		// candidate route with the best route so far.
-		current, ok := distance[fromVertex]
+		heapKey := heapKey{
+			from: fromVertex,
+			to:   toNodeDist.node,
+		}
+
+		current, ok := distance[heapKey]
 		if ok {
 			// If this route is worse than what we already found,
 			// skip this route.
@@ -897,17 +886,18 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		// The new better distance is recorded, and also our "next hop"
 		// map is populated with this edge.
 		withDist := &nodeWithDist{
-			dist:              tempDist,
-			weight:            tempWeight,
-			node:              fromVertex,
-			netAmountReceived: netAmountToReceive,
-			outboundFee:       lnwire.MilliSatoshi(outboundFee),
-			incomingCltv:      incomingCltv,
-			probability:       probability,
-			nextHop:           edge,
-			routingInfoSize:   routingInfoSize,
+			dist:            tempDist,
+			weight:          tempWeight,
+			node:            fromVertex,
+			amtToSend:       amountToSend,
+			outboundFee:     lnwire.MilliSatoshi(outboundFee),
+			incomingCltv:    incomingCltv,
+			probability:     probability,
+			nextHop:         edge,
+			routingInfoSize: routingInfoSize,
+			next:            toNodeDist,
 		}
-		distance[fromVertex] = withDist
+		distance[heapKey] = withDist
 
 		// Either push withDist onto the heap if the node
 		// represented by fromVertex is not already on the heap OR adjust
@@ -995,7 +985,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 			)
 		}
 
-		netAmountReceived := partialPath.netAmountReceived
+		netAmountReceived := partialPath.amtToSend +
+			partialPath.outboundFee
 
 		// Expand all connections using the optimal policy for each
 		// connection.
@@ -1041,7 +1032,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		}
 
 		if nodeHeap.Len() == 0 {
-			break
+			return nil, 0, errNoPathFound
 		}
 
 		// Fetch the node within the smallest distance from our source
@@ -1059,28 +1050,18 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	// Use the distance map to unravel the forward path from source to
 	// target.
 	var pathEdges []*unifiedEdge
-	currentNode := source
+	currentNode := partialPath
 	for {
-		// Determine the next hop forward using the next map.
-		currentNodeWithDist, ok := distance[currentNode]
-		if !ok {
-			// If the node doesn't have a next hop it means we
-			// didn't find a path.
-			return nil, 0, errNoPathFound
+		policy := currentNode.nextHop
+		if policy == nil {
+			break
 		}
 
 		// Add the next hop to the list of path edges.
-		pathEdges = append(pathEdges, currentNodeWithDist.nextHop)
+		pathEdges = append(pathEdges, currentNode.nextHop)
 
 		// Advance current node.
-		currentNode = currentNodeWithDist.nextHop.policy.ToNodePubKey()
-
-		// Check stop condition at the end of this loop. This prevents
-		// breaking out too soon for self-payments that have target set
-		// to source.
-		if currentNode == target {
-			break
-		}
+		currentNode = currentNode.next
 	}
 
 	// For the final hop, we'll set the node features to those determined
@@ -1097,10 +1078,10 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	pathEdges[len(pathEdges)-1].policy.ToNodeFeatures = features
 
 	log.Debugf("Found route: probability=%v, hops=%v, fee=%v",
-		distance[source].probability, len(pathEdges),
-		distance[source].netAmountReceived-amt)
+		partialPath.probability, len(pathEdges),
+		partialPath.amtToSend-amt)
 
-	return pathEdges, distance[source].probability, nil
+	return pathEdges, partialPath.probability, nil
 }
 
 // getProbabilityBasedDist converts a weight into a distance that takes into