-
Notifications
You must be signed in to change notification settings - Fork 211
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 MuSig2 adaptor sig exercise in example code #188
base: master
Are you sure you want to change the base?
Add MuSig2 adaptor sig exercise in example code #188
Conversation
I would like to see in the example |
427ca95
to
e502240
Compare
@rage-proof year late, but I came back around to the API and would have found it useful so done. |
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.
Concept ACK
I wonder if this example should go to a separate function. Adaptors add quite a lot to the flow and increase the brain power necessary to follow this example, so this is probably confusing for users who want to see a minimal example. If that means duplicating some of the code, this is not a problem, I think.
if (!secp256k1_musig_adapt(ctx, sig64, presig, adaptor_key, nonce_parity)) { | ||
return 0; | ||
} | ||
/* With sig64 "on-chain" now, other party can grab the revealed adaptor secret */ |
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.
/* With sig64 "on-chain" now, other party can grab the revealed adaptor secret */ | |
/* With sig64 "on-chain" now, other party can grab the revealed adaptor secret */ |
wrong indentation
if (!secp256k1_musig_nonce_parity(ctx, &nonce_parity, &session)) { | ||
return 0; | ||
} | ||
if (!secp256k1_musig_partial_sig_agg(ctx, presig, &session, partial_sigs, N_SIGNERS)){ |
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.
if (!secp256k1_musig_partial_sig_agg(ctx, presig, &session, partial_sigs, N_SIGNERS)){ | |
if (!secp256k1_musig_partial_sig_agg(ctx, presig, &session, partial_sigs, N_SIGNERS)) { |
if (!secp256k1_musig_extract_adaptor(ctx, extracted_adaptor, sig64, presig, nonce_parity)) { | ||
return 0; | ||
} | ||
if (memcmp(extracted_adaptor, adaptor_key, sizeof(adaptor_key)) != 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.
Can you assert
this? We normally use assert
for these comparisons in the other examples.
@@ -201,7 +233,7 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st | |||
return 1; | |||
} | |||
printf("ok\n"); | |||
printf("Verifying signature....."); | |||
printf("Verifying signature and revealed adaptor....."); |
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.
I don't think we're verifying the adaptor here?
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.
errr extracting revealed secret, I guess is the phrase?
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.
I'm only saying that the printf not match the code below, which still only verifies a sig.
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.
oh, I added this in the wrong spot
Code examples are clearer than doc explanations in a few ways, so figured I'd just submit this.