-
Notifications
You must be signed in to change notification settings - Fork 10
Add support for musig2 #93
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
Conversation
fd246e9 to
581c617
Compare
951c843 to
962cefb
Compare
9ed75a4 to
0c79edd
Compare
0c79edd to
a3355d2
Compare
7b9c200 to
f5219aa
Compare
t-bast
left a comment
There was a problem hiding this 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 👍
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.
f5219aa to
0976292
Compare
t-bast
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| allprojects { | ||
| group = "fr.acinq.secp256k1" | ||
| version = "0.14.0-SNAPSHOT" | ||
| version = "0.14.0-MUSIG2-SNAPSHOT" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
We use https://github.com/jonasnick/secp256k1/tree/musig2-module instead of https://github.com/bitcoin-core/secp256k1, we'll switch back to
secp256k1once 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.