Skip to content

Add low-level taproot helpers #3086

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
Merged
merged 10 commits into from
May 23, 2025
Merged

Add low-level taproot helpers #3086

merged 10 commits into from
May 23, 2025
+527 −677

Conversation

sstone
Copy link
Member

@sstone sstone commented May 15, 2025

We add a new commitment format for simple taproot channels, and implement creating and verifying musig2 and taproot signatures for these channels.
Wire format and channel creation/update workflows have not been modified, this will be done in another PR.

@sstone sstone requested a review from t-bast May 15, 2025 13:26
@sstone sstone marked this pull request as ready for review May 15, 2025 13:26
@sstone sstone mentioned this pull request May 15, 2025
Closed
t-bast
t-bast reviewed May 16, 2025
View reviewed changes
Copy link
Member

@t-bast t-bast left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think there's a bit more work to do around taproot scripts and witnesses to make it cleaner by introducing better types and encapsulation of script trees and their corresponding witnesses. My main comment that requires more work is #3086 (comment), the rest is mostly nits for now.

t-bast
t-bast reviewed May 19, 2025
View reviewed changes
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

This is looking much better than the attempts we made before the transactions architecture refactoring, that was worth it! I think we're almost there, but can still simplify it a bit.

@t-bast
Copy link
Member

t-bast commented May 22, 2025

Needs rebase!

sstone added 8 commits May 22, 2025 10:10
@sstone
Add a new commiment format for simple taproot channels
8016502 
There are no functional changes, we just define a new format that is not used anywhere yet, with dummy "not implemented" blocks when needed.
@sstone
Implement low-level tooling support for simple taproot channels
444f152 
We can create, sign, spend commit and htlc transactions for the new simple taproot channels commitment format.
Wire messages and channel creation/update workflow have not been modified.
@sstone
Fixup: address review comments
b53328d
@sstone
Fixup: Add new types to simplify how we create script trees
0ca8e38 
We only either single leaves or branches with 2 single leaves (a timeout and a success branch for example), having
specific types for them makes to code easier to read.
@sstone
Fixup: remove unused import
0be491c
@sstone
Simplify and extend transaction tests
a9c411c
@sstone
Fixup: Simplify makeCommitTxOutputs()
7520a9f
@sstone
Use Musig2.verifyTaprootSignature from bitcoin-lib 0.39
8fcc0f9
@sstone sstone force-pushed the add-taproot-helpers branch from 668da46 to 8fcc0f9 Compare May 22, 2025 08:30
t-bast
t-bast reviewed May 22, 2025
View reviewed changes
t-bast
t-bast approved these changes May 23, 2025
View reviewed changes
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM! It's really nice to see that this PR adds a lot of features and improves test coverage while decreasing the number of lines of code!

@t-bast t-bast merged commit dd622ad into master May 23, 2025
1 check passed
@t-bast t-bast deleted the add-taproot-helpers branch May 23, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-bast t-bast t-bast approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sstone @t-bast