From a040f8fafa2922ac023da79638becf0d0d54db58 Mon Sep 17 00:00:00 2001 From: ziggie Date: Sat, 12 Oct 2024 18:30:34 +0200 Subject: [PATCH 1/4] htlcswitch: increase log --- htlcswitch/link.go | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 383a28546..029198edf 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2814,6 +2814,10 @@ func (l *channelLink) exceedsFeeExposureLimit( // future commitment transaction with the fee-rate. totalLocalDust := localDustSum + lnwire.NewMSatFromSatoshis(localFee) if totalLocalDust > l.cfg.MaxFeeExposure { + l.log.Debugf("ChannelLink(%v): exceeds fee exposure limit: "+ + "local dust: %v, local fee: %v", l.ShortChanID(), + totalLocalDust, localFee) + return true, nil } @@ -2821,7 +2825,15 @@ func (l *channelLink) exceedsFeeExposureLimit( remoteFee, ) - return totalRemoteDust > l.cfg.MaxFeeExposure, nil + if totalRemoteDust > l.cfg.MaxFeeExposure { + l.log.Debugf("ChannelLink(%v): exceeds fee exposure limit: "+ + "remote dust: %v, remote fee: %v", l.ShortChanID(), + totalRemoteDust, remoteFee) + + return true, nil + } + + return false, nil } // isOverexposedWithHtlc calculates whether the proposed HTLC will make the @@ -2860,8 +2872,10 @@ func (l *channelLink) isOverexposedWithHtlc(htlc *lnwire.UpdateAddHTLC, commitFee = l.getCommitFee(true) } - localDustSum += lnwire.NewMSatFromSatoshis(commitFee) - remoteDustSum += lnwire.NewMSatFromSatoshis(commitFee) + commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee) + + localDustSum += commitFeeMSat + remoteDustSum += commitFeeMSat // Calculate the additional fee increase if this is a non-dust HTLC. weight := lntypes.WeightUnit(input.HTLCWeight) @@ -2881,6 +2895,10 @@ func (l *channelLink) isOverexposedWithHtlc(htlc *lnwire.UpdateAddHTLC, if localDustSum > l.cfg.MaxFeeExposure { // The max fee exposure was exceeded. + l.log.Debugf("ChannelLink(%v): HTLC %v makes the channel "+ + "overexposed, total local dust: %v (current commit "+ + "fee: %v)", l.ShortChanID(), htlc, localDustSum) + return true } @@ -2894,7 +2912,16 @@ func (l *channelLink) isOverexposedWithHtlc(htlc *lnwire.UpdateAddHTLC, remoteDustSum += additional } - return remoteDustSum > l.cfg.MaxFeeExposure + if remoteDustSum > l.cfg.MaxFeeExposure { + // The max fee exposure was exceeded. + l.log.Debugf("ChannelLink(%v): HTLC %v makes the channel "+ + "overexposed, total remote dust: %v (current commit "+ + "fee: %v)", l.ShortChanID(), htlc, remoteDustSum) + + return true + } + + return false } // dustClosure is a function that evaluates whether an HTLC is dust. It returns From 7b2da94750deb792de39352d5aafa013d8d15fe4 Mon Sep 17 00:00:00 2001 From: ziggie Date: Sat, 12 Oct 2024 20:29:08 +0200 Subject: [PATCH 2/4] multi: deprecate dust-treshold config value Replace ambigious config value "dust-treshold" with a more clear "channel-max-fee-exposure" exposure value. The old value is deprecated and will be removed in the near future. --- config.go | 26 ++++++++++++++++++++++++-- htlcswitch/switch_test.go | 4 ++-- itest/lnd_payment_test.go | 2 +- lntest/harness.go | 2 +- sample-lnd.conf | 30 ++++++++++++++++++++++++++---- 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/config.go b/config.go index 5ac4dc307..9b9e9573e 100644 --- a/config.go +++ b/config.go @@ -450,7 +450,9 @@ type Config struct { GcCanceledInvoicesOnTheFly bool `long:"gc-canceled-invoices-on-the-fly" description:"If true, we'll delete newly canceled invoices on the fly."` - MaxFeeExposure uint64 `long:"dust-threshold" description:"Sets the max fee exposure in satoshis for a channel after which HTLC's will be failed."` + DustThreshold uint64 `long:"dust-threshold" description:"DEPRECATED: Sets the max fee exposure in satoshis for a channel after which HTLC's will be failed." hidden:"true"` + + MaxFeeExposure uint64 `long:"channel-max-fee-exposure" description:" Limits the maximum fee exposure in satoshis of a channel. This value is enforced for all channels and is independent of the channel initiator."` Fee *lncfg.Fee `group:"fee" namespace:"fee"` @@ -710,7 +712,6 @@ func DefaultConfig() Config { MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte, - MaxFeeExposure: uint64(htlcswitch.DefaultMaxFeeExposure.ToSatoshis()), LogRotator: build.NewRotatingLogWriter(), DB: lncfg.DefaultDB(), Cluster: lncfg.DefaultCluster(), @@ -1714,6 +1715,27 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, cfg.Pprof.MutexProfile = cfg.MutexProfile } + // Don't allow both the old dust-threshold and the new + // channel-max-fee-exposure to be set. + if cfg.DustThreshold != 0 && cfg.MaxFeeExposure != 0 { + return nil, mkErr("cannot set both dust-threshold and " + + "channel-max-fee-exposure") + } + + switch { + // Use the old dust-threshold as the max fee exposure if it is set and + // the new option is not. + case cfg.DustThreshold != 0: + cfg.MaxFeeExposure = cfg.DustThreshold + + // Use the default max fee exposure if the new option is not set and + // the old one is not set either. + case cfg.MaxFeeExposure == 0: + cfg.MaxFeeExposure = uint64( + htlcswitch.DefaultMaxFeeExposure.ToSatoshis(), + ) + } + // Validate the subconfigs for workers, caches, and the tower client. err = lncfg.Validate( cfg.Workers, diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 5d24e2daa..f47deb080 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -4516,8 +4516,8 @@ func TestSwitchDustForwarding(t *testing.T) { } // sendDustHtlcs is a helper function used to send many dust HTLC's to test the -// Switch's dust-threshold logic. It takes a boolean denoting whether or not -// Alice is the sender. +// Switch's channel-max-fee-exposure logic. It takes a boolean denoting whether +// or not Alice is the sender. func sendDustHtlcs(t *testing.T, n *threeHopNetwork, alice bool, amt lnwire.MilliSatoshi, sid lnwire.ShortChannelID, numHTLCs int) { diff --git a/itest/lnd_payment_test.go b/itest/lnd_payment_test.go index e49f5f6fe..501221206 100644 --- a/itest/lnd_payment_test.go +++ b/itest/lnd_payment_test.go @@ -820,7 +820,7 @@ func testBidirectionalAsyncPayments(ht *lntest.HarnessTest) { args := []string{ // Increase the dust threshold to avoid the payments fail due // to threshold limit reached. - "--dust-threshold=10000000", + "--channel-max-fee-exposure=10000000", // Increase the pending commit interval since there are lots of // commitment dances. diff --git a/lntest/harness.go b/lntest/harness.go index 2f4278e25..c68c3ad08 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -316,7 +316,7 @@ func (h *HarnessTest) SetupStandbyNodes() { lndArgs := []string{ "--default-remote-max-htlcs=483", - "--dust-threshold=5000000", + "--channel-max-fee-exposure=5000000", } // Start the initial seeder nodes within the test network. diff --git a/sample-lnd.conf b/sample-lnd.conf index 6d64603e8..6af5e4b57 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -474,10 +474,32 @@ ; propagation ; max-commit-fee-rate-anchors=10 -; A threshold defining the maximum amount of dust a given channel can have -; after which forwarding and sending dust HTLC's to and from the channel will -; fail. This amount is expressed in satoshis. -; dust-threshold=500000 +; DEPRECATED: This value will be deprecated please use the new setting +; "channel-max-fee-exposure". This value is equivalent to the new fee exposure +; limit but was removed because the name was ambigious. +; dust-threshold= + +; This value replaces the old 'dust-threshold' setting and defines the maximum +; amount of satoshis that a channel pays in fees in case the commitment +; transaction is broadcasted. This is enforced in both directions either when +; we are the channel intiator hence paying the fees but also applies to the +; channel fee if we are NOT the channel initiator. It is +; important to note that every HTLC adds fees to the channel state. Non-dust +; HTLCs add just a new output onto the commitment transaction whereas dust +; HTLCs are completely attributed the commitment fee. So this limit can also +; influence adding new HTLCs onto the state. When the limit is reached we won't +; allow any new HTLCs onto the channel state (outgoing and incoming). So +; choosing a right limit here must be done with caution. Moreover this is a +; limit for all channels universally meaning there is no difference made due to +; the channel size. So it is recommended to use the default value. However if +; you have a very small channel average size you might want to reduce this +; value. +; WARNING: Setting this value too low might cause force closes because the +; lightning protocol has no way to roll back a channel state when your peer +; proposes a channel update which exceeds this limit. There are only two options +; to resolve this situation, either increasing the limit or one side force +; closes the channel. +; channel-max-fee-exposure=500000 ; If true, lnd will abort committing a migration if it would otherwise have been ; successful. This leaves the database unmodified, and still compatible with the From 643e415448fff822e8aea6d3d7e14fe467d4a4e3 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 6 Nov 2024 18:34:00 +0100 Subject: [PATCH 3/4] scripts: dont check new config value. Because we need to remain backwards compatible with the old `dust-threshold` value we set the default value for `channel-max-fee-exposure` later in the program flow. Given the restrictions of the sample config check we need to exclude this value from the check. --- scripts/check-sample-lnd-conf.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/check-sample-lnd-conf.sh b/scripts/check-sample-lnd-conf.sh index ba96a637c..48cbad7f6 100755 --- a/scripts/check-sample-lnd-conf.sh +++ b/scripts/check-sample-lnd-conf.sh @@ -54,11 +54,12 @@ OPTIONS_NO_CONF="help lnddir configfile version end" # OPTIONS_NO_LND_DEFAULT_VALUE_CHECK is a list of options with default values # set, but there aren't any returned defaults by lnd --help. Defaults have to be # included in sample-lnd.conf but no further checks are performed. -OPTIONS_NO_LND_DEFAULT_VALUE_CHECK="adminmacaroonpath readonlymacaroonpath \ - invoicemacaroonpath rpclisten restlisten listen backupfilepath maxchansize \ - bitcoin.chaindir bitcoin.defaultchanconfs bitcoin.defaultremotedelay \ - bitcoin.dnsseed signrpc.signermacaroonpath walletrpc.walletkitmacaroonpath \ - chainrpc.notifiermacaroonpath routerrpc.routermacaroonpath" +OPTIONS_NO_LND_DEFAULT_VALUE_CHECK="channel-max-fee-exposure adminmacaroonpath \ + readonlymacaroonpath invoicemacaroonpath rpclisten restlisten listen \ + backupfilepath maxchansize bitcoin.chaindir bitcoin.defaultchanconfs \ + bitcoin.defaultremotedelay bitcoin.dnsseed signrpc.signermacaroonpath \ + walletrpc.walletkitmacaroonpath chainrpc.notifiermacaroonpath \ + routerrpc.routermacaroonpath" # EXITCODE is returned at the end after all checks are performed and set to 1 From aebf03aca4785c826487487a2471781c07f4c24b Mon Sep 17 00:00:00 2001 From: ziggie Date: Sat, 19 Oct 2024 09:17:30 +0200 Subject: [PATCH 4/4] docs: add release-notes for 19. --- docs/release-notes/release-notes-0.19.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 11613fcec..08988ddeb 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -110,6 +110,11 @@ these options have also been increased from max 3 log files to 10 and from max 10 MB to 20 MB. +* [Deprecate `dust-threshold` +config option](https://github.com/lightningnetwork/lnd/pull/9182) and introduce +a new option `channel-max-fee-exposure` which is unambiguous in its description. +The underlying functionality between those two options remain the same. + ## Breaking Changes ## Performance Improvements