multi: add taproot support to the new RBF close flow #10063
Conversation
Pull reviewers statsStats of the last 30 days for lnd:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Roasbeef, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly extends LND's cooperative channel closing capabilities by integrating Taproot channel support into the RBF (Replace-by-Fee) flow. It introduces a sophisticated Just-In-Time nonce management system using MuSig2 signatures, along with necessary wire protocol and state machine adjustments, ensuring a secure and efficient closing process for modern Lightning channels while maintaining full backward compatibility.
Highlights
- Taproot RBF Cooperative Close Support: This PR implements comprehensive support for Taproot channels within the existing RBF (Replace-by-Fee) cooperative channel closing protocol. This includes using MuSig2 signatures and a JIT (Just-In-Time) nonce pattern for secure and efficient nonce management during RBF iterations.
- Wire Protocol Extensions: The
shutdown,closing_complete, andclosing_sigmessages have been extended with Taproot-specific TLV fields. Theshutdownmessage now carries the sender's closee nonce.closing_completeusesPartialSigWithNonceto bundle partial signatures with next-round nonces, andclosing_siguses separatePartialSigandNextCloseeNoncefields. - State Machine Enhancements: The cooperative close state machine has been updated to detect Taproot channels via an
IsTaproot()method and manage nonce states. It now maintains separateLocalMusigSessionandRemoteMusigSessioncontexts to handle the asymmetric closer/closee roles in the Taproot closing flow. - Signature Handling and Compatibility: New helpers for signature extraction and validation have been introduced to properly handle both Taproot partial signatures and traditional ECDSA signatures. All changes maintain full backward compatibility, activating Taproot functionality only when MuSig2 sessions are present.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This PR introduces comprehensive support for Taproot channels in the RBF cooperative close flow. The changes are extensive, touching the wire protocol, state machine, and adding MuSig2 signing logic with a JIT nonce pattern. The refactoring of integration tests to cover both taproot and non-taproot channels is well-executed.
My review has identified a critical issue in the nonce handling logic for taproot channels that could lead to signature failures, and a style guide violation regarding line length.
Roasbeef
force-pushed
the
splice-nonces
branch
from
1b95798 to
6d31dc2
Compare
In this commit we, add support for taproot partial signatures with nonces to the ClosingComplete message. This is the foundation for taproot RBF cooperative channel closing, implementing the JIT nonce pattern required for the modern taproot closing flow. The changes include a new TaprootClosingSigs struct that mirrors the existing ClosingSigs but uses PartialSigWithNonce for taproot channels. The decoding and encoding functions are updated to handle both regular ECDSA signatures and taproot partial signatures. For taproot channels, the TaprootClosingSigs field is populated while ClosingSigs remains empty, maintaining backward compatibility. We also fix a minor typo in the comment for CloserNoClosee field (clsoee -> closee).
In this commit we, extend the ClosingSig message to support taproot partial signatures for the RBF cooperative close flow. The ClosingSig message is sent by the closee in response to a ClosingComplete message. For taproot channels, we add TaprootPartialSigs which contains partial signatures without nonces since the remote party already knows our nonce from the previous ClosingComplete message. We also add a NextCloseeNonce field for RBF iterations, allowing the closee to provide a new nonce for the next potential RBF round. The decoding and encoding functions are updated to handle both regular signatures and taproot partial signatures, maintaining backward compatibility with existing non-taproot channels while enabling the advanced taproot RBF flow.
In this commit we, add support for closee nonces in the Shutdown message to enable taproot RBF cooperative channel closing. The ShutdownNonce field allows taproot channels to exchange the initial nonces required for the MuSig2 signing process during cooperative closure. This nonce represents the closee nonce that the remote party will use when they act as the closer in the RBF flow. The nonce is transmitted as part of the shutdown flow and is essential for the JIT nonce pattern used in the modern taproot closing protocol. The changes maintain backward compatibility as the ShutdownNonce field is optional and only used for taproot channels that support the enhanced RBF flow.
In this commit we, update the test message utilities to include the new taproot signature fields added to ClosingComplete and ClosingSig messages. This ensures the wire protocol tests properly exercise the new taproot partial signature functionality.
…ment In this commit we, add the fundamental infrastructure for taproot RBF cooperative channel closing. This includes adding taproot channel detection, MuSig2 session management, and nonce state tracking throughout the closing state machine. Key additions include the IsTaproot method on Environment to detect taproot channels based on the presence of MuSig sessions, and LocalMusigSession/RemoteMusigSession fields for managing the different signing contexts. We add NonceState tracking to maintain closee nonces exchanged during the shutdown phase. The SendShutdown and ShutdownReceived events are extended to carry closee nonces for taproot channels, and we add proper error handling for missing nonces in taproot shutdown messages. These changes provide the foundation for the taproot-specific state transitions while maintaining compatibility with existing non-taproot channels.
In this commit we, implement the complete taproot RBF cooperative close state machine transitions. This is a comprehensive change that adds all the necessary components for taproot channel closing support. The implementation includes several key areas: First, we add nonce management helpers including initLocalMusigCloseeNonce and initRemoteMusigCloseeNonce for properly initializing MuSig2 sessions with the appropriate closee nonces during the RBF flow. Second, we implement signature extraction and validation helpers including partialSigToWireSig for converting partial signatures to wire format, and extractTaprootSigAndNonce, extractSigAndNonce, and validateAndExtractSigAndNonce for handling both taproot and regular signatures with proper validation. Third, we add comprehensive signature encoding logic with encodeClosingSignatures that creates appropriate signature structures for both channel types, and helper functions like processRemoteTaprootSig, createLocalCloseeSignature, and createClosingSigMessage for managing the complex taproot signing flow. Fourth, we extend the shutdown validation logic to require nonces for taproot channels and update all state transitions to properly handle nonce exchange, MuSig2 session initialization, and the dual signature paths for taproot vs non-taproot channels. Finally, we add signature preparation logic with prepareClosingSignatures and extraction helpers like extractSigAndNonceFromComplete that handle the complex musig signature combination required for taproot channels while maintaining compatibility with existing ECDSA signatures. The changes maintain backward compatibility with existing non-taproot channels while enabling the full taproot RBF cooperative close flow with proper nonce rotation and signature handling.
In this commit we, extend the RBF cooperative close test suite to support taproot channels. This includes adding schnorr signature test constants, taproot channel test helpers, and comprehensive test coverage for the taproot RBF flow. The changes add localSchnorrSig and remoteSchnorrSig test constants to mirror the existing ECDSA signatures, and include proper imports for musig2, chainhash, and lnwallet to support the taproot testing infrastructure. The test modifications ensure that both taproot and non-taproot channels are properly tested throughout the RBF cooperative close state machine, validating the dual signature handling paths and nonce management logic introduced in the main implementation.
In this commit we, update the chancloser test utilities and message mapping functions to properly handle the new taproot-specific fields in the RBF cooperative close flow. The changes ensure that test harnesses and message mapping functions are aware of the taproot signature fields and nonce handling required for the extended wire protocol support. This maintains test coverage for both existing non-taproot functionality and the new taproot capabilities.
In this commit we, extend the integration test suite to cover taproot RBF cooperative close scenarios. The tests validate the complete end-to-end flow for taproot channels including nonce exchange during shutdown, MuSig2 signature handling, and proper RBF iteration with nonce rotation. The integration tests ensure that the taproot closing flow works correctly in a real network environment, exercising both the happy path and various edge cases specific to taproot channel closing. This provides comprehensive coverage for the enhanced cooperative close functionality.
In this commit we, integrate the taproot RBF cooperative close functionality throughout the LND stack. This includes updating protocol configuration and peer connection handling to support the new taproot closing flow. The changes wire through the taproot channel detection, nonce exchange during shutdown, and proper handling of the enhanced wire protocol messages in the peer layer. This completes the integration of taproot RBF cooperative close functionality, providing a complete alternate closing path for taproot channels that leverages MuSig2 signatures and implements proper nonce rotation for secure RBF scenarios.
In this commit we, update the RBF cooperative close documentation to comprehensively cover the taproot channel closing flow. The documentation now explains the JIT nonce pattern, asymmetric signature roles, and the complete nonce exchange protocol for taproot channels. Key additions include detailed explanations of how nonces flow through the RBF process, the distinction between closer and closee roles, and the specific wire message extensions for PartialSigWithNonce and NextCloseeNonce fields. The documentation also covers validation requirements and implementation notes specific to taproot channels. This documentation provides a complete reference for understanding and implementing the enhanced taproot RBF cooperative close protocol.
In this commit, we revise the sig type parsing to make the control flow clearer, and also to be spec compliant. Before we would error out if _both_ the CloserNoClosee and the CloserAndClosee fields were set.
|
Tacked on a commit to revise the way to handling the parsing of the various sigs. I still find the spec somewhat confusing (eg: why would I ever send both sig types? the sig can only cover one txn...), but I think this should fix a regression reported by t-bast when testing, and also make the logic here a bit more explicit. |
yyforyongyu
left a comment
yyforyongyu
left a comment
There was a problem hiding this comment.
I'm halfway there, meanwhile I just realized there are some commits in the base branch, i think we should rebase on master?
| // once we get their ClosingComplete message. | ||
| if localCloseeNonce.IsNone() { | ||
| remoteMusig := env.RemoteMusigSession | ||
| if remoteMusig != nil { |
There was a problem hiding this comment.
Why would the remoteMusig be nil at this stage?
There was a problem hiding this comment.
Correct that it shouldn't be, this is just a defensive check.
| upfrontAddr fn.Option[lnwire.DeliveryAddress], | ||
| msg *ShutdownReceived, chanPoint wire.OutPoint, | ||
| chainParams chaincfg.Params) error { | ||
| chainParams chaincfg.Params, isTaproot bool) error { |
There was a problem hiding this comment.
in a hindsight this method could be made on the env, which can make things a bit cleaner.
There was a problem hiding this comment.
Typically the env is meant to be mostly static, it just gives the fundamental params/interfaces to the state machine.
| if env.LocalMusigSession != nil { | ||
| remoteCloseeNonce.WhenSome(func(nonce lnwire.Musig2Nonce) { | ||
| remoteMusigNonce := musig2.Nonces{PubNonce: nonce} | ||
| env.LocalMusigSession.InitRemoteNonce(&remoteMusigNonce) |
There was a problem hiding this comment.
Environment should be static and read-only? I think musig session should be part of the ProtocolState.
There was a problem hiding this comment.
Yes it is static and read only. However interfaces it exposes might have side effects or permit mutations.
We have other interfaces that are exposed in env that have underlying side effects, eg: MarkShutdownSent, MarkCoopBroadcasted.
I think musig session should be part of the ProtocolState.
So you mean thread it through for all relevant states? That could work too, but I don't think it fundamentally changes anything here.
There was a problem hiding this comment.
Yeah I'm concerned about side effects. For instance, at L63 in sendShutdownEvents, we call remoteMusig.ClosingNonce()to generate a nonce, which, iiuc, modifies the state in env.RemoteMusigSession. If there's an error down the line in sendShutdownEvents, does it mean the evn needs to be re-initialized for us to retry the close flow?
| ShutdownScript: msg.Address, | ||
| BlockHeight: r.blockHeight, | ||
| ShutdownScript: msg.Address, | ||
| RemoteShutdownNonce: remoteShutdownNonce, |
There was a problem hiding this comment.
We should stay consistent and pass the wire msg similar to how it's done in OfferReceivedEvent, and LocalSigReceived. Or better, we should parse the wire msg first, and add fields to LocalSigReceived after validation, such that the state machine can be more clean - it shouldn't need to know about the lnwire types. But that's a lager refactor - here I think provide consistency helps.
There was a problem hiding this comment.
such that the state machine can be more clean - it shouldn't need to know about the lnwire types. But that's a lager refactor - here I think provide consistency helps.
So the state machine itself doesn't know about the wire types persay. This message mapper is run on a higher level: the state machine executor is what runs this.
See StateMachine.SendMessage. This is how we map normal wire messages to state machine events (the MultiMsgRouter calls this from the readHandler in the peer).
Or perhaps you mean that the state machine shouldn't reuse the lnwire.Musig2Nonce type? It already knows about lnwire as it currently explicitly populates the relevant wire messages. Something needs to be able to populate those messages, and the state machine executor in general knows of lnwire.Message.
There was a problem hiding this comment.
Yeah i was referring that OfferReceivedEvent and LocalSigReceived wrap the entire lnwire msg, while ShutdownReceived extracts the fields, which makes me wondering why the difference. Tho this is just a minor point so non-blocking.
| closeTerms := CloseChannelTerms{ | ||
| ShutdownScripts: c.ShutdownScripts, | ||
| ShutdownBalances: msg.ShutdownBalances, | ||
| NonceState: c.NonceState, |
There was a problem hiding this comment.
Q: so they all share the same nonce state here?
lnd/lnwallet/chancloser/rbf_coop_transitions.go
Lines 556 to 566 in 85a5bf2
There was a problem hiding this comment.
Yeah nonce state is shared, as it contains the closee nonces (a.k.a verification nonces, or the nonces send up front that'll be used to verify incoming sigantures). The closer nonces (signer nonces) are always generated right before they're sent.
| mockLocalMusig = newMockMusigSession() | ||
| mockRemoteMusig = newMockMusigSession() | ||
| cfg.localMusigSession = fn.Some[MusigSession](mockLocalMusig) | ||
| cfg.remoteMusigSession = fn.Some[MusigSession](mockRemoteMusig) |
There was a problem hiding this comment.
these lines seem quite repetitive
There was a problem hiding this comment.
You mean I should formulate the tests differently, like use TDD here instead of calling twice?
There was a problem hiding this comment.
I was referring to these lines, which have been seen multiple times,
cfg := &harnessCfg{
initialState: fn.Some[ProtocolState](firstState),
localUpfrontAddr: fn.Some(localAddr),
}
if isTaproot {
mockLocalMusig = newMockMusigSession()
mockRemoteMusig = newMockMusigSession()
cfg.localMusigSession = fn.Some[MusigSession](mockLocalMusig)
cfg.remoteMusigSession = fn.Some[MusigSession](mockRemoteMusig)
}
closeHarness := newCloser(t, cfg)
defer closeHarness.stopAndAssert()Tho a second thought is, i actually prefer the repetitive lines, as long as the tests are split into tap vs non tap, as it's easier to follow the overflow by viewing a single test, so nvm.
So it's built on a few branches stacked on top of each other, it can't be directly based on master. |
In this commit, we fix a nonce handling bug. The bug was unnoticed until interop testing due to some inadvertent mutation. Before this commit, in peer/brontide.go, we used the _same_ instance of the musig2 chan closer, which masked the bug. The issue was that we would attempt to generate a siganture for the remote party _before_ we had applied their JIT nonce to our remote (used to sign their close txn) musig session. We first created a new test to confirm the issue (in peer, as it needed to be in order to avoid a circular dep test). Without these changes, the test fails. The fix is two fold: 1. Create two independent musig2 chan closers. 2. Update the ordering to apply their nonce before we generate a signature.
|
Pushed a major bug fix, discovered during interop testing: 7fa6395 |
yyforyongyu
left a comment
yyforyongyu
left a comment
There was a problem hiding this comment.
Cool I think this is close, left a few questions in the comments. Meanwhile I think we can fix the CI here to make this PR into a mergeable state.
So it's built on a few branches stacked on top of each other, it can't be directly based on master.
Is the diff large there? I think we need to open and merge that dependent PR first?
| ShutdownScript: msg.Address, | ||
| BlockHeight: r.blockHeight, | ||
| ShutdownScript: msg.Address, | ||
| RemoteShutdownNonce: remoteShutdownNonce, |
There was a problem hiding this comment.
Yeah i was referring that OfferReceivedEvent and LocalSigReceived wrap the entire lnwire msg, while ShutdownReceived extracts the fields, which makes me wondering why the difference. Tho this is just a minor point so non-blocking.
|
|
||
| // testSendOfferRbfIterationLoop is a helper function that tests the RBF iteration | ||
| // loop scenario for both taproot and non-taproot channels. | ||
| func testSendOfferRbfIterationLoop(t *testing.T, closeTerms *CloseChannelTerms, |
There was a problem hiding this comment.
Similarly we can also split this into testSendOfferRbfIterationLoopNonTap and testSendOfferRbfIterationLoopTaproot, same goes for testRecvOfferRbfLoopIterations and testSendOfferIterationNoDust, think it will be easier to maintain the tests.
| mockLocalMusig = newMockMusigSession() | ||
| mockRemoteMusig = newMockMusigSession() | ||
| cfg.localMusigSession = fn.Some[MusigSession](mockLocalMusig) | ||
| cfg.remoteMusigSession = fn.Some[MusigSession](mockRemoteMusig) |
There was a problem hiding this comment.
I was referring to these lines, which have been seen multiple times,
cfg := &harnessCfg{
initialState: fn.Some[ProtocolState](firstState),
localUpfrontAddr: fn.Some(localAddr),
}
if isTaproot {
mockLocalMusig = newMockMusigSession()
mockRemoteMusig = newMockMusigSession()
cfg.localMusigSession = fn.Some[MusigSession](mockLocalMusig)
cfg.remoteMusigSession = fn.Some[MusigSession](mockRemoteMusig)
}
closeHarness := newCloser(t, cfg)
defer closeHarness.stopAndAssert()Tho a second thought is, i actually prefer the repetitive lines, as long as the tests are split into tap vs non tap, as it's easier to follow the overflow by viewing a single test, so nvm.
| // MusigPartialSig from the wire signature. We'll need to create | ||
| // a new CreateCloseProposal to get the proper MusigPartialSig | ||
| // that CompleteCooperativeClose expects. | ||
| rawLocalSig, _, _, err := env.CloseSigner.CreateCloseProposal( |
There was a problem hiding this comment.
Q: is CreateCloseProposal idempotent? Thinking about that we may call this twice, once in LocalCloseStart to send the offer, then here when combining the sigs. I guess if the tests are passing this should work.
| // generate it using the RemoteMusigSession, as that'll set our | ||
| // localNonce, we'll receive their remoteNonce for this session | ||
| // once we get their ClosingComplete message. | ||
| if localCloseeNonce.IsNone() { |
There was a problem hiding this comment.
nit: can just do if env.IsTaproot() && localCloseeNonce.IsNone() {
| var selectedField fn.Option[SigType] | ||
| if localIsDust { | ||
| // Spec step 1: Local output is dust, use CloserNoClosee. | ||
| selectedField = fields.CloserNoClosee |
There was a problem hiding this comment.
Looks like validateSigFields is redundant as we can just check IsNone here and return the ErrCloserAndClosee? also think we should use wrapped error here to add more context for future debugging.
| // can be sent. Each signature includes the nonce for the next RBF | ||
| // round (implementing the JIT nonce pattern). | ||
| // | ||
| // NOTE: This field is only populated for taproot channels. When present, |
There was a problem hiding this comment.
Should we enforce this when decoding the msg? or use sth like fn.Either[ClosingSigs, TaprootClosingSigs]?
| IdealFeeRate: feeRate, | ||
| DeliveryAddr: shutdownStartAddr( | ||
| shutdown, | ||
| ), |
There was a problem hiding this comment.
Should we also generate the closee nonce here? Feels like it should be the closer's responsibility to generate the nonce?
| aliceChan, bobChan, err := lnwallet.CreateTestChannels(t, chanType) | ||
| require.NoError(t, err) | ||
|
|
||
| // Create TWO SEPARATE MusigChanCloser instances. This is the key to |
| ) | ||
| } | ||
|
|
||
| type mockCloseSigner struct{} |
There was a problem hiding this comment.
I think for new mockers we usually use mock.Mock so we have a tight control in our unit tests.
MPins
left a comment
MPins
left a comment
There was a problem hiding this comment.
While reviewing the PR, I noticed a potential mismatch between this flow and the flow defined in the BOLT specification.
lnwallet/chancloser/rbf_close.md
Outdated
| 2. **First Close Attempt** (Alice as closer): | ||
| - Alice sends `closing_complete`: | ||
| - Uses Bob's nonce NB (from shutdown) to create her closer signature | ||
| - Includes `PartialSigWithNonce` with her next closee nonce `NA2` | ||
| - Bob sends `closing_sig`: | ||
| - Uses Alice's nonce NA (from shutdown) to create his closee signature | ||
| - Includes `NextCloseeNonce` with his next closee nonce `NB2` |
There was a problem hiding this comment.
My understanding from the BOLT spec is that:
Alice
- Includes `PartialSigWithNonce` with her locally generated closer nonce
Bob
- Uses Alice's closer nonce just received in the `PartialSigWithNonce` to create his closee signature
The spec:
The sender of closing_complete (aka. "the closer"):
For taproot channels (when option_simple_taproot was negotiated):
MUST provide PartialSigWithNonce (98 bytes) in the appropriate TLV field based on output presence
MUST compute the aggregated musig2 public key from the sorted funding_pubkeys using the KeyAgg algorithm from bip-musig2
For the first closing transaction:
MUST use the shutdown_nonce received from the peer as their closee nonce
MUST use a locally generated nonce as the closer nonce
For RBF iterations:
MUST use the nonce extracted from the peer's previous closing_sig message as their closee nonce
MUST use a fresh locally generated nonce as the closer nonce
MUST generate the partial signature using the Sign algorithm from bip-musig2
The receiver of closing_complete (aka. "the closee"):
For taproot channels:
MUST verify that signature fields contain PartialSigWithNonce (98 bytes)
MUST extract the partial signature (first 32 bytes) and the sender's closer nonce (remaining 66 bytes)
MUST compute the aggregate nonce by combining the closer nonce received with:
For the first closing transaction:
the shutdown_nonce the receiver previously sent (their closee nonce)
For RBF iterations:
the next_closee_nonce the receiver sent in their previous closing_sig message
MUST verify the partial signature using the PartialSigVerifyInternal algorithm of bip-musig2
MUST store (in memory) the extracted nonce for potential future RBF iterations
lnwallet/chancloser/rbf_close.md
Outdated
| 3. **RBF Iteration** (Bob as closer): | ||
| - Bob sends `closing_complete`: | ||
| - Uses Alice's nonce NA2 (from previous `PartialSigWithNonce`) to create | ||
| his closer signature | ||
| - Includes `PartialSigWithNonce` with his next closee nonce `NB3` | ||
| - Alice sends `closing_sig`: | ||
| - Uses Bob's nonce NB2 (from previous `NextCloseeNonce`) to create her | ||
| closee signature | ||
| - Includes `NextCloseeNonce` with her next closee nonce `NA3` |
There was a problem hiding this comment.
My understanding from the BOLT spec is that:
Bob
- Use Alice's nonce NA (from `Shutdown`) to create his closer signature
- Includes `PartialSigWithNonce` with his locally generate closer nonce
Alice
- Uses Bob's closer nonce just received in the `PartialSigWithNonce` to create her closee signature
- Includes `NextCloseeNonce` with her next closee nonce `NA2`
The spec:
RBF Iterations: Subsequent nonces are delivered with signatures
closing_complete (from closer): Uses PartialSigWithNonce (98 bytes)
Bundles the partial signature with the sender's closer nonce
The bundling is necessary because the closee doesn't know this nonce yet
closing_sig (from closee): Uses PartialSig (32 bytes) + NextCloseeNonce field
Separates the signature from the next closee nonce because this nonce is for the next RBF attempt, not the one currently being signed
We weren't properly using the latest closee nonce from the remote party when signing.
We need to parse the sigs in a strict order, as it's possible for a party to send more than one siganture.
|
@Roasbeef, remember to re-request review from reviewers when ready |
In this PR, we implement comprehensive taproot RBF cooperative channel closing support for LND. The implementation extends the existing RBF cooperative close protocol to support taproot channels using MuSig2 signatures and introduces a JIT (Just-In-Time) nonce pattern for secure and efficient nonce management during RBF iterations.
wire + state machine changes
The implementation introduces taproot-specific wire protocol extensions for the
shutdown,closing_complete, andclosing_sigmessages. Theshutdownmessage now carries closee nonces for taproot channels, whileclosing_completeusesPartialSigWithNoncestructures that bundle partial signatures with next-round nonces. Theclosing_sigmessage uses separatePartialSigandNextCloseeNoncefields to optimize nonce delivery timing.The cooperative close state machine has been extended with taproot channel detection through the
IsTaproot()method and comprehensive nonce state management. The implementation maintains separateLocalMusigSessionandRemoteMusigSessioncontexts for handling the asymmetric closer/closee roles in the taproot closing flow.New signature extraction and validation helpers handle both taproot partial signatures and traditional ECDSA signatures with proper type validation. The
partialSigToWireSig,extractTaprootSigAndNonce, andvalidateAndExtractSigAndNoncefunctions provide robust signature handling while maintaining backward compatibility.The state transitions have been updated to properly initialize MuSig2 sessions with appropriate closee nonces, handle signature preparation for both channel types, and manage the complex nonce rotation required for secure RBF iterations. The implementation includes comprehensive error handling for taproot-specific validation requirements.
Fixes #9662