Merge pull request #9182 from ziggie1984/fix-feebuffer

Deprecate dust-threshold config value

Author: Roasbeef

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,

docs/release-notes/release-notes-0.19.0.md

@@ -139,6 +139,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
 

htlcswitch/link.go

@@ -2814,14 +2814,26 @@ 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
 	}
 
 	totalRemoteDust := remoteDustSum + lnwire.NewMSatFromSatoshis(
 		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

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) {
 

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.

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.

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

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