From 16f6284d979fb4a0d47ee47fbd28c3b7aea81817 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 25 Jun 2024 15:02:41 +0200 Subject: [PATCH 01/10] routing: reorganize BuildRoute --- routing/router.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/routing/router.go b/routing/router.go index 5551c5345..9704039c5 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1409,8 +1409,7 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, hops []route.Vertex, outgoingChan *uint64, finalCltvDelta int32, payAddr *[32]byte) (*route.Route, error) { - log.Tracef("BuildRoute called: hopsCount=%v, amt=%v", - len(hops), amt) + log.Tracef("BuildRoute called: hopsCount=%v, amt=%v", len(hops), amt) var outgoingChans map[uint64]struct{} if outgoingChan != nil { @@ -1419,6 +1418,17 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, } } + // We'll attempt to obtain a set of bandwidth hints that helps us select + // the best outgoing channel to use in case no outgoing channel is set. + bandwidthHints, err := newBandwidthManager( + r.cfg.RoutingGraph, r.cfg.SelfNode, r.cfg.GetLink, + ) + if err != nil { + return nil, err + } + + sourceNode := r.cfg.SelfNode + // If no amount is specified, we need to build a route for the minimum // amount that this route can carry. useMinAmt := amt == nil @@ -1436,23 +1446,6 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, runningAmt = *amt } - // We'll attempt to obtain a set of bandwidth hints that helps us select - // the best outgoing channel to use in case no outgoing channel is set. - bandwidthHints, err := newBandwidthManager( - r.cfg.RoutingGraph, r.cfg.SelfNode, r.cfg.GetLink, - ) - if err != nil { - return nil, err - } - - // Fetch the current block height outside the routing transaction, to - // prevent the rpc call blocking the database. - _, height, err := r.cfg.Chain.GetBestBlock() - if err != nil { - return nil, err - } - - sourceNode := r.cfg.SelfNode unifiers, senderAmt, err := getRouteUnifiers( sourceNode, hops, useMinAmt, runningAmt, outgoingChans, r.cfg.RoutingGraph, bandwidthHints, @@ -1468,6 +1461,13 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, return nil, err } + // Fetch the current block height outside the routing transaction, to + // prevent the rpc call blocking the database. + _, height, err := r.cfg.Chain.GetBestBlock() + if err != nil { + return nil, err + } + // Build and return the final route. return newRoute( sourceNode, pathEdges, uint32(height), From 2edbe6b87848abfabaf446be3f37cbc41f37c4df Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 25 Jun 2024 11:11:18 +0200 Subject: [PATCH 02/10] routing: extract getEdgeUnifiers We split up the functionality in getRouteUnifiers into checking that all edges exist via getEdgeUnifiers and then add a backward pass that will be responsible for determining the sender amount. We remove the node pub key from the error string, as in route building this is duplicate info, which can be determined from the input keys, further it's not available in the backward pass anymore. We refactor the BuildRoute test to use the require library and add a test case for a max HTLC violation on the last hop. --- routing/router.go | 84 ++++++++++++++------------- routing/router_test.go | 128 +++++++++++++++++++++++------------------ 2 files changed, 118 insertions(+), 94 deletions(-) diff --git a/routing/router.go b/routing/router.go index 9704039c5..85a4afd55 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1393,13 +1393,12 @@ func (r *ChannelRouter) extractChannelUpdate( // channels that satisfy all requirements. type ErrNoChannel struct { position int - fromNode route.Vertex } // Error returns a human-readable string describing the error. func (e ErrNoChannel) Error() string { return fmt.Sprintf("no matching outgoing channel available for "+ - "node %v (%v)", e.position, e.fromNode) + "node index %v", e.position) } // BuildRoute returns a fully specified route based on a list of pubkeys. If @@ -1429,6 +1428,15 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, sourceNode := r.cfg.SelfNode + // We check that each node in the route has a connection to others that + // can forward in principle. + unifiers, err := getEdgeUnifiers( + r.cfg.SelfNode, hops, outgoingChans, r.cfg.RoutingGraph, + ) + if err != nil { + return nil, err + } + // If no amount is specified, we need to build a route for the minimum // amount that this route can carry. useMinAmt := amt == nil @@ -1446,16 +1454,15 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, runningAmt = *amt } - unifiers, senderAmt, err := getRouteUnifiers( - sourceNode, hops, useMinAmt, runningAmt, outgoingChans, - r.cfg.RoutingGraph, bandwidthHints, + senderAmt, err := senderAmtBackwardPass( + unifiers, useMinAmt, runningAmt, bandwidthHints, ) if err != nil { return nil, err } pathEdges, receiverAmt, err := getPathEdges( - sourceNode, senderAmt, unifiers, bandwidthHints, hops, + senderAmt, unifiers, bandwidthHints, ) if err != nil { return nil, err @@ -1481,12 +1488,10 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, ) } -// getRouteUnifiers returns a list of edge unifiers for the given route. -func getRouteUnifiers(source route.Vertex, hops []route.Vertex, - useMinAmt bool, runningAmt lnwire.MilliSatoshi, - outgoingChans map[uint64]struct{}, graph Graph, - bandwidthHints *bandwidthManager) ([]*edgeUnifier, lnwire.MilliSatoshi, - error) { +// getEdgeUnifiers returns a list of edge unifiers for the given route. +func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, + outgoingChans map[uint64]struct{}, + graph Graph) ([]*edgeUnifier, error) { // Allocate a list that will contain the edge unifiers for this route. unifiers := make([]*edgeUnifier, len(hops)) @@ -1502,8 +1507,6 @@ func getRouteUnifiers(source route.Vertex, hops []route.Vertex, fromNode = hops[i-1] } - localChan := i == 0 - // Build unified policies for this hop based on the channels // known in the graph. Don't use inbound fees. // @@ -1514,19 +1517,34 @@ func getRouteUnifiers(source route.Vertex, hops []route.Vertex, err := u.addGraphPolicies(graph) if err != nil { - return nil, 0, err + return nil, err } // Exit if there are no channels. edgeUnifier, ok := u.edgeUnifiers[fromNode] if !ok { log.Errorf("Cannot find policy for node %v", fromNode) - return nil, 0, ErrNoChannel{ - fromNode: fromNode, - position: i, - } + return nil, ErrNoChannel{position: i} } + unifiers[i] = edgeUnifier + } + + return unifiers, nil +} + +// senderAmtBackwardPass determines the amount that should be sent to fulfill +// min HTLC requirements. The minimal sender amount can be searched for by +// activating useMinAmt. +func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, + runningAmt lnwire.MilliSatoshi, + bandwidthHints *bandwidthManager) (lnwire.MilliSatoshi, error) { + + // Traverse hops backwards to accumulate fees in the running amounts. + for i := len(unifiers) - 1; i >= 0; i-- { + localChan := i == 0 + edgeUnifier := unifiers[i] + // If using min amt, increase amt if needed. if useMinAmt { min := edgeUnifier.minAmt() @@ -1538,11 +1556,10 @@ func getRouteUnifiers(source route.Vertex, hops []route.Vertex, // Get an edge for the specific amount that we want to forward. edge := edgeUnifier.getEdge(runningAmt, bandwidthHints, 0) if edge == nil { - log.Errorf("Cannot find policy with amt=%v for node %v", - runningAmt, fromNode) + log.Errorf("Cannot find policy with amt=%v for hop %v", + runningAmt, i) - return nil, 0, ErrNoChannel{ - fromNode: fromNode, + return 0, ErrNoChannel{ position: i, } } @@ -1554,19 +1571,16 @@ func getRouteUnifiers(source route.Vertex, hops []route.Vertex, log.Tracef("Select channel %v at position %v", edge.policy.ChannelID, i) - - unifiers[i] = edgeUnifier } - return unifiers, runningAmt, nil + return runningAmt, nil } // getPathEdges returns the edges that make up the path and the total amount, // including fees, to send the payment. -func getPathEdges(source route.Vertex, receiverAmt lnwire.MilliSatoshi, - unifiers []*edgeUnifier, bandwidthHints *bandwidthManager, - hops []route.Vertex) ([]*unifiedEdge, - lnwire.MilliSatoshi, error) { +func getPathEdges(receiverAmt lnwire.MilliSatoshi, unifiers []*edgeUnifier, + bandwidthHints *bandwidthManager) ([]*unifiedEdge, lnwire.MilliSatoshi, + error) { // Now that we arrived at the start of the route and found out the route // total amount, we make a forward pass. Because the amount may have @@ -1576,15 +1590,7 @@ func getPathEdges(source route.Vertex, receiverAmt lnwire.MilliSatoshi, for i, unifier := range unifiers { edge := unifier.getEdge(receiverAmt, bandwidthHints, 0) if edge == nil { - fromNode := source - if i > 0 { - fromNode = hops[i-1] - } - - return nil, 0, ErrNoChannel{ - fromNode: fromNode, - position: i, - } + return nil, 0, ErrNoChannel{position: i} } if i > 0 { diff --git a/routing/router_test.go b/routing/router_test.go index f631669a9..6b80bd86e 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1536,10 +1536,10 @@ func TestBuildRoute(t *testing.T) { }, 6), // Create two channels from b to c. For building routes, we - // expect the lowest cost channel to be selected. Note that this - // isn't a situation that we are expecting in reality. Routing - // nodes are recommended to keep their channel policies towards - // the same peer identical. + // expect the highest cost channel to be selected. Note that + // this isn't a situation that we are expecting in reality. + // Routing nodes are recommended to keep their channel policies + // towards the same peer identical. symmetricTestChannel("b", "c", chanCapSat, &testChannelPolicy{ Expiry: 144, FeeRate: 50000, @@ -1555,6 +1555,8 @@ func TestBuildRoute(t *testing.T) { Features: paymentAddrFeatures, }, 7), + // Create some channels that have conflicting min/max + // constraints. symmetricTestChannel("a", "e", chanCapSat, &testChannelPolicy{ Expiry: 144, FeeRate: 80000, @@ -1569,9 +1571,28 @@ func TestBuildRoute(t *testing.T) { MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), Features: paymentAddrFeatures, }, 4), + + // Create some channels that have a conflicting max HTLC + // constraint for one node pair, similar to the b->c channels. + symmetricTestChannel("b", "z", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 50000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + MaxHTLC: lnwire.NewMSatFromSatoshis(25), + Features: paymentAddrFeatures, + }, 3), + symmetricTestChannel("b", "z", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 60000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + MaxHTLC: lnwire.MilliSatoshi(20100), + Features: paymentAddrFeatures, + }, 8), } - testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") + testGraph, err := createTestGraphFromChannels( + t, true, testChannels, "a", + ) require.NoError(t, err, "unable to create graph") const startingBlockHeight = 101 @@ -1583,15 +1604,10 @@ func TestBuildRoute(t *testing.T) { t.Helper() - if len(rt.Hops) != len(expected) { - t.Fatal("hop count mismatch") - } + require.Len(t, rt.Hops, len(expected)) + for i, hop := range rt.Hops { - if hop.ChannelID != expected[i] { - t.Fatalf("expected channel %v at pos %v, but "+ - "got channel %v", - expected[i], i, hop.ChannelID) - } + require.Equal(t, expected[i], hop.ChannelID) } lastHop := rt.Hops[len(rt.Hops)-1] @@ -1603,64 +1619,67 @@ func TestBuildRoute(t *testing.T) { _, err = rand.Read(payAddr[:]) require.NoError(t, err) + // Create hop list for an unknown destination. + hops := []route.Vertex{ctx.aliases["b"], ctx.aliases["y"]} + _, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + noChanErr := ErrNoChannel{} + require.ErrorAs(t, err, &noChanErr) + require.Equal(t, 1, noChanErr.position) + // Create hop list from the route node pubkeys. - hops := []route.Vertex{ - ctx.aliases["b"], ctx.aliases["c"], - } + hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["c"]} amt := lnwire.NewMSatFromSatoshis(100) // Build the route for the given amount. - rt, err := ctx.router.BuildRoute( - &amt, hops, nil, 40, &payAddr, - ) - if err != nil { - t.Fatal(err) - } + rt, err := ctx.router.BuildRoute(&amt, hops, nil, 40, &payAddr) + require.NoError(t, err) // Check that we get the expected route back. The total amount should be // the amount to deliver to hop c (100 sats) plus the max fee for the // connection b->c (6 sats). checkHops(rt, []uint64{1, 7}, payAddr) - if rt.TotalAmount != 106000 { - t.Fatalf("unexpected total amount %v", rt.TotalAmount) - } + require.Equal(t, lnwire.MilliSatoshi(106000), rt.TotalAmount) // Build the route for the minimum amount. - rt, err = ctx.router.BuildRoute( - nil, hops, nil, 40, &payAddr, - ) - if err != nil { - t.Fatal(err) - } + rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + require.NoError(t, err) // Check that we get the expected route back. The minimum that we can // send from b to c is 20 sats. Hop b charges 1200 msat for the // forwarding. The channel between hop a and b can carry amounts in the // range [5, 100], so 21200 msats is the minimum amount for this route. checkHops(rt, []uint64{1, 7}, payAddr) - if rt.TotalAmount != 21200 { - t.Fatalf("unexpected total amount %v", rt.TotalAmount) - } + require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) + + // The receiver gets sent the minimal HTLC amount. + require.Equal(t, lnwire.MilliSatoshi(20000), rt.Hops[1].AmtToForward) // Test a route that contains incompatible channel htlc constraints. // There is no amount that can pass through both channel 5 and 4. - hops = []route.Vertex{ - ctx.aliases["e"], ctx.aliases["c"], - } - _, err = ctx.router.BuildRoute( - nil, hops, nil, 40, nil, - ) - errNoChannel, ok := err.(ErrNoChannel) - if !ok { - t.Fatalf("expected incompatible policies error, but got %v", - err) - } - if errNoChannel.position != 0 { - t.Fatalf("unexpected no channel error position") - } - if errNoChannel.fromNode != ctx.aliases["a"] { - t.Fatalf("unexpected no channel error node") - } + hops = []route.Vertex{ctx.aliases["e"], ctx.aliases["c"]} + _, err = ctx.router.BuildRoute(nil, hops, nil, 40, nil) + require.Error(t, err) + noChanErr = ErrNoChannel{} + require.ErrorAs(t, err, &noChanErr) + require.Equal(t, 0, noChanErr.position) + + // Test a route that contains channel constraints that lead to a + // different selection of a unified edge, when the amount is rescaled + // for the final edge. From a backward pass we expect the policy of + // channel 8 to be used, because its policy has the highest fee rate, + // bumping the amount to 20000 msat leading to a sender amount of 21200 + // msat including the fees for hop over channel 8. In the forward pass + // however, we discover that the max HTLC constraint of channel 8 is + // violated after subtracting the fee, which is why we change the policy + // to the one of channel 3. This leads to a delivered amount of 20191 + // msat, in contrast to the naive 20000 msat from the min HTLC + // constraint of both channels. + hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["z"]} + rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{1, 3}, payAddr) + require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) + require.Equal(t, lnwire.MilliSatoshi(20191), rt.Hops[1].AmtToForward) } // TestGetPathEdges tests that the getPathEdges function returns the expected @@ -1689,14 +1708,13 @@ func TestGetPathEdges(t *testing.T) { localChan: true, }, }, - expectedErr: fmt.Sprintf("no matching outgoing channel "+ - "available for node 0 (%v)", ctx.aliases["roasbeef"]), + expectedErr: fmt.Sprintf("no matching outgoing channel " + + "available for node index 0"), }} for _, tc := range testCases { pathEdges, amt, err := getPathEdges( - tc.sourceNode, tc.amt, tc.unifiers, tc.bandwidthHints, - tc.hops, + tc.amt, tc.unifiers, tc.bandwidthHints, ) if tc.expectedErr != "" { From e4c7c106184af7392b5fd842550bcf4b6b3ee300 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 25 Jun 2024 11:26:08 +0200 Subject: [PATCH 03/10] routing: handle both amounts equally We duplicate the function calls to handle the min amount and known amount cases in a similar manner, to make the next diff easier to parse. --- routing/router.go | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/routing/router.go b/routing/router.go index 85a4afd55..3d2c34eac 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1441,31 +1441,45 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // amount that this route can carry. useMinAmt := amt == nil - var runningAmt lnwire.MilliSatoshi + var ( + receiverAmt lnwire.MilliSatoshi + pathEdges []*unifiedEdge + ) + if useMinAmt { // For minimum amount routes, aim to deliver at least 1 msat to // the destination. There are nodes in the wild that have a // min_htlc channel policy of zero, which could lead to a zero // amount payment being made. - runningAmt = 1 + senderAmt, err := senderAmtBackwardPass( + unifiers, useMinAmt, 1, bandwidthHints, + ) + if err != nil { + return nil, err + } + + pathEdges, receiverAmt, err = getPathEdges( + senderAmt, unifiers, bandwidthHints, + ) + if err != nil { + return nil, err + } } else { // If an amount is specified, we need to build a route that // delivers exactly this amount to the final destination. - runningAmt = *amt - } + senderAmt, err := senderAmtBackwardPass( + unifiers, useMinAmt, *amt, bandwidthHints, + ) + if err != nil { + return nil, err + } - senderAmt, err := senderAmtBackwardPass( - unifiers, useMinAmt, runningAmt, bandwidthHints, - ) - if err != nil { - return nil, err - } - - pathEdges, receiverAmt, err := getPathEdges( - senderAmt, unifiers, bandwidthHints, - ) - if err != nil { - return nil, err + pathEdges, receiverAmt, err = getPathEdges( + senderAmt, unifiers, bandwidthHints, + ) + if err != nil { + return nil, err + } } // Fetch the current block height outside the routing transaction, to From c18694ff5eec31fc9070ca844246aa558181b29e Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 25 Jun 2024 11:45:36 +0200 Subject: [PATCH 04/10] routing: backward pass determines unified edges We shift the duty of determining the policies to the backward pass as the forward pass will only be responsible for finding the corrected receiver amount. Note that this is not a pure refactor as demonstrated in the test, as the forward pass doesn't select new policies anymore, which is less flexible and doesn't lead to the highest possible receiver amount. This is however neccessary as we otherwise won't be able to compute forwarding amounts involving inbound fees and this edge case is unlikely to occur, because we search for a min amount for a route that was most likely constructed for a larger amount. --- routing/router.go | 63 ++++++------ routing/router_test.go | 220 ++++++++++++++++++++++++++++++++++------- 2 files changed, 215 insertions(+), 68 deletions(-) diff --git a/routing/router.go b/routing/router.go index 3d2c34eac..1fce71fb3 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1451,35 +1451,29 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // the destination. There are nodes in the wild that have a // min_htlc channel policy of zero, which could lead to a zero // amount payment being made. - senderAmt, err := senderAmtBackwardPass( + var senderAmt lnwire.MilliSatoshi + pathEdges, senderAmt, err = senderAmtBackwardPass( unifiers, useMinAmt, 1, bandwidthHints, ) if err != nil { return nil, err } - pathEdges, receiverAmt, err = getPathEdges( - senderAmt, unifiers, bandwidthHints, - ) + receiverAmt, err = receiverAmtForwardPass(senderAmt, pathEdges) if err != nil { return nil, err } } else { // If an amount is specified, we need to build a route that // delivers exactly this amount to the final destination. - senderAmt, err := senderAmtBackwardPass( + pathEdges, _, err = senderAmtBackwardPass( unifiers, useMinAmt, *amt, bandwidthHints, ) if err != nil { return nil, err } - pathEdges, receiverAmt, err = getPathEdges( - senderAmt, unifiers, bandwidthHints, - ) - if err != nil { - return nil, err - } + receiverAmt = *amt } // Fetch the current block height outside the routing transaction, to @@ -1547,12 +1541,15 @@ func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, return unifiers, nil } -// senderAmtBackwardPass determines the amount that should be sent to fulfill -// min HTLC requirements. The minimal sender amount can be searched for by -// activating useMinAmt. +// senderAmtBackwardPass returns a list of unified edges for the given route and +// determines the amount that should be sent to fulfill min HTLC requirements. +// The minimal sender amount can be searched for by activating useMinAmt. func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, runningAmt lnwire.MilliSatoshi, - bandwidthHints *bandwidthManager) (lnwire.MilliSatoshi, error) { + bandwidthHints bandwidthHints) ([]*unifiedEdge, lnwire.MilliSatoshi, + error) { + + var unifiedEdges = make([]*unifiedEdge, len(unifiers)) // Traverse hops backwards to accumulate fees in the running amounts. for i := len(unifiers) - 1; i >= 0; i-- { @@ -1573,7 +1570,7 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, log.Errorf("Cannot find policy with amt=%v for hop %v", runningAmt, i) - return 0, ErrNoChannel{ + return nil, 0, ErrNoChannel{ position: i, } } @@ -1585,37 +1582,37 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, log.Tracef("Select channel %v at position %v", edge.policy.ChannelID, i) + + unifiedEdges[i] = edge } - return runningAmt, nil + return unifiedEdges, runningAmt, nil } -// getPathEdges returns the edges that make up the path and the total amount, -// including fees, to send the payment. -func getPathEdges(receiverAmt lnwire.MilliSatoshi, unifiers []*edgeUnifier, - bandwidthHints *bandwidthManager) ([]*unifiedEdge, lnwire.MilliSatoshi, - error) { +// receiverAmtForwardPass returns the amount that a receiver will receive after +// deducting all fees from the sender amount. +func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, + unifiedEdges []*unifiedEdge) (lnwire.MilliSatoshi, error) { // Now that we arrived at the start of the route and found out the route // total amount, we make a forward pass. Because the amount may have // been increased in the backward pass, fees need to be recalculated and // amount ranges re-checked. - var pathEdges []*unifiedEdge - for i, unifier := range unifiers { - edge := unifier.getEdge(receiverAmt, bandwidthHints, 0) - if edge == nil { - return nil, 0, ErrNoChannel{position: i} - } - + for i, edge := range unifiedEdges { if i > 0 { // Decrease the amount to send while going forward. - receiverAmt -= edge.policy.ComputeFeeFromIncoming( - receiverAmt, + runningAmt -= edge.policy.ComputeFeeFromIncoming( + runningAmt, ) } - pathEdges = append(pathEdges, edge) + if !edge.amtInRange(runningAmt) { + log.Errorf("Amount %v not in range for hop %v", + runningAmt, i) + + return 0, ErrNoChannel{position: i} + } } - return pathEdges, receiverAmt, nil + return runningAmt, nil } diff --git a/routing/router_test.go b/routing/router_test.go index 6b80bd86e..3371678c1 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1669,53 +1669,122 @@ func TestBuildRoute(t *testing.T) { // channel 8 to be used, because its policy has the highest fee rate, // bumping the amount to 20000 msat leading to a sender amount of 21200 // msat including the fees for hop over channel 8. In the forward pass - // however, we discover that the max HTLC constraint of channel 8 is - // violated after subtracting the fee, which is why we change the policy - // to the one of channel 3. This leads to a delivered amount of 20191 - // msat, in contrast to the naive 20000 msat from the min HTLC - // constraint of both channels. + // however, we subtract that fee again, resulting in the min HTLC + // amount. The forward pass doesn't check for a different policy that + // could me more applicable, which is why we don't get back the highest + // amount that could be delivered to the receiver of 21819 msat, using + // policy of channel 3. hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["z"]} rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) require.NoError(t, err) - checkHops(rt, []uint64{1, 3}, payAddr) + checkHops(rt, []uint64{1, 8}, payAddr) require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) - require.Equal(t, lnwire.MilliSatoshi(20191), rt.Hops[1].AmtToForward) + require.Equal(t, lnwire.MilliSatoshi(20000), rt.Hops[1].AmtToForward) + } -// TestGetPathEdges tests that the getPathEdges function returns the expected -// edges and amount when given a set of unifiers and does not panic. -func TestGetPathEdges(t *testing.T) { +// TestReceiverAmtForwardPass tests that the forward pass returns the expected +// receiver amount when given a set of edges and does not panic. +func TestReceiverAmtForwardPass(t *testing.T) { t.Parallel() - const startingBlockHeight = 101 - ctx := createTestCtxFromFile(t, startingBlockHeight, basicGraphFilePath) - testCases := []struct { - sourceNode route.Vertex - amt lnwire.MilliSatoshi - unifiers []*edgeUnifier - bandwidthHints *bandwidthManager - hops []route.Vertex + name string + amt lnwire.MilliSatoshi + unifiedEdges []*unifiedEdge + hops []route.Vertex - expectedEdges []*models.CachedEdgePolicy - expectedAmt lnwire.MilliSatoshi - expectedErr string - }{{ - sourceNode: ctx.aliases["roasbeef"], - unifiers: []*edgeUnifier{ - { - edges: []*unifiedEdge{}, - localChan: true, - }, + expectedAmt lnwire.MilliSatoshi + expectedErr string + }{ + { + name: "empty", }, - expectedErr: fmt.Sprintf("no matching outgoing channel " + - "available for node index 0"), - }} + { + name: "single edge, no valid policy", + amt: 1000, + unifiedEdges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + MinHTLC: 1001, + }, + }, + }, + expectedErr: fmt.Sprintf("no matching outgoing " + + "channel available for node index 0"), + }, + { + name: "single edge", + amt: 1000, + unifiedEdges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + MinHTLC: 1000, + }, + }, + }, + expectedAmt: 1000, + }, + { + name: "outbound fee, no rounding", + amt: 1e9, + unifiedEdges: []*unifiedEdge{ + { + // The first hop's outbound fee is + // irrelevant in fee calculation. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1234, + FeeProportionalMillionths: 1234, + }, + }, + { + // No rounding is done here. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1000, + FeeProportionalMillionths: 1000, + }, + }, + }, + // From an outgoing amount of 999000000 msat, we get + // in = out + base + out * rate = 1000000000.0 + // + // The inverse outgoing amount for this is + // out = (in - base) / (1 + rate) = + // (1e9 - 1000) / (1 + 1e-3) = 999000000.0000001, + // which is rounded down. + expectedAmt: 999000000, + }, + { + name: "outbound fee, rounding", + amt: 1e9, + unifiedEdges: []*unifiedEdge{ + { + // The first hop's outbound fee is + // irrelevant in fee calculation. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1234, + FeeProportionalMillionths: 1234, + }, + }, + { + // This policy is chosen such that we + // round down. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1000, + FeeProportionalMillionths: 999, + }, + }, + }, + // The float amount for this is + // out = (in - base) / (1 + rate) = + // (1e9 - 1000) / (1 + 999e-6) = 999000998.002995, + // which is rounded up. + expectedAmt: 999000999, + }, + } for _, tc := range testCases { - pathEdges, amt, err := getPathEdges( - tc.amt, tc.unifiers, tc.bandwidthHints, - ) + amt, err := receiverAmtForwardPass(tc.amt, tc.unifiedEdges) if tc.expectedErr != "" { require.Error(t, err) @@ -1725,11 +1794,92 @@ func TestGetPathEdges(t *testing.T) { } require.NoError(t, err) - require.Equal(t, pathEdges, tc.expectedEdges) require.Equal(t, amt, tc.expectedAmt) } } +// TestSenderAmtBackwardPass tests that the computation of the sender amount is +// done correctly for route building. +func TestSenderAmtBackwardPass(t *testing.T) { + bandwidthHints := bandwidthManager{ + getLink: func(chanId lnwire.ShortChannelID) ( + htlcswitch.ChannelLink, error) { + + return nil, nil + }, + localChans: make(map[lnwire.ShortChannelID]struct{}), + } + + var ( + capacity btcutil.Amount = 1_000_000 + testReceiverAmt lnwire.MilliSatoshi = 1_000_000 + minHTLC lnwire.MilliSatoshi = 1_000 + ) + + edgeUnifiers := []*edgeUnifier{ + { + edges: []*unifiedEdge{ + { + // This outbound fee doesn't have an + // effect (sender doesn't pay outbound). + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 112, + }, + capacity: capacity, + }, + }, + }, + { + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 222, + }, + capacity: capacity, + }, + }, + }, + { + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 333, + MinHTLC: minHTLC, + }, + capacity: capacity, + }, + }, + }, + } + + // A search for an amount that is below the minimum HTLC amount should + // fail. + _, _, err := senderAmtBackwardPass( + edgeUnifiers, false, minHTLC-1, &bandwidthHints, + ) + require.Error(t, err) + + // Do a min amount search. + unifiedEdges, senderAmount, err := senderAmtBackwardPass( + edgeUnifiers, true, 1, &bandwidthHints, + ) + require.NoError(t, err) + require.Equal(t, minHTLC+333+222, senderAmount) + + // Do a search for a specific amount. + unifiedEdges, senderAmount, err = senderAmtBackwardPass( + edgeUnifiers, false, testReceiverAmt, &bandwidthHints, + ) + require.NoError(t, err) + require.Equal(t, testReceiverAmt+333+222, senderAmount) + + // Check that we arrive at the same receiver amount by doing a forward + // pass. + receiverAmt, err := receiverAmtForwardPass(senderAmount, unifiedEdges) + require.NoError(t, err) + require.Equal(t, testReceiverAmt, receiverAmt) +} + // TestSendToRouteSkipTempErrSuccess validates a successful payment send. func TestSendToRouteSkipTempErrSuccess(t *testing.T) { t.Parallel() From 2c79bf9635256d0ea6ab42286e516facc54e28cd Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 25 Jun 2024 19:32:55 +0200 Subject: [PATCH 05/10] routing: refactor backward and forward pass --- routing/router.go | 114 ++++++++++++++++++++++++++++++----------- routing/router_test.go | 8 ++- 2 files changed, 91 insertions(+), 31 deletions(-) diff --git a/routing/router.go b/routing/router.go index 1fce71fb3..1a5c85397 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1545,48 +1545,90 @@ func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, // determines the amount that should be sent to fulfill min HTLC requirements. // The minimal sender amount can be searched for by activating useMinAmt. func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, - runningAmt lnwire.MilliSatoshi, + receiverAmt lnwire.MilliSatoshi, bandwidthHints bandwidthHints) ([]*unifiedEdge, lnwire.MilliSatoshi, error) { + if len(unifiers) == 0 { + return nil, 0, fmt.Errorf("no unifiers provided") + } + var unifiedEdges = make([]*unifiedEdge, len(unifiers)) - // Traverse hops backwards to accumulate fees in the running amounts. - for i := len(unifiers) - 1; i >= 0; i-- { - localChan := i == 0 - edgeUnifier := unifiers[i] + // We traverse the route backwards and handle the last hop separately. + edgeUnifier := unifiers[len(unifiers)-1] - // If using min amt, increase amt if needed. + // incomingAmt tracks the amount that is forwarded on the edges of a + // route. The last hop only forwards the amount that the receiver should + // receive, as there are no fees paid to the last node. + incomingAmt := receiverAmt + + // If using min amt, increase the amount if needed to fulfill min HTLC + // requirements. + if useMinAmt { + min := edgeUnifier.minAmt() + if min > incomingAmt { + incomingAmt = min + } + } + + // Get an edge for the specific amount that we want to forward. + edge := edgeUnifier.getEdge(incomingAmt, bandwidthHints, 0) + if edge == nil { + log.Errorf("Cannot find policy with amt=%v "+ + "for hop %v", incomingAmt, len(unifiers)-1) + + return nil, 0, ErrNoChannel{position: len(unifiers) - 1} + } + + unifiedEdges[len(unifiers)-1] = edge + + // Handle the rest of the route except the last hop. + for i := len(unifiers) - 2; i >= 0; i-- { + edgeUnifier = unifiers[i] + + // If using min amt, increase the amount if needed to fulfill + // min HTLC requirements. if useMinAmt { min := edgeUnifier.minAmt() - if min > runningAmt { - runningAmt = min + if min > incomingAmt { + incomingAmt = min } } - // Get an edge for the specific amount that we want to forward. - edge := edgeUnifier.getEdge(runningAmt, bandwidthHints, 0) + // A --current hop-- B --next hop: incomingAmt-- C + // The outbound fee paid to the current end node B is based on + // the amount that the next hop forwards and B's policy for that + // hop. + outboundFee := unifiedEdges[i+1].policy.ComputeFee( + incomingAmt, + ) + + netAmount := incomingAmt + outboundFee + + // We need to select an edge that can forward the requested + // amount. + edge = edgeUnifier.getEdge( + netAmount, bandwidthHints, outboundFee, + ) if edge == nil { - log.Errorf("Cannot find policy with amt=%v for hop %v", - runningAmt, i) - - return nil, 0, ErrNoChannel{ - position: i, - } + return nil, 0, ErrNoChannel{position: i} } - // Add fee for this hop. - if !localChan { - runningAmt += edge.policy.ComputeFee(runningAmt) - } + fee := outboundFee log.Tracef("Select channel %v at position %v", edge.policy.ChannelID, i) + // Finally, we update the amount that needs to flow into node B + // from A, which is the next hop's forwarding amount plus the + // fee for B: A --current hop: incomingAmt-- B --next hop-- C + incomingAmt += fee + unifiedEdges[i] = edge } - return unifiedEdges, runningAmt, nil + return unifiedEdges, incomingAmt, nil } // receiverAmtForwardPass returns the amount that a receiver will receive after @@ -1594,20 +1636,32 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, unifiedEdges []*unifiedEdge) (lnwire.MilliSatoshi, error) { + if len(unifiedEdges) == 0 { + return 0, fmt.Errorf("no edges to forward through") + } + + inEdge := unifiedEdges[0] + if !inEdge.amtInRange(runningAmt) { + log.Errorf("Amount %v not in range for hop index %v", + runningAmt, 0) + + return 0, ErrNoChannel{position: 0} + } + // Now that we arrived at the start of the route and found out the route // total amount, we make a forward pass. Because the amount may have // been increased in the backward pass, fees need to be recalculated and // amount ranges re-checked. - for i, edge := range unifiedEdges { - if i > 0 { - // Decrease the amount to send while going forward. - runningAmt -= edge.policy.ComputeFeeFromIncoming( - runningAmt, - ) - } + for i := 1; i < len(unifiedEdges); i++ { + outEdge := unifiedEdges[i] - if !edge.amtInRange(runningAmt) { - log.Errorf("Amount %v not in range for hop %v", + // Decrease the amount to send while going forward. + runningAmt -= outEdge.policy.ComputeFeeFromIncoming( + runningAmt, + ) + + if !outEdge.amtInRange(runningAmt) { + log.Errorf("Amount %v not in range for hop index %v", runningAmt, i) return 0, ErrNoChannel{position: i} diff --git a/routing/router_test.go b/routing/router_test.go index 3371678c1..7ee5817b4 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1619,6 +1619,11 @@ func TestBuildRoute(t *testing.T) { _, err = rand.Read(payAddr[:]) require.NoError(t, err) + // Test that we can't build a route when no hops are given. + hops = []route.Vertex{} + _, err = ctx.router.BuildRoute(nil, hops, nil, 40, nil) + require.Error(t, err) + // Create hop list for an unknown destination. hops := []route.Vertex{ctx.aliases["b"], ctx.aliases["y"]} _, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) @@ -1698,7 +1703,8 @@ func TestReceiverAmtForwardPass(t *testing.T) { expectedErr string }{ { - name: "empty", + name: "empty", + expectedErr: "no edges to forward through", }, { name: "single edge, no valid policy", From 36cd03669b4fe8b4487fb6abe792005b6f84d71e Mon Sep 17 00:00:00 2001 From: bitromortac Date: Mon, 1 Jul 2024 13:34:45 +0200 Subject: [PATCH 06/10] routing: add outgoingFromIncoming amount calc Adds a utility function to be able to compute the outgoing routing amount from the incoming amount by taking inbound and outbound fees into account. The discussion was contributed by user feelancer21, see https://github.com/feelancer21/lnd/commit/f6f05fa930985aac0d27c3f6681aada1b599162a. --- routing/router.go | 223 +++++++++++++++++++++++++++++++++++++++++ routing/router_test.go | 150 +++++++++++++++++++++++++++ 2 files changed, 373 insertions(+) diff --git a/routing/router.go b/routing/router.go index 1a5c85397..d67fc2ebd 100644 --- a/routing/router.go +++ b/routing/router.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "math" + "math/big" "sort" "sync" "sync/atomic" @@ -836,6 +837,7 @@ func generateSphinxPacket(rt *route.Route, paymentHash []byte, hopCopy := sphinxPath[i] path[i] = hopCopy } + return spew.Sdump(path) }), ) @@ -1670,3 +1672,224 @@ func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, return runningAmt, nil } + +// incomingFromOutgoing computes the incoming amount based on the outgoing +// amount by adding fees to the outgoing amount, replicating the path finding +// and routing process, see also CheckHtlcForward. +func incomingFromOutgoing(outgoingAmt lnwire.MilliSatoshi, + incoming, outgoing *unifiedEdge) lnwire.MilliSatoshi { + + outgoingFee := outgoing.policy.ComputeFee(outgoingAmt) + + // Net amount is the amount the inbound fees are calculated with. + netAmount := outgoingAmt + outgoingFee + + inboundFee := incoming.inboundFees.CalcFee(netAmount) + + // The inbound fee is not allowed to reduce the incoming amount below + // the outgoing amount. + if int64(outgoingFee)+inboundFee < 0 { + return outgoingAmt + } + + return netAmount + lnwire.MilliSatoshi(inboundFee) +} + +// outgoingFromIncoming computes the outgoing amount based on the incoming +// amount by subtracting fees from the incoming amount. Note that this is not +// exactly the inverse of incomingFromOutgoing, because of some rounding. +func outgoingFromIncoming(incomingAmt lnwire.MilliSatoshi, + incoming, outgoing *unifiedEdge) lnwire.MilliSatoshi { + + // Convert all quantities to big.Int to be able to hande negative + // values. The formulas to compute the outgoing amount involve terms + // with PPM*PPM*A, which can easily overflow an int64. + A := big.NewInt(int64(incomingAmt)) + Ro := big.NewInt(int64(outgoing.policy.FeeProportionalMillionths)) + Bo := big.NewInt(int64(outgoing.policy.FeeBaseMSat)) + Ri := big.NewInt(int64(incoming.inboundFees.Rate)) + Bi := big.NewInt(int64(incoming.inboundFees.Base)) + PPM := big.NewInt(1_000_000) + + // The following discussion was contributed by user feelancer21, see + //nolint:lll + // https://github.com/feelancer21/lnd/commit/f6f05fa930985aac0d27c3f6681aada1b599162a. + + // The incoming amount Ai based on the outgoing amount Ao is computed by + // Ai = max(Ai(Ao), Ao), which caps the incoming amount such that the + // total node fee (Ai - Ao) is non-negative. This is commonly enforced + // by routing nodes. + + // The function Ai(Ao) is given by: + // Ai(Ao) = (Ao + Bo + Ro/PPM) + (Bi + (Ao + Ro/PPM + Bo)*Ri/PPM), where + // the first term is the net amount (the outgoing amount plus the + // outbound fee), and the second is the inbound fee computed based on + // the net amount. + + // Ai(Ao) can potentially become more negative in absolute value than + // Ao, which is why the above mentioned capping is needed. We can + // abbreviate Ai(Ao) with Ai(Ao) = m*Ao + n, where m and n are: + // m := (1 + Ro/PPM) * (1 + Ri/PPM) + // n := Bi + Bo*(1 + Ri/PPM) + + // If we know that m > 0, this is equivalent of Ri/PPM > -1, because Ri + // is the only factor that can become negative. A value or Ri/PPM = -1, + // means that the routing node is willing to give up on 100% of the + // net amount (based on the fee rate), which is likely to not happen in + // practice. This condition will be important for a later trick. + + // If we want to compute the incoming amount based on the outgoing + // amount, which is the reverse problem, we need to solve Ai = + // max(Ai(Ao), Ao) for Ao(Ai). Given an incoming amount A, + // we look for an Ao such that A = max(Ai(Ao), Ao). + + // The max function separates this into two cases. The case to take is + // not clear yet, because we don't know Ao, but later we see a trick + // how to determine which case is the one to take. + + // first case: Ai(Ao) <= Ao: + // Therefore, A = max(Ai(Ao), Ao) = Ao, we find Ao = A. + // This also leads to Ai(A) <= A by substitution into the condition. + + // second case: Ai(Ao) > Ao: + // Therefore, A = max(Ai(Ao), Ao) = Ai(Ao) = m*Ao + n. Solving for Ao + // gives Ao = (A - n)/m. + // + // We know + // Ai(Ao) > Ao <=> A = Ai(Ao) > Ao = (A - n)/m, + // so A > (A - n)/m. + // + // **Assuming m > 0**, by multiplying with m, we can transform this to + // A * m + n > A. + // + // We know Ai(A) = A*m + n, therefore Ai(A) > A. + // + // This means that if we apply the incoming amount calculation to the + // **incoming** amount, and this condition holds, then we know that we + // deal with the second case, being able to compute the outgoing amount + // based off the formula Ao = (A - n)/m, otherwise we will just return + // the incoming amount. + + // In case the inbound fee rate is less than -1 (-100%), we fail to + // compute the outbound amount and return the incoming amount. This also + // protects against zero division later. + + // We compute m in terms of big.Int to be safe from overflows and to be + // consistent with later calculations. + // m := (PPM*PPM + Ri*PPM + Ro*PPM + Ro*Ri)/(PPM*PPM) + + // Compute terms in (PPM*PPM + Ri*PPM + Ro*PPM + Ro*Ri). + m1 := new(big.Int).Mul(PPM, PPM) + m2 := new(big.Int).Mul(Ri, PPM) + m3 := new(big.Int).Mul(Ro, PPM) + m4 := new(big.Int).Mul(Ro, Ri) + + // Add up terms m1..m4. + m := big.NewInt(0) + m.Add(m, m1) + m.Add(m, m2) + m.Add(m, m3) + m.Add(m, m4) + + // Since we compare to 0, we can multiply by PPM*PPM to avoid the + // division. + if m.Int64() <= 0 { + return incomingAmt + } + + // In order to decide if the total fee is negative, we apply the fee + // to the *incoming* amount as mentioned before. + + // We compute the test amount in terms of big.Int to be safe from + // overflows and to be consistent later calculations. + // testAmtF := A*m + n = + // = A + Bo + Bi + (PPM*(A*Ri + A*Ro + Ro*Ri) + A*Ri*Ro)/(PPM*PPM) + + // Compute terms in (A*Ri + A*Ro + Ro*Ri). + t1 := new(big.Int).Mul(A, Ri) + t2 := new(big.Int).Mul(A, Ro) + t3 := new(big.Int).Mul(Ro, Ri) + + // Sum up terms t1-t3. + t4 := big.NewInt(0) + t4.Add(t4, t1) + t4.Add(t4, t2) + t4.Add(t4, t3) + + // Compute PPM*(A*Ri + A*Ro + Ro*Ri). + t6 := new(big.Int).Mul(PPM, t4) + + // Compute A*Ri*Ro. + t7 := new(big.Int).Mul(A, Ri) + t7.Mul(t7, Ro) + + // Compute (PPM*(A*Ri + A*Ro + Ro*Ri) + A*Ri*Ro)/(PPM*PPM). + num := new(big.Int).Add(t6, t7) + denom := new(big.Int).Mul(PPM, PPM) + fraction := new(big.Int).Div(num, denom) + + // Sum up all terms. + testAmt := big.NewInt(0) + testAmt.Add(testAmt, A) + testAmt.Add(testAmt, Bo) + testAmt.Add(testAmt, Bi) + testAmt.Add(testAmt, fraction) + + // Protect against negative values for the integer cast to Msat. + if testAmt.Int64() < 0 { + return incomingAmt + } + + // If the second case holds, we have to compute the outgoing amount. + if lnwire.MilliSatoshi(testAmt.Int64()) > incomingAmt { + // Compute the outgoing amount by integer ceiling division. This + // precision is needed because PPM*PPM*A and other terms can + // easily overflow with int64, which happens with about + // A = 10_000 sat. + + // out := (A - n) / m = numerator / denominator + // numerator := PPM*(PPM*(A - Bo - Bi) - Bo*Ri) + // denominator := PPM*(PPM + Ri + Ro) + Ri*Ro + + var numerator big.Int + + // Compute (A - Bo - Bi). + temp1 := new(big.Int).Sub(A, Bo) + temp2 := new(big.Int).Sub(temp1, Bi) + + // Compute terms in (PPM*(A - Bo - Bi) - Bo*Ri). + temp3 := new(big.Int).Mul(PPM, temp2) + temp4 := new(big.Int).Mul(Bo, Ri) + + // Compute PPM*(PPM*(A - Bo - Bi) - Bo*Ri) + temp5 := new(big.Int).Sub(temp3, temp4) + numerator.Mul(PPM, temp5) + + var denominator big.Int + + // Compute (PPM + Ri + Ro). + temp1 = new(big.Int).Add(PPM, Ri) + temp2 = new(big.Int).Add(temp1, Ro) + + // Compute PPM*(PPM + Ri + Ro) + Ri*Ro. + temp3 = new(big.Int).Mul(PPM, temp2) + temp4 = new(big.Int).Mul(Ri, Ro) + denominator.Add(temp3, temp4) + + // We overestimate the outgoing amount by taking the ceiling of + // the division. This means that we may round slightly up by a + // MilliSatoshi, but this helps to ensure that we don't hit min + // HTLC constrains in the context of finding the minimum amount + // of a route. + // ceil = floor((numerator + denominator - 1) / denominator) + ceil := new(big.Int).Add(&numerator, &denominator) + ceil.Sub(ceil, big.NewInt(1)) + ceil.Div(ceil, &denominator) + + return lnwire.MilliSatoshi(ceil.Int64()) + } + + // Otherwise the inbound fee made up for the outbound fee, which is why + // we just return the incoming amount. + return incomingAmt +} diff --git a/routing/router_test.go b/routing/router_test.go index 7ee5817b4..ec095d787 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1886,6 +1886,156 @@ func TestSenderAmtBackwardPass(t *testing.T) { require.Equal(t, testReceiverAmt, receiverAmt) } +// TestInboundOutbound tests the functions that computes the incoming and +// outgoing amounts based on the fees of the incoming and outgoing channels. +func TestInboundOutbound(t *testing.T) { + var outgoingAmt uint64 = 10_000_000 + + tests := []struct { + name string + incomingBase int32 + incomingRate int32 + outgoingBase uint64 + outgoingRate uint64 + }{ + { + name: "only outbound fee", + incomingBase: 0, + incomingRate: 0, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "positive inbound and outbound fee", + incomingBase: 20, + incomingRate: 100, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "small negative inbound and outbound fee", + incomingBase: -10, + incomingRate: -50, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "equal negative inbound and outbound fee", + incomingBase: -20, + incomingRate: -100, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "large negative inbound and outbound fee", + incomingBase: -30, + incomingRate: -200, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "order of PPM negative inbound and " + + "outbound fee (m=0)", + incomingBase: -30, + incomingRate: -1_000_000, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "huge negative inbound and " + + "outbound fee (m<0)", + incomingBase: -30, + incomingRate: -2_000_000, + outgoingBase: 20, + outgoingRate: 100, + }, + } + + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(tt *testing.T) { + testInboundOutboundFee( + tt, outgoingAmt, tc.incomingBase, + tc.incomingRate, tc.outgoingBase, + tc.outgoingRate, + ) + }) + } +} + +// testInboundOutboundFee is a helper function that tests the outgoing and +// incoming amount relationship. +func testInboundOutboundFee(t *testing.T, outgoingAmt uint64, inBase, + inRate int32, outBase, outRate uint64) { + + debugStr := fmt.Sprintf( + "outAmt=%d, inBase=%d, inRate=%d, outBase=%d, outRate=%d", + outgoingAmt, inBase, inRate, outBase, outRate, + ) + + incomingEdge := &unifiedEdge{ + policy: &models.CachedEdgePolicy{}, + inboundFees: models.InboundFee{ + Base: inBase, + Rate: inRate, + }, + } + + outgoingEdge := &unifiedEdge{ + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: lnwire.MilliSatoshi( + outBase, + ), + FeeProportionalMillionths: lnwire.MilliSatoshi( + outRate, + ), + }, + } + + // We compute the incoming amount based on the outgoing amount, which + // mimicks the path finding process. + incomingAmt := incomingFromOutgoing( + lnwire.MilliSatoshi(outgoingAmt), incomingEdge, + outgoingEdge, + ) + + // We do the reverse and compute the outgoing amount based on the + // incoming amount. + outgoingAmtNew := outgoingFromIncoming( + incomingAmt, incomingEdge, outgoingEdge, + ) + + // We require that the incoming amount is always larger than or equal to + // the outgoing amount, because total fees (=incoming-outgoing) should + // not become negative. + require.GreaterOrEqual( + t, int64(incomingAmt), int64(outgoingAmtNew), debugStr, + "expected incomingAmt >= outgoingAmtNew", + ) + + // We check that up to rounding the amounts are equal. + require.InDelta( + t, int64(outgoingAmt), int64(outgoingAmtNew), 1.0, debugStr, + "expected |outgoingAmt - outgoingAmtNew | <= 1", + ) + + // If we round, the computed outgoing amount should be larger than the + // exact outgoing amount, to not hit any min HTLC limits. + require.GreaterOrEqual( + t, int64(outgoingAmtNew), int64(outgoingAmt), debugStr, + "expected outgoingAmtNew >= outgoingAmt", + ) +} + +// FuzzInboundOutbound tests the incoming and outgoing amount calculation +// functions with fuzzing. +func FuzzInboundOutboundFee(f *testing.F) { + f.Add(uint64(0), int32(0), int32(0), uint64(0), uint64(0)) + + f.Fuzz(testInboundOutboundFee) +} + // TestSendToRouteSkipTempErrSuccess validates a successful payment send. func TestSendToRouteSkipTempErrSuccess(t *testing.T) { t.Parallel() From 8b32e3e78570d6c533a4bae6907465177486a2f2 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 30 Jul 2024 14:17:47 +0200 Subject: [PATCH 07/10] routing: add inbound fee support for BuildRoute --- channeldb/graph_test.go | 7 +- channeldb/models/cached_edge_policy.go | 11 --- channeldb/models/channel_edge_policy.go | 16 ----- routing/router.go | 21 +++--- routing/router_test.go | 93 ++++++++++++++++++++++++- 5 files changed, 105 insertions(+), 43 deletions(-) diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 256851238..00e30d5b2 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -3688,7 +3688,7 @@ func TestLightningNodeSigVerification(t *testing.T) { } } -// TestComputeFee tests fee calculation based on both in- and outgoing amt. +// TestComputeFee tests fee calculation based on the outgoing amt. func TestComputeFee(t *testing.T) { var ( policy = models.ChannelEdgePolicy{ @@ -3703,11 +3703,6 @@ func TestComputeFee(t *testing.T) { if fee != expectedFee { t.Fatalf("expected fee %v, got %v", expectedFee, fee) } - - fwdFee := policy.ComputeFeeFromIncoming(outgoingAmt + fee) - if fwdFee != expectedFee { - t.Fatalf("expected fee %v, but got %v", fee, fwdFee) - } } // TestBatchedAddChannelEdge asserts that BatchedAddChannelEdge properly diff --git a/channeldb/models/cached_edge_policy.go b/channeldb/models/cached_edge_policy.go index 25bef567e..b770ec1fb 100644 --- a/channeldb/models/cached_edge_policy.go +++ b/channeldb/models/cached_edge_policy.go @@ -71,17 +71,6 @@ func (c *CachedEdgePolicy) ComputeFee( return c.FeeBaseMSat + (amt*c.FeeProportionalMillionths)/feeRateParts } -// ComputeFeeFromIncoming computes the fee to forward an HTLC given the incoming -// amount. -func (c *CachedEdgePolicy) ComputeFeeFromIncoming( - incomingAmt lnwire.MilliSatoshi) lnwire.MilliSatoshi { - - return incomingAmt - divideCeil( - feeRateParts*(incomingAmt-c.FeeBaseMSat), - feeRateParts+c.FeeProportionalMillionths, - ) -} - // NewCachedPolicy turns a full policy into a minimal one that can be cached. func NewCachedPolicy(policy *ChannelEdgePolicy) *CachedEdgePolicy { return &CachedEdgePolicy{ diff --git a/channeldb/models/channel_edge_policy.go b/channeldb/models/channel_edge_policy.go index 93902d31f..322ce3cd0 100644 --- a/channeldb/models/channel_edge_policy.go +++ b/channeldb/models/channel_edge_policy.go @@ -113,19 +113,3 @@ func (c *ChannelEdgePolicy) ComputeFee( return c.FeeBaseMSat + (amt*c.FeeProportionalMillionths)/feeRateParts } - -// divideCeil divides dividend by factor and rounds the result up. -func divideCeil(dividend, factor lnwire.MilliSatoshi) lnwire.MilliSatoshi { - return (dividend + factor - 1) / factor -} - -// ComputeFeeFromIncoming computes the fee to forward an HTLC given the incoming -// amount. -func (c *ChannelEdgePolicy) ComputeFeeFromIncoming( - incomingAmt lnwire.MilliSatoshi) lnwire.MilliSatoshi { - - return incomingAmt - divideCeil( - feeRateParts*(incomingAmt-c.FeeBaseMSat), - feeRateParts+c.FeeProportionalMillionths, - ) -} diff --git a/routing/router.go b/routing/router.go index d67fc2ebd..83cd3c734 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1518,11 +1518,11 @@ func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, } // Build unified policies for this hop based on the channels - // known in the graph. Don't use inbound fees. - // - // TODO: Add inbound fees support for BuildRoute. + // known in the graph. Inbound fees are only active if the edge + // is not the last hop. + isExitHop := i == len(hops)-1 u := newNodeEdgeUnifier( - source, toNode, false, outgoingChans, + source, toNode, !isExitHop, outgoingChans, ) err := u.addGraphPolicies(graph) @@ -1617,7 +1617,13 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, return nil, 0, ErrNoChannel{position: i} } - fee := outboundFee + // The fee paid to B depends on the current hop's inbound fee + // policy and on the outbound fee for the next hop as any + // inbound fee discount is capped by the outbound fee such that + // the total fee for B can't become negative. + inboundFee := calcCappedInboundFee(edge, netAmount, outboundFee) + + fee := lnwire.MilliSatoshi(int64(outboundFee) + inboundFee) log.Tracef("Select channel %v at position %v", edge.policy.ChannelID, i) @@ -1655,12 +1661,11 @@ func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, // been increased in the backward pass, fees need to be recalculated and // amount ranges re-checked. for i := 1; i < len(unifiedEdges); i++ { + inEdge := unifiedEdges[i-1] outEdge := unifiedEdges[i] // Decrease the amount to send while going forward. - runningAmt -= outEdge.policy.ComputeFeeFromIncoming( - runningAmt, - ) + runningAmt = outgoingFromIncoming(runningAmt, inEdge, outEdge) if !outEdge.amtInRange(runningAmt) { log.Errorf("Amount %v not in range for hop index %v", diff --git a/routing/router_test.go b/routing/router_test.go index ec095d787..a766325a5 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1588,6 +1588,28 @@ func TestBuildRoute(t *testing.T) { MaxHTLC: lnwire.MilliSatoshi(20100), Features: paymentAddrFeatures, }, 8), + + // Create a route with inbound fees. + symmetricTestChannel("a", "d", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 20000, + MinHTLC: lnwire.NewMSatFromSatoshis(5), + MaxHTLC: lnwire.NewMSatFromSatoshis( + chanCapSat, + ), + InboundFeeBaseMsat: -1000, + InboundFeeRate: -1000, + }, 9), + symmetricTestChannel("d", "f", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 60000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + MaxHTLC: lnwire.NewMSatFromSatoshis(120), + Features: paymentAddrFeatures, + // The inbound fee will not be active for the last hop. + InboundFeeBaseMsat: 2000, + InboundFeeRate: 2000, + }, 10), } testGraph, err := createTestGraphFromChannels( @@ -1686,6 +1708,30 @@ func TestBuildRoute(t *testing.T) { require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) require.Equal(t, lnwire.MilliSatoshi(20000), rt.Hops[1].AmtToForward) + // Check that we compute a correct forwarding amount that involves + // inbound fees. We expect a similar amount as for the above case of + // b->c, but reduced by the inbound discount on the channel a->d. + // We get 106000 - 1000 (base in) - 0.001 * 106000 (rate in) = 104894. + hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} + amt = lnwire.NewMSatFromSatoshis(100) + rt, err = ctx.router.BuildRoute(&amt, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{9, 10}, payAddr) + require.EqualValues(t, 104894, rt.TotalAmount) + + // Also check the min amount with inbound fees. The min amount bumps + // this to 20000 msat for the last hop. The outbound fee is 1200 msat, + // the inbound fee is -1021.2 msat (rounded down). This results in a + // total fee of 179 msat, giving a sender amount of 20179 msat. The + // determined receiver amount however reduces this to 20001 msat again + // due to rounding. This would not be compatible with the sender amount + // of 20179 msat, which results in underpayment of 1 msat in fee. There + // is a third pass through newRoute in which this gets corrected to end + hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} + rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{9, 10}, payAddr) + require.EqualValues(t, 20180, rt.TotalAmount, "%v", rt.TotalAmount) } // TestReceiverAmtForwardPass tests that the forward pass returns the expected @@ -1831,6 +1877,9 @@ func TestSenderAmtBackwardPass(t *testing.T) { policy: &models.CachedEdgePolicy{ FeeBaseMSat: 112, }, + inboundFees: models.InboundFee{ + Base: 111, + }, capacity: capacity, }, }, @@ -1841,6 +1890,9 @@ func TestSenderAmtBackwardPass(t *testing.T) { policy: &models.CachedEdgePolicy{ FeeBaseMSat: 222, }, + inboundFees: models.InboundFee{ + Base: 222, + }, capacity: capacity, }, }, @@ -1852,6 +1904,12 @@ func TestSenderAmtBackwardPass(t *testing.T) { FeeBaseMSat: 333, MinHTLC: minHTLC, }, + // In pathfinding, inbound fees are not + // populated for exit hops because the + // newNodeEdgeUnifier enforces this. + // This is important as otherwise we + // would not fail the min HTLC check in + // getEdge. capacity: capacity, }, }, @@ -1870,20 +1928,51 @@ func TestSenderAmtBackwardPass(t *testing.T) { edgeUnifiers, true, 1, &bandwidthHints, ) require.NoError(t, err) - require.Equal(t, minHTLC+333+222, senderAmount) + require.Equal(t, minHTLC+333+222+222+111, senderAmount) // Do a search for a specific amount. unifiedEdges, senderAmount, err = senderAmtBackwardPass( edgeUnifiers, false, testReceiverAmt, &bandwidthHints, ) require.NoError(t, err) - require.Equal(t, testReceiverAmt+333+222, senderAmount) + require.Equal(t, testReceiverAmt+333+222+222+111, senderAmount) // Check that we arrive at the same receiver amount by doing a forward // pass. receiverAmt, err := receiverAmtForwardPass(senderAmount, unifiedEdges) require.NoError(t, err) require.Equal(t, testReceiverAmt, receiverAmt) + + // Insert a policy that leads to rounding. + edgeUnifiers[1] = &edgeUnifier{ + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 20, + FeeProportionalMillionths: 100, + }, + inboundFees: models.InboundFee{ + Base: -10, + Rate: -50, + }, + capacity: capacity, + }, + }, + } + + unifiedEdges, senderAmount, err = senderAmtBackwardPass( + edgeUnifiers, false, testReceiverAmt, &bandwidthHints, + ) + require.NoError(t, err) + + // For this route, we have some rounding errors, so we can't expect the + // exact amount, but it should be higher than the exact amount, to not + // end up below a min HTLC constraint. + receiverAmt, err = receiverAmtForwardPass(senderAmount, unifiedEdges) + require.NoError(t, err) + require.NotEqual(t, testReceiverAmt, receiverAmt) + require.InDelta(t, int64(testReceiverAmt), int64(receiverAmt), 1) + require.GreaterOrEqual(t, int64(receiverAmt), int64(testReceiverAmt)) } // TestInboundOutbound tests the functions that computes the incoming and From 452db01ad7716b2a97ec8bd5e5496b00c47d3dd7 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Wed, 31 Jul 2024 14:32:48 +0200 Subject: [PATCH 08/10] lnrpc+routing: convert amt pointer to fn.Option --- lnrpc/routerrpc/router_server.go | 5 ++- routing/router.go | 73 +++++++++++++++----------------- routing/router_test.go | 31 ++++++++------ 3 files changed, 55 insertions(+), 54 deletions(-) diff --git a/lnrpc/routerrpc/router_server.go b/lnrpc/routerrpc/router_server.go index 85b6b2b2c..0391fe825 100644 --- a/lnrpc/routerrpc/router_server.go +++ b/lnrpc/routerrpc/router_server.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" "github.com/lightningnetwork/lnd/lntypes" @@ -1411,10 +1412,10 @@ func (s *Server) BuildRoute(_ context.Context, } // Prepare BuildRoute call parameters from rpc request. - var amt *lnwire.MilliSatoshi + var amt fn.Option[lnwire.MilliSatoshi] if req.AmtMsat != 0 { rpcAmt := lnwire.MilliSatoshi(req.AmtMsat) - amt = &rpcAmt + amt = fn.Some(rpcAmt) } var outgoingChan *uint64 diff --git a/routing/router.go b/routing/router.go index 83cd3c734..dbbec74c4 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1406,7 +1406,7 @@ func (e ErrNoChannel) Error() string { // BuildRoute returns a fully specified route based on a list of pubkeys. If // amount is nil, the minimum routable amount is used. To force a specific // outgoing channel, use the outgoingChan parameter. -func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, +func (r *ChannelRouter) BuildRoute(amt fn.Option[lnwire.MilliSatoshi], hops []route.Vertex, outgoingChan *uint64, finalCltvDelta int32, payAddr *[32]byte) (*route.Route, error) { @@ -1439,43 +1439,36 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, return nil, err } - // If no amount is specified, we need to build a route for the minimum - // amount that this route can carry. - useMinAmt := amt == nil - var ( receiverAmt lnwire.MilliSatoshi + senderAmt lnwire.MilliSatoshi pathEdges []*unifiedEdge ) - if useMinAmt { - // For minimum amount routes, aim to deliver at least 1 msat to - // the destination. There are nodes in the wild that have a - // min_htlc channel policy of zero, which could lead to a zero - // amount payment being made. - var senderAmt lnwire.MilliSatoshi - pathEdges, senderAmt, err = senderAmtBackwardPass( - unifiers, useMinAmt, 1, bandwidthHints, - ) - if err != nil { - return nil, err - } + // We determine the edges compatible with the requested amount, as well + // as the amount to send, which can be used to determine the final + // receiver amount, if a minimal amount was requested. + pathEdges, senderAmt, err = senderAmtBackwardPass( + unifiers, amt, bandwidthHints, + ) + if err != nil { + return nil, err + } - receiverAmt, err = receiverAmtForwardPass(senderAmt, pathEdges) - if err != nil { - return nil, err - } - } else { - // If an amount is specified, we need to build a route that - // delivers exactly this amount to the final destination. - pathEdges, _, err = senderAmtBackwardPass( - unifiers, useMinAmt, *amt, bandwidthHints, - ) - if err != nil { - return nil, err - } - - receiverAmt = *amt + // For the minimal amount search, we need to do a forward pass to find a + // larger receiver amount due to possible min HTLC bumps, otherwise we + // just use the requested amount. + receiverAmt, err = fn.ElimOption( + amt, + func() fn.Result[lnwire.MilliSatoshi] { + return fn.NewResult( + receiverAmtForwardPass(senderAmt, pathEdges), + ) + }, + fn.Ok[lnwire.MilliSatoshi], + ).Unpack() + if err != nil { + return nil, err } // Fetch the current block height outside the routing transaction, to @@ -1545,9 +1538,9 @@ func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, // senderAmtBackwardPass returns a list of unified edges for the given route and // determines the amount that should be sent to fulfill min HTLC requirements. -// The minimal sender amount can be searched for by activating useMinAmt. -func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, - receiverAmt lnwire.MilliSatoshi, +// The minimal sender amount can be searched for by using amt=None. +func senderAmtBackwardPass(unifiers []*edgeUnifier, + amt fn.Option[lnwire.MilliSatoshi], bandwidthHints bandwidthHints) ([]*unifiedEdge, lnwire.MilliSatoshi, error) { @@ -1563,11 +1556,15 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, // incomingAmt tracks the amount that is forwarded on the edges of a // route. The last hop only forwards the amount that the receiver should // receive, as there are no fees paid to the last node. - incomingAmt := receiverAmt + // For minimum amount routes, aim to deliver at least 1 msat to + // the destination. There are nodes in the wild that have a + // min_htlc channel policy of zero, which could lead to a zero + // amount payment being made. + incomingAmt := amt.UnwrapOr(1) // If using min amt, increase the amount if needed to fulfill min HTLC // requirements. - if useMinAmt { + if amt.IsNone() { min := edgeUnifier.minAmt() if min > incomingAmt { incomingAmt = min @@ -1591,7 +1588,7 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, // If using min amt, increase the amount if needed to fulfill // min HTLC requirements. - if useMinAmt { + if amt.IsNone() { min := edgeUnifier.minAmt() if min > incomingAmt { incomingAmt = min diff --git a/routing/router_test.go b/routing/router_test.go index a766325a5..9a394a9fd 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -24,6 +24,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/graph" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" @@ -1641,14 +1642,16 @@ func TestBuildRoute(t *testing.T) { _, err = rand.Read(payAddr[:]) require.NoError(t, err) + noAmt := fn.None[lnwire.MilliSatoshi]() + // Test that we can't build a route when no hops are given. hops = []route.Vertex{} - _, err = ctx.router.BuildRoute(nil, hops, nil, 40, nil) + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, nil) require.Error(t, err) // Create hop list for an unknown destination. hops := []route.Vertex{ctx.aliases["b"], ctx.aliases["y"]} - _, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) noChanErr := ErrNoChannel{} require.ErrorAs(t, err, &noChanErr) require.Equal(t, 1, noChanErr.position) @@ -1658,7 +1661,7 @@ func TestBuildRoute(t *testing.T) { amt := lnwire.NewMSatFromSatoshis(100) // Build the route for the given amount. - rt, err := ctx.router.BuildRoute(&amt, hops, nil, 40, &payAddr) + rt, err := ctx.router.BuildRoute(fn.Some(amt), hops, nil, 40, &payAddr) require.NoError(t, err) // Check that we get the expected route back. The total amount should be @@ -1668,7 +1671,7 @@ func TestBuildRoute(t *testing.T) { require.Equal(t, lnwire.MilliSatoshi(106000), rt.TotalAmount) // Build the route for the minimum amount. - rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + rt, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) require.NoError(t, err) // Check that we get the expected route back. The minimum that we can @@ -1684,7 +1687,7 @@ func TestBuildRoute(t *testing.T) { // Test a route that contains incompatible channel htlc constraints. // There is no amount that can pass through both channel 5 and 4. hops = []route.Vertex{ctx.aliases["e"], ctx.aliases["c"]} - _, err = ctx.router.BuildRoute(nil, hops, nil, 40, nil) + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, nil) require.Error(t, err) noChanErr = ErrNoChannel{} require.ErrorAs(t, err, &noChanErr) @@ -1702,7 +1705,7 @@ func TestBuildRoute(t *testing.T) { // amount that could be delivered to the receiver of 21819 msat, using // policy of channel 3. hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["z"]} - rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + rt, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) require.NoError(t, err) checkHops(rt, []uint64{1, 8}, payAddr) require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) @@ -1714,7 +1717,7 @@ func TestBuildRoute(t *testing.T) { // We get 106000 - 1000 (base in) - 0.001 * 106000 (rate in) = 104894. hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} amt = lnwire.NewMSatFromSatoshis(100) - rt, err = ctx.router.BuildRoute(&amt, hops, nil, 40, &payAddr) + rt, err = ctx.router.BuildRoute(fn.Some(amt), hops, nil, 40, &payAddr) require.NoError(t, err) checkHops(rt, []uint64{9, 10}, payAddr) require.EqualValues(t, 104894, rt.TotalAmount) @@ -1728,7 +1731,7 @@ func TestBuildRoute(t *testing.T) { // of 20179 msat, which results in underpayment of 1 msat in fee. There // is a third pass through newRoute in which this gets corrected to end hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} - rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + rt, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) require.NoError(t, err) checkHops(rt, []uint64{9, 10}, payAddr) require.EqualValues(t, 20180, rt.TotalAmount, "%v", rt.TotalAmount) @@ -1919,20 +1922,20 @@ func TestSenderAmtBackwardPass(t *testing.T) { // A search for an amount that is below the minimum HTLC amount should // fail. _, _, err := senderAmtBackwardPass( - edgeUnifiers, false, minHTLC-1, &bandwidthHints, + edgeUnifiers, fn.Some(minHTLC-1), &bandwidthHints, ) require.Error(t, err) // Do a min amount search. - unifiedEdges, senderAmount, err := senderAmtBackwardPass( - edgeUnifiers, true, 1, &bandwidthHints, + _, senderAmount, err := senderAmtBackwardPass( + edgeUnifiers, fn.None[lnwire.MilliSatoshi](), &bandwidthHints, ) require.NoError(t, err) require.Equal(t, minHTLC+333+222+222+111, senderAmount) // Do a search for a specific amount. - unifiedEdges, senderAmount, err = senderAmtBackwardPass( - edgeUnifiers, false, testReceiverAmt, &bandwidthHints, + unifiedEdges, senderAmount, err := senderAmtBackwardPass( + edgeUnifiers, fn.Some(testReceiverAmt), &bandwidthHints, ) require.NoError(t, err) require.Equal(t, testReceiverAmt+333+222+222+111, senderAmount) @@ -1961,7 +1964,7 @@ func TestSenderAmtBackwardPass(t *testing.T) { } unifiedEdges, senderAmount, err = senderAmtBackwardPass( - edgeUnifiers, false, testReceiverAmt, &bandwidthHints, + edgeUnifiers, fn.Some(testReceiverAmt), &bandwidthHints, ) require.NoError(t, err) From b0e1a722c6aa6a9b0bf17a91bf4505b4d9a4aff5 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Wed, 7 Aug 2024 11:34:30 +0200 Subject: [PATCH 09/10] routerrpc: add check for empty hops --- lnrpc/routerrpc/router_server.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lnrpc/routerrpc/router_server.go b/lnrpc/routerrpc/router_server.go index 0391fe825..a88bb4d0c 100644 --- a/lnrpc/routerrpc/router_server.go +++ b/lnrpc/routerrpc/router_server.go @@ -1401,6 +1401,10 @@ func (s *Server) trackPaymentStream(context context.Context, func (s *Server) BuildRoute(_ context.Context, req *BuildRouteRequest) (*BuildRouteResponse, error) { + if len(req.HopPubkeys) == 0 { + return nil, errors.New("no hops specified") + } + // Unmarshall hop list. hops := make([]route.Vertex, len(req.HopPubkeys)) for i, pubkeyBytes := range req.HopPubkeys { From f622b43b5dd21dedff7084d7b4caefb09664467e Mon Sep 17 00:00:00 2001 From: bitromortac Date: Tue, 2 Jul 2024 12:25:18 +0200 Subject: [PATCH 10/10] docs: add to release notes --- docs/release-notes/release-notes-0.18.3.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.3.md b/docs/release-notes/release-notes-0.18.3.md index 4cb59b76d..de34239b6 100644 --- a/docs/release-notes/release-notes-0.18.3.md +++ b/docs/release-notes/release-notes-0.18.3.md @@ -84,6 +84,9 @@ * [`ChanInfoRequest`](https://github.com/lightningnetwork/lnd/pull/8813) adds support for channel points. +* [BuildRoute](https://github.com/lightningnetwork/lnd/pull/8886) now supports + inbound fees. + ## lncli Updates * [`importmc`](https://github.com/lightningnetwork/lnd/pull/8779) now accepts