From 0fd76e53b80e346468730fd27be2002861cee13d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 10 Dec 2020 14:16:53 +0100 Subject: [PATCH 1/3] multi: cap anchors feerate at configurable maximum This commit caps the update fee the initiator will send when the anchors channel type is used. We do not limit anything on the receiver side. 10 sat/vbyte is the current default max fee rate we use. This should be enough to ensure propagation before anchoring down the commitment transaction. --- config.go | 10 +++++++++ fundingmanager.go | 33 ++++++++++++++++++--------- htlcswitch/link.go | 9 +++++++- htlcswitch/test_utils.go | 1 + lnwallet/channel.go | 16 +++++++++++--- lnwallet/channel_test.go | 48 ++++++++++++++++++++++++++++++---------- lnwallet/commitment.go | 5 +++++ peer/brontide.go | 5 +++++ server.go | 6 ++++- 9 files changed, 106 insertions(+), 27 deletions(-) diff --git a/config.go b/config.go index d7e3aefd..7a13e58a 100644 --- a/config.go +++ b/config.go @@ -33,6 +33,7 @@ import ( "github.com/lightningnetwork/lnd/lncfg" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lnrpc/signrpc" + "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/routing" "github.com/lightningnetwork/lnd/tor" ) @@ -289,6 +290,8 @@ type Config struct { MaxChannelFeeAllocation float64 `long:"max-channel-fee-allocation" description:"The maximum percentage of total funds that can be allocated to a channel's commitment fee. This only applies for the initiator of the channel. Valid values are within [0.1, 1]."` + MaxCommitFeeRateAnchors uint64 `long:"max-commit-fee-rate-anchors" description:"The maximum fee rate in sat/vbyte that will be used for commitments of channels of the anchors type. Must be large enough to ensure transaction propagation"` + DryRunMigration bool `long:"dry-run-migration" description:"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 previously active version of lnd."` net tor.Net @@ -475,6 +478,7 @@ func DefaultConfig() Config { }, MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, + MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte, LogWriter: build.NewRotatingLogWriter(), DB: lncfg.DefaultDB(), registeredChains: chainreg.NewChainRegistry(), @@ -735,6 +739,12 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) { cfg.MaxChannelFeeAllocation) } + if cfg.MaxCommitFeeRateAnchors < 1 { + return nil, fmt.Errorf("invalid max commit fee rate anchors: "+ + "%v, must be at least 1 sat/vbyte", + cfg.MaxCommitFeeRateAnchors) + } + // Validate the Tor config parameters. socks, err := lncfg.ParseAddressString( cfg.Tor.SOCKS, strconv.Itoa(defaultTorSOCKSPort), diff --git a/fundingmanager.go b/fundingmanager.go index 9d1e234c..219f4c58 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -366,6 +366,10 @@ type fundingConfig struct { // RegisteredChains keeps track of all chains that have been registered // with the daemon. RegisteredChains *chainreg.ChainRegistry + + // MaxAnchorsCommitFeeRate is the max commitment fee rate we'll use as + // the initiator for channels of the anchor type. + MaxAnchorsCommitFeeRate chainfee.SatPerKWeight } // fundingManager acts as an orchestrator/bridge between the wallet's @@ -3122,16 +3126,6 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { msg.pushAmt, msg.chainHash, peerKey.SerializeCompressed(), ourDustLimit, msg.minConfs) - // First, we'll query the fee estimator for a fee that should get the - // commitment transaction confirmed by the next few blocks (conf target - // of 3). We target the near blocks here to ensure that we'll be able - // to execute a timely unilateral channel closure if needed. - commitFeePerKw, err := f.cfg.FeeEstimator.EstimateFeePerKW(3) - if err != nil { - msg.err <- err - return - } - // We set the channel flags to indicate whether we want this channel to // be announced to the network. var channelFlags lnwire.FundingFlag @@ -3190,6 +3184,25 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { commitType := commitmentType( msg.peer.LocalFeatures(), msg.peer.RemoteFeatures(), ) + + // First, we'll query the fee estimator for a fee that should get the + // commitment transaction confirmed by the next few blocks (conf target + // of 3). We target the near blocks here to ensure that we'll be able + // to execute a timely unilateral channel closure if needed. + commitFeePerKw, err := f.cfg.FeeEstimator.EstimateFeePerKW(3) + if err != nil { + msg.err <- err + return + } + + // For anchor channels cap the initial commit fee rate at our defined + // maximum. + if commitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx && + commitFeePerKw > f.cfg.MaxAnchorsCommitFeeRate { + + commitFeePerKw = f.cfg.MaxAnchorsCommitFeeRate + } + req := &lnwallet.InitFundingReserveMsg{ ChainHash: &msg.chainHash, PendingChanID: chanID, diff --git a/htlcswitch/link.go b/htlcswitch/link.go index df6bafc9..9d6ad673 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -269,6 +269,10 @@ type ChannelLinkConfig struct { // initiator of the channel. MaxFeeAllocation float64 + // MaxAnchorsCommitFeeRate is the max commitment fee rate we'll use as + // the initiator for channels of the anchor type. + MaxAnchorsCommitFeeRate chainfee.SatPerKWeight + // NotifyActiveLink allows the link to tell the ChannelNotifier when a // link is first started. NotifyActiveLink func(wire.OutPoint) @@ -1090,7 +1094,10 @@ func (l *channelLink) htlcManager() { // based on our current set fee rate. We'll cap the new // fee rate to our max fee allocation. commitFee := l.channel.CommitFeeRate() - maxFee := l.channel.MaxFeeRate(l.cfg.MaxFeeAllocation) + maxFee := l.channel.MaxFeeRate( + l.cfg.MaxFeeAllocation, + l.cfg.MaxAnchorsCommitFeeRate, + ) newCommitFee := chainfee.SatPerKWeight( math.Min(float64(netFee), float64(maxFee)), ) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 6fc6b1dd..7d14abce 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1185,6 +1185,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, OutgoingCltvRejectDelta: 3, MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, MaxFeeAllocation: DefaultMaxLinkFeeAllocation, + MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte(10 * 1000).FeePerKWeight(), NotifyActiveLink: func(wire.OutPoint) {}, NotifyActiveChannel: func(wire.OutPoint) {}, NotifyInactiveChannel: func(wire.OutPoint) {}, diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 90f30e58..6cd4389a 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6809,11 +6809,14 @@ func (lc *LightningChannel) CalcFee(feeRate chainfee.SatPerKWeight) btcutil.Amou // MaxFeeRate returns the maximum fee rate given an allocation of the channel // initiator's spendable balance. This can be useful to determine when we should -// stop proposing fee updates that exceed our maximum allocation. +// stop proposing fee updates that exceed our maximum allocation. We also take +// a fee rate cap that should be used for anchor type channels. // // NOTE: This should only be used for channels in which the local commitment is // the initiator. -func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWeight { +func (lc *LightningChannel) MaxFeeRate(maxAllocation float64, + maxAnchorFeeRate chainfee.SatPerKWeight) chainfee.SatPerKWeight { + lc.RLock() defer lc.RUnlock() @@ -6828,9 +6831,16 @@ func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWe // Ensure the fee rate doesn't dip below the fee floor. _, weight := lc.availableBalance() maxFeeRate := maxFee / (float64(weight) / 1000) - return chainfee.SatPerKWeight( + feeRate := chainfee.SatPerKWeight( math.Max(maxFeeRate, float64(chainfee.FeePerKwFloor)), ) + + // Cap anchor fee rates. + if lc.channelState.ChanType.HasAnchors() && feeRate > maxAnchorFeeRate { + return maxAnchorFeeRate + } + + return feeRate } // RemoteNextRevocation returns the channelState's RemoteNextRevocation. diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index b69b2faf..ead91598 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -7996,6 +7996,19 @@ func TestForceCloseBorkedState(t *testing.T) { func TestChannelMaxFeeRate(t *testing.T) { t.Parallel() + assertMaxFeeRate := func(c *LightningChannel, + maxAlloc float64, anchorMax, expFeeRate chainfee.SatPerKWeight) { + + t.Helper() + + maxFeeRate := c.MaxFeeRate(maxAlloc, anchorMax) + if maxFeeRate != expFeeRate { + t.Fatalf("expected max fee rate of %v with max "+ + "allocation of %v, got %v", expFeeRate, + maxAlloc, maxFeeRate) + } + } + aliceChannel, _, cleanUp, err := CreateTestChannels( channeldb.SingleFunderTweaklessBit, ) @@ -8004,21 +8017,32 @@ func TestChannelMaxFeeRate(t *testing.T) { } defer cleanUp() - assertMaxFeeRate := func(maxAlloc float64, - expFeeRate chainfee.SatPerKWeight) { + assertMaxFeeRate(aliceChannel, 1.0, 0, 690607734) + assertMaxFeeRate(aliceChannel, 0.001, 0, 690607) + assertMaxFeeRate(aliceChannel, 0.000001, 0, 690) + assertMaxFeeRate(aliceChannel, 0.0000001, 0, chainfee.FeePerKwFloor) - maxFeeRate := aliceChannel.MaxFeeRate(maxAlloc) - if maxFeeRate != expFeeRate { - t.Fatalf("expected max fee rate of %v with max "+ - "allocation of %v, got %v", expFeeRate, - maxAlloc, maxFeeRate) - } + // Check that anchor channels are capped at their max fee rate. + anchorChannel, _, cleanUp, err := CreateTestChannels( + channeldb.SingleFunderTweaklessBit | channeldb.AnchorOutputsBit, + ) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) } + defer cleanUp() - assertMaxFeeRate(1.0, 690607734) - assertMaxFeeRate(0.001, 690607) - assertMaxFeeRate(0.000001, 690) - assertMaxFeeRate(0.0000001, chainfee.FeePerKwFloor) + // Anchor commitments are heavier, hence will the same allocation lead + // to slightly lower fee rates. + assertMaxFeeRate( + anchorChannel, 1.0, chainfee.FeePerKwFloor, + chainfee.FeePerKwFloor, + ) + assertMaxFeeRate(anchorChannel, 0.001, 1000000, 444839) + assertMaxFeeRate(anchorChannel, 0.001, 300000, 300000) + assertMaxFeeRate(anchorChannel, 0.000001, 700, 444) + assertMaxFeeRate( + anchorChannel, 0.0000001, 1000000, chainfee.FeePerKwFloor, + ) } // TestChannelFeeRateFloor asserts that valid commitments can be proposed and diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 9dbf19b1..4d58822d 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -17,6 +17,11 @@ import ( // anchorSize is the constant anchor output size. const anchorSize = btcutil.Amount(330) +// DefaultAnchorsCommitMaxFeeRateSatPerVByte is the default max fee rate in +// sat/vbyte the initiator will use for anchor channels. This should be enough +// to ensure propagation before anchoring down the commitment transaction. +const DefaultAnchorsCommitMaxFeeRateSatPerVByte = 10 + // CommitmentKeyRing holds all derived keys needed to construct commitment and // HTLC transactions. The keys are derived differently depending whether the // commitment transaction is ours or the remote peer's. Private keys associated diff --git a/peer/brontide.go b/peer/brontide.go index d3089313..49c3d8e4 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -294,6 +294,10 @@ type Config struct { // commitment fee. This only applies for the initiator of the channel. MaxChannelFeeAllocation float64 + // MaxAnchorsCommitFeeRate is the maximum fee rate we'll use as an + // initiator for anchor channel commitments. + MaxAnchorsCommitFeeRate chainfee.SatPerKWeight + // ServerPubKey is the serialized, compressed public key of our lnd node. // It is used to determine which policy (channel edge) to pass to the // ChannelLink. @@ -815,6 +819,7 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint, TowerClient: towerClient, MaxOutgoingCltvExpiry: p.cfg.MaxOutgoingCltvExpiry, MaxFeeAllocation: p.cfg.MaxChannelFeeAllocation, + MaxAnchorsCommitFeeRate: p.cfg.MaxAnchorsCommitFeeRate, NotifyActiveLink: p.cfg.ChannelNotifier.NotifyActiveLinkEvent, NotifyActiveChannel: p.cfg.ChannelNotifier.NotifyActiveChannelEvent, NotifyInactiveChannel: p.cfg.ChannelNotifier.NotifyInactiveChannelEvent, diff --git a/server.go b/server.go index 5291b566..d59c25a1 100644 --- a/server.go +++ b/server.go @@ -1185,6 +1185,8 @@ func newServer(cfg *Config, listenAddrs []net.Addr, NotifyPendingOpenChannelEvent: s.channelNotifier.NotifyPendingOpenChannelEvent, EnableUpfrontShutdown: cfg.EnableUpfrontShutdown, RegisteredChains: cfg.registeredChains, + MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte( + s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(), }) if err != nil { return nil, err @@ -3119,7 +3121,9 @@ func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq, UnsafeReplay: s.cfg.UnsafeReplay, MaxOutgoingCltvExpiry: s.cfg.MaxOutgoingCltvExpiry, MaxChannelFeeAllocation: s.cfg.MaxChannelFeeAllocation, - Quit: s.quit, + MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte( + s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(), + Quit: s.quit, } copy(pCfg.PubKeyBytes[:], peerAddr.IdentityKey.SerializeCompressed()) From 99bbd992d2f9a0e4d1c23dd6dab21919402865a8 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 14 Dec 2020 21:07:12 +0100 Subject: [PATCH 2/3] sample config: add no-anchors and max-commit-fee-rate-anchors --- sample-lnd.conf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sample-lnd.conf b/sample-lnd.conf index fc788a7b..85d69c82 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -307,6 +307,11 @@ ; values are within [0.1, 1]. (default: 0.5) ; max-channel-fee-allocation=0.9 +; The maximum fee rate in sat/vbyte that will be used for commitments of +; channels of the anchors type. Must be large enough to ensure transaction +; propagation (default: 10) +; max-commit-fee-rate-anchors=5 + ; 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 ; previously active version of lnd. From 413ece2ccc225d8172d21295abdaae744d8922a5 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 15 Dec 2020 11:22:14 +0100 Subject: [PATCH 3/3] itest: account for capped anchor commit fee rate in balance calc --- lntest/itest/lnd_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index b22bcf54..83e07e7f 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1166,8 +1166,12 @@ func (c commitType) calcStaticFee(numHTLCs int) btcutil.Amount { // The anchor commitment type is slightly heavier, and we must also add // the value of the two anchors to the resulting fee the initiator - // pays. + // pays. In addition the fee rate is capped at 10 sat/vbyte for anchor + // channels. if c == commitTypeAnchors { + feePerKw = chainfee.SatPerKVByte( + lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte * 1000, + ).FeePerKWeight() commitWeight = input.AnchorCommitWeight anchors = 2 * anchorSize }