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.

Sign up for GitHub

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

Jump to bottom

Implement "Simple Taproot Channels" BOLT proposal #2868

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

Implement "Simple Taproot Channels" BOLT proposal #2868

wants to merge 17 commits into from

Conversation

sstone
Copy link
Member

@sstone sstone commented Jun 13, 2024 •
edited
Loading

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 mentioned this pull request Jul 4, 2024
Draft
@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
@remyers remyers mentioned this pull request Sep 24, 2024
Open
remyers
remyers reviewed Oct 2, 2024
View reviewed changes
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.

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala Show resolved Hide resolved
@t-bast
Add detailed error message when splice feerate is incorrect (#2920)
96d0c9a 
This can be helpful when troubleshooting cross-compatibility issues.

Replaces #2911.
@sstone sstone force-pushed the simple-taproot-channels branch 2 times, most recently from 1d85d22 to 258a479 Compare October 22, 2024 14:18
@t-bast
Use shared input's txOut in shouldSignFirst (#2934)
4ca8ea0 
This is clearer that way and yields the same result: we only look at
`txOut` amounts when computing each node's contributions.
pm47 and others added 2 commits November 8, 2024 14:04
@pm47 @t-bast
Don't relay buggy extra payments (#2937)
f4efd64 
If a payer is buggy and tries to pay the same invoice multiple times, it can lead to an edge case where the recipient accepted the first one and purchased liquidity for it, but didn't purchase additional liquidity and thus cannot receive the duplicate payments.

Also, when replaying parts that were waiting for an on-the-fly funding, we set `commit = false` to all parts, instead of just the last one. This optimization caused the payment to be stuck if the last part was unexpectedly rejected (which would for example happen in the buggy payer case described above, before we rejected those extra payments).

---------

Co-authored-by: t-bast <[email protected]>
@pm47
Make cluster serialization support unknown messages (#2938)
f02c98b 
The codec that we were using to serialize `LightningMessage`s in the eclair cluster didn't support `UnknownMessage`. This resulted in those messages being dropped by the front and never reaching the backend node.
t-bast and others added 13 commits November 12, 2024 12:13
@t-bast @sstone
Add option_simple_close feature bit
4526df4
@t-bast @sstone
Add codecs for option_simple_close
64208b2 
This feature adds two new messages:

- `closing_complete` sent after exchanging `shutdown`
- `closing_sig` sent in response to `closing_complete`
@t-bast @sstone
Add support for OP_RETURN closing scripts
43e1204 
The spec allows the closer to use an OP_RETURN output if their amount is
too low when using `option_simple_close`.
@t-bast @sstone
Implement the option_simple_close protocol
d442a4a 
We introduce a new `NEGOTIATING_SIMPLE` state where we exchange the
`closing_complete` and `closing_sig` messages, and allow RBF-ing previous
transactions and updating our closing script.

We stay in that state until one of the transactions confirms, or a force
close is detected. This is important to ensure we're able to correctly
reconnect and negotiate RBF candidates.

We keep this separate from the previous NEGOTIATING state to make it
easier to remove support for the older mutual close protocols once we're
confident the network has been upgraded.
@t-bast @sstone
Require strict exchange of shutdown
98cc0b1 
Whenever one side sends `shutdown`, we restart a signing round from
scratch. To be compatible with future taproot channels, we require
the receiver to also send `shutdown` before moving on to exchanging
`closing_complete` and `closing_sig`. This will give nodes a message
to exchange fresh musig2 nonces before producing signatures.

On reconnection, we also restart a signing session from scratch and
discard pending partial signatures.
@sstone
Refactor tx signing (no functional changes)
eff09a3
@sstone
Upgrade input info class to allow spending from taproot transactions
86dfadd 
Our InputInfo class contains a tx output and the matching redeem script, which is enough to spend segwit v0 transactions.
For taproot transactions, instead of a redeem script, we need a script tree instead, and the appropriate internal pubkey.
@sstone @pm47
Fixup! Update legacy InputInfo codec
9d84dfa 
Co-authored-by: Pierre-Marie Padiou <[email protected]>
@sstone
Get ready to store partial signatures
7bcee44 
We currently store our peer's signature for our remote commit tx, so we can publish it if needed.
If we upgrade funding tx to use musig2 instead of multisig 2-of-2 we will need to store a partial signature instead.
@sstone
Fixup! fix typo
2ae762f
@sstone
Implement simple taproot channels
7c5cb89 
This commit implements:
  - feature bits for simple taproot channels
  - TLV extensions for funding/closing wire messages
  - modifications to how we handle channel funding, splicing and mutual closing
  - changes to the commitment structures

Since the current "simple taproot channels" proposal is not compatible with splices, we extend it 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 nonce for the new commit tx that is being built, here it has been added to `SpliceInit` and `SpliceAck`.
@sstone
Adapt the simple_close protocol to simple taproot channels
c34ef62 
`partial_signature_with_nonce` TLVs are added to `closing_complete` and `closing_sig` with the same format as in `commitment_signed`

The closing workflow is similar to the standard "simple close" workflow:
- Alice and Bob exchange `shutdown`, which includes a "closing nonce" (no changes here compared to the "simple taproot channels" spec).
- Alice selects possible closing transaction (closer_output_only, closee_output_only, closer_and_closee_output) and for each of them creates
a partial_signature_with_nonce using a new random local nonce and Bob's closing nonce (which she received in Bob's `shutdown` message).
- Alice send a `closing_complete` message to Bob that include these partial_signature_with_nonce.
- Bob receive Alice's `closing_complete` message, selects one of Alice's partial_signature_with_nonce, creates partial_signature_with_nonce using.
his closing nonce and the nonce attached to the partial_signature_with_nonce and sends it to Alice in a `closing_sig` message.
- Alice receives Bob's `closing_sig` and creates a partial signature for her closing tx using her closing nonce and the nonce attached Bob's partial_signature_with_nonce.
- Alice combines this signature with Bob's and can broadcat her closing tx.
@sstone
Fix splicing issues
827bcd9 
Upon re-connection, when a splice has not been fully completed, nodes will re-send signatures for the previous remote commit tx. This signature will
be ignored by the receiving nodes if it has already received it before it was disconnected, simply by comparing them (signatures are deterministic).

With taproot channels, we also need to attach musig2 nonces for splices in progress to channel_reestablish, which are needed to re-generate the signature
for the old commit tx.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remyers remyers remyers left review comments

Assignees
No one assigned
Labels
Optech Make Me Famous!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sstone @codecov-commenter @remyers @t-bast @pm47