lnwallet/chancloser: use AND for IsTaproot check, use partialSigToWireSig
What the developer wrote
Authored by Olaoluwa Osuntokun
lnwallet/chancloser: use AND for IsTaproot check, use partialSigToWireSig
Change IsTaproot() from OR to AND: both LocalMusigSession and
RemoteMusigSession must be set to consider the channel taproot. This
prevents panics from nil session access when only one session is
initialized.
Also use partialSigToWireSig helper for consistency in
extractSigAndNonceFromComplete instead of inline conversion.
What changed, and why it matters
This commit fixes a logic bug in LND's cooperative channel-closing code. The program previously treated a channel as a modern 'taproot' channel if either side's special signing session was set, but it really needs both. When only one was set, the code could later try to use a missing session and crash. The patch also swaps an inline signature conversion for an existing helper to keep the code consistent. It is a defensive bug fix that prevents a panic rather than a clear remote-exploitable vulnerability.
Treat as a stability/defensive fix. Review whether any reachable state in production can trigger the half-initialized session condition and confirm the panic cannot be induced by a peer. If it is reachable via protocol messages, prioritize backporting; otherwise include in the next regular release.
Security signals we found
Logic bug in feature-gate predicate (OR vs AND)
Potential nil-pointer dereference / panic in channel-closing state machine
Defensive hardening of taproot/MuSig2 cooperative close path
Refactor to use existing helper for signature serialization consistency
Evidence from the diff
In lnwallet/chancloser, Environment.IsTaproot() was changed from an OR to an AND: it now returns true only when both LocalMusigSession and RemoteMusigSession are non-nil. The prior OR meant a taproot code path could be entered with one nil MuSig2 session, leading to nil-dereference panics. A test was updated to supply both sessions. In extractSigAndNonceFromComplete, the inline conversion of a partial Schnorr signature to a wire.Signature was replaced by the existing partialSigToWireSig helper, which is functionally equivalent but less error-prone.
Changed components
lnwallet/chancloser/rbf_coop_states.golnwallet/chancloser/rbf_coop_transitions.golnwallet/chancloser/rbf_coop_test.goInspect captured patch +10 / −13
diff --git a/lnwallet/chancloser/rbf_coop_states.go b/lnwallet/chancloser/rbf_coop_states.go
index 4cdd583..675d0bc 100644
--- a/lnwallet/chancloser/rbf_coop_states.go
+++ b/lnwallet/chancloser/rbf_coop_states.go
@@ -373,9 +373,9 @@ func (e *Environment) Name() string {
}
// IsTaproot returns true if this is a taproot channel. A channel is considered
-// taproot if either the LocalMusigSession or RemoteMusigSession is set.
+// taproot if both the LocalMusigSession and RemoteMusigSession are set.
func (e *Environment) IsTaproot() bool {
- return e.LocalMusigSession != nil || e.RemoteMusigSession != nil
+ return e.LocalMusigSession != nil && e.RemoteMusigSession != nil
}
// CloseStateTransition is the StateTransition type specific to the coop close
diff --git a/lnwallet/chancloser/rbf_coop_test.go b/lnwallet/chancloser/rbf_coop_test.go
index c02adec..86f93a3 100644
--- a/lnwallet/chancloser/rbf_coop_test.go
+++ b/lnwallet/chancloser/rbf_coop_test.go
@@ -3208,11 +3208,12 @@ func TestLocalOfferSentNonceInitOrder(t *testing.T) {
LocalSig: localSchnorrSig,
}
- // The environment needs LocalMusigSession set for taproot path.
- // IsTaproot() returns true when LocalMusigSession is non-nil.
+ // The environment needs both musig sessions set for taproot path.
+ // IsTaproot() returns true when both sessions are non-nil.
env := &Environment{
- ChanPoint: randOutPoint(t),
- LocalMusigSession: strictLocalMusig,
+ ChanPoint: randOutPoint(t),
+ LocalMusigSession: strictLocalMusig,
+ RemoteMusigSession: newMockMusigSession(),
}
// Create a LocalSigReceived event with NextCloseeNonce.
diff --git a/lnwallet/chancloser/rbf_coop_transitions.go b/lnwallet/chancloser/rbf_coop_transitions.go
index 50732d3..7d6ddb8 100644
--- a/lnwallet/chancloser/rbf_coop_transitions.go
+++ b/lnwallet/chancloser/rbf_coop_transitions.go
@@ -1850,13 +1850,9 @@ func selectAndExtractSig(fields SigFieldSet, localIsDust bool) (
})
// Otherwise, for taproot, extract the partial sig and nonce.
- sigType.WhenRight(func(partialSig lnwire.PartialSigWithNonce) {
- nonce = fn.Some(partialSig.Nonce)
-
- sigBytes := partialSig.Sig.Bytes()
- copy(sig.RawBytes()[:32], sigBytes[:])
-
- sig.ForceSchnorr()
+ sigType.WhenRight(func(ps lnwire.PartialSigWithNonce) {
+ nonce = fn.Some(ps.Nonce)
+ sig = partialSigToWireSig(ps.PartialSig)
})
return sig, nonce, isNoClosee, nil
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.