Skip to content

peer: never use RBF coop close for aux channels#10962

Open
jtobin wants to merge 2 commits into
lightningnetwork:masterfrom
jtobin:rbf-guard
Open

peer: never use RBF coop close for aux channels#10962
jtobin wants to merge 2 commits into
lightningnetwork:masterfrom
jtobin:rbf-guard

Conversation

@jtobin

@jtobin jtobin commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

The RBF coop close flow is selected purely from the peer-level feature bits: if both peers signal option_simple_close, every channel with that peer uses the RBF close state machine. But the RBF state machine has no aux-closer integration — it neither includes the aux records in the Shutdown message nor invokes the aux hooks when constructing the close transaction.

For a taproot overlay (asset) channel, the funding output commits to more than the musig2 key: it anchors a Taproot Assets commitment. Closing such a channel through the RBF flow produces a close transaction that spends the funding output without re-committing the assets, irrevocably destroying them on-chain. The aux closer then fails to finalize the confirmed close, which blocks the chain watcher's coop close handler and leaves the channel stuck in waiting-close.

The existing protection is a default only — server.go skips auto-enabling RBF when overlay channels are enabled — and is bypassed by an explicit protocol.rbf-coop-close=true, or by removing the overlay flag while still holding overlay channels. v0.21.0-beta.rc1 force-enabled RBF for any node with a taproot flag set, with no opt-out, but only that (pre-)release.

Fix

Make the exclusion structural rather than configurational: introduce rbfCoopCloseAllowedForChan, which requires the negotiated feature bits AND that the channel type carries no tapscript root, and use it at every site that chooses between the RBF closer and the legacy negotiate closer. The tapscript root bit is set exactly and only for overlay channels, and semantically marks any channel whose funding output commits to state a vanilla close transaction would destroy.

Aux channels now always fall back to the legacy negotiate closer (which is aux-aware), regardless of feature bits or configuration. Since no RBF msg-router endpoint is registered for them, an incoming Shutdown from the peer likewise falls through to the legacy handling. As a backstop, initRbfChanCloser refuses to construct an RBF closer for aux channels outright. Plain taproot channels are unaffected and retain full RBF coop close support.

Note this makes no attempt to support RBF closes for overlay channels — that requires threading the aux hooks through the RBF state machine and is left as follow-up work.

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.

Introduce rbfCoopCloseAllowedForChan, which requires the RBF feature
bits AND that the channel type carries no tapscript root, and use it
at every site that chooses between the RBF closer and the legacy
negotiate closer. 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.
@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Jul 10, 2026
@github-actions

Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

gh pr view | 3 files | 155 lines changed (140 additions, 15 deletions)

🔴 Critical (1 file)
  • peer/brontide.go (+39/-15) - Falls under peer/*: this PR modifies the coop-close closer-selection logic (rbfCoopCloseAllowedForChan, initRbfChanCloser, and the RBF-vs-legacy closer dispatch) that decides whether an incoming/outgoing channel shutdown uses the RBF state machine or the legacy negotiate closer. A regression here can misroute aux (Taproot Assets overlay) channel closes into the RBF closer, which does not integrate aux hooks — resulting in on-chain destruction of committed assets and a stuck waiting-close state, per the PR description.
🟢 Low (2 files)
  • peer/brontide_test.go (+97/-0) - Test-only change, excluded from severity/bump counting.
  • docs/release-notes/release-notes-0.21.2.md (+4/-0) - Release notes update.

Analysis

The only non-test source file touched, peer/brontide.go, sits squarely in the peer/* critical-path category (encrypted peer connection / channel-close negotiation logic). The change is small in absolute size (54 lines in the source file, well below the >500-line or >20-file bump thresholds, and touches only one critical package), so no severity bump applies beyond the base CRITICAL classification.

The change itself is a targeted, structural fix: it makes RBF coop-close exclusion for aux (Taproot Assets overlay) channels unconditional on the channel type (presence of a tapscript root) rather than relying solely on feature-bit negotiation and a configurable default. Given the PR's own description of the failure mode — irrevocable on-chain destruction of committed assets and a stuck waiting-close if this logic is wrong — this is exactly the kind of change that warrants expert review of the closer-selection state machine and the interaction between feature bits, channel type, and the RBF/legacy closer dispatch paths.


To override, add a severity-override-{critical,high,medium,low} label.

Start the 0.21.2 release notes with an entry for the aux channel RBF
coop close fix. The PR link placeholder should be filled in once the
pull request is opened.
@saubyk saubyk added this to lnd v0.22 Jul 10, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in lnd v0.22 Jul 10, 2026
@saubyk saubyk moved this from Backlog to In progress in lnd v0.22 Jul 10, 2026
@saubyk saubyk removed this from lnd v0.22 Jul 10, 2026
@saubyk saubyk added this to v0.21 Jul 10, 2026
@saubyk saubyk moved this to In progress in v0.21 Jul 10, 2026
@saubyk saubyk added this to the v0.21.2 milestone Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

severity-critical Requires expert review - security/consensus critical

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[bug]: Assets never become available onchain after cooperative channel closure

2 participants