cmd/lncli: add fee limit flags to routing related commands

cmd/lncli/commands.go

@@ -103,37 +103,6 @@ func actionDecorator(f func(*cli.Context) error) func(*cli.Context) error {
 	}
 }
 
-// calculateFeeLimit takes ctx and the amount for the payment in satoshis.
-// It checks the fee_limit and fee_limit_percent flags and determines the
-// the feeLimit based on those values. If both flags are set, calculateFeeLimit
-// sets feeLimit to the Minumum value between them. If neither flag is set,
-// feeLimit is set to the maximum int64 value as to avoid enforcing a limit.
-func calculateFeeLimit(ctx *cli.Context, amount int64) int64 {
-	var (
-		feeLimit        int64
-		feeLimitPercent int64
-	)
-
-	const MaxInt64Value int64 = 1<<63 - 1 // 9223372036854775807
-	feeLimit = MaxInt64Value
-
-	if ctx.IsSet("fee_limit_percent") {
-		feeRatio := float64(ctx.Int64("fee_limit_percent")) / 100
-		feeLimitPercent = int64(float64(amount) * feeRatio)
-		feeLimit = feeLimitPercent
-	}
-
-	if ctx.IsSet("fee_limit") {
-		feeLimit = ctx.Int64("fee_limit")
-	}
-
-	// If both fee_limit and fee_limit_percent are set, pick the minimum
-	if ctx.IsSet("fee_limit") && ctx.IsSet("fee_limit_percent") {
-		feeLimit = int64(math.Min(float64(feeLimit), float64(feeLimitPercent)))
-	}
-	return feeLimit
-}
-
 var newAddressCommand = cli.Command{
 	Name:      "newaddress",
 	Category:  "Wallet",
@@ -1648,6 +1617,16 @@ var sendPaymentCommand = cli.Command{
 			Name:  "amt, a",
 			Usage: "number of satoshis to send",
 		},
+		cli.Int64Flag{
+			Name: "fee_limit",
+			Usage: "maximum fee allowed in satoshis when sending" +
+				"the payment",
+		},
+		cli.Int64Flag{
+			Name: "fee_limit_percent",
+			Usage: "percentage of the payment's amount used as the" +
+				"maximum fee allowed when sending the payment",
+		},
 		cli.StringFlag{
 			Name:  "payment_hash, r",
 			Usage: "the hash to use within the payment's HTLC",
@@ -1664,18 +1643,36 @@ var sendPaymentCommand = cli.Command{
 			Name:  "final_cltv_delta",
 			Usage: "the number of blocks the last hop has to reveal the preimage",
 		},
-		cli.Int64Flag{
-			Name:  "fee_limit",
-			Usage: "maximum total fees for the payment in satoshis",
-		},
-		cli.Int64Flag{
-			Name:  "fee_limit_percent",
-			Usage: "maximum total fees as a percentage of amt",
-		},
 	},
 	Action: sendPayment,
 }
 
