routing: allow specifying a fee limit during route construction

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
+	// ErrFeeLimitExceeded is returned when the total fees of a route exceed
-	// exceed the user-specified maximum payment for the fee.
+	// the user-specified fee limit.
-	ErrFeeCutoffExceeded
+	ErrFeeLimitExceeded
 )
 
 // routerError is a structure that represent the error inside the routing package,

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

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
+		// Invalidate this route if its total fees exceed our fee limit.
-		// payment, stop hopping the route and return
+		if route.TotalFees > feeLimit {
-		if route.TotalFees.ToSatoshis() > feeLimit {
+			err := fmt.Sprintf("total route fees exceeded fee "+
-			err := fmt.Sprintf("Route fee exceeded fee limit of %v",
+				"limit of %v", feeLimit)
-				feeLimit)
+			return nil, newErrf(ErrFeeLimitExceeded, err)
-			return nil, newErrf(ErrFeeCutoffExceeded, err)
 		}
 
 		// As a sanity check, we ensure that the selected channel has

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,
+	route, err := newRoute(
-		finalHopCLTV, feeLimit)
+		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,
+	route, err = newRoute(
-		finalHopCLTV, feeLimit)
+		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, noFeeLimit, 100)
-	routes, err := ctx.router.FindRoutes(carol, amt, feeLimit, 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)
 	}

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,
+func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop,
-	amt lnwire.MilliSatoshi, feeLimit btcutil.Amount,
+	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,
+			amt, feeLimit, source, path[1:], currentHeight,
-			feeLimit,
+			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

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 {