← Watch feed
peer: never use RBF coop close for aux channels
Public commit record
76/100 · Adequate
What the developer wrote
Authored by Jared Tobin
peer: never use RBF coop close for aux channels
The RBF coop close flow was selected purely from the peer-level
feature bits (rbfCoopCloseAllowed), with no per-channel exclusion.
The RBF close state machine does not invoke any of the aux closer
hooks: the Shutdown message it sends carries no aux custom records,
and the close transaction it negotiates contains no aux outputs. For
a taproot asset (overlay) channel this means the funding output --
which anchors the asset commitment -- is spent by a transaction that
does not re-commit the assets, irrevocably destroying them on-chain.
The aux closer then fails to finalize the confirmed close (it was
never asked to produce vPackets), which blocks the chain watcher's
coop close handler and leaves the channel stuck in waiting-close.
See lightninglabs/taproot-assets#2196 for an instance of this
happening in the wild.
Extend rbfCoopCloseAllowed to take the channel type: it now requires
the RBF feature bits AND that the channel type carries no tapscript
root, and is used at every site that chooses between the RBF closer
and the legacy negotiate closer. The RBF close actor's own eligibility
check is dropped entirely: an actor is only ever registered after
initRbfChanCloser has vetted the channel, so the check was redundant.
Aux channels now always fall back to the legacy closer, which is
aux-aware, regardless of the negotiated feature bits. Since no RBF
msg-router endpoint is registered for aux channels, an incoming Shutdown
from the peer likewise falls through to the legacy close handling. As a
backstop, initRbfChanCloser now refuses to construct an RBF closer for
aux channels outright.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit fixes a bug in LND where a new cooperative channel-closing mechanism (RBF coop close) was being used for special 'aux' channels that carry Taproot Assets. Those aux channels need extra steps to safely move the assets out, but the RBF close machine skipped those steps. If triggered, the closing transaction would spend the channel's funding output without preserving the asset commitment, effectively destroying the committed assets on-chain and leaving the channel stuck in a 'waiting-close' state. The fix forces aux channels to use the older, aux-aware close path even when both peers support the new RBF feature.
Recommended action
Upgrade LND nodes that may open or operate taproot asset overlay channels to a release containing this commit. Verify that no aux channels were closed via the RBF path while running affected code; affected closes would require manual recovery and asset-state inspection. Review other feature-gated flows for similar channel-type assumptions.
Security signals we found
01
Loss-of-funds / asset-destruction bug for aux (taproot asset overlay) channels
02
Channel-stuck / denial-of-service side effect (waiting-close state)
03
Feature-bit selection bypassed per-channel safety checks
04
Missing integration of aux closer hooks in RBF close state machine
05
Fix includes defense-in-depth: predicate change, redundant actor check removal, and constructor backstop
Evidence from the diff
The RBF cooperative close flow was selected based only on peer-level feature bits (RbfCoopCloseOptional / RbfCoopCloseOptionalStaging). It did not call aux closer hooks, so the Shutdown message carried no aux custom records and the negotiated close tx had no aux outputs. For taproot asset overlay channels (identified by a tapscript root in the channel type), spending the funding output without re-committing the assets would burn the asset commitment. The aux closer then could not finalize the confirmed close because it was never asked to produce vPackets, blocking the chain watcher’s coop close handler. The patch makes rbfCoopCloseAllowed() channel-type-aware, requiring both the RBF feature bit and the absence of a tapscript root. It applies this check at every site that chooses between RBF and legacy closer, drops the redundant eligibility check inside the RBF actor, and adds a backstop in initRbfChanCloser() that refuses to construct an RBF closer for aux channels. A unit test verifies the predicate.
Changed components
peer/brontide.gopeer/rbf_close_wrapper_actor.goRBF cooperative close state machineLegacy negotiate closer fallback pathTaproot Assets / aux channel close handlingInspect captured patch +145 / −30
diff --git a/peer/brontide.go b/peer/brontide.go
index 193f20d..e9c258d 100644
--- a/peer/brontide.go
+++ b/peer/brontide.go
@@ -1089,16 +1089,26 @@ func (p *Brontide) taprootShutdownAllowed() bool {
p.LocalFeatures().HasFeature(lnwire.ShutdownAnySegwitOptional)
}
-// rbfCoopCloseAllowed returns true if both parties have negotiated the new RBF
-// coop close feature.
-func (p *Brontide) rbfCoopCloseAllowed() bool {
+// rbfCoopCloseAllowed returns true if the new RBF coop close flow can be
+// used for a channel of the given type: both parties must have negotiated
+// the RBF coop close feature, and the channel must not be an aux channel.
+// Aux channels (taproot overlay channels, marked by a tapscript root) are
+// excluded even when both peers signal the RBF feature bit: the RBF close
+// state machine does not invoke any of the aux closer hooks, so closing an
+// aux channel through it would produce a close transaction without the aux
+// outputs (destroying the committed assets), which the aux closer is then
+// unable to finalize once the transaction confirms. Such channels fall back
+// to the legacy negotiate closer, which is aux-aware.
+func (p *Brontide) rbfCoopCloseAllowed(chanType chanstate.ChannelType) bool {
bothHaveBit := func(bit lnwire.FeatureBit) bool {
return p.RemoteFeatures().HasFeature(bit) &&
p.LocalFeatures().HasFeature(bit)
}
- return bothHaveBit(lnwire.RbfCoopCloseOptional) ||
+ featureNegotiated := bothHaveBit(lnwire.RbfCoopCloseOptional) ||
bothHaveBit(lnwire.RbfCoopCloseOptionalStaging)
+
+ return featureNegotiated && !chanType.HasTapscriptRoot()
}
// QuitSignal is a method that should return a channel which will be sent upon
@@ -1377,9 +1387,9 @@ func (p *Brontide) loadActiveChannels(chans []*chanstate.OpenChannel) (
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() {
+ // If we can use the new RBF close feature for this
+ // channel, we don't need to create the legacy closer.
+ if p.rbfCoopCloseAllowed(dbChan.ChanType) {
return
}
@@ -1455,9 +1465,9 @@ func (p *Brontide) loadActiveChannels(chans []*chanstate.OpenChannel) (
p.storeActiveChannel(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() {
+ // We're using the old co-op close for this channel, so we
+ // don't need to init the new RBF chan closer.
+ if !p.rbfCoopCloseAllowed(dbChan.ChanType) {
continue
}
@@ -3640,13 +3650,15 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) (
// the LocalUpfrontShutdownScript or generate a script.
c := lnChan.State()
_, err := c.BroadcastedCooperative()
- if err != nil && err != channeldb.ErrNoCloseTx {
- // An error other than ErrNoCloseTx was encountered.
+
+ // Any error other than "no close tx" is a real failure.
+ if err != nil && !errors.Is(err, channeldb.ErrNoCloseTx) {
return nil, err
- } else if err == nil && !p.rbfCoopCloseAllowed() {
- // This is a channel that doesn't support RBF coop close, and it
- // already had a coop close txn broadcast. As a result, we can
- // just exit here as all we can do is wait for it to confirm.
+ }
+
+ // A close tx was already broadcast and this channel can't use RBF
+ // coop close, so all we can do is wait for it to confirm.
+ if err == nil && !p.rbfCoopCloseAllowed(c.ChanType) {
return nil, nil
}
@@ -3681,10 +3693,10 @@ 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() {
+ // If the new RBF co-op close is negotiated and usable for this
+ // channel, then we'll init and start that state machine, skipping the
+ // steps for the negotiate machine below.
+ if p.rbfCoopCloseAllowed(c.ChanType) {
_, err := p.initRbfChanCloser(lnChan)
if err != nil {
return nil, fmt.Errorf("unable to init rbf chan "+
@@ -4172,6 +4184,15 @@ func (p *Brontide) chanFlushEventSentinel(chanCloser *chancloser.RbfChanCloser,
func (p *Brontide) initRbfChanCloser(
channel *lnwallet.LightningChannel) (*chancloser.RbfChanCloser, error) {
+ // Aux channels can't use the RBF coop close flow, as the state
+ // machine doesn't invoke the aux closer hooks needed to construct an
+ // aux-aware close transaction.
+ if channel.ChanType().HasTapscriptRoot() {
+ return nil, fmt.Errorf("ChannelPoint(%v): RBF coop close "+
+ "not supported for aux channels",
+ channel.ChannelPoint())
+ }
+
chanID := lnwire.NewChanIDFromOutPoint(channel.ChannelPoint())
link := p.fetchLinkFromKeyAndCid(chanID)
@@ -4595,7 +4616,7 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) {
// iteration, in which case we'll be obtaining a new
// transaction w/ a higher fee rate.
//
- case p.rbfCoopCloseAllowed():
+ case p.rbfCoopCloseAllowed(channel.ChanType()):
err = p.startRbfChanCloser(
newRPCShutdownInit(req), channel.ChannelPoint(),
)
@@ -5625,9 +5646,9 @@ 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() {
+ // We're using the old co-op close for this channel, so we don't need
+ // to init the new RBF chan closer.
+ if !p.rbfCoopCloseAllowed(lnChan.ChanType()) {
return nil
}
diff --git a/peer/brontide_test.go b/peer/brontide_test.go
index ee6efbb..910f0de 100644
--- a/peer/brontide_test.go
+++ b/peer/brontide_test.go
@@ -1838,3 +1838,100 @@ func TestHasActiveChannels(t *testing.T) {
require.False(t, peer.hasActiveChannels())
require.Equal(t, int32(0), peer.numActiveChans.Load())
}
+
+// TestRbfCoopCloseAllowed asserts that the per-channel RBF coop close
+// predicate excludes aux channels (channel types carrying a tapscript root)
+// even when both peers have negotiated the RBF coop close feature, while
+// permitting it for all other channel types.
+func TestRbfCoopCloseAllowed(t *testing.T) {
+ t.Parallel()
+
+ newPeer := func(local, remote *lnwire.RawFeatureVector) *Brontide {
+ return &Brontide{
+ cfg: Config{
+ Features: lnwire.NewFeatureVector(
+ local, lnwire.Features,
+ ),
+ },
+ remoteFeatures: lnwire.NewFeatureVector(
+ remote, lnwire.Features,
+ ),
+ }
+ }
+
+ var (
+ noBits = lnwire.NewRawFeatureVector()
+ rbfBit = lnwire.NewRawFeatureVector(
+ lnwire.RbfCoopCloseOptional,
+ )
+ stagingBit = lnwire.NewRawFeatureVector(
+ lnwire.RbfCoopCloseOptionalStaging,
+ )
+
+ overlayChan = chanstate.SimpleTaprootFeatureBit |
+ chanstate.TapscriptRootBit
+ )
+
+ tests := []struct {
+ name string
+ peer *Brontide
+ chanType chanstate.ChannelType
+ allowed bool
+ }{
+ {
+ name: "both signal, plain channel",
+ peer: newPeer(rbfBit, rbfBit),
+ chanType: chanstate.SingleFunderTweaklessBit,
+ allowed: true,
+ },
+ {
+ name: "both signal staging, plain channel",
+ peer: newPeer(stagingBit, stagingBit),
+ chanType: chanstate.SingleFunderTweaklessBit,
+ allowed: true,
+ },
+ {
+ name: "both signal, simple taproot channel",
+ peer: newPeer(rbfBit, rbfBit),
+ chanType: chanstate.SimpleTaprootFeatureBit,
+ allowed: true,
+ },
+ {
+ name: "both signal, aux (overlay) channel",
+ peer: newPeer(rbfBit, rbfBit),
+ chanType: overlayChan,
+ allowed: false,
+ },
+ {
+ name: "both signal staging, aux (overlay) channel",
+ peer: newPeer(stagingBit, stagingBit),
+ chanType: overlayChan,
+ allowed: false,
+ },
+ {
+ name: "only local signals, plain channel",
+ peer: newPeer(rbfBit, noBits),
+ chanType: chanstate.SingleFunderTweaklessBit,
+ allowed: false,
+ },
+ {
+ name: "neither signals, aux (overlay) channel",
+ peer: newPeer(noBits, noBits),
+ chanType: overlayChan,
+ allowed: false,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+
+ require.Equal(
+ t, test.allowed,
+ test.peer.rbfCoopCloseAllowed(
+ test.chanType,
+ ),
+ )
+ })
+ }
+}
diff --git a/peer/rbf_close_wrapper_actor.go b/peer/rbf_close_wrapper_actor.go
index 9cb1cde..90adf2f 100644
--- a/peer/rbf_close_wrapper_actor.go
+++ b/peer/rbf_close_wrapper_actor.go
@@ -131,12 +131,9 @@ func (r *rbfCloseActor) Receive(_ context.Context,
type retType = *CoopCloseUpdates
- // If RBF coop close isn't permitted, then we'll return an error.
- if !r.chanPeer.rbfCoopCloseAllowed() {
- return fn.Errf[retType]("rbf coop close not enabled for " +
- "channel")
- }
-
+ // Note that no eligibility check is needed here: an actor is only
+ // ever registered after initRbfChanCloser has vetted the channel for
+ // RBF coop close.
closeUpdates := &CoopCloseUpdates{
UpdateChan: make(chan interface{}, 1),
ErrChan: make(chan error, 1),
Risk score
Our methodology →Why this scored 74/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.