From bfa1cf17b94628fb52ea308b19202047d6632848 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 2 Feb 2022 14:24:52 +0100 Subject: [PATCH 1/4] lnwallet: introduce default routing fee calculation --- lnwallet/parameters.go | 29 +++++++++++++++++++++++++++ lnwallet/parameters_test.go | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/lnwallet/parameters.go b/lnwallet/parameters.go index ccd5583d0..9e4b67422 100644 --- a/lnwallet/parameters.go +++ b/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 diff --git a/lnwallet/parameters_test.go b/lnwallet/parameters_test.go index 021cb7bc6..cde35c05e 100644 --- a/lnwallet/parameters_test.go +++ b/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) { From d22f543906e1d600b54765d631f6dd4e3bbb30ec Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 2 Feb 2022 14:24:53 +0100 Subject: [PATCH 2/4] lnrpc: use amount based default routing fee --- lnrpc/lightning.pb.go | 6 ++++-- lnrpc/lightning.proto | 6 ++++-- lnrpc/lightning.swagger.json | 2 +- lnrpc/marshall_utils.go | 7 +++---- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lnrpc/lightning.pb.go b/lnrpc/lightning.pb.go index ba6bff1eb..42b937825 100644 --- a/lnrpc/lightning.pb.go +++ b/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 diff --git a/lnrpc/lightning.proto b/lnrpc/lightning.proto index 38e7f0aec..905f839c4 100644 --- a/lnrpc/lightning.proto +++ b/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; diff --git a/lnrpc/lightning.swagger.json b/lnrpc/lightning.swagger.json index 9cec28593..7ae032167 100644 --- a/lnrpc/lightning.swagger.json +++ b/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", diff --git a/lnrpc/marshall_utils.go b/lnrpc/marshall_utils.go index d0e501160..013e1f9be 100644 --- a/lnrpc/marshall_utils.go +++ b/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 - // amount as an upper bound in order to avoid payment attempts - // from incurring fees higher than the payment amount itself. - return amount + // Fall back to a sane default value that is based on the amount + // itself. + return lnwallet.DefaultRoutingFeeLimitForAmount(amount) } } From d252653624305745a5278c0e5c78e452c83f2f76 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 2 Feb 2022 14:24:54 +0100 Subject: [PATCH 3/4] lncli: use amount based default routing fee --- cmd/lncli/cmd_payments.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/lncli/cmd_payments.go b/cmd/lncli/cmd_payments.go index 057d923dc..c6c6bf940 100644 --- a/cmd/lncli/cmd_payments.go +++ b/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%). - return amt, nil + // If no fee limit is set, use a default value based on the amount. + amtMsat := lnwire.NewMSatFromSatoshis(btcutil.Amount(amt)) + limitMsat := lnwallet.DefaultRoutingFeeLimitForAmount(amtMsat) + return int64(limitMsat.ToSatoshis()), nil } func confirmPayReq(resp *lnrpc.PayReq, amt, feeLimit int64) error { From 84c8c18a9e1f39a9e280d137ea0741a7f58c3a0e Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 2 Feb 2022 14:24:56 +0100 Subject: [PATCH 4/4] docs: update release notes --- docs/release-notes/release-notes-0.14.2.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.2.md b/docs/release-notes/release-notes-0.14.2.md index 167dc04ab..3169c3cf2 100644 --- a/docs/release-notes/release-notes-0.14.2.md +++ b/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