lnwallet/chancloser: move nonce rotation to LocalOfferSent
What the developer wrote
Authored by Olaoluwa Osuntokun
lnwallet/chancloser: move nonce rotation to LocalOfferSent
Move the RemoteCloseeNonce update from updateAndValidateCloseTerms to
LocalOfferSent.ProcessEvent. This keeps updateAndValidateCloseTerms
focused on close term validation, and makes the nonce rotation point
explicit in the state machine — it happens when processing the
LocalSigReceived event, alongside signature extraction.
Update TestNextCloseeNonceStorageFromClosingSig to verify that
updateAndValidateCloseTerms no longer modifies RemoteCloseeNonce.
What changed, and why it matters
This is a small internal code cleanup in LND's cooperative channel-closing logic. It moves where a cryptographic nonce is updated from one function to another within the same state machine, with no indication of a security bug or user-facing behavior change. The change is framed by the author as improving code organization, not fixing a vulnerability.
No security action required. Treat as normal code-quality refactor. If reviewing for a release, verify that the moved nonce rotation is still invoked for all intended taproot cooperative-close paths and that no duplicate or missed rotation occurs.
Security signals we found
No security-relevant keywords in commit title or message
Refactor only: existing nonce extraction logic moved to a different state-machine event handler
Test updated to assert absence of side effect in previous function
No validation rules weakened or bypassed in the diff
No vendor disclosure of security relevance
Evidence from the diff
The commit refactors nonce handling in lnwallet/chancloser. Previously, updateAndValidateCloseTerms both validated close terms and updated RemoteCloseeNonce from a ClosingSig message. The patch removes that side effect from updateAndValidateCloseTerms and places the nonce rotation in LocalOfferSent.ProcessEvent when processing LocalSigReceived. The accompanying test is updated to assert that updateAndValidateCloseTerms no longer modifies RemoteCloseeNonce. The diff alone shows only a relocation of existing logic; it does not reveal a security defect, missing validation, or incorrect nonce handling.
Changed components
lnwallet/chancloser/rbf_coop_transitions.golnwallet/chancloser/rbf_coop_test.goInspect captured patch +15 / −26
diff --git a/lnwallet/chancloser/rbf_coop_test.go b/lnwallet/chancloser/rbf_coop_test.go
index d14d0d2..0806d08 100644
--- a/lnwallet/chancloser/rbf_coop_test.go
+++ b/lnwallet/chancloser/rbf_coop_test.go
@@ -3082,13 +3082,15 @@ func TestTaprootNonceHandling(t *testing.T) {
)
}
-// TestNextCloseeNonceStorageFromClosingSig tests that NextCloseeNonce from
-// LocalSigReceived (ClosingSig message) is properly stored for the next RBF
-// round in updateAndValidateCloseTerms.
+// TestNextCloseeNonceStorageFromClosingSig tests that
+// updateAndValidateCloseTerms does NOT modify RemoteCloseeNonce. The nonce
+// rotation is handled by LocalOfferSent.ProcessEvent, keeping close term
+// validation separate from nonce state management.
func TestNextCloseeNonceStorageFromClosingSig(t *testing.T) {
t.Parallel()
- // Create a closing negotiation state with taproot
+ // Create a closing negotiation state with taproot.
+ originalNonce := lnwire.Musig2Nonce{4, 5, 6}
closeTerms := &CloseChannelTerms{
ShutdownScripts: ShutdownScripts{
LocalDeliveryScript: localAddr,
@@ -3096,7 +3098,7 @@ func TestNextCloseeNonceStorageFromClosingSig(t *testing.T) {
},
NonceState: NonceState{
LocalCloseeNonce: fn.Some(lnwire.Musig2Nonce{1, 2, 3}),
- RemoteCloseeNonce: fn.Some(lnwire.Musig2Nonce{4, 5, 6}),
+ RemoteCloseeNonce: fn.Some(originalNonce),
},
}
@@ -3128,21 +3130,19 @@ func TestNextCloseeNonceStorageFromClosingSig(t *testing.T) {
},
}
- // Test that updateAndValidateCloseTerms properly stores the
- // NextCloseeNonce.
+ // updateAndValidateCloseTerms should only validate close terms, not
+ // update the nonce. The nonce rotation happens in
+ // LocalOfferSent.ProcessEvent.
err := negotiation.updateAndValidateCloseTerms(sigEvent, true)
require.NoError(t, err)
- // Verify the NextCloseeNonce was stored for the next round.
- require.True(
- t, negotiation.NonceState.RemoteCloseeNonce.IsSome(),
- "NextCloseeNonce should be stored for next round",
- )
-
+ // Verify the RemoteCloseeNonce was NOT modified — it should still
+ // hold the original nonce from shutdown.
storedNonce := negotiation.NonceState.RemoteCloseeNonce.UnwrapOrFail(t)
require.Equal(
- t, nextCloseeNonce, storedNonce,
- "stored nonce should match the NextCloseeNonce from ClosingSig",
+ t, originalNonce, storedNonce,
+ "updateAndValidateCloseTerms should not modify "+
+ "RemoteCloseeNonce",
)
}
diff --git a/lnwallet/chancloser/rbf_coop_transitions.go b/lnwallet/chancloser/rbf_coop_transitions.go
index f31099c..eb8b1d6 100644
--- a/lnwallet/chancloser/rbf_coop_transitions.go
+++ b/lnwallet/chancloser/rbf_coop_transitions.go
@@ -961,17 +961,6 @@ func (c *ClosingNegotiation) updateAndValidateCloseTerms(event ProtocolEvent,
return err
}
- // For taproot channels, extract the NextCloseeNonce from
- // ClosingSig if present. This will be used for the next RBF
- // iteration when we act as closer. This is their new closee
- // nonce.
- _, nextCloseeNonce := validateAndExtractSigAndNonce(
- msg.SigMsg, isTaproot,
- )
- nextCloseeNonce.WhenSome(func(nonce lnwire.Musig2Nonce) {
- c.NonceState.RemoteCloseeNonce = fn.Some(nonce)
- })
-
return nil
}
Why this scored 12/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.