-
Notifications
You must be signed in to change notification settings - Fork 10
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
Conversation
fd246e9
to
581c617
Compare
951c843
to
962cefb
Compare
9ed75a4
to
0c79edd
Compare
0c79edd
to
a3355d2
Compare
7b9c200
to
f5219aa
Compare
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
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
@@ -22,7 +22,7 @@ buildscript { | |||
|
|||
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
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.