lnd: add max fee rate check to closechannel rpc

Author: ziggie1984

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"
 )
 
@@ -246,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

lntest/harness.go

@@ -1296,14 +1296,10 @@ func (h *HarnessTest) ForceCloseChannel(hn *node.HarnessNode,
 // CloseChannelAssertErr closes the given channel and asserts an error
 // returned.
 func (h *HarnessTest) CloseChannelAssertErr(hn *node.HarnessNode,
-	cp *lnrpc.ChannelPoint, force bool) error {
+	req *lnrpc.CloseChannelRequest) error {
 
 	// Calls the rpc to close the channel.
-	closeReq := &lnrpc.CloseChannelRequest{
-		ChannelPoint: cp,
-		Force:        force,
-	}
-	stream := hn.RPC.CloseChannel(closeReq)
+	stream := hn.RPC.CloseChannel(req)
 
 	// Consume the "channel close" update in order to wait for the closing
 	// transaction to be broadcast, then wait for the closing tx to be seen

lnwallet/chancloser/chancloser.go

@@ -360,6 +360,9 @@ func (c *ChanCloser) initFeeBaseline() {
 		)
 	}
 
+	// TODO(ziggie): Make sure the ideal fee is not higher than the max fee.
+	// Either error out or cap the ideal fee at the max fee.
+
 	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))

rpcserver.go

@@ -2878,6 +2878,15 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest,
 		maxFee := chainfee.SatPerKVByte(
 			in.MaxFeePerVbyte * 1000,
 		).FeePerKWeight()
+
+		// In case the max fee was specified, we check if it's less than
+		// the initial fee rate and abort if it is.
+		if maxFee != 0 && maxFee < feeRate {
+			return fmt.Errorf("max_fee_per_vbyte (%v) is less "+
+				"than the required fee rate (%v)", maxFee,
+				feeRate)
+		}
+
 		updateChan, errChan = r.server.htlcSwitch.CloseLink(
 			chanPoint, contractcourt.CloseRegular, feeRate,
 			maxFee, deliveryScript,
@@ -2909,7 +2918,9 @@ out:
 		case err := <-errChan:
 			rpcsLog.Errorf("[closechannel] unable to close "+
 				"ChannelPoint(%v): %v", chanPoint, err)
+
 			return err
+
 		case closingUpdate := <-updateChan:
 			rpcClosingUpdate, err := createRPCCloseUpdate(
 				closingUpdate,
@@ -2948,6 +2959,7 @@ out:
 					"txid(%v)", h)
 				break out
 			}
+
 		case <-r.quit:
 			return nil
 		}