Merge pull request #9491 from ziggie1984/closechannel-rpc

Allow coop closing a channel with HTLCs on it via lncli

Author: guggero

cmd/commands/commands.go

@@ -1011,6 +1011,11 @@ var closeChannelCommand = cli.Command{
 	comparison is the end boundary of the fee negotiation, if not specified
 	it's always x3 of the starting value. Increasing this value increases
 	the chance of a successful negotiation.
+	Moreover if the channel has active HTLCs on it, the coop close will
+	wait until all HTLCs are resolved and will not allow any new HTLCs on
+	the channel. The channel will appear as disabled in the listchannels
+	output. The command will block in that case until the channel close tx
+	is broadcasted.
 
 	In the case of a cooperative closure, one can manually set the address
 	to deliver funds to upon closure. This is optional, and may only be used
@@ -1042,8 +1047,10 @@ var closeChannelCommand = cli.Command{
 			Usage: "attempt an uncooperative closure",
 		},
 		cli.BoolFlag{
-			Name:  "block",
-			Usage: "block until the channel is closed",
+			Name: "block",
+			Usage: `block will wait for the channel to be closed,
+			"meaning that it will wait for the channel close tx to
+			get 1 confirmation.`,
 		},
 		cli.Int64Flag{
 			Name: "conf_target",
@@ -1117,6 +1124,9 @@ func closeChannel(ctx *cli.Context) error {
 		SatPerVbyte:     ctx.Uint64(feeRateFlag),
 		DeliveryAddress: ctx.String("delivery_addr"),
 		MaxFeePerVbyte:  ctx.Uint64("max_fee_rate"),
+		// This makes sure that a coop close will also be executed if
+		// active HTLCs are present on the channel.
+		NoWait: true,
 	}
 
 	// After parsing the request, we'll spin up a goroutine that will
@@ -1154,7 +1164,9 @@ func closeChannel(ctx *cli.Context) error {
 // executeChannelClose attempts to close the channel from a request. The closing
 // transaction ID is sent through `txidChan` as soon as it is broadcasted to the
 // network. The block boolean is used to determine if we should block until the
-// closing transaction receives all of its required confirmations.
+// closing transaction receives a confirmation of 1 block. The logging outputs
+// are sent to stderr to avoid conflicts with the JSON output of the command
+// and potential work flows which depend on a proper JSON output.
 func executeChannelClose(ctxc context.Context, client lnrpc.LightningClient,
 	req *lnrpc.CloseChannelRequest, txidChan chan<- string, block bool) error {
 
@@ -1173,22 +1185,40 @@ func executeChannelClose(ctxc context.Context, client lnrpc.LightningClient,
 
 		switch update := resp.Update.(type) {
 		case *lnrpc.CloseStatusUpdate_CloseInstant:
-			if req.NoWait {
-				return nil
+			fmt.Fprintln(os.Stderr, "Channel close successfully "+
+				"initiated")
+
+			pendingHtlcs := update.CloseInstant.NumPendingHtlcs
+			if pendingHtlcs > 0 {
+				fmt.Fprintf(os.Stderr, "Cooperative channel "+
+					"close waiting for %d HTLCs to be "+
+					"resolved before the close process "+
+					"can kick off\n", pendingHtlcs)
 			}
+
 		case *lnrpc.CloseStatusUpdate_ClosePending:
 			closingHash := update.ClosePending.Txid
 			txid, err := chainhash.NewHash(closingHash)
 			if err != nil {
 				return err
 			}
 
+			fmt.Fprintf(os.Stderr, "Channel close transaction "+
+				"broadcasted: %v\n", txid)
+
 			txidChan <- txid.String()
 
 			if !block {
 				return nil
 			}
+
+			fmt.Fprintln(os.Stderr, "Waiting for channel close "+
+				"confirmation ...")
+
 		case *lnrpc.CloseStatusUpdate_ChanClose:
+			fmt.Fprintln(os.Stderr, "Channel close successfully "+
+				"confirmed")
+
 			return nil
 		}
 	}

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

@@ -93,6 +93,10 @@
  config is added `disable-backup-archive`, with default set to false, to 
  determine if previous channel backups should be archived or not.
 
+* [The max fee rate](https://github.com/lightningnetwork/lnd/pull/9491) is now
+  respected when a coop close is initiated. Before the max fee rate would only
+  be effective for the remote party in the negotiation.
+
 ## Functional Enhancements
 * [Add ability](https://github.com/lightningnetwork/lnd/pull/8998) to paginate 
  wallet transactions.
@@ -137,6 +141,11 @@
   the misnomer of `chan_id` which was describing the short channel
   id to `scid` to represent what it really is.
 
+* [In the coop close](https://github.com/lightningnetwork/lnd/pull/9491) case
+  we always initiate the cooperative close flow even if there are HTLCs active
+  on the channel. LND will disable the channel for new HTLCs and kick off the
+  cooperative close flow automatically when the channel has no HTLCs left.
+
 # Improvements
 ## Functional Updates
 

htlcswitch/switch.go

@@ -1556,7 +1556,7 @@ out:
 			s.indexMtx.RUnlock()
 
 			peerPub := link.PeerPubKey()
-			log.Debugf("Requesting local channel close: peer=%v, "+
+			log.Debugf("Requesting local channel close: peer=%x, "+
 				"chan_id=%x", link.PeerPubKey(), chanID[:])
 
 			go s.cfg.LocalChannelClose(peerPub[:], req)

itest/list_on_test.go

@@ -602,6 +602,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "coop close with htlcs",
 		TestFunc: testCoopCloseWithHtlcs,
 	},
+	{
+		Name:     "coop close exceeds max fee",
+		TestFunc: testCoopCloseExceedsMaxFee,
+	},
 	{
 		Name:     "open channel locked balance",
 		TestFunc: testOpenChannelLockedBalance,

itest/lnd_channel_backup_test.go

@@ -283,14 +283,16 @@ func (c *chanRestoreScenario) testScenario(ht *lntest.HarnessTest,
 
 	// We don't get an error directly but only when reading the first
 	// message of the stream.
-	err := ht.CloseChannelAssertErr(
-		dave, &lnrpc.ChannelPoint{
+	req := &lnrpc.CloseChannelRequest{
+		ChannelPoint: &lnrpc.ChannelPoint{
 			FundingTxid: &lnrpc.ChannelPoint_FundingTxidStr{
 				FundingTxidStr: chanPointParts[0],
 			},
 			OutputIndex: uint32(chanPointIndex),
-		}, true,
-	)
+		},
+		Force: true,
+	}
+	err := ht.CloseChannelAssertErr(dave, req)
 	require.Contains(ht, err.Error(), "cannot close channel with state: ")
 	require.Contains(ht, err.Error(), "ChanStatusRestored")
 

itest/lnd_coop_close_with_htlcs_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/stretchr/testify/require"
 )
 
@@ -93,14 +94,17 @@ func coopCloseWithHTLCs(ht *lntest.HarnessTest) {
 	// closure is set up. Let's settle the invoice.
 	alice.RPC.SettleInvoice(preimage[:])
 
-	// Pull the instant update off the wire to clear the path for the
-	// close pending update.
-	_, err := closeClient.Recv()
+	// Pull the instant update off the wire and make sure the number of
+	// pending HTLCs is as expected.
+	update, err := closeClient.Recv()
 	require.NoError(ht, err)
+	closeInstant := update.GetCloseInstant()
+	require.NotNil(ht, closeInstant)
+	require.Equal(ht, closeInstant.NumPendingHtlcs, int32(1))
 
 	// Wait for the next channel closure update. Now that we have settled
 	// the only HTLC this should be imminent.
-	update, err := closeClient.Recv()
+	update, err = closeClient.Recv()
 	require.NoError(ht, err)
 
 	// This next update should be a GetClosePending as it should be the
@@ -243,3 +247,76 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) {
 	// Show that the address used is the one she requested.
 	require.Equal(ht, outputDetail.Address, newAddr.Address)
 }
+
+// testCoopCloseExceedsMaxFee tests that we fail the coop close process if
+// the max fee rate exceeds the expected fee rate for the initial closing fee
+// proposal.
+func testCoopCloseExceedsMaxFee(ht *lntest.HarnessTest) {
+	const chanAmt = 1000000
+
+	// Create a channel Alice->Bob.
+	chanPoints, nodes := ht.CreateSimpleNetwork(
+		[][]string{nil, nil}, lntest.OpenChannelParams{
+			Amt: chanAmt,
+		},
+	)
+
+	alice, _ := nodes[0], nodes[1]
+	chanPoint := chanPoints[0]
+
+	// Set the fee estimate for one block to 10 sat/vbyte.
+	ht.SetFeeEstimateWithConf(chainfee.SatPerVByte(10).FeePerKWeight(), 1)
+
+	// Have alice attempt to close the channel. We expect the initial fee
+	// rate to exceed the max fee rate for the closing transaction so we
+	// fail the closing process.
+	req := &lnrpc.CloseChannelRequest{
+		ChannelPoint:   chanPoint,
+		MaxFeePerVbyte: 5,
+		NoWait:         true,
+		TargetConf:     1,
+	}
+	err := ht.CloseChannelAssertErr(alice, req)
+	require.Contains(ht, err.Error(), "max_fee_per_vbyte (1250 sat/kw) is "+
+		"less than the required fee rate (2500 sat/kw)")
+
+	// Now close the channel with a appropriate max fee rate.
+	closeClient := alice.RPC.CloseChannel(&lnrpc.CloseChannelRequest{
+		ChannelPoint:   chanPoint,
+		NoWait:         true,
+		TargetConf:     1,
+		MaxFeePerVbyte: 10,
+	})
+
+	// Pull the instant update off the wire to clear the path for the
+	// close pending update. Moreover confirm that there are no pending
+	// HTLCs on the channel.
+	update, err := closeClient.Recv()
+	require.NoError(ht, err)
+	closeInstant := update.GetCloseInstant()
+	require.NotNil(ht, closeInstant)
+	require.Equal(ht, closeInstant.NumPendingHtlcs, int32(0))
+
+	// Wait for the channel to be closed.
+	update, err = closeClient.Recv()
+	require.NoError(ht, err)
+
+	// This next update should be a GetClosePending as it should be the
+	// negotiation of the coop close tx.
+	closePending := update.GetClosePending()
+	require.NotNil(ht, closePending)
+
+	// Convert the txid we get from the PendingUpdate to a Hash so we can
+	// wait for it to be mined.
+	var closeTxid chainhash.Hash
+	require.NoError(
+		ht, closeTxid.SetBytes(closePending.Txid),
+		"invalid closing txid",
+	)
+
+	// Wait for the close tx to be in the Mempool.
+	ht.AssertTxInMempool(closeTxid)
+
+	// Wait for it to get mined and finish tearing down.
+	ht.AssertStreamChannelCoopClosed(alice, chanPoint, false, closeClient)
+}

itest/lnd_funding_test.go

@@ -674,7 +674,10 @@ func runExternalFundingScriptEnforced(ht *lntest.HarnessTest) {
 	// First, we'll try to close the channel as Carol, the initiator. This
 	// should fail as a frozen channel only allows the responder to
 	// initiate a channel close.
-	err := ht.CloseChannelAssertErr(carol, chanPoint2, false)
+	req := &lnrpc.CloseChannelRequest{
+		ChannelPoint: chanPoint2,
+	}
+	err := ht.CloseChannelAssertErr(carol, req)
 	require.Contains(ht, err.Error(), "cannot co-op close frozen channel")
 
 	// Before Dave closes the channel, he needs to check the invoice is
@@ -831,7 +834,10 @@ func runExternalFundingTaproot(ht *lntest.HarnessTest) {
 	// First, we'll try to close the channel as Carol, the initiator. This
 	// should fail as a frozen channel only allows the responder to
 	// initiate a channel close.
-	err := ht.CloseChannelAssertErr(carol, chanPoint2, false)
+	req := &lnrpc.CloseChannelRequest{
+		ChannelPoint: chanPoint2,
+	}
+	err := ht.CloseChannelAssertErr(carol, req)
 	require.Contains(ht, err.Error(), "cannot co-op close frozen channel")
 
 	// Before Dave closes the channel, he needs to check the invoice is