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

Add support for musig2 #93

Merged
merged 10 commits into from
Feb 14, 2024
Merged

Add support for musig2 #93

merged 10 commits into from
Feb 14, 2024

Conversation

sstone
Copy link
Member

@sstone sstone commented Dec 13, 2023

We use https://github.com/jonasnick/secp256k1/tree/musig2-module instead of https://github.com/bitcoin-core/secp256k1, we'll switch back to secp256k1 once the musig2 module has been included.

This branch is based on https://github.com/ACINQ/secp256k1-kmp/tree/snapshot/kotlin-1.9 because it's extremely tedious to work with kotlin's c-interop on kotlin 1.8 but changes could easily be backported if needed before we upgrade.

This PR "tracks" bitcoin-core/secp256k1#1479 and should be ready very soon once it has been merged.

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, once the secp256k1 PR is merged we should be good to go 👍

sstone and others added 7 commits January 23, 2024 22:28
Usage of the Musig2 functions isn't intuitive at all, especially with
the key aggregation cache and session data. It's important to provide
accurate documentation to help users understand how to correctly produce
musig2 signatures.

We also change argument names to match Kotlin best practices instead of
using the same argument names as C functions.
@sstone sstone marked this pull request as ready for review February 14, 2024 11:51
@sstone sstone requested a review from t-bast February 14, 2024 12:23
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

@@ -22,7 +22,7 @@ buildscript {

allprojects {
group = "fr.acinq.secp256k1"
version = "0.14.0-SNAPSHOT"
version = "0.14.0-MUSIG2-SNAPSHOT"
Copy link
Member

@t-bast t-bast Feb 14, 2024

Choose a reason for hiding this comment

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

This isn't necessary anymore, is it? We'll even change this to 0.14.0 for the release, right? We can change that in a release PR though that sets it explicitly to 0.14.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I'll release version 0.14.0 as soon as the PR is merged

@sstone sstone merged commit 202b0c9 into master Feb 14, 2024
6 checks passed
@sstone sstone deleted the snapshot/musig2 branch February 14, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants