Merge pull request #9669 from Roasbeef/rbf-taproot-downgrade

multi: downgrade to legacy coop close for taproot channels
This commit is contained in:
Olaoluwa Osuntokun authored and GitHub committed 2025-04-04 14:38:36 -07:00
commit 580935a4a4
6 files changed
+133 -67

No files matched your search

+4
View File
@@ -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,
+96 -44
View File
@@ -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.
+1 -1
View File
@@ -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.
+6
View File
@@ -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{
+26 -11
View File
@@ -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
}
-11
View File
@@ -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(),