diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 168465fb8..e4dbc32ca 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -610,6 +610,10 @@ var allTestCases = []*lntest.TestCase{ Name: "coop close with htlcs", TestFunc: testCoopCloseWithHtlcs, }, + { + Name: "coop close with htlcs restart", + TestFunc: testCoopCloseWithHtlcsWithRestart, + }, { Name: "coop close exceeds max fee", TestFunc: testCoopCloseExceedsMaxFee, diff --git a/itest/lnd_coop_close_with_htlcs_test.go b/itest/lnd_coop_close_with_htlcs_test.go index ae9ec9089..06d4dc9ab 100644 --- a/itest/lnd_coop_close_with_htlcs_test.go +++ b/itest/lnd_coop_close_with_htlcs_test.go @@ -18,60 +18,110 @@ import ( "github.com/stretchr/testify/require" ) +type flagCombo struct { + isRbf bool + isTaproot bool + testName string + flags []string + commitType lnrpc.CommitmentType + private bool +} + +// createFlagCombos generates a slice of flagCombo structs representing +// different test configurations for RBF and Taproot channels. +func createFlagCombos() []flagCombo { + var testCases []flagCombo + for _, isRbf := range []bool{true, false} { + for _, isTaproot := range []bool{true, false} { + flagCombo := flagCombo{ + isRbf: isRbf, + isTaproot: isTaproot, + } + + var flags []string + + if isRbf { + flags = append(flags, node.CfgRbfClose...) + } + + if isTaproot { + flags = append(flags, node.CfgSimpleTaproot...) + flagCombo.commitType = lnrpc.CommitmentType_SIMPLE_TAPROOT //nolint:ll + flagCombo.private = true + } + + flagCombo.flags = flags + flagCombo.testName = fmt.Sprintf( + "is_rbf=%v is_taproot=%v", isRbf, isTaproot, + ) + + testCases = append(testCases, flagCombo) + } + } + + return testCases +} + // testCoopCloseWithHtlcs tests whether we can successfully issue a coop close -// request while there are still active htlcs on the link. In all the tests, we +// request while there are still active htlcs on the link. In this test, we // will set up an HODL invoice to suspend settlement. Then we will attempt to // close the channel which should appear as a noop for the time being. Then we // will have the receiver settle the invoice and observe that the channel gets -// torn down after settlement. +// torn down after settlement. This test covers scenarios without node restarts. func testCoopCloseWithHtlcs(ht *lntest.HarnessTest) { - rbfCoopFlags := []string{"--protocol.rbf-coop-close"} + testCases := createFlagCombos() - for _, isRbf := range []bool{true, false} { - testName := fmt.Sprintf("no restart is_rbf=%v", isRbf) - ht.Run(testName, func(t *testing.T) { + for _, testCase := range testCases { + testCase := testCase // Capture range variable. + ht.Run(testCase.testName, func(t *testing.T) { tt := ht.Subtest(t) - var flags []string - if isRbf { - flags = rbfCoopFlags - } + alice := ht.NewNodeWithCoins("Alice", testCase.flags) + bob := ht.NewNodeWithCoins("bob", testCase.flags) - alice := ht.NewNodeWithCoins("Alice", flags) - bob := ht.NewNodeWithCoins("bob", flags) - - coopCloseWithHTLCs(tt, alice, bob) - }) - } - - for _, isRbf := range []bool{true, false} { - testName := fmt.Sprintf("with restart is_rbf=%v", isRbf) - ht.Run(testName, func(t *testing.T) { - tt := ht.Subtest(t) - - var flags []string - if isRbf { - flags = rbfCoopFlags - } - - alice := ht.NewNodeWithCoins("Alice", flags) - bob := ht.NewNodeWithCoins("bob", flags) - - coopCloseWithHTLCsWithRestart(tt, alice, bob) + runCoopCloseWithHTLCsScenario(tt, alice, bob, testCase) }) } } -// coopCloseWithHTLCs tests the basic coop close scenario which occurs when one -// channel party initiates a channel shutdown while an HTLC is still pending on -// the channel. -func coopCloseWithHTLCs(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) { +// testCoopCloseWithHtlcsWithRestart tests whether we can successfully issue a +// coop close request while there are still active htlcs on the link, even +// after a node restart. In this test, we will set up an HODL invoice to +// suspend settlement. Then we will attempt to close the channel, restart the +// nodes, and then have the receiver settle the invoice, observing that the +// channel gets torn down after settlement and restart. +func testCoopCloseWithHtlcsWithRestart(ht *lntest.HarnessTest) { + testCases := createFlagCombos() + + for _, testCase := range testCases { + testCase := testCase // Capture range variable. + ht.Run(testCase.testName, func(t *testing.T) { + tt := ht.Subtest(t) + + alice := ht.NewNodeWithCoins("Alice", testCase.flags) + bob := ht.NewNodeWithCoins("bob", testCase.flags) + + runCoopCloseWithHTLCsWithRestartScenario( + tt, alice, bob, testCase, + ) + }) + } +} + +// runCoopCloseWithHTLCsScenario tests the basic coop close scenario which +// occurs when one channel party initiates a channel shutdown while an HTLC is +// still pending on the channel. +func runCoopCloseWithHTLCsScenario(ht *lntest.HarnessTest, alice, + bob *node.HarnessNode, testCase flagCombo) { + ht.ConnectNodes(alice, bob) // Here we set up a channel between Alice and Bob, beginning with a // balance on Bob's side. chanPoint := ht.OpenChannel(bob, alice, lntest.OpenChannelParams{ - Amt: btcutil.Amount(1000000), + Amt: btcutil.Amount(1000000), + CommitmentType: testCase.commitType, + Private: testCase.private, }) // Wait for Bob to understand that the channel is ready to use. @@ -153,12 +203,12 @@ func coopCloseWithHTLCs(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) { ht.AssertStreamChannelCoopClosed(alice, chanPoint, false, closeClient) } -// coopCloseWithHTLCsWithRestart also tests the coop close flow when an HTLC -// is still pending on the channel but this time it ensures that the shutdown -// process continues as expected even if a channel re-establish happens after -// one party has already initiated the shutdown. -func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest, alice, - bob *node.HarnessNode) { +// runCoopCloseWithHTLCsWithRestartScenario also tests the coop close flow when +// an HTLC is still pending on the channel but this time it ensures that the +// shutdown process continues as expected even if a channel re-establish +// happens after one party has already initiated the shutdown. +func runCoopCloseWithHTLCsWithRestartScenario(ht *lntest.HarnessTest, alice, + bob *node.HarnessNode, testCase flagCombo) { ht.ConnectNodes(alice, bob) @@ -167,8 +217,10 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest, alice, // so that we can assert that the correct delivery address gets used by // the channel close initiator. chanPoint := ht.OpenChannel(bob, alice, lntest.OpenChannelParams{ - Amt: btcutil.Amount(1000000), - PushAmt: btcutil.Amount(1000000 / 2), + Amt: btcutil.Amount(1000000), + PushAmt: btcutil.Amount(1000000 / 2), + CommitmentType: testCase.commitType, + Private: testCase.private, }) // Wait for Bob to understand that the channel is ready to use. diff --git a/lncfg/protocol.go b/lncfg/protocol.go index 71b81fe75..3c5220d72 100644 --- a/lncfg/protocol.go +++ b/lncfg/protocol.go @@ -37,7 +37,7 @@ type ProtocolOptions struct { // RbfCoopClose should be set if we want to signal that we support for // the new experimental RBF coop close feature. - RbfCoopClose bool `long:"rbf-coop-close" description:"if set, then lnd will signal that it supports the new RBF based coop close protocol"` + RbfCoopClose bool `long:"rbf-coop-close" description:"if set, then lnd will signal that it supports the new RBF based coop close protocol, taproot channels are not supported"` // NoAnchors should be set if we don't want to support opening or accepting // channels having the anchor commitment type. diff --git a/lntest/node/config.go b/lntest/node/config.go index d71723aac..168dc87ec 100644 --- a/lntest/node/config.go +++ b/lntest/node/config.go @@ -68,6 +68,12 @@ var ( "--protocol.simple-taproot-chans", } + // CfgRbfCoopClose specifies the config used to create a node that + // supports the new RBF close protocol. + CfgRbfClose = []string{ + "--protocol.rbf-coop-close", + } + // CfgZeroConf specifies the config used to create a node that uses the // zero-conf channel feature. CfgZeroConf = []string{ diff --git a/peer/brontide.go b/peer/brontide.go index 98414d7ef..bfc603ae8 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -1231,14 +1231,17 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( return nil, err } + isTaprootChan := lnChan.ChanType().IsTaproot() + var ( shutdownMsg fn.Option[lnwire.Shutdown] shutdownInfoErr error ) shutdownInfo.WhenSome(func(info channeldb.ShutdownInfo) { // If we can use the new RBF close feature, we don't - // need to create the legacy closer. - if p.rbfCoopCloseAllowed() { + // need to create the legacy closer. However for taproot + // channels, we'll continue to use the legacy closer. + if p.rbfCoopCloseAllowed() && !isTaprootChan { return } @@ -1315,8 +1318,10 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( p.activeChannels.Store(chanID, lnChan) // We're using the old co-op close, so we don't need to init - // the new RBF chan closer. - if !p.rbfCoopCloseAllowed() { + // the new RBF chan closer. If we have a taproot chan, then + // we'll also use the legacy type, so we don't need to make the + // new closer. + if !p.rbfCoopCloseAllowed() || isTaprootChan { continue } @@ -3268,6 +3273,8 @@ func chooseDeliveryScript(upfront, requested lnwire.DeliveryAddress, func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( *lnwire.Shutdown, error) { + isTaprootChan := lnChan.ChanType().IsTaproot() + // If this channel has status ChanStatusCoopBroadcasted and does not // have a closing transaction, then the cooperative close process was // started but never finished. We'll re-create the chanCloser state @@ -3320,8 +3327,8 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( // If the new RBF co-op close is negotiated, then we'll init and start // that state machine, skipping the steps for the negotiate machine - // below. - if p.rbfCoopCloseAllowed() { + // below. We don't support this close type for taproot channels though. + if p.rbfCoopCloseAllowed() && !isTaprootChan { _, err := p.initRbfChanCloser(lnChan) if err != nil { return nil, fmt.Errorf("unable to init rbf chan "+ @@ -4045,7 +4052,7 @@ func (p *Brontide) startRbfChanCloser(shutdown shutdownInit, chanID := lnwire.NewChanIDFromOutPoint(chanPoint) chanCloser, found := p.activeChanCloses.Load(chanID) if !found { - return fmt.Errorf("rbf can closer not found for channel %v", + return fmt.Errorf("rbf chan closer not found for channel %v", chanPoint) } @@ -4165,6 +4172,8 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { return } + isTaprootChan := channel.ChanType().IsTaproot() + switch req.CloseType { // A type of CloseRegular indicates that the user has opted to close // out this channel on-chain, so we execute the cooperative channel @@ -4176,7 +4185,10 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { // it to send the shutdown message. This also might be an RBF // iteration, in which case we'll be obtaining a new // transaction w/ a higher fee rate. - case p.rbfCoopCloseAllowed(): + // + // We don't support this close type for taproot channels yet + // however. + case !isTaprootChan && p.rbfCoopCloseAllowed(): err = p.startRbfChanCloser( newRPCShutdownInit(req), channel.ChannelPoint(), ) @@ -5135,9 +5147,12 @@ func (p *Brontide) addActiveChannel(c *lnpeer.NewChannel) error { "peer", chanPoint) } - // We're using the old co-op close, so we don't need to init the new - // RBF chan closer. - if !p.rbfCoopCloseAllowed() { + isTaprootChan := c.ChanType.IsTaproot() + + // We're using the old co-op close, so we don't need to init the new RBF + // chan closer. If this is a taproot channel, then we'll also fall + // through, as we don't support this type yet w/ rbf close. + if !p.rbfCoopCloseAllowed() || isTaprootChan { return nil } diff --git a/server.go b/server.go index 59c084f3a..e816c3ca4 100644 --- a/server.go +++ b/server.go @@ -612,17 +612,6 @@ func newServer(cfg *Config, listenAddrs []net.Addr, "aux controllers") } - // For now, the RBF coop close flag and the taproot channel type cannot - // be used together. - // - // TODO(roasbeef): fix - if cfg.ProtocolOptions.RbfCoopClose && - cfg.ProtocolOptions.TaprootChans { - - return nil, fmt.Errorf("RBF coop close and taproot " + - "channels cannot be used together") - } - //nolint:ll featureMgr, err := feature.NewManager(feature.Config{ NoTLVOnion: cfg.ProtocolOptions.LegacyOnion(),