[5/? ] - lnwallet: add taproot funding support to the internal wallet flow (reservations)#7344
Conversation
be2c89c to
58e6bef
Compare
8b3a3d6 to
6fc2387
Compare
58e6bef to
ce6879c
Compare
6fc2387 to
947096b
Compare
f8bc68e to
e895d9d
Compare
|
Initial round of comments addressed, PTAL! |
There was a problem hiding this comment.
So this is the nonce that's sent in open_channel by the funder and can be used to generate the final sig when receiving funding_signed.nonce from the fundee right?
There was a problem hiding this comment.
Correct, this is where we use the counter based method to generate nonces we can always arrive at again.
In part 4, I started to also bind the nonce creation with the tx hash of the tx we're about to sign. However at this point, we need to send a nonce, and don't yet know the tx hash so we can't do so.
We'll still do that each time we go to sign a new commitment, as the tx hash then is already known.
691ecd9 to
d22a0f9
Compare
d76ae7a to
bfaefbe
Compare
|
Latest round of comments addressed, PTAL! |
yyforyongyu
left a comment
There was a problem hiding this comment.
Verrry close🤙🤙🤙 Need to fix the itest compile error, plus the linter error.
There was a problem hiding this comment.
just occurs to me, why is this named simple taproot🤓?
There was a problem hiding this comment.
So initial AJ had a much more feature complete and ambitious proposal to basically do everything (taproot, PTLCs, new commitment type, scriptless scripts) all in one. I thought it was a bit too much to bite off at once, so two spec meetings around I proposed a staged roll out proposal: musig2 output first, then PTLC+scriptless script, then commitment format changes). Thus was born "simple taproot".
d22a0f9 to
7c85f02
Compare
bfaefbe to
4cab0ce
Compare
ellemouton
left a comment
There was a problem hiding this comment.
LGTM 🔥 🥕⚡
A couple of linter errors to catch - most of them are lll ones
There was a problem hiding this comment.
non blocking
am assuming we will calculate the witness script then for taproot chans at the time of spending since we would need to know which which branch we are spending in any case to build the control block at spend time. Other option would be to add a isLocalCommit bool to the CommitScriptToSelf method and then pre-compute and return the witness script & control block. That way we also dont have to derive the tree twice.
There was a problem hiding this comment.
See later in the series, this is modified further to include the full script tree information.
yyforyongyu
left a comment
There was a problem hiding this comment.
Let's fix the linter errors and we can⛵️
There was a problem hiding this comment.
Leaving some notes for other reviewers, was wondering if lc.currentHeight is always updated to date here since NewAnchorResolutions makes a separate call to db and creates a new LightningWallet. This is fine as we'd only call it when StateCommitmentBroadcasted, which means the db has the latest view of the local commit, and no other goroutines can update it anymore because it's closed.
4cab0ce to
781a70f
Compare
In this commit, we add a new wallet level channel type, along with the new fields we'll need to accept from both parties within the contribution messages. In this case, we now have a local nonce, along with the internal musig session.
We also update some of the resolutions (even though they aren't hooked up yet), as they need to be able to properly re-create the set of scripts.
In this commit, we build on all the prior commits and integrate the new taproot channels into the existing internal funding flow. Along the way, we do some refactoring to unify things like signing and verifying incoming commitment transaction signatures. For our local nonce, we use the existing functional option type to derive the nonce based on the initial shachain pre-image we'll use as our revocation.
In this commit, we modify the starting logic to note attempt to add a tower client for taproot channels. Instead, we'll just log that this isn't available yet.
781a70f to
2d4df80
Compare
Change Description
This depends on #7340.
In this PR, we build on all the prior commits and integrate the new taproot channels into the existing internal funding flow. Along the way, we do some refactoring to unify things like signing and verifying incoming commitment transaction signatures.
For our local nonce, we use the existing functional option type to derive the nonce based on the initial shachain pre-image we'll use as our revocation.
We also hook up the new funding flow to the existing internal wallet integration tests:
The last ~14 commits are new. Along the way some rebase issues were found and fixed in commits marked as
[temp].The next PR in this series will modify the channel state machine to understand the new commitment dance.
Steps to Test
Steps for reviewers to follow to test the change.
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.