Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "Simple Taproot Channels" BOLT proposal #2868

Draft
wants to merge 6 commits into
base: store-partial-signatures
Choose a base branch
from

Conversation

sstone
Copy link
Member

@sstone sstone commented Jun 13, 2024

This PR implements lightning/bolts#995 which introduces a new channel format where funding transactions send to an aggregated musig2 public key instead of a 2-of-2 multisig address:

  • funding and closing transactions become cheaper (by about 15%)
  • on-chain footprint becomes more private: funding and closing transactions are impossible to distinguish from other p2tr transactions

Basic interop testing (opening/closing channels, sending/receiving payments) pass with lnd v0.18

The trickiest part of this PR is the update to the splicing protocol: splicing means that there can be multiple commitment transactions that are active at the same time, and signatures must be exchanged for all of them. With the new musig2-based funding transaction scheme, this means generating and exchanging musig2 nonces in advance, which requires modifications to the interactive tx and splicing protocols that are not (yet) covered by the BOLT proposal.

This is still a WIP, we also need to add support for simple taproot channels to https://github.com/ACINQ/lightning-kmp.

Based on #2896

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 95.17974% with 59 lines in your changes missing coverage. Please review.

Project coverage is 86.38%. Comparing base (40f13f4) to head (51ec562).
Report is 6 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2868      +/-   ##
==========================================
+ Coverage   85.92%   86.38%   +0.46%     
==========================================
  Files         219      220       +1     
  Lines       18488    19477     +989     
  Branches      794      814      +20     
==========================================
+ Hits        15886    16826     +940     
- Misses       2602     2651      +49     
Files Coverage Δ
...core/src/main/scala/fr/acinq/eclair/Features.scala 100.00% <100.00%> (ø)
...r/acinq/eclair/blockchain/fee/OnChainFeeConf.scala 95.65% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 100.00% <ø> (ø)
...cala/fr/acinq/eclair/channel/ChannelFeatures.scala 100.00% <100.00%> (ø)
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 87.52% <100.00%> (+0.52%) ⬆️
...inq/eclair/channel/fsm/CommonFundingHandlers.scala 92.00% <100.00%> (+0.88%) ⬆️
...q/eclair/channel/publish/ReplaceableTxFunder.scala 84.81% <100.00%> (-0.53%) ⬇️
...q/eclair/crypto/keymanager/ChannelKeyManager.scala 88.88% <100.00%> (+1.38%) ⬆️
...air/crypto/keymanager/LocalChannelKeyManager.scala 100.00% <100.00%> (ø)
...air/crypto/keymanager/LocalOnChainKeyManager.scala 87.50% <100.00%> (ø)
... and 21 more

... and 5 files with indirect coverage changes

@sstone sstone force-pushed the simple-taproot-channels branch 2 times, most recently from 786cfa1 to f9fe228 Compare July 22, 2024 12:41
@sstone sstone changed the base branch from master to store-partial-signatures August 8, 2024 19:09
@sstone sstone force-pushed the store-partial-signatures branch 2 times, most recently from 452c0e0 to 16e75b0 Compare September 10, 2024 17:59
Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good so far. I focused this preliminary review on splicing to identifiy any potential nonce reuse issues early.

I found one potential issue if we allow tx_abort after we send commit_sig.

I also think RBF of splices will need to be done carefully to avoid nonce resuse. I think we will need to include the RBF attempt count when generating nonces. I'll review again once you've rebased on the latest splice branch that includes splice RBF.

This commit implements:
- feature bits for simple taproot channels
- TLV extensions for funding/closing wire messages
- modifications to how we handle channel funding and mutual closing
- changes to the commitment structures
It does not cover dual-funding and splices, which are not part of the original extension proposal.
The current "simple taproot channels" proposal is not compatible with splices.
Supporting splices means supporting multiple commitment transactions that are valid at the same time, with the same commitment index but with different funding transactions.
We need to extend the taproot proposal to include a list of musig2 nonces (one for each active commitment transaction).
Similar to how commitment points are handled, `firstRemoteNonce` and `secondRemoteNonce` fields have been added to `SpliceInit` and `SpliceAck`, encoded as a list of nonces (instead of 2 expicit nonces)
We also need a  for the new commit tx that is being built, here it has been added to `SpliceInit` and `SpliceAck`.

The funding tx that is being built during the interactive session needs to spend the current funding tx.
For this, we re-use the scheme that we developped for our custome "swaproot" musig swap-ins: we add musig2 nonces to the `TxComplete` message, one nonce for each input that requires one, ordered by serial id.
The life-cycle of these nonces is tied to the life-cycle of the interactive session which is never persisted (nonces here do not have to be deterministic).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants