Improve swap-in protocol with taproot and musig2 #563
Merged
Conversation
This file contains 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
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
cf0c81b
to
294b474
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
3 times, most recently
from
6c784da
to
b159915
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
b159915
to
4a14bf8
Compare
t-bast
reviewed
Jan 5, 2024
src/commonMain/kotlin/fr/acinq/lightning/transactions/Transactions.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
4237120
to
3b231d8
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
49bec2e
to
bae7be8
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
4 times, most recently
from
2f2b29c
to
d22dfd1
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
3 times, most recently
from
91ea457
to
361faa9
Compare
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
4ca344d
to
bd7644c
Compare
t-bast
reviewed
Feb 2, 2024
There was a problem hiding this comment.
I've opened #592 to leverage the musig2 helpers and fix my remaining open comments. Once we converge on it and merge it into this PR, I think that the code will be in pretty good shape, the only thing that will remain will be to write unit tests that exercise the musig2 swaps and fix the SwapInManager tests!
t-bast
reviewed
Feb 5, 2024
|
The swap-in manager tests were failing when we experimented with not sending the full tx (I was creating wallet UTXOs manually), they've been restored now. |
|
Right, that was why! That makes sense, then we just need to add tests to |
t-bast
reviewed
Feb 6, 2024
src/commonTest/kotlin/fr/acinq/lightning/channel/InteractiveTxTestsCommon.kt
Show resolved
Hide resolved
src/commonTest/kotlin/fr/acinq/lightning/channel/InteractiveTxTestsCommon.kt
Show resolved
Hide resolved
src/commonTest/kotlin/fr/acinq/lightning/channel/InteractiveTxTestsCommon.kt
Outdated
Show resolved
Hide resolved
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
d69d4c0
to
02d89fb
Compare
t-bast
reviewed
Feb 6, 2024
src/commonTest/kotlin/fr/acinq/lightning/channel/InteractiveTxTestsCommon.kt
Outdated
Show resolved
Hide resolved
src/commonTest/kotlin/fr/acinq/lightning/channel/InteractiveTxTestsCommon.kt
Show resolved
Hide resolved
They use taproot v1, providing the tx output and not the entire tx is safe (see #579). Here we add the swap-in input output and txout to the swap-in TLV, so this change does not interface with proposed changes to the LN spec.
This reverts commit c0a6d5a.
We use the musig2 helpers exposed by ACINQ/bitcoin-kmp#114 to simplify the swap-in protocol and hide all of the musig2 internal details (key aggregation cache, control block, internal taproot key, opaque session object, nonce aggregation). The code is simpler to reason about and signing is more similar to signing normal single-sig inputs.
They were disabled when we experimented with not sending the full tx.
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
ecbc4c6
to
f963c21
Compare
t-bast
reviewed
Feb 12, 2024
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
2 times, most recently
from
48d7454
to
be62fc5
Compare
t-bast
reviewed
Feb 13, 2024
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
be62fc5
to
5e18d0b
Compare
t-bast
reviewed
Feb 13, 2024
The current recovery process needed to be updated to derive the correct master priv key from the seed by specifying our custom BIP32 path (m/52h/0h/2h/0) when we create the wallet. We also export 2 descriptor methods: one to get the private swap-in wallet descriptor, which can be used as-is, and the other to get the public swap-in wallet descriptor, which can be used to create a watch-only wallet to monitor swap-in funds and to recovery funds using our recovery procedure. Both descriptor use the refund master key, and not the master key itself because we use hardened paths to derive the refund key, which means that it is not possible to compute the refund master public key from the master public: importing the descriptor would fail.
sstone
force-pushed
the
snapshot/swap-in-potentiam-taproot
branch
from
5e18d0b
to
7d17663
Compare
t-bast
reviewed
Feb 14, 2024
Co-authored-by: Bastien Teinturier <[email protected]>
t-bast
approved these changes
Feb 15, 2024
pm47
approved these changes
Feb 15, 2024
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 an evolution of our p2swh-based swap-in that uses Musig2 and taproot for better privacy and lower on-chain fees.
Our swap-in protocol becomes
user key + server key OR user refund key + delay, and is implemented with taproot which provides a clean separation between the mutual use case (handled by the internal key, which is a Musig2 aggregation of the user's and server's key) and the refund case (handled by a taproot script with a single leaf: the refund script).Musig2 requires both parties to generate secret nonces and exchange public nonces before they can compute partial Musig2 signatures, which can then be combined into a single Schnorr signature:
TxComplete, which includes public nonces for all swap-in inputsTxSignatures, which includes partial Musig2 signatures for all swap-in inputsNonces are never persisted and their lifecycle is tied to the interactive tx session lifecycle: while the session is alive, it remembers nonces that it sent before i.e if
TxCompleteis sent several times, nonces for the same input id will be re-used. I believe that it is safe since they will be used only once for signing.A specific user refund key is used in the refund script: this allows us to easily rotate swap-in addresses (each time we change th user refund key) and generate a single taproot descriptor that will allow bitcoin core to find and spend (after the refund delay) all swap-in transactions for a given user (i.e for a given phoenix wallet).
This is a first step towards fixing ACINQ/phoenix#403: this new scheme makes swap-in address rotation easy to implement but this is out of the scope of this PR.
Swap-in addresses are now regular
p2traddresses, and swap-in transactions become indistinguishable from otherp2trtransactions.And since spending swap-in inputs requires a single 64 bytes Schnorr transaction (instead of 2 73 bytes DER transaction + a 77 bytes redeem script), the weight of a swap-in input is reduced by 40% for cheaper on-chain fees.
This PR is built on top of #565 because of ACINQ/secp256k1-kmp#93 but if needed changes to
secp256k1-kmpandbitcoin-kmpcould be back-ported to work with kotlin 1.8.