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 != "" {