+// retrieveFeeLimit retrieves the fee limit based on the different fee limit
+// flags passed.
+func retrieveFeeLimit(ctx *cli.Context) (*lnrpc.FeeLimit, error) {
+	switch {
+	case ctx.IsSet("fee_limit") && ctx.IsSet("fee_limit_percent"):
+		return nil, fmt.Errorf("either fee_limit or fee_limit_percent " +
+			"can be set, but not both")
+	case ctx.IsSet("fee_limit"):
+		return &lnrpc.FeeLimit{
+			Limit: &lnrpc.FeeLimit_Fixed{
+				Fixed: ctx.Int64("fee_limit"),
+			},
+		}, nil
+	case ctx.IsSet("fee_limit_percent"):
+		return &lnrpc.FeeLimit{
+			Limit: &lnrpc.FeeLimit_Percent{
+				Percent: ctx.Int64("fee_limit_percent"),
+			},
+		}, nil
+	}
+
+	// Since the fee limit flags aren't required, we don't return an error
+	// if they're not set.
+	return nil, nil
+}
+
 func sendPayment(ctx *cli.Context) error {
 	// Show command help if no arguments provided
 	if ctx.NArg() == 0 && ctx.NumFlags() == 0 {
@@ -1683,91 +1680,99 @@ func sendPayment(ctx *cli.Context) error {
 		return nil
 	}
 
-	var req *lnrpc.SendRequest
+	// First, we'll retrieve the fee limit value passed since it can apply
+	// to both ways of sending payments (with the payment request or
+	// providing the details manually).
+	feeLimit, err := retrieveFeeLimit(ctx)
+	if err != nil {
+		return err
+	}
+
+	// If a payment request was provided, we can exit early since all of the
+	// details of the payment are encoded within the request.
 	if ctx.IsSet("pay_req") {
-		req = &lnrpc.SendRequest{
+		req := &lnrpc.SendRequest{
 			PaymentRequest: ctx.String("pay_req"),
 			Amt:            ctx.Int64("amt"),
+			FeeLimit:       feeLimit,
 		}
-	} else {
-		args := ctx.Args()
 
-		var (
+		return sendPaymentRequest(ctx, req)
-			destNode []byte
+	}
-			err      error
+
-			amount   int64
+	var (
-			feeLimit int64
+		destNode []byte
-		)
+		amount   int64
+	)
+
+	args := ctx.Args()
+
+	switch {
+	case ctx.IsSet("dest"):
+		destNode, err = hex.DecodeString(ctx.String("dest"))
+	case args.Present():
+		destNode, err = hex.DecodeString(args.First())
+		args = args.Tail()
+	default:
+		return fmt.Errorf("destination txid argument missing")
+	}
+	if err != nil {
+		return err
+	}
+
+	if len(destNode) != 33 {
+		return fmt.Errorf("dest node pubkey must be exactly 33 bytes, is "+
+			"instead: %v", len(destNode))
+	}
+
+	if ctx.IsSet("amt") {
+		amount = ctx.Int64("amt")
+	} else if args.Present() {
+		amount, err = strconv.ParseInt(args.First(), 10, 64)
+		args = args.Tail()
+		if err != nil {
+			return fmt.Errorf("unable to decode payment amount: %v", err)
+		}
+	}
+
+	req := &lnrpc.SendRequest{
+		Dest:     destNode,
+		Amt:      amount,
+		FeeLimit: feeLimit,
+	}
+
+	if ctx.Bool("debug_send") && (ctx.IsSet("payment_hash") || args.Present()) {
+		return fmt.Errorf("do not provide a payment hash with debug send")
+	} else if !ctx.Bool("debug_send") {
+		var rHash []byte
 
 		switch {
-		case ctx.IsSet("dest"):
+		case ctx.IsSet("payment_hash"):
-			destNode, err = hex.DecodeString(ctx.String("dest"))
+			rHash, err = hex.DecodeString(ctx.String("payment_hash"))
 		case args.Present():
-			destNode, err = hex.DecodeString(args.First())
+			rHash, err = hex.DecodeString(args.First())
-			args = args.Tail()
 		default:
-			return fmt.Errorf("destination txid argument missing")
+			return fmt.Errorf("payment hash argument missing")
 		}
+
 		if err != nil {
 			return err
 		}
-
+		if len(rHash) != 32 {
-		if len(destNode) != 33 {
+			return fmt.Errorf("payment hash must be exactly 32 "+
-			return fmt.Errorf("dest node pubkey must be exactly 33 bytes, is "+
+				"bytes, is instead %v", len(rHash))
-				"instead: %v", len(destNode))
 		}
+		req.PaymentHash = rHash
 
-		if ctx.IsSet("amt") {
+		switch {
-			amount = ctx.Int64("amt")
+		case ctx.IsSet("final_cltv_delta"):
-		} else if args.Present() {
+			req.FinalCltvDelta = int32(ctx.Int64("final_cltv_delta"))
-			amount, err = strconv.ParseInt(args.First(), 10, 64)
+		case args.Present():
-			args = args.Tail()
+			delta, err := strconv.ParseInt(args.First(), 10, 64)
-			if err != nil {
-				return fmt.Errorf("unable to decode payment amount: %v", err)
-			}
-		}
-
-		feeLimit = calculateFeeLimit(ctx, amount)
-
-		req = &lnrpc.SendRequest{
-			Dest:     destNode,
-			Amt:      amount,
-			FeeLimit: feeLimit,
-		}
-
-		if ctx.Bool("debug_send") && (ctx.IsSet("payment_hash") || args.Present()) {
-			return fmt.Errorf("do not provide a payment hash with debug send")
-		} else if !ctx.Bool("debug_send") {
-			var rHash []byte
-
-			switch {
-			case ctx.IsSet("payment_hash"):
-				rHash, err = hex.DecodeString(ctx.String("payment_hash"))
-			case args.Present():
-				rHash, err = hex.DecodeString(args.First())
-			default:
-				return fmt.Errorf("payment hash argument missing")
-			}
-
 			if err != nil {
 				return err
 			}
-			if len(rHash) != 32 {
+			req.FinalCltvDelta = int32(delta)
-				return fmt.Errorf("payment hash must be exactly 32 "+
-					"bytes, is instead %v", len(rHash))
-			}
-			req.PaymentHash = rHash
-
-			switch {
-			case ctx.IsSet("final_cltv_delta"):
-				req.FinalCltvDelta = int32(ctx.Int64("final_cltv_delta"))
-			case args.Present():
-				delta, err := strconv.ParseInt(args.First(), 10, 64)
-				if err != nil {
-					return err
-				}
-				req.FinalCltvDelta = int32(delta)
-			}
 		}
 	}
 
@@ -1823,12 +1828,14 @@ var payInvoiceCommand = cli.Command{
 				"invoice",
 		},
 		cli.Int64Flag{
-			Name:  "fee_limit",
+			Name: "fee_limit",
-			Usage: "maximum total fees for the payment in satoshis",
+			Usage: "maximum fee allowed in satoshis when sending" +
+				"the payment",
 		},
 		cli.Int64Flag{
-			Name:  "fee_limit_percent",
+			Name: "fee_limit_percent",
-			Usage: "maximum total fees as a percentage of amt",
+			Usage: "percentage of the payment's amount used as the" +
+				"maximum fee allowed when sending the payment",
 		},
 	},
 	Action: actionDecorator(payInvoice),
@@ -1838,9 +1845,6 @@ func payInvoice(ctx *cli.Context) error {
 	args := ctx.Args()
 
 	var payReq string
-	var feeLimit int64
-	var amt int64
-
 	switch {
 	case ctx.IsSet("pay_req"):
 		payReq = ctx.String("pay_req")
@@ -1850,13 +1854,14 @@ func payInvoice(ctx *cli.Context) error {
 		return fmt.Errorf("pay_req argument missing")
 	}
 
-	amt = ctx.Int64("amt")
+	feeLimit, err := retrieveFeeLimit(ctx)
-
+	if err != nil {
-	feeLimit = calculateFeeLimit(ctx, amt)
+		return err
+	}
 
 	req := &lnrpc.SendRequest{
 		PaymentRequest: payReq,
-		Amt:            amt,
+		Amt:            ctx.Int64("amt"),
 		FeeLimit:       feeLimit,
 	}
 
@@ -2558,12 +2563,14 @@ var queryRoutesCommand = cli.Command{
 			Usage: "the amount to send expressed in satoshis",
 		},
 		cli.Int64Flag{
-			Name:  "fee_limit",
+			Name: "fee_limit",
-			Usage: "maximum total fees for the payment in satoshis",
+			Usage: "maximum fee allowed in satoshis when sending" +
+				"the payment",
 		},
 		cli.Int64Flag{
-			Name:  "fee_limit_percent",
+			Name: "fee_limit_percent",
-			Usage: "maximum total fees as a percentage of amt",
+			Usage: "percentage of the payment's amount used as the" +
+				"maximum fee allowed when sending the payment",
 		},
 		cli.Int64Flag{
 			Name:  "num_max_routes",
@@ -2585,10 +2592,9 @@ func queryRoutes(ctx *cli.Context) error {
 	defer cleanUp()
 
 	var (
-		dest     string
+		dest string
-		amt      int64
+		amt  int64
-		err      error
+		err  error
-		feeLimit int64
 	)
 
 	args := ctx.Args()
@@ -2615,11 +2621,15 @@ func queryRoutes(ctx *cli.Context) error {
 		return fmt.Errorf("amt argument missing")
 	}
 
-	feeLimit = calculateFeeLimit(ctx, amt)
+	feeLimit, err := retrieveFeeLimit(ctx)
+	if err != nil {
+		return err
+	}
 
 	req := &lnrpc.QueryRoutesRequest{
 		PubKey:         dest,
 		Amt:            amt,
+		FeeLimit:       feeLimit,
 		NumRoutes:      int32(ctx.Int("num_max_routes")),
 		FinalCltvDelta: int32(ctx.Int("final_cltv_delta")),
 	}