diff --git a/routing/errors.go b/routing/errors.go index a879cfe7d..58a92b904 100644 --- a/routing/errors.go +++ b/routing/errors.go @@ -43,9 +43,9 @@ const ( // attempt timed out before we were able to successfully route an HTLC. ErrPaymentAttemptTimeout - // ErrFeeCutoffExceeded is returned when the total fees of a route - // exceed the user-specified maximum payment for the fee. - ErrFeeCutoffExceeded + // ErrFeeLimitExceeded is returned when the total fees of a route exceed + // the user-specified fee limit. + ErrFeeLimitExceeded ) // routerError is a structure that represent the error inside the routing package, diff --git a/routing/missioncontrol.go b/routing/missioncontrol.go index c6eaad9a7..9e0fa9086 100644 --- a/routing/missioncontrol.go +++ b/routing/missioncontrol.go @@ -384,7 +384,8 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, // a route by applying the time-lock and fee requirements. sourceVertex := Vertex(p.mc.selfNode.PubKeyBytes) route, err := newRoute( - payment.Amount, sourceVertex, path, height, finalCltvDelta, + payment.Amount, payment.FeeLimit, sourceVertex, path, height, + finalCltvDelta, ) if err != nil { // TODO(roasbeef): return which edge/vertex didn't work diff --git a/routing/pathfind.go b/routing/pathfind.go index 18a6cce8e..b85b149f0 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -248,9 +248,9 @@ func (r *Route) ToHopPayloads() []sphinx.HopData { // // NOTE: The passed slice of ChannelHops MUST be sorted in forward order: from // the source to the target node of the path finding attempt. -func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex Vertex, +func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex, pathEdges []*ChannelHop, currentHeight uint32, - finalCLTVDelta uint16, feeLimit btcutil.Amount) (*Route, error) { + finalCLTVDelta uint16) (*Route, error) { // First, we'll create a new empty route with enough hops to match the // amount of path edges. We set the TotalTimeLock to the current block @@ -339,12 +339,11 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex Vertex, route.TotalFees += nextHop.Fee - // If the total fees exceed the fee limit established for this - // payment, stop hopping the route and return - if route.TotalFees.ToSatoshis() > feeLimit { - err := fmt.Sprintf("Route fee exceeded fee limit of %v", - feeLimit) - return nil, newErrf(ErrFeeCutoffExceeded, err) + // Invalidate this route if its total fees exceed our fee limit. + if route.TotalFees > feeLimit { + err := fmt.Sprintf("total route fees exceeded fee "+ + "limit of %v", feeLimit) + return nil, newErrf(ErrFeeLimitExceeded, err) } // As a sanity check, we ensure that the selected channel has diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index f3a4d0060..7a7be0da2 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "io/ioutil" + "math" "math/big" "net" "os" @@ -40,6 +41,11 @@ const ( // implementations will use in order to ensure that they're calculating // the payload for each hop in path properly. specExampleFilePath = "testdata/spec_example.json" + + // noFeeLimit is the maximum value of a payment through Lightning. We + // can use this value to signal there is no fee limit since payments + // should never be larger than this. + noFeeLimit = lnwire.MilliSatoshi(math.MaxUint32) ) var ( @@ -310,7 +316,6 @@ func TestBasicGraphPathFinding(t *testing.T) { ) paymentAmt := lnwire.NewMSatFromSatoshis(100) - feeLimit := paymentAmt.ToSatoshis() target := aliases["sophon"] path, err := findPath( nil, graph, nil, sourceNode, target, ignoredVertexes, @@ -320,8 +325,10 @@ func TestBasicGraphPathFinding(t *testing.T) { t.Fatalf("unable to find path: %v", err) } - route, err := newRoute(paymentAmt, sourceVertex, path, startingHeight, - finalHopCLTV, feeLimit) + route, err := newRoute( + paymentAmt, noFeeLimit, sourceVertex, path, startingHeight, + finalHopCLTV, + ) if err != nil { t.Fatalf("unable to create path: %v", err) } @@ -463,8 +470,10 @@ func TestBasicGraphPathFinding(t *testing.T) { t.Fatalf("unable to find route: %v", err) } - route, err = newRoute(paymentAmt, sourceVertex, path, startingHeight, - finalHopCLTV, feeLimit) + route, err = newRoute( + paymentAmt, noFeeLimit, sourceVertex, path, startingHeight, + finalHopCLTV, + ) if err != nil { t.Fatalf("unable to create path: %v", err) } @@ -732,6 +741,8 @@ func TestPathInsufficientCapacity(t *testing.T) { // TestRouteFailMinHTLC tests that if we attempt to route an HTLC which is // smaller than the advertised minHTLC of an edge, then path finding fails. func TestRouteFailMinHTLC(t *testing.T) { + t.Parallel() + graph, cleanUp, aliases, err := parseTestGraph(basicGraphFilePath) defer cleanUp() if err != nil { @@ -763,6 +774,8 @@ func TestRouteFailMinHTLC(t *testing.T) { // that's disabled, then that edge is disqualified, and the routing attempt // will fail. func TestRouteFailDisabledEdge(t *testing.T) { + t.Parallel() + graph, cleanUp, aliases, err := parseTestGraph(basicGraphFilePath) defer cleanUp() if err != nil { @@ -810,6 +823,48 @@ func TestRouteFailDisabledEdge(t *testing.T) { } } +// TestRouteExceededFeeLimit tests that routes respect the fee limit imposed. +func TestRouteExceededFeeLimit(t *testing.T) { + t.Parallel() + + graph, cleanUp, aliases, err := parseTestGraph(basicGraphFilePath) + defer cleanUp() + if err != nil { + t.Fatalf("unable to create graph: %v", err) + } + + sourceNode, err := graph.SourceNode() + if err != nil { + t.Fatalf("unable to fetch source node: %v", err) + } + sourceVertex := Vertex(sourceNode.PubKeyBytes) + + ignoredVertices := make(map[Vertex]struct{}) + ignoredEdges := make(map[uint64]struct{}) + + // Find a path to send 100 satoshis from roasbeef to sophon. + target := aliases["sophon"] + amt := lnwire.NewMSatFromSatoshis(100) + path, err := findPath( + nil, graph, nil, sourceNode, target, ignoredVertices, + ignoredEdges, amt, nil, + ) + if err != nil { + t.Fatalf("unable to find path from roasbeef to phamnuwen for "+ + "100 satoshis: %v", err) + } + + // We'll now purposefully set a fee limit of 0 to trigger the exceeded + // fee limit error. This should work since the path retrieved spans + // multiple hops incurring a fee. + feeLimit := lnwire.NewMSatFromSatoshis(0) + + _, err = newRoute(amt, feeLimit, sourceVertex, path, 100, 1) + if !IsError(err, ErrFeeLimitExceeded) { + t.Fatalf("route should've exceeded fee limit: %v", err) + } +} + func TestPathInsufficientCapacityWithFee(t *testing.T) { t.Parallel() @@ -855,8 +910,7 @@ func TestPathFindSpecExample(t *testing.T) { // Query for a route of 4,999,999 mSAT to carol. carol := ctx.aliases["C"] const amt lnwire.MilliSatoshi = 4999999 - feeLimit := amt.ToSatoshis() - routes, err := ctx.router.FindRoutes(carol, amt, feeLimit, 100) + routes, err := ctx.router.FindRoutes(carol, amt, noFeeLimit, 100) if err != nil { t.Fatalf("unable to find route: %v", err) } @@ -916,7 +970,7 @@ func TestPathFindSpecExample(t *testing.T) { // We'll now request a route from A -> B -> C. ctx.router.routeCache = make(map[routeTuple][]*Route) - routes, err = ctx.router.FindRoutes(carol, amt, feeLimit, 100) + routes, err = ctx.router.FindRoutes(carol, amt, noFeeLimit, 100) if err != nil { t.Fatalf("unable to find routes: %v", err) } diff --git a/routing/router.go b/routing/router.go index eef768b60..4f3a5bfb6 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1258,8 +1258,8 @@ func pruneChannelFromRoutes(routes []*Route, skipChan uint64) []*Route { // fee information attached. The set of routes returned may be less than the // initial set of paths as it's possible we drop a route if it can't handle the // total payment flow after fees are calculated. -func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop, finalCLTVDelta uint16, - amt lnwire.MilliSatoshi, feeLimit btcutil.Amount, +func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop, + finalCLTVDelta uint16, amt, feeLimit lnwire.MilliSatoshi, currentHeight uint32) ([]*Route, error) { validRoutes := make([]*Route, 0, len(paths)) @@ -1268,8 +1268,8 @@ func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop, finalCLTVDelta // hop in the path as it contains a "self-hop" that is inserted // by our KSP algorithm. route, err := newRoute( - amt, source, path[1:], currentHeight, finalCLTVDelta, - feeLimit, + amt, feeLimit, source, path[1:], currentHeight, + finalCLTVDelta, ) if err != nil { // TODO(roasbeef): report straw breaking edge? @@ -1318,7 +1318,7 @@ func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop, finalCLTVDelta // route that will be ranked the highest is the one with the lowest cumulative // fee along the route. func (r *ChannelRouter) FindRoutes(target *btcec.PublicKey, - amt lnwire.MilliSatoshi, feeLimit btcutil.Amount, numPaths uint32, + amt, feeLimit lnwire.MilliSatoshi, numPaths uint32, finalExpiry ...uint16) ([]*Route, error) { var finalCLTVDelta uint16 diff --git a/routing/router_test.go b/routing/router_test.go index 511842eb9..97944064c 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -174,10 +174,9 @@ func TestFindRoutesFeeSorting(t *testing.T) { // Execute a query for all possible routes between roasbeef and luo ji. paymentAmt := lnwire.NewMSatFromSatoshis(100) - feeLimit := paymentAmt.ToSatoshis() target := ctx.aliases["luoji"] routes, err := ctx.router.FindRoutes( - target, paymentAmt, feeLimit, defaultNumRoutes, + target, paymentAmt, noFeeLimit, defaultNumRoutes, DefaultFinalCLTVDelta, ) if err != nil { @@ -209,6 +208,59 @@ func TestFindRoutesFeeSorting(t *testing.T) { } } +// TestFindRoutesWithFeeLimit asserts that routes found by the FindRoutes method +// within the channel router contain a total fee less than or equal to the fee +// limit. +func TestFindRoutesWithFeeLimit(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) + } + + // This test will attempt to find routes from roasbeef to sophon for 100 + // satoshis with a fee limit of 10 satoshis. There are two routes from + // roasbeef to sophon: + // 1. roasbeef -> songoku -> sophon + // 2. roasbeef -> phamnuwen -> sophon + // The second route violates our fee limit, so we should only expect to + // see the first route. + target := ctx.aliases["sophon"] + paymentAmt := lnwire.NewMSatFromSatoshis(100) + feeLimit := lnwire.NewMSatFromSatoshis(10) + + routes, err := ctx.router.FindRoutes( + target, paymentAmt, feeLimit, defaultNumRoutes, + DefaultFinalCLTVDelta, + ) + if err != nil { + t.Fatalf("unable to find any routes: %v", err) + } + + if len(routes) != 1 { + t.Fatalf("expected 1 route, got %d", len(routes)) + } + + if routes[0].TotalFees > feeLimit { + t.Fatalf("route exceeded fee limit: %v", spew.Sdump(routes[0])) + } + + hops := routes[0].Hops + if len(hops) != 2 { + t.Fatalf("expected 2 hops, got %d", len(hops)) + } + + if hops[0].Channel.Node.Alias != "songoku" { + t.Fatalf("expected first hop through songoku, got %s", + hops[0].Channel.Node.Alias) + } +} + // TestSendPaymentRouteFailureFallback tests that when sending a payment, if // one of the target routes is seen as unavailable, then the next route in the // queue is used instead. This process should continue until either a payment @@ -227,12 +279,11 @@ func TestSendPaymentRouteFailureFallback(t *testing.T) { // to luo ji for 1000 satoshis, with a maximum of 1000 satoshis in fees. var payHash [32]byte paymentAmt := lnwire.NewMSatFromSatoshis(1000) - feeLimit := paymentAmt.ToSatoshis() payment := LightningPayment{ Target: ctx.aliases["luoji"], Amount: paymentAmt, + FeeLimit: noFeeLimit, PaymentHash: payHash, - FeeLimit: feeLimit, } var preImage [32]byte @@ -305,9 +356,11 @@ func TestSendPaymentErrorRepeatedFeeInsufficient(t *testing.T) { // Craft a LightningPayment struct that'll send a payment from roasbeef // to luo ji for 100 satoshis. var payHash [32]byte + amt := lnwire.NewMSatFromSatoshis(1000) payment := LightningPayment{ Target: ctx.aliases["sophon"], - Amount: lnwire.NewMSatFromSatoshis(1000), + Amount: amt, + FeeLimit: noFeeLimit, PaymentHash: payHash, } @@ -403,9 +456,11 @@ func TestSendPaymentErrorNonFinalTimeLockErrors(t *testing.T) { // Craft a LightningPayment struct that'll send a payment from roasbeef // to sophon for 1k satoshis. var payHash [32]byte + amt := lnwire.NewMSatFromSatoshis(1000) payment := LightningPayment{ Target: ctx.aliases["sophon"], - Amount: lnwire.NewMSatFromSatoshis(1000), + Amount: amt, + FeeLimit: noFeeLimit, PaymentHash: payHash, } @@ -533,12 +588,11 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { // to luo ji for 1000 satoshis, with a maximum of 1000 satoshis in fees. var payHash [32]byte paymentAmt := lnwire.NewMSatFromSatoshis(1000) - feeLimit := paymentAmt.ToSatoshis() payment := LightningPayment{ Target: ctx.aliases["luoji"], Amount: paymentAmt, + FeeLimit: noFeeLimit, PaymentHash: payHash, - FeeLimit: feeLimit, } var preImage [32]byte @@ -974,10 +1028,9 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { // We should now be able to find two routes to node 2. paymentAmt := lnwire.NewMSatFromSatoshis(100) - feeLimit := paymentAmt.ToSatoshis() targetNode := priv2.PubKey() routes, err := ctx.router.FindRoutes( - targetNode, paymentAmt, feeLimit, defaultNumRoutes, + targetNode, paymentAmt, noFeeLimit, defaultNumRoutes, DefaultFinalCLTVDelta, ) if err != nil { @@ -1022,7 +1075,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { // Should still be able to find the routes, and the info should be // updated. routes, err = ctx.router.FindRoutes( - targetNode, paymentAmt, feeLimit, defaultNumRoutes, + targetNode, paymentAmt, noFeeLimit, defaultNumRoutes, DefaultFinalCLTVDelta, ) if err != nil {