lnwallet/chancloser: account for aux close outputs in initial coop close fee baseline

Author: darioAnongba

docs/release-notes/release-notes-0.21.0.md

@@ -21,7 +21,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
@@ -69,12 +69,16 @@
   channel that was only expected to be used for a single message. The erring
   goroutine would block on the second send, leading to a deadlock at shutdown.
 
-* [Fixed `lncli unlock` to wait until the wallet is ready to be
+- [Fixed `lncli unlock` to wait until the wallet is ready to be
   unlocked](https://github.com/lightningnetwork/lnd/pull/10536)
   before sending the unlock request. The command now reports wallet state
   transitions during startup, avoiding lost unlocks during slow database
   initialization.
 
+- [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 for [onion messaging forwarding](https://github.com/lightningnetwork/lnd/pull/9868) 

lnwallet/chancloser/chancloser.go

@@ -255,7 +255,7 @@ type ChanCloser struct {
 // calcCoopCloseFee computes an "ideal" absolute co-op close fee given the
 // delivery scripts of both parties 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 +276,9 @@ func calcCoopCloseFee(chanType channeldb.ChannelType,
 	if remoteOutput != nil {
 		weightEstimator.AddTxOutput(remoteOutput)
 	}
+	for _, extraOutput := range extraOutputs {
+		weightEstimator.AddTxOutput(extraOutput)
+	}
 
 	totalWeight := weightEstimator.Weight()
 
@@ -295,7 +298,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 to
+	// account for extra output weight, 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
@@ -327,7 +387,7 @@ 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() {
+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 +406,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 +433,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 +809,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 {

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"
@@ -300,6 +301,37 @@ func (m *mockMusigSession) ClosingNonce() (*musig2.Nonces, error) {
 func (m *mockMusigSession) InitRemoteNonce(nonce *musig2.Nonces) {
 }
 
+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
 }
@@ -367,13 +399,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.
@@ -533,7 +629,7 @@ func TestTaprootFastClose(t *testing.T) {
 			},
 		}, DeliveryAddrWithKey{}, idealFee, 0, nil, lntypes.Local,
 	)
-	aliceCloser.initFeeBaseline()
+	require.NoError(t, aliceCloser.initFeeBaseline())
 
 	bobCloser := NewChanCloser(
 		ChanCloseCfg{
@@ -550,7 +646,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.