lnwire+lnwallet: add LocalNonces field for splice nonce coordination w/ taproot channels #9982
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This commit introduces a new TLV structure LocalNoncesData that contains a map of transaction IDs to MuSig2 nonces. This structure enables coordinating multiple nonces for different purposes (e.g., channel commits, splice operations) within a single wire message. The implementation includes: - LocalNoncesData struct with deterministic TLV encoding/decoding - Comprehensive unit tests covering encoding, decoding, and edge cases - Helper functions for creating optional LocalNonces values This is a prep for upcoming spec changes to allow a party that has in-prorgess splices to tell the remote party which nonces to use for which splice.
Extends the ChannelReestablish message to include a new optional LocalNonces field alongside the existing LocalNonce field. This enables backwards-compatible transmission of multiple nonces for different purposes during channel reestablishment. Changes include: - Add LocalNonces field to ChannelReestablish struct - Update Encode/Decode methods to handle the new TLV field - Extend property-based testing to randomly include LocalNonces - Maintain full backwards compatibility with existing LocalNonce field
Updates channel synchronization to support the new LocalNonces field while maintaining backwards compatibility with the legacy LocalNonce field. This enables coordinated nonce exchange for splice operations and other multi-commitment scenarios. Key changes: - ChanSyncMsg now populates both LocalNonce and LocalNonces fields - ProcessChanSyncMsg prioritizes LocalNonces over LocalNonce - Uses empty hash key for primary commitment nonce in LocalNonces map - Maintains full backwards compatibility with existing implementations
Adds extensive test coverage for the new LocalNonces functionality in channel synchronization, ensuring both legacy and new nonce fields work correctly together. Test additions include: - Helper function to extract nonces from either LocalNonces or LocalNonce - Comprehensive TestChanSyncTaprootLocalNonces covering all scenarios - Tests for backwards compatibility when only one field is present - Error handling tests for missing nonce scenarios - Updates to existing sync tests to handle both nonce field types
|
This is dependent on #9639 |
4 tasks
|
Hope to see this soon! |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements support for the upcoming BOLT specification change that enables coordinated nonce exchange for splice operations in taproot channels. The current
LocalNoncefield inChannelReestablishonly supports a single nonce, but splice operations require coordination of multiple nonces for different transaction purposes.Note that we don't support splicing yet, but we aim to make sure the final version of taproot channels is fully compatible with splicing. This new set of fields is only intended to be set with the production taproot channel feature bit. A follow up PR will actually start to gate the script and p2p message changes (like this).
Ultimately for the foreseeable future, we'll support both the staging and prod taproot channel feature bit. Dynamic commitments can be used in the future to upgrade all the staging versions off-chain, which will allow us to eventually remove that feature bit.
Changes
This PR introduces a new
LocalNoncesfield alongside the existingLocalNoncefield to enable:Implementation Details
Wire Protocol Changes
LocalNoncesDataTLV structure containing a map of transaction IDs to MuSig2 noncesChannelReestablishmessage with optionalLocalNoncesfield (TLV)LocalNoncefieldChannel Synchronization Logic
ChanSyncMsg()now populates both legacy and new nonce fields for taproot channelsProcessChanSyncMsg()prioritizesLocalNoncesoverLocalNoncewhen both are presentchainhash.Hash{}for primary commitment nonce in the mapKey Design Decisions
Testing
LocalNoncesDataencoding/decodingRandTestMessageTestChanSyncTaprootLocalNoncescovering all sync scenarios