-
Notifications
You must be signed in to change notification settings - Fork 492
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
extension-bolt: simple taproot channels (feature 80/81) #995
base: master
Are you sure you want to change the base?
Conversation
d7b1fe6
to
ec8c7b4
Compare
Some things that came up in meatspace discussions:
|
I think the commit_sig should contain the sender's "remote nonce" and the revoke_and_ack contain the sender's "local nonce". Also since funding_locked will be sent repeatedly with scid-alias when that is merged and deployed, then there should probably be language to define that the nonces are only sent the first time? |
let's try to pick naming conventions for nonces that doesn't make me cry over the asymmetry |
Some points: This interacts with the 2-of-3 goal of @moneyball . If one participant uses a 2-of-3 and owns ALL 3 keys, then it is fine and we can just have MuSig2 with both channel endpoints. But the 2-of-3 goal is that one channel endpoint is really a nodelet-like setup: there is one sub-participant with 2 keys and another "server" participant with 1 key, a la GreenWallet. This requires composable MuSig2. Now I think composable MuSig2, if it can be proven safe, just requires two -- This interacts with VLS as well @ksedgwic . The nonce A similar technique may also be useful for the server in the 2-of-3 of @moneyball; rather than maintain a state for each channel of each client, the client could store the per-channel |
So I talked to @jonasnick, and as I understand it, we can work with just two |
Re recursive musig2: I'm gonna give the implementation a shot (outside the LN context, just the musig-within-musig) just to double check my assumptions re not needing to modify the (revised) nonce exchange flow. |
i made a pull request on this pull request with script fixes |
Why not the revocation key? When i publish an old state, the remote party can claim my output and htlcs with the key path, but not his own output, and also has to wait a block. If we set the internal key to the revocation key it will give the remote party more privacy, nobody on chain can see which outputs were to local and to remote (and htlcs if they are swept along). It will also give more consistency with other output as they also have the revocation key as internal key. it will also be cheaper (or get a higher fee rate with the same amount of sats), this only requires a signature from a taptweaked revocation key (65) instead of a signature (65), the script (36) and the controlblock (34) (incl length prefix) |
#995 (comment) makes it invisible for outside observers to identify the to_remote output in case of a revoked commitment. if there are some htlcs on it that are long expired and the second stage is broadcasted (like in the fee siphoning attack), the funds go to the local delayed pubkey + relative timelock. outside observers can now see which output was the to_local one, just search the output of an htlc 2nd stage tx in the commitment transaction. example ctx: 15c262aeaa0c5a44e9e5f25dd6ad51b4162ec4e23668d568dc2c6ad98ae31023 (testnet) the transaction with the expired htlc reveals the to_local output. (it is already revealed by the script, but this wouldnt be the case with a revoked taproot ctx) this can be fixed by tweaking the local delayed pubkey with the hash of EDIT: no secret is needed, instead a taptweak like tweak can be done. everywhere where a local delayed pubkey is used, it is tweaked with for clarity: htlc outputs that send funds to the local delayed pubkey use a tweaked local delayed pubkey where the output index of the htlc output on the commitment transaction is used, not the htlc success or timeout tx |
this would preserve privacy, but you'd also need to do this for the |
hmmm true, so it is either privacy, with no key reuse or no utxo set bloat. btw another idea about anchors and less utxo set bloat: |
The |
if this is a problem B can tweak the key before using it without A even knowing (also A has to do this because the lexicographically smaller key is tweaked), but i dont think it is, lnd uses a separate bip32 tree for this (separate from the wallet) (btw without taproot funding pubkeys were revealed every time a channel was closed) |
afaict the algorithm in this bip is generalized for 32 byte pubkeys and more than 2 signers, the 'simple' musig2 with the pubkey's with the parity bit known looks like this equation i used, btw i got it here https://github.com/t-bast/lightning-docs/blob/master/schnorr.md#musig2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some old comments I forgot to submit
Most recent comment is noting that partial sigs are 32 bytes, so this needs explicit defining somewhere, since signature types seem to assume 64(may have missed it).
nvm this wouldn't work because keys are only revealed when swept without signature to make this problem somewhat easier i suggest to remove the now that the this special case can of course be seen from both sides:
even more rare: revocation no anchor keys are revealed here because with the revocation key the taproot key path is used. i don't know to make anchor sweepable in this case long story short:
Questions/feedback welcome! |
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), see lightning/bolts#995 (comment)). We also need a new "next remote nonce" 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 needs to spend the current funding tx, for this we re-use the current remote nonce (no need to send a new one).
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), see lightning/bolts#995 (comment)). We also need a new "next remote nonce" 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 needs to spend the current funding tx, for this we re-use the current remote nonce (no need to send a new one).
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), see lightning/bolts#995 (comment)). We also need a new "next remote nonce" 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.
In order to force close a channel, the holder of a verification nonce must use | ||
that same nonce to counter sign the commitment transaction with the other half | ||
of the musig2 partial signature. Rather than force an implementation to retain | ||
additional signing state (the verification nonce) to avoid holding as "hot" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional signing state (the verification nonce) to avoid holding as "hot" | |
additional signing state (the verification nonce) to avoid holding a "hot" |
the ASCII string `taproot-rev-root`. | ||
|
||
3. Given a commitment height/number (`N`), the verification nonce to send to | ||
the remote party party can be derive by obtaining the `Nth` shachain leaf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the remote party party can be derive by obtaining the `Nth` shachain leaf | |
the remote party party can be derived by obtaining the `Nth` shachain leaf |
construct a `musig2` partial signature for the sender's remote commitment | ||
using the `Sign` algorithm from `bip-musig2`. | ||
|
||
- MUST include the partial signature and the public counterpart of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section should be updated: the sender will use the nonces they exchanged with the recipient in their 'shutdown' message to generate and send a partial signature.
- the `shutdown_nonce` field the recipient previously sent in the | ||
`shutdown` message. | ||
|
||
- the `public_nonce` included as part of the `partial_signature_with_nonce` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, the remote public nonce was included in the received shutdown
message.
to accept a prior offer by a peer that it would have accepted in the current | ||
round. For musig2, as each signature comes with nonce state, the prior offer | ||
may actually be using distinct nonce state, rendering it unable to be comined | ||
for the final transaction braodcast. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the final transaction braodcast. | |
for the final transaction broadcast. |
Instead, the responder will simply accept what the initiator proposes. The | ||
responder can always CPFP after the fact if they require a higher fee rate. The | ||
initiator is the one that pays fees directly (coming out of their settled | ||
output), so the responder will always have their full funds develiered to them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
output), so the responder will always have their full funds develiered to them. | |
output), so the responder will always have their full funds delivered to them. |
|
||
* `OP_1 to_remote_output_key` | ||
* where: | ||
* `taproot_nums_point = 0245b18183a06ee58228f07d9716f0f121cd194e4d924b037522503a7160432f15` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the NUMS point defined in https://github.com/lightning/bolts/blob/e25132d8de0164224578964fcd3f7328ddfc3281/bolt-simple-taproot.md#nothing-up-my-sleeves-points ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct! The linked point matches what we use in lnd today: https://github.com/lightningnetwork/lnd/blob/2f2efc782436944ce98b1e0e13bde125951b2b36/input/script_utils.go#L46-L48
Can't recall why the text has this incorrect point.
As a side question @sstone, here's the code we used to generate the NUMs point: https://github.com/lightninglabs/lightning-node-connect/blob/master/mailbox/numsgen/main.go. Perhaps Eclair is interested in replicating the derivation in Scala/Kotlin to increase confidence in the integrity of the point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add a test to check that we can re-generate this NUMS point here: https://github.com/ACINQ/eclair/blob/37553dc4ae92c2950b7fe77e3ad0090e10001421/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala#L1178
along side `<revocationpubkey>` to derive the private key needed to sweep the | ||
top-level key spend path. The control block can be crafted as such: | ||
``` | ||
revoke_control_block = (output_key_y_parity | 0xc0) || taproot_nums_point || revoke_script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the actual script here but an inclusion proof as described in https://github.com/lightning/bolts/blob/e25132d8de0164224578964fcd3f7328ddfc3281/bolt-simple-taproot.md#taproot-script-path-spends
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, would we prefer that the derivation of the control block is more implicit in the spec? I think it's relevant at time to spell it out, as the internal key isn't always taproot_nums_point
. So maybe we just need to specify what internal key should be used, then refer to the section you've linked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think that specifying the script and internal key used to build the proof that must be included in the control block should be enough.
only `33` bytes, as it just includes the internal key (along with the y-parity | ||
bit and leaf version): | ||
``` | ||
delay_control_block = (output_key_y_parity | 0xc0) || taproot_nums-point || to_delay_srcipt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, an inclusion proof not an actual script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did a close reading in preparation to reviewing #2868 and found some nits/typos.
- MUST specify the `next_local_nonce` field. | ||
- MUST use the `NonceGen` algorithm defined in `bip-musig2` to generate | ||
`next_local_nonce` to ensure it generates nonces in a safe manner. | ||
- MOST not set the `announce_channel` bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- MOST not set the `announce_channel` bit. | |
- MUST not set the `announce_channel` bit. |
|
||
### Cooperative Closure | ||
|
||
Compared to the base segwit v0 channel type, for simple taproot channels, then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compared to the base segwit v0 channel type, for simple taproot channels, then | |
Compared to the base segwit v0 channel type, for simple taproot channels, the |
- MUST reject the channel if `next_local_nonce` is absent, or cannot be | ||
parsed as two compressed secp256k1 points | ||
|
||
- the specified public nonce cannot be parsed as two compressed secp256k1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the specified public nonce cannot be parsed as two compressed secp256k1 |
This is redundant with the above MUST clause
1. `tlv_stream`: `shutdown_tlvs` | ||
2. types: | ||
1. type: 8 (`shutdown_nonce`) | ||
2: data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2: data: | |
2. data: |
2. types: | ||
1. type: 6 (`partial_signature`) | ||
2. data: | ||
* [`32*byte`: `partial_signature` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [`32*byte`: `partial_signature` | |
* [`32*byte`: `partial_signature`] |
root, the verification nonce sent by that state can be deterministically | ||
reproduced: | ||
|
||
1. Given the shachain root used to generate revocation pre-images, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has any thought been given on how this handles/could be extended to handle splicing?
you're going to possibly be signing multiple times at a given height, and sharing nonces before knowing the final txid/state of the commit tx.
Without giving much thought the splices could be given a counter and incorporated to the shachain scheme.
cc @ddustin
also see: https://github.com/bitcoin-core/secp256k1/blob/master/include/secp256k1_musig.h#L425
This PR puts forth two concepts:
The extensions described in this document have purposefully excluded any gossip related changes, as the there doesn't yet appear to be a predominant direction we'd all like to head in (nu nu gossip vs kick the can and add schnorr).
Most of the changes here described are pretty routine: use musig2 when relevant, and create simple tapscript trees to fold in areas where the script has multiple conditional paths. The main consideration with
musig2
is ofc: how to handle nonces. This document takes a very conservative stance, and simply proposes that all nonces be 100% ephemeral, and forgotten, even after a connection has been dropped. This has some non-obvious implications w.r.t the retransmission flow. Beyond that, it's mostly: piggy back the nonce set of nonces (4 public nonces total, since there're "two" messages) on a message to avoid having to add additional round trips.The other "new" thing this adds is the generation/existence of a NUMs point, which is used to ensure that certain paths can only be spent via the script spend path (like the to remote output for the remote party, as this inherits anchor outputs semantics).
This is still marked as draft, as it's just barely to the point of being readable, and still has a lot of clean ups to be done w.r.t notation, clarify, wording, and full specification.