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
WIP: Add aggsig example code #505
Conversation
08a63fb
to
fa7dba6
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.
Tested ACK
examples/aggsig.c
Outdated
return 1; | ||
} | ||
|
||
/* Sign a message hash with the given key pairs and and store the result in 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.
and and
examples/aggsig.c
Outdated
} | ||
|
||
/* Verify an aggregated signature of n_pubkeys pubkeys over msghash */ | ||
int verify(const secp256k1_context* ctx, const unsigned char *sig, const unsigned char *msghash, const secp256k1_pubkey *pubkeys, size_t n_pubkeys) { |
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.
You use N_PUBKEYS
in other functions but here you add a size_t n_pubkeys
. Particular reason or just ended up that way? (You only call this with N_PUBKEYS
as argument I think.)
examples/aggsig.c
Outdated
scratch_size = 9000; | ||
} | ||
scratch = secp256k1_scratch_space_create(ctx, 0, scratch_size); | ||
if(scratch == NULL) { |
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.
Nit: You're mixing if()
and if ()
style, and the same for for()
and for ()
. Personally prefer the latter, but probably shouldn't mix.
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.
good catch, thanks!
int i; | ||
unsigned char seckeys[N_PUBKEYS][32]; | ||
secp256k1_pubkey pubkeys[N_PUBKEYS]; | ||
unsigned char msghash[32] = "this_should_actually_be_msg_hash"; |
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 will actually overbuffer, writing a \0
into the 33rd byte of msghash
. Surprised compilers don't warn about that.
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.
Hm, http://port70.net/~nsz/c/c89/c89-draft.html#3.5.6 says terminating null characters are only added "if there is room".
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.
That would explain the lack of compiler warnings! First thing I ever heard of such an arbitrary rule. C can be freaky.
fa7dba6
to
c5a4486
Compare
Closing as this will be probably superseded by MuSig |
Based on #461 + some things I suggested for that PR.
I think it'd be a good idea to have some example code to show how to use the aggsig module. For example, the seed of the nonce RNG could be misused and it's not all that intuitive how a scratch space is used. This PR addresses this by adding an
aggsig.c
file into anexamples/
directory and building it asexample_aggsig
(it's also run as part ofmake check
).Since #461 does not allow for multi party signing at the moment,
aggsig.c
demonstrates single user signing and verification for now.