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

multi: merge simple taproot channels staging branch into master #7904

Merged
merged 134 commits into from Aug 23, 2023

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Aug 22, 2023

In this PR, we propose merging the simple taproot channels staging branch into the master branch. Each of the side branches has been progressively merged into the staging branch over the past few months in an incremental manner. The final PR was merged earlier today with a clean build on CI. The purpose of this PR is to catch up the master branch with all the changes up until now.

All the tests pass for me locally, but I had to make some changes for the on chain force close zero conf tests. The master branch now mines all the transactions in a single block, even if they're zero conf. While the master branch when the feature was branched off on would only mine again it it wasn't private && zeroConf. Even though all the blocks are a mined now, I still spot an occasional flake where even though Carol should have gone to chain, she doesn't seem to execute on that.

Fixes #6691

@Roasbeef
Copy link
Member Author

One other thing I noticed while catching up the branch is that signrpc.MuSig2SessionRequest now needs to be updated to properly serialize the new set of options into the proto. This only applies to the remote signer context. The new options are what allows us to generate the nonces early, and then later bind them to the musig2 session.

@github-actions
Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments
guggero
🥇
10
▀▀▀
1d 8h 8m
20
▀▀▀▀▀▀
yyforyongyu
🥈
9
▀▀▀
1d 6h 40m
5
Roasbeef
🥉
3
4d 10h 3m
▀▀
3
ellemouton
3
6d 18h 11m
▀▀▀
0
ProofOfKeags
2
2d 5h
1
positiveblue
1
5h 1m
0
Crypt-iQ
1
1d 1h 55m
1
morehouse
1
1d 1h 55m
1
saubyk
1
7h 15m
0
blckbx
1
50m
2
ziggie1984
1
20h 48m
3

@saubyk saubyk added this to the v0.17.0 milestone Aug 22, 2023
@Roasbeef
Copy link
Member Author

Took another stab at the rebased version, wasn't as bad as I thought it'd be after getting thru the first 30 or so, pushing that up shortly.

@Roasbeef Roasbeef force-pushed the simple-taproot-chans-staging branch 2 times, most recently from eac49f8 to 13f3419 Compare August 22, 2023 23:26
In this commit, we update the Sig type to support ECDSA and schnorr
signatures. We need to do this as the HTLC signatures will become
schnorr sigs for taproot channels. The current spec draft opts to
overload this field since both the sigs are actually 64 bytes in length.
The only consideration with this move is that callers need to "coerce" a
sig to the proper type if they need schnorr signatures.
In this commit, we add the new types that'll house musig signatures with
and without their nonces. We send the nonce along with the sig
everywhere but the co-op close flow.
This ensures that the caller doesn't need to worry about the TLV type
ordering of the records the pass into the function.
In this commit, we add a helper function to take a taproot output key
and turn it into a v1 witness program.
In this commit, we add GenTaprootFundingScript, which'll return the
taproot pkScript and output for a taproot+musig2 channel. This uses
musig2 key aggregation with sorting activated.

The final key produced uses a bip86 tweak, meaning that the output key
provably doesn't commit to any script path. In the future, we may want
to permit this, as then it allows for a greater degree of
programmability of the funding output.
This is a preparatory change for the upcoming "simple channel close"
feature which'll utilize RBF to allow either side to resign the co-op
close transaction for broadcast at any point.
The local nonce needs to be the one that's finalized to simulate us
receiving the remote nonce, then generating the local nonce in a JIT
style.
Otherwise, in the itests (which are mainly based on implicit
negotiation), we won't try to open taproot chans when we actually need
to.

We may want to revisit this however, since it may lock in parties trying
to use the defaults that aren't currently setting the explicit commit
type during funding.
We keep in line with the existing test that uses implicit negotiation.
In the future, we should modify to also have explicit negotiation as
well.
In this commit, we remove the internal call to `InitRemoteMusigNonces`.
We don't need this since when we go to process the remote party's chan
reest message, we'll already call this method. Otherwise, we'll get an
error here since the pending verification nonce has been wiped out after
each call.
The first and second level taptweaks need to be stored in order to
ensure the breach arb can play revocations at both the first and second
level.
Co-op close for musig2 chans needs to use the proper witness size.
We also remove the old implicit negotiation as well, as we'll be
updating tests to use explciit when required.
We use a helper function to ensure that anytime we're about to make a
normal sighash, we consult the channel type to check if we should use
the default value or sighash all explicitly.
This adds some extra assertions to ensure things like the taproot
commitment weight estimation is correct.
In this commit, we carry out a new notion introduced during a recent
spec meeting to use a feature bit plus 100 before the feature has been
finalized in the spec.

We split into the Final and Staging bits.
The prior commit removed implicit negotiation, so we'll need to make
sure to use the explicit chan type feature vector when we go to
negotiate.
In the master branch, the blocks are mined earlier, so we don't need to
mine an extra set.
@Roasbeef Roasbeef force-pushed the simple-taproot-chans-staging branch from 13f3419 to 92da6b1 Compare August 22, 2023 23:35
@saubyk saubyk self-requested a review August 23, 2023 02:20
Copy link
Collaborator

@saubyk saubyk left a comment

Choose a reason for hiding this comment

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

Concept Ack 🥕
Go Taproot channels 😁 🚀

@Roasbeef Roasbeef merged commit e2f5374 into master Aug 23, 2023
22 of 25 checks passed
@guggero guggero deleted the simple-taproot-chans-staging branch August 23, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Epic] funding+contractcourt+lnwallet: implement an initial version of Simple Taproot Channels
2 participants