diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index 23b93a0fd0d..40091e737d4 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -22,7 +22,7 @@ # Bug Fixes -* [Fixed `OpenChannel` with +- [Fixed `OpenChannel` with `fund_max`](https://github.com/lightningnetwork/lnd/pull/10488) to use the protocol-level maximum channel size instead of the user-configured `maxchansize`. The `maxchansize` config option is intended only for limiting @@ -88,6 +88,10 @@ RBF close state machine does not yet thread through the `AuxCloser` hook that overlay channels rely on to build aux-aware close transactions. +* [Fixed coop close fee baseline for channels with auxiliary close outputs](https://github.com/lightningnetwork/lnd/pull/10615) + by including extra outputs in initial fee estimation, preventing underpriced + taproot/custom channel cooperative closes from failing mempool acceptance. + # New Features - [Basic Support](https://github.com/lightningnetwork/lnd/pull/9868) for onion diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 6e5eaf39356..991f1ad6f36 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -253,9 +253,10 @@ type ChanCloser struct { } // calcCoopCloseFee computes an "ideal" absolute co-op close fee given the -// delivery scripts of both parties and our ideal fee rate. +// delivery scripts of both parties, any extra (e.g. auxiliary) close outputs +// to include in the weight estimate, and our ideal fee rate. func calcCoopCloseFee(chanType channeldb.ChannelType, - localOutput, remoteOutput *wire.TxOut, + localOutput, remoteOutput *wire.TxOut, extraOutputs []*wire.TxOut, idealFeeRate chainfee.SatPerKWeight) btcutil.Amount { var weightEstimator input.TxWeightEstimator @@ -276,6 +277,9 @@ func calcCoopCloseFee(chanType channeldb.ChannelType, if remoteOutput != nil { weightEstimator.AddTxOutput(remoteOutput) } + for _, extraOutput := range extraOutputs { + weightEstimator.AddTxOutput(extraOutput) + } totalWeight := weightEstimator.Weight() @@ -295,7 +299,64 @@ func (d *SimpleCoopFeeEstimator) EstimateFee(chanType channeldb.ChannelType, localTxOut, remoteTxOut *wire.TxOut, idealFeeRate chainfee.SatPerKWeight) btcutil.Amount { - return calcCoopCloseFee(chanType, localTxOut, remoteTxOut, idealFeeRate) + return calcCoopCloseFee( + chanType, localTxOut, remoteTxOut, nil, idealFeeRate, + ) +} + +// estimateCloseFee computes a close fee for the given fee rate while taking +// into account any optional auxiliary close outputs. +func (c *ChanCloser) estimateCloseFee(localTxOut, remoteTxOut *wire.TxOut, + feeRate chainfee.SatPerKWeight) (btcutil.Amount, error) { + + // Historical behavior uses the default channel type here and doesn't + // differentiate between channel feature variants. + var defaultChanType channeldb.ChannelType + fee := c.cfg.FeeEstimator.EstimateFee( + defaultChanType, localTxOut, remoteTxOut, feeRate, + ) + + if c.cfg.AuxCloser.IsNone() { + return fee, nil + } + + // Aux output selection can depend on CloseFee. After we bump the fee, + // the aux closer can return a different output set (for example + // around dust thresholds). Run a second pass so the fee and output + // set are self-consistent, while keeping this bounded. + for range 2 { + auxOutputs, err := c.auxCloseOutputs(fee) + if err != nil { + return 0, err + } + + var extraOutputs []*wire.TxOut + auxOutputs.WhenSome(func(outs AuxCloseOutputs) { + extraOutputs = make( + []*wire.TxOut, 0, len(outs.ExtraCloseOutputs), + ) + for _, closeOutput := range outs.ExtraCloseOutputs { + txOut := closeOutput.TxOut + extraOutputs = append(extraOutputs, &txOut) + } + }) + + if len(extraOutputs) == 0 { + return fee, nil + } + + feeWithAuxOutputs := calcCoopCloseFee( + defaultChanType, localTxOut, remoteTxOut, + extraOutputs, feeRate, + ) + if feeWithAuxOutputs <= fee { + return fee, nil + } + + fee = feeWithAuxOutputs + } + + return fee, nil } // NewChanCloser creates a new instance of the channel closure given the passed @@ -326,8 +387,10 @@ func NewChanCloser(cfg ChanCloseCfg, deliveryScript DeliveryAddrWithKey, } // initFeeBaseline computes our ideal fee rate, and also the largest fee we'll -// accept given information about the delivery script of the remote party. -func (c *ChanCloser) initFeeBaseline() { +// accept given information about the delivery script of the remote party. It +// returns an error if the auxiliary close outputs cannot be enumerated to +// include in the fee estimate. +func (c *ChanCloser) initFeeBaseline() error { // Depending on if a balance ends up being dust or not, we'll pass a // nil TxOut into the EstimateFee call which can handle it. var localTxOut, remoteTxOut *wire.TxOut @@ -346,18 +409,25 @@ func (c *ChanCloser) initFeeBaseline() { // Given the target fee-per-kw, we'll compute what our ideal _total_ // fee will be starting at for this fee negotiation. - c.idealFeeSat = c.cfg.FeeEstimator.EstimateFee( - 0, localTxOut, remoteTxOut, c.idealFeeRate, + var err error + c.idealFeeSat, err = c.estimateCloseFee( + localTxOut, remoteTxOut, c.idealFeeRate, ) + if err != nil { + return err + } // When we're the initiator, we'll want to also factor in the highest // fee we want to pay. This'll either be 3x the ideal fee, or the // specified explicit max fee. c.maxFee = c.idealFeeSat * defaultMaxFeeMultiplier if c.cfg.MaxFee > 0 { - c.maxFee = c.cfg.FeeEstimator.EstimateFee( - 0, localTxOut, remoteTxOut, c.cfg.MaxFee, + c.maxFee, err = c.estimateCloseFee( + localTxOut, remoteTxOut, c.cfg.MaxFee, ) + if err != nil { + return err + } } // TODO(ziggie): Make sure the ideal fee is not higher than the max fee. @@ -366,6 +436,8 @@ func (c *ChanCloser) initFeeBaseline() { chancloserLog.Infof("Ideal fee for closure of ChannelPoint(%v) "+ "is: %v sat (max_fee=%v sat)", c.cfg.Channel.ChannelPoint(), int64(c.idealFeeSat), int64(c.maxFee)) + + return nil } // initChanShutdown begins the shutdown process by un-registering the channel, @@ -740,14 +812,17 @@ func (c *ChanCloser) BeginNegotiation() (fn.Option[lnwire.ClosingSigned], case closeAwaitingFlush: // Now that we know their desired delivery script, we can // compute what our max/ideal fee will be. - c.initFeeBaseline() + err := c.initFeeBaseline() + if err != nil { + return noClosingSigned, err + } // Before continuing, mark the channel as cooperatively closed // with a nil txn. Even though we haven't negotiated the final // txn, this guarantees that our listchannels rpc will be // externally consistent, and reflect that the channel is being // shutdown by the time the closing request returns. - err := c.cfg.Channel.MarkCoopBroadcasted( + err = c.cfg.Channel.MarkCoopBroadcasted( nil, c.closer, ) if err != nil { diff --git a/lnwallet/chancloser/chancloser_test.go b/lnwallet/chancloser/chancloser_test.go index 5d96b96f31b..3e2ada24a1f 100644 --- a/lnwallet/chancloser/chancloser_test.go +++ b/lnwallet/chancloser/chancloser_test.go @@ -21,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/lnutils" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + wallettypes "github.com/lightningnetwork/lnd/lnwallet/types" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/tlv" "github.com/stretchr/testify/require" @@ -308,6 +309,38 @@ func (m *mockMusigSession) ClosingNonce() (*musig2.Nonces, error) { }, nil } +type mockAuxChanCloser struct { + extraScript []byte +} + +func (m *mockAuxChanCloser) ShutdownBlob( + req wallettypes.AuxShutdownReq, +) (fn.Option[lnwire.CustomRecords], error) { + + return fn.None[lnwire.CustomRecords](), nil +} + +func (m *mockAuxChanCloser) AuxCloseOutputs( + desc wallettypes.AuxCloseDesc) (fn.Option[AuxCloseOutputs], error) { + + closeOutputs := []lnwallet.CloseOutput{{ + TxOut: wire.TxOut{ + PkScript: m.extraScript, + Value: 0, + }, + }} + + return fn.Some(AuxCloseOutputs{ + ExtraCloseOutputs: closeOutputs, + }), nil +} + +func (m *mockAuxChanCloser) FinalizeClose(desc wallettypes.AuxCloseDesc, + closeTx *wire.MsgTx) error { + + return nil +} + type mockCoopFeeEstimator struct { targetFee btcutil.Amount } @@ -375,13 +408,77 @@ func TestMaxFeeClamp(t *testing.T) { // We'll call initFeeBaseline early here since we need // the populate these internal variables. - chanCloser.initFeeBaseline() + require.NoError(t, chanCloser.initFeeBaseline()) require.Equal(t, test.maxFee, chanCloser.maxFee) }) } } +// TestInitFeeBaselineWithAuxCloseOutputs tests that aux close outputs are +// accounted for in the initial fee baseline calculation. +func TestInitFeeBaselineWithAuxCloseOutputs(t *testing.T) { + t.Parallel() + + localScript := bytes.Repeat([]byte{0x11}, 34) + remoteScript := bytes.Repeat([]byte{0x22}, 34) + extraScript := bytes.Repeat([]byte{0x33}, 34) + + channel := &mockChannel{ + initiator: true, + } + + newCloser := func(auxCloser fn.Option[AuxChanCloser]) *ChanCloser { + closer := NewChanCloser( + ChanCloseCfg{ + Channel: channel, + FeeEstimator: &SimpleCoopFeeEstimator{}, + AuxCloser: auxCloser, + }, + DeliveryAddrWithKey{ + DeliveryAddress: localScript, + }, + chainfee.FeePerKwFloor, 0, nil, lntypes.Local, + ) + closer.remoteDeliveryScript = remoteScript + + return closer + } + + closerNoAux := newCloser(fn.None[AuxChanCloser]()) + require.NoError(t, closerNoAux.initFeeBaseline()) + + closerWithAux := newCloser(fn.Some[AuxChanCloser](&mockAuxChanCloser{ + extraScript: extraScript, + })) + require.NoError(t, closerWithAux.initFeeBaseline()) + + localOutput := &wire.TxOut{ + PkScript: localScript, + Value: 0, + } + remoteOutput := &wire.TxOut{ + PkScript: remoteScript, + Value: 0, + } + extraOutput := &wire.TxOut{ + PkScript: extraScript, + Value: 0, + } + + expectedFeeNoAux := calcCoopCloseFee( + 0, localOutput, remoteOutput, nil, chainfee.FeePerKwFloor, + ) + expectedFeeWithAux := calcCoopCloseFee( + 0, localOutput, remoteOutput, []*wire.TxOut{extraOutput}, + chainfee.FeePerKwFloor, + ) + + require.Equal(t, expectedFeeNoAux, closerNoAux.idealFeeSat) + require.Equal(t, expectedFeeWithAux, closerWithAux.idealFeeSat) + require.Greater(t, closerWithAux.idealFeeSat, closerNoAux.idealFeeSat) +} + // TestMaxFeeBailOut tests that once the negotiated fee rate rises above our // maximum fee, we'll return an error and refuse to process a co-op close // message. @@ -541,7 +638,7 @@ func TestTaprootFastClose(t *testing.T) { }, }, DeliveryAddrWithKey{}, idealFee, 0, nil, lntypes.Local, ) - aliceCloser.initFeeBaseline() + require.NoError(t, aliceCloser.initFeeBaseline()) bobCloser := NewChanCloser( ChanCloseCfg{ @@ -558,7 +655,7 @@ func TestTaprootFastClose(t *testing.T) { }, }, DeliveryAddrWithKey{}, idealFee, 0, nil, lntypes.Remote, ) - bobCloser.initFeeBaseline() + require.NoError(t, bobCloser.initFeeBaseline()) // With our set up complete, we'll now initialize the shutdown // procedure kicked off by Alice.