Merge pull request #6770 from Roasbeef/co-op-close-with-more-spec-innit

lnwallet/chancloser: remove the commit fee clamp, introduce max fee

Author: Roasbeef

docs/release-notes/release-notes-0.15.1.md

@@ -1,6 +1,10 @@
 # Release Notes
 
-## Protocol Extensions
+## Protocol/Spec Updates
+
+* [We'll now no longer clamp the co-op close fee to the commitment
+ fee](https://github.com/lightningnetwork/lnd/pull/6770). Instead, if users are
+ the initiator, they can now specify a max fee that should be respected.
 
 ### Zero-Conf Channel Opens
 * [Introduces support for zero-conf channel opens and non-zero-conf option_scid_alias channels.](https://github.com/lightningnetwork/lnd/pull/5955)

htlcswitch/switch.go

@@ -106,6 +106,12 @@ type ChanClose struct {
 	// process for the cooperative closure transaction kicks off.
 	TargetFeePerKw chainfee.SatPerKWeight
 
+	// MaxFee is the highest fee the caller is willing to pay.
+	//
+	// NOTE: This field is only respected if the caller is the initiator of
+	// the channel.
+	MaxFee chainfee.SatPerKWeight
+
 	// DeliveryScript is an optional delivery script to pay funds out to.
 	DeliveryScript lnwire.DeliveryAddress
 
@@ -1656,7 +1662,7 @@ func (s *Switch) teardownCircuit(pkt *htlcPacket) error {
 // optional parameter which sets a user specified script to close out to.
 func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
 	closeType contractcourt.ChannelCloseType,
-	targetFeePerKw chainfee.SatPerKWeight,
+	targetFeePerKw, maxFee chainfee.SatPerKWeight,
 	deliveryScript lnwire.DeliveryAddress) (chan interface{}, chan error) {
 
 	// TODO(roasbeef) abstract out the close updates.
@@ -1668,6 +1674,7 @@ func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
 		ChanPoint:      chanPoint,
 		Updates:        updateChan,
 		TargetFeePerKw: targetFeePerKw,
+		MaxFee:         maxFee,
 		DeliveryScript: deliveryScript,
 		Err:            errChan,
 	}

lnrpc/lightning.pb.go

@@ -1910,6 +1910,11 @@ message CloseChannelRequest {
     // A manual fee rate set in sat/vbyte that should be used when crafting the
     // closure transaction.
     uint64 sat_per_vbyte = 6;
+
+    // The maximum fee rate the closer is willing to pay.
+    //
+    // NOTE: This field is only respected if we're the initiator of the channel.
+    uint64 max_fee_per_vbyte = 7;
 }
 
 message CloseStatusUpdate {

lnrpc/lightning.proto

@@ -811,6 +811,14 @@
             "required": false,
             "type": "string",
             "format": "uint64"
+          },
+          {
+            "name": "max_fee_per_vbyte",
+            "description": "The maximum fee rate the closer is willing to pay.\n\nNOTE: This field is only respected if we're the initiator of the channel.",
+            "in": "query",
+            "required": false,
+            "type": "string",
+            "format": "uint64"
           }
         ],
         "tags": [

lnrpc/lightning.swagger.json

@@ -6,10 +6,12 @@ import (
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/txscript"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightningnetwork/lnd/htlcswitch"
+	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/labels"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
@@ -35,6 +37,12 @@ var (
 	// shutdown script previously set for that party.
 	ErrUpfrontShutdownScriptMismatch = fmt.Errorf("shutdown script does not " +
 		"match upfront shutdown script")
+
+	// ErrProposalExeceedsMaxFee is returned when as the initiator, the
+	// latest fee proposal sent by the responder exceed our max fee.
+	// responder.
+	ErrProposalExeceedsMaxFee = fmt.Errorf("latest fee proposal exceeds " +
+		"max fee")
 )
 
 // closeState represents all the possible states the channel closer state
@@ -73,11 +81,63 @@ const (
 	closeFinished
 )
 
+const (
+	// defaultMaxFeeMultiplier is a multiplier we'll apply to the ideal fee
+	// of the initiator, to decide when the negotiated fee is too high. By
+	// default, we want to bail out if we attempt to negotiate a fee that's
+	// 3x higher than our max fee.
+	defaultMaxFeeMultiplier = 3
+)
+
+// Channel abstracts away from the core channel state machine by exposing an
+// interface that requires only the methods we need to carry out the channel
+// closing process.
+type Channel interface {
+	// CalcFee returns the absolute fee for the given fee rate.
+	CalcFee(chainfee.SatPerKWeight) btcutil.Amount
+
+	// ChannelPoint returns the channel point of the target channel.
+	ChannelPoint() *wire.OutPoint
+
+	// MarkCoopBroadcasted persistently marks that the channel close
+	// transaction has been broadcast.
+	MarkCoopBroadcasted(*wire.MsgTx, bool) error
+
+	// IsInitiator returns true we are the initiator of the channel.
+	IsInitiator() bool
+
+	// ShortChanID returns the scid of the channel.
+	ShortChanID() lnwire.ShortChannelID
+
+	// AbsoluteThawHeight returns the absolute thaw height of the channel.
+	// If the channel is pending, or an unconfirmed zero conf channel, then
+	// an error should be returned.
+	AbsoluteThawHeight() (uint32, error)
+
+	// RemoteUpfrontShutdownScript returns the upfront shutdown script of
+	// the remote party. If the remote party didn't specify such a script,
+	// an empty delivery address should be returned.
+	RemoteUpfrontShutdownScript() lnwire.DeliveryAddress
+
+	// CreateCloseProposal creates a new co-op close proposal in the form
+	// of a valid signature, the chainhash of the final txid, and our final
+	// balance in the created state.
+	CreateCloseProposal(proposedFee btcutil.Amount, localDeliveryScript []byte,
+		remoteDeliveryScript []byte) (input.Signature, *chainhash.Hash,
+		btcutil.Amount, error)
+
+	// CompleteCooperativeClose persistently "completes" the cooperative
+	// close by producing a fully signed co-op close transaction.
+	CompleteCooperativeClose(localSig, remoteSig input.Signature,
+		localDeliveryScript, remoteDeliveryScript []byte,
+		proposedFee btcutil.Amount) (*wire.MsgTx, btcutil.Amount, error)
+}
+
 // ChanCloseCfg holds all the items that a ChanCloser requires to carry out its
 // duties.
 type ChanCloseCfg struct {
 	// Channel is the channel that should be closed.
-	Channel *lnwallet.LightningChannel
+	Channel Channel
 
 	// BroadcastTx broadcasts the passed transaction to the network.
 	BroadcastTx func(*wire.MsgTx, string) error
@@ -89,6 +149,10 @@ type ChanCloseCfg struct {
 	// Disconnect will disconnect from the remote peer in this close.
 	Disconnect func() error
 
+	// MaxFee, is non-zero represents the highest fee that the initiator is
+	// willing to pay to close the channel.
+	MaxFee chainfee.SatPerKWeight
+
 	// Quit is a channel that should be sent upon in the occasion the state
 	// machine should cease all progress and shutdown.
 	Quit chan struct{}
@@ -122,6 +186,11 @@ type ChanCloser struct {
 	// offer when starting negotiation. This will be used as a baseline.
 	idealFeeSat btcutil.Amount
 
+	// maxFee is the highest fee the initiator is willing to pay to close
+	// out the channel. This is either a use specified value, or a default
+	// multiplier based of the initial starting ideal fee.
+	maxFee btcutil.Amount
+
 	// lastFeeProposal is the last fee that we proposed to the remote party.
 	// We'll use this as a pivot point to ratchet our next offer up, down, or
 	// simply accept the remote party's prior offer.
@@ -162,23 +231,16 @@ func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte,
 	idealFeePerKw chainfee.SatPerKWeight, negotiationHeight uint32,
 	closeReq *htlcswitch.ChanClose, locallyInitiated bool) *ChanCloser {
 
-	// Given the target fee-per-kw, we'll compute what our ideal _total_ fee
-	// will be starting at for this fee negotiation.
-	//
-	// TODO(roasbeef): should factor in minimal commit
+	// Given the target fee-per-kw, we'll compute what our ideal _total_
+	// fee will be starting at for this fee negotiation.
 	idealFeeSat := cfg.Channel.CalcFee(idealFeePerKw)
 
-	// If this fee is greater than the fee currently present within the
-	// commitment transaction, then we'll clamp it down to be within the proper
-	// range.
-	//
-	// TODO(roasbeef): clamp fee func?
-	channelCommitFee := cfg.Channel.StateSnapshot().CommitFee
-	if idealFeeSat > channelCommitFee {
-		chancloserLog.Infof("Ideal starting fee of %v is greater than commit "+
-			"fee of %v, clamping", int64(idealFeeSat), int64(channelCommitFee))
-
-		idealFeeSat = channelCommitFee
+	// 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.
+	maxFee := idealFeeSat * defaultMaxFeeMultiplier
+	if cfg.MaxFee > 0 {
+		maxFee = cfg.Channel.CalcFee(cfg.MaxFee)
 	}
 
 	chancloserLog.Infof("Ideal fee for closure of ChannelPoint(%v) is: %v sat",
@@ -193,6 +255,7 @@ func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte,
 		cfg:                 cfg,
 		negotiationHeight:   negotiationHeight,
 		idealFeeSat:         idealFeeSat,
+		maxFee:              maxFee,
 		localDeliveryScript: deliveryScript,
 		priorFeeOffers:      make(map[btcutil.Amount]*lnwire.ClosingSigned),
 		locallyInitiated:    locallyInitiated,
@@ -282,9 +345,13 @@ func (c *ChanCloser) CloseRequest() *htlcswitch.ChanClose {
 	return c.closeReq
 }
 
-// Channel returns the channel stored in the config.
+// Channel returns the channel stored in the config as a
+// *lnwallet.LightningChannel.
+//
+// NOTE: This method will PANIC if the underlying channel implementation isn't
+// the desired type.
 func (c *ChanCloser) Channel() *lnwallet.LightningChannel {
-	return c.cfg.Channel
+	return c.cfg.Channel.(*lnwallet.LightningChannel)
 }
 
 // NegotiationHeight returns the negotiation height.
@@ -352,9 +419,8 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message,
 		// initiator of the channel opening, then we'll deny their close
 		// attempt.
 		chanInitiator := c.cfg.Channel.IsInitiator()
-		chanState := c.cfg.Channel.State()
 		if !chanInitiator {
-			absoluteThawHeight, err := chanState.AbsoluteThawHeight()
+			absoluteThawHeight, err := c.cfg.Channel.AbsoluteThawHeight()
 			if err != nil {
 				return nil, false, err
 			}
@@ -483,9 +549,10 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message,
 			feeProposal := calcCompromiseFee(c.chanPoint, c.idealFeeSat,
 				c.lastFeeProposal, remoteProposedFee,
 			)
-			if feeProposal > c.idealFeeSat*3 {
-				return nil, false, fmt.Errorf("couldn't find" +
-					" compromise fee")
+			if c.cfg.Channel.IsInitiator() && feeProposal > c.maxFee {
+				return nil, false, fmt.Errorf("%w: %v > %v",
+					ErrProposalExeceedsMaxFee, feeProposal,
+					c.maxFee)
 			}
 
 			// With our new fee proposal calculated, we'll craft a new close