diff --git a/routing/heap.go b/routing/heap.go index 7670637d..25745cb9 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -7,7 +7,7 @@ import "github.com/lightningnetwork/lnd/channeldb" type nodeWithDist struct { // dist is the distance to this node from the source node in our // current context. - dist float64 + dist int64 // node is the vertex itself. This pointer can be used to explore all // the outgoing edges (channels) emanating from a node. diff --git a/routing/pathfind.go b/routing/pathfind.go index f9270dfd..f9a6a33b 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -25,7 +25,7 @@ const ( HopLimit = 20 // infinity is used as a starting distance in our shortest path search. - infinity = math.MaxFloat64 + infinity = math.MaxInt64 ) // ChannelHop is an intermediate hop within the network with a greater @@ -74,7 +74,9 @@ type Hop struct { // computeFee computes the fee to forward an HTLC of `amt` milli-satoshis over // the passed active payment channel. This value is currently computed as // specified in BOLT07, but will likely change in the near future. -func computeFee(amt lnwire.MilliSatoshi, edge *ChannelHop) lnwire.MilliSatoshi { +func computeFee(amt lnwire.MilliSatoshi, + edge *channeldb.ChannelEdgePolicy) lnwire.MilliSatoshi { + return edge.FeeBaseMSat + (amt*edge.FeeProportionalMillionths)/1000000 } @@ -293,7 +295,7 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex Vertex, // amount of satoshis incoming into this hop to // properly pay the required fees. prevAmount := prevHop.AmtToForward - fee = computeFee(prevAmount, prevEdge) + fee = computeFee(prevAmount, prevEdge.ChannelEdgePolicy) // With the fee computed, we increment the total amount // as we need to pay this fee. This value represents @@ -398,12 +400,28 @@ type edgeWithPrev struct { // edgeWeight computes the weight of an edge. This value is used when searching // for the shortest path within the channel graph between two nodes. Currently -// this is just 1 + the cltv delta value required at this hop, this value -// should be tuned with experimental and empirical data. +// a component is just 1 + the cltv delta value required at this hop, this +// value should be tuned with experimental and empirical data. We'll also +// factor in the "pure fee" through this hop, using the square of this fee as +// part of the weighting. The goal here is to bias more heavily towards fee +// ranking, and fallback to a time-lock based value in the case of a fee tie. // // TODO(roasbeef): compute robust weight metric -func edgeWeight(e *channeldb.ChannelEdgePolicy) float64 { - return float64(1 + e.TimeLockDelta) +func edgeWeight(amt lnwire.MilliSatoshi, e *channeldb.ChannelEdgePolicy) int64 { + // First, we'll compute the "pure" fee through this hop. We say pure, + // as this may not be what's ultimately paid as fees are properly + // calculated backwards, while we're going in the reverse direction. + pureFee := computeFee(amt, e) + + // We'll then square the fee itself in order to more heavily weight our + // edge selection to bias towards lower fees. + feeWeight := int64(pureFee * pureFee) + + // The final component is then 1 plus the timelock delta. + timeWeight := int64(1 + e.TimeLockDelta) + + // The final weighting is: fee^2 + time_lock_delta. + return feeWeight + timeWeight } // findPath attempts to find a path from the source node within the @@ -514,7 +532,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, // Compute the tentative distance to this new // channel/edge which is the distance to our current // pivot node plus the weight of this edge. - tempDist := distance[pivot].dist + edgeWeight(outEdge) + tempDist := distance[pivot].dist + edgeWeight(amt, outEdge) // If this new tentative distance is better than the // current best known distance to this node, then we diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 21db48d5..f4d7db3f 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -390,7 +390,9 @@ func TestBasicGraphPathFinding(t *testing.T) { // Additionally, we'll ensure that the amount to forward, and fees // computed for each hop are correct. - firstHopFee := computeFee(paymentAmt, route.Hops[1].Channel) + firstHopFee := computeFee( + paymentAmt, route.Hops[1].Channel.ChannelEdgePolicy, + ) if route.Hops[0].Fee != firstHopFee { t.Fatalf("first hop fee incorrect: expected %v, got %v", firstHopFee, route.Hops[0].Fee) @@ -507,7 +509,9 @@ func TestKShortestPathFinding(t *testing.T) { paymentAmt := lnwire.NewMSatFromSatoshis(100) target := aliases["luoji"] - paths, err := findPaths(nil, graph, sourceNode, target, paymentAmt) + paths, err := findPaths( + nil, graph, sourceNode, target, paymentAmt, 100, + ) if err != nil { t.Fatalf("unable to find paths between roasbeef and "+ "luo ji: %v", err) diff --git a/routing/router_test.go b/routing/router_test.go index 48193c96..552684ce 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1334,3 +1334,56 @@ func TestRouterChansClosedOfflinePruneGraph(t *testing.T) { t.Fatalf("channel was found in graph but shouldn't have been") } } + +// TestFindPathFeeWeighting tests that the findPath method will properly prefer +// routes with lower fees over routes with lower time lock values. This is +// meant to exercise the fact that the internal findPath method ranks edges +// with the square of the total fee in order bias towards lower fees. +func TestFindPathFeeWeighting(t *testing.T) { + t.Parallel() + + const startingBlockHeight = 101 + ctx, cleanUp, err := createTestCtx(startingBlockHeight, basicGraphFilePath) + defer cleanUp() + if err != nil { + t.Fatalf("unable to create router: %v", err) + } + + var preImage [32]byte + copy(preImage[:], bytes.Repeat([]byte{9}, 32)) + + sourceNode, err := ctx.graph.SourceNode() + if err != nil { + t.Fatalf("unable to fetch source node: %v", err) + } + + ignoreVertex := make(map[Vertex]struct{}) + ignoreEdge := make(map[uint64]struct{}) + + amt := lnwire.MilliSatoshi(100) + + target := ctx.aliases["luoji"] + if target == nil { + t.Fatalf("unable to find target node") + } + + // We'll now attempt a path finding attempt using this set up. Due to + // the edge weighting, we should select the direct path over the 2 hop + // path even though the direct path has a higher potential time lock. + path, err := findPath( + nil, ctx.graph, sourceNode, target, ignoreVertex, ignoreEdge, + amt, + ) + if err != nil { + t.Fatalf("unable to find path: %v", err) + } + + // The route that was chosen should be exactly one hop, and should be + // directly to luoji. + if len(path) != 1 { + t.Fatalf("expected path length of 1, instead was: %v", len(path)) + } + if path[0].Node.Alias != "luoji" { + t.Fatalf("wrong node: %v", path[0].Node.Alias) + } +} diff --git a/routing/testdata/basic_graph.json b/routing/testdata/basic_graph.json index 64849b63..c5fa8647 100644 --- a/routing/testdata/basic_graph.json +++ b/routing/testdata/basic_graph.json @@ -135,7 +135,7 @@ "channel_id": 689530843, "channel_point": "25376aa6cb81913ad30416bd22d4083241bd6d68e811d0284d3c3a17795c458a:0", "flags": 0, - "expiry": 1, + "expiry": 10, "min_htlc": 1, "fee_base_msat": 10, "fee_rate": 1000,