From f458844412f6c99170cd83f56b74f90579a36708 Mon Sep 17 00:00:00 2001 From: ziggie Date: Sun, 9 Feb 2025 12:29:55 +0100 Subject: [PATCH] lnd: add max fee rate check to closechannel rpc --- itest/list_on_test.go | 4 ++ itest/lnd_channel_backup_test.go | 10 ++-- itest/lnd_coop_close_with_htlcs_test.go | 74 +++++++++++++++++++++++++ itest/lnd_funding_test.go | 10 +++- lntest/harness.go | 8 +-- lnwallet/chancloser/chancloser.go | 3 + rpcserver.go | 12 ++++ 7 files changed, 109 insertions(+), 12 deletions(-) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 2cf71feec..091d306e6 100644 --- a/itest/list_on_test.go +++ b/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, diff --git a/itest/lnd_channel_backup_test.go b/itest/lnd_channel_backup_test.go index 5baf3e1a1..8b12f4fdc 100644 --- a/itest/lnd_channel_backup_test.go +++ b/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") diff --git a/itest/lnd_coop_close_with_htlcs_test.go b/itest/lnd_coop_close_with_htlcs_test.go index 5f00e5da9..8707fada7 100644 --- a/itest/lnd_coop_close_with_htlcs_test.go +++ b/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) +} diff --git a/itest/lnd_funding_test.go b/itest/lnd_funding_test.go index bde5bebc3..c2704c3f4 100644 --- a/itest/lnd_funding_test.go +++ b/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 diff --git a/lntest/harness.go b/lntest/harness.go index fe0caa84d..c89f773b3 100644 --- a/lntest/harness.go +++ b/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 diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 5081f0beb..0751c2906 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/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)) diff --git a/rpcserver.go b/rpcserver.go index 75a864bcd..30664eac7 100644 --- a/rpcserver.go +++ b/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 }