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 _ge_set_all_gej and use it in musig for own public nonces #1614

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/group.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ static void secp256k1_ge_set_gej(secp256k1_ge *r, secp256k1_gej *a);
/** Set a group element equal to another which is given in jacobian coordinates. */
static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a);

/** Set a batch of group elements equal to the inputs given in jacobian coordinates */
/** Set group elements r[0:len] (affine) equal to group elements a[0:len] (jacobian).
* None of the group elements in a[0:len] may be infinity. Constant time. */
static void secp256k1_ge_set_all_gej(secp256k1_ge *r, const secp256k1_gej *a, size_t len);

/** Set group elements r[0:len] (affine) equal to group elements a[0:len] (jacobian). */
static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a, size_t len);

/** Bring a batch of inputs to the same global z "denominator", based on ratios between
Expand Down
38 changes: 38 additions & 0 deletions src/group_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,44 @@ static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a) {
SECP256K1_GE_VERIFY(r);
}

static void secp256k1_ge_set_all_gej(secp256k1_ge *r, const secp256k1_gej *a, size_t len) {
secp256k1_fe u;
size_t i;
#ifdef VERIFY
for (i = 0; i < len; i++) {
SECP256K1_GEJ_VERIFY(&a[i]);
VERIFY_CHECK(!secp256k1_gej_is_infinity(&a[i]));
}
#endif

if (len == 0) {
return;
}

/* Use destination's x coordinates as scratch space */
r[0].x = a[0].z;
for (i = 1; i < len; i++) {
secp256k1_fe_mul(&r[i].x, &r[i - 1].x, &a[i].z);
}
secp256k1_fe_inv(&u, &r[len - 1].x);

for (i = len - 1; i > 0; i--) {
secp256k1_fe_mul(&r[i].x, &r[i - 1].x, &u);
secp256k1_fe_mul(&u, &u, &a[i].z);
}
r[0].x = u;

for (i = 0; i < len; i++) {
secp256k1_ge_set_gej_zinv(&r[i], &a[i], &r[i].x);
}

#ifdef VERIFY
for (i = 0; i < len; i++) {
SECP256K1_GE_VERIFY(&r[i]);
}
#endif
}

static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a, size_t len) {
secp256k1_fe u;
size_t i;
Expand Down
10 changes: 6 additions & 4 deletions src/modules/musig/session_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
secp256k1_scalar k[2];
secp256k1_ge nonce_pts[2];
secp256k1_gej nonce_ptj[2];
int i;
unsigned char pk_ser[33];
size_t pk_ser_len = sizeof(pk_ser);
Expand Down Expand Up @@ -445,12 +446,13 @@ int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_m
secp256k1_musig_secnonce_invalidate(ctx, secnonce, !ret);

for (i = 0; i < 2; i++) {
secp256k1_gej nonce_ptj;
secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj, &k[i]);
secp256k1_ge_set_gej(&nonce_pts[i], &nonce_ptj);
secp256k1_declassify(ctx, &nonce_pts[i], sizeof(nonce_pts));
secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj[i], &k[i]);
secp256k1_scalar_clear(&k[i]);
}
secp256k1_ge_set_all_gej(nonce_pts, nonce_ptj, 2);
for (i = 0; i < 2; i++) {
secp256k1_declassify(ctx, &nonce_pts[i], sizeof(nonce_pts));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not directly related to this PR, as the _declassify call is only moved but still called with the same values, but the size passed seems too large, if I'm not missing anything (should be sizeof(nonce_pts[i]), rather than the full array size, otherwise we mark beyond the array on the second iteration, and the second half of the array twice).

}
/* None of the nonce_pts will be infinity because k != 0 with overwhelming
* probability */
secp256k1_musig_pubnonce_save(pubnonce, nonce_pts);
Expand Down
26 changes: 25 additions & 1 deletion src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -3822,14 +3822,38 @@ static void test_ge(void) {

/* Test batch gej -> ge conversion without known z ratios. */
{
secp256k1_ge *ge_set_all_var = (secp256k1_ge *)checked_malloc(&CTX->error_callback, (4 * runs + 1) * sizeof(secp256k1_ge));
secp256k1_ge *ge_set_all = (secp256k1_ge *)checked_malloc(&CTX->error_callback, (4 * runs + 1) * sizeof(secp256k1_ge));
secp256k1_ge_set_all_gej_var(ge_set_all, gej, 4 * runs + 1);
secp256k1_ge_set_all_gej_var(&ge_set_all_var[0], &gej[0], 4 * runs + 1);
for (i = 0; i < 4 * runs + 1; i++) {
secp256k1_fe s;
testutil_random_fe_non_zero(&s);
secp256k1_gej_rescale(&gej[i], &s);
CHECK(secp256k1_gej_eq_ge_var(&gej[i], &ge_set_all_var[i]));
}

/* Skip infinity at &gej[0]. */
secp256k1_ge_set_all_gej(&ge_set_all[1], &gej[1], 4 * runs);
for (i = 1; i < 4 * runs + 1; i++) {
secp256k1_fe s;
testutil_random_fe_non_zero(&s);
secp256k1_gej_rescale(&gej[i], &s);
CHECK(secp256k1_gej_eq_ge_var(&gej[i], &ge_set_all[i]));
CHECK(secp256k1_ge_eq_var(&ge_set_all_var[i], &ge_set_all[i]));
}

/* Test with an array of length 1. */
secp256k1_ge_set_all_gej_var(ge_set_all_var, &gej[1], 1);
secp256k1_ge_set_all_gej(ge_set_all, &gej[1], 1);
CHECK(secp256k1_gej_eq_ge_var(&gej[1], &ge_set_all_var[1]));
CHECK(secp256k1_gej_eq_ge_var(&gej[1], &ge_set_all[1]));
CHECK(secp256k1_ge_eq_var(&ge_set_all_var[1], &ge_set_all[1]));

/* Test with an array of length 0. */
secp256k1_ge_set_all_gej_var(NULL, NULL, 0);
secp256k1_ge_set_all_gej(NULL, NULL, 0);

free(ge_set_all_var);
free(ge_set_all);
}

Expand Down
Loading