Merge pull request #6226 from guggero/fee-limit-fix

Fix default value for fee limit

cmd/lncli/cmd_payments.go

@@ -22,6 +22,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -217,8 +218,10 @@ func retrieveFeeLimit(ctx *cli.Context, amt int64) (int64, error) {
 		return feeLimitRoundedUp, nil
 	}
 
-	// If no fee limit is set, use the payment amount as a limit (100%).
+	// If no fee limit is set, use a default value based on the amount.
-	return amt, nil
+	amtMsat := lnwire.NewMSatFromSatoshis(btcutil.Amount(amt))
+	limitMsat := lnwallet.DefaultRoutingFeeLimitForAmount(amtMsat)
+	return int64(limitMsat.ToSatoshis()), nil
 }
 
 func confirmPayReq(resp *lnrpc.PayReq, amt, feeLimit int64) error {

docs/release-notes/release-notes-0.14.2.md

@@ -19,6 +19,17 @@ connection from the watch-only node.
   In other words, freshly-installed LND can now be initialized with multiple
   channels from an external (e.g. hardware) wallet *in a single transaction*.
 
+* A bug that allowed fees to be up to 100% of the payment amount was fixed by
+  [introducing a more sane default
+  value](https://github.com/lightningnetwork/lnd/pull/6226) of 5% routing fees
+  (except for small amounts <= 50 satoshis where the 100% routing fees are kept
+  to accommodate for the base fee in channels). To avoid falling back to a
+  default value, users should always set their own fee limits by using the
+  `--fee_limit` or `--fee_limit_percent` flags on the `lncli payinvoice`,
+  `lncli sendpayment` and `lncli queryroutes` commands. Users of the gRPC or
+  REST API should set the `fee_limit` field on the corresponding calls
+  (`SendPayment`, `SendPaymentSync`, `QueryRoutes`).
+
 ## Database
 
 * [Speed up graph cache loading on startup with

lnrpc/lightning.pb.go

@@ -1938,7 +1938,8 @@ type SendRequest struct {
 	//The maximum number of satoshis that will be paid as a fee of the payment.
 	//This value can be represented either as a percentage of the amount being
 	//sent, or as a fixed amount of the maximum fee the user is willing the pay to
-	//send the payment.
+	//send the payment. If not specified, lnd will use a default value of 100%
+	//fees for small amounts (<=50 sat) or 5% fees for larger amounts.
 	FeeLimit *FeeLimit `protobuf:"bytes,8,opt,name=fee_limit,json=feeLimit,proto3" json:"fee_limit,omitempty"`
 	//
 	//The channel id of the channel that must be taken to the first hop. If zero,
@@ -8517,7 +8518,8 @@ type QueryRoutesRequest struct {
 	//The maximum number of satoshis that will be paid as a fee of the payment.
 	//This value can be represented either as a percentage of the amount being
 	//sent, or as a fixed amount of the maximum fee the user is willing the pay to
-	//send the payment.
+	//send the payment. If not specified, lnd will use a default value of 100%
+	//fees for small amounts (<=50 sat) or 5% fees for larger amounts.
 	FeeLimit *FeeLimit `protobuf:"bytes,5,opt,name=fee_limit,json=feeLimit,proto3" json:"fee_limit,omitempty"`
 	//
 	//A list of nodes to ignore during path finding. When using REST, these fields

lnrpc/lightning.proto

@@ -753,7 +753,8 @@ message SendRequest {
     The maximum number of satoshis that will be paid as a fee of the payment.
     This value can be represented either as a percentage of the amount being
     sent, or as a fixed amount of the maximum fee the user is willing the pay to
-    send the payment.
+    send the payment. If not specified, lnd will use a default value of 100%
+    fees for small amounts (<=50 sat) or 5% fees for larger amounts.
     */
     FeeLimit fee_limit = 8;
 
@@ -2574,7 +2575,8 @@ message QueryRoutesRequest {
     The maximum number of satoshis that will be paid as a fee of the payment.
     This value can be represented either as a percentage of the amount being
     sent, or as a fixed amount of the maximum fee the user is willing the pay to
-    send the payment.
+    send the payment. If not specified, lnd will use a default value of 100%
+    fees for small amounts (<=50 sat) or 5% fees for larger amounts.
     */
     FeeLimit fee_limit = 5;
 

lnrpc/lightning.swagger.json

@@ -6238,7 +6238,7 @@
         },
         "fee_limit": {
           "$ref": "#/definitions/lnrpcFeeLimit",
-          "description": "The maximum number of satoshis that will be paid as a fee of the payment.\nThis value can be represented either as a percentage of the amount being\nsent, or as a fixed amount of the maximum fee the user is willing the pay to\nsend the payment."
+          "description": "The maximum number of satoshis that will be paid as a fee of the payment.\nThis value can be represented either as a percentage of the amount being\nsent, or as a fixed amount of the maximum fee the user is willing the pay to\nsend the payment. If not specified, lnd will use a default value of 100%\nfees for small amounts (\u003c=50 sat) or 5% fees for larger amounts."
         },
         "outgoing_chan_id": {
           "type": "string",

lnrpc/marshall_utils.go

@@ -40,10 +40,9 @@ func CalculateFeeLimit(feeLimit *FeeLimit,
 		return amount * lnwire.MilliSatoshi(feeLimit.GetPercent()) / 100
 
 	default:
-		// If a fee limit was not specified, we'll use the payment's
+		// Fall back to a sane default value that is based on the amount
-		// amount as an upper bound in order to avoid payment attempts
+		// itself.
-		// from incurring fees higher than the payment amount itself.
+		return lnwallet.DefaultRoutingFeeLimitForAmount(amount)
-		return amount
 	}
 }
 

lnwallet/parameters.go

@@ -5,8 +5,37 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/input"
+	"github.com/lightningnetwork/lnd/lnwire"
 )
 
+const (
+	// RoutingFee100PercentUpTo is the cut-off amount we allow 100% fees to
+	// be charged up to.
+	RoutingFee100PercentUpTo lnwire.MilliSatoshi = 50_000
+
+	// DefaultRoutingFeePercentage is the default off-chain routing fee we
+	// allow to be charged for a payment over the RoutingFee100PercentUpTo
+	// size.
+	DefaultRoutingFeePercentage lnwire.MilliSatoshi = 5
+)
+
+// DefaultRoutingFeeLimitForAmount returns the default off-chain routing fee
+// limit lnd uses if the user does not specify a limit manually. The fee is
+// amount dependent because of the base routing fee that is set on many
+// channels. For example the default base fee is 1 satoshi. So sending a payment
+// of one satoshi will cost 1 satoshi in fees over most channels, which comes to
+// a fee of 100%. That's why for very small amounts we allow 100% fee.
+func DefaultRoutingFeeLimitForAmount(a lnwire.MilliSatoshi) lnwire.MilliSatoshi {
+	// Allow 100% fees up to a certain amount to accommodate for base fees.
+	if a <= RoutingFee100PercentUpTo {
+		return a
+	}
+
+	// Everything larger than the cut-off amount will get a default fee
+	// percentage.
+	return a * DefaultRoutingFeePercentage / 100
+}
+
 // DustLimitForSize retrieves the dust limit for a given pkscript size. Given
 // the size, it automatically determines whether the script is a witness script
 // or not. It calls btcd's GetDustThreshold method under the hood. It must be

lnwallet/parameters_test.go

@@ -1,13 +1,52 @@
 package lnwallet
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/input"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
 )
 
+// TestDefaultRoutingFeeLimitForAmount tests that we use the correct default
+// routing fee depending on the amount.
+func TestDefaultRoutingFeeLimitForAmount(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		amount        lnwire.MilliSatoshi
+		expectedLimit lnwire.MilliSatoshi
+	}{
+		{
+			amount:        1,
+			expectedLimit: 1,
+		},
+		{
+			amount:        50_000,
+			expectedLimit: 50_000,
+		},
+		{
+			amount:        50_001,
+			expectedLimit: 2_500,
+		},
+		{
+			amount:        5_000_000_000,
+			expectedLimit: 250_000_000,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(fmt.Sprintf("%d sats", test.amount), func(t *testing.T) {
+			feeLimit := DefaultRoutingFeeLimitForAmount(test.amount)
+			require.Equal(t, test.expectedLimit, feeLimit)
+		})
+	}
+}
+
 // TestDustLimitForSize tests that we receive the expected dust limits for
 // various script types from btcd's GetDustThreshold function.
 func TestDustLimitForSize(t *testing.T) {