Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1265: Remove bits argument from secp256k…
Browse files Browse the repository at this point in the history 
…1_wnaf_const{_xonly}

a575339 Remove bits argument from secp256k1_wnaf_const (always 256) (Pieter Wuille)

Pull request description:

  There is little reason for having the number of bits in the scalar as a parameter, as I don't think there are any (current) use cases for non-256-bit scalars.

ACKs for top commit:
  jonasnick:
    ACK a575339
  real-or-random:
    utACK a575339

Tree-SHA512: 994b1f19b4c513f6d070ed259a5d6f221a0c2450271ec824c5eba1cd0ecace276de391c170285bfeae96aaf8f1e0f7fe6260966ded0336c75c522ab6c56d182c
  • Loading branch information
@real-or-random
real-or-random committed Apr 18, 2023
2 parents 566faa1 + a575339 commit 9ce9984
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 63 deletions.
2 changes: 1 addition & 1 deletion src/bench_ecmult.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ static void bench_ecmult_const(void* arg, int iters) {
int i;

for (i = 0; i < iters; ++i) {
secp256k1_ecmult_const(&data->output[i], &data->pubkeys[(data->offset1+i) % POINTS], &data->scalars[(data->offset2+i) % POINTS], 256);
secp256k1_ecmult_const(&data->output[i], &data->pubkeys[(data->offset1+i) % POINTS], &data->scalars[(data->offset2+i) % POINTS]);
}
}

Expand Down
5 changes: 1 addition & 4 deletions src/ecmult_const.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@

/**
* Multiply: R = q*A (in constant-time)
* Here `bits` should be set to the maximum bitlength of the _absolute value_ of `q`, plus
* one because we internally sometimes add 2 to the number during the WNAF conversion.
* A must not be infinity.
*/
static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *q, int bits);
static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *q);

/**
* Same as secp256k1_ecmult_const, but takes in an x coordinate of the base point
Expand All @@ -35,7 +33,6 @@ static int secp256k1_ecmult_const_xonly(
const secp256k1_fe *n,
const secp256k1_fe *d,
const secp256k1_scalar *q,
int bits,
int known_on_curve
);

Expand Down
61 changes: 22 additions & 39 deletions src/ecmult_const_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ static int secp256k1_wnaf_const(int *wnaf, const secp256k1_scalar *scalar, int w
return skew;
}

static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *scalar, int size) {
static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *scalar) {
secp256k1_ge pre_a[ECMULT_TABLE_SIZE(WINDOW_A)];
secp256k1_ge tmpa;
secp256k1_fe Z;
Expand All @@ -145,18 +145,10 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
int i;

/* build wnaf representation for q. */
int rsize = size;
if (size > 128) {
rsize = 128;
/* split q into q_1 and q_lam (where q = q_1 + q_lam*lambda, and q_1 and q_lam are ~128 bit) */
secp256k1_scalar_split_lambda(&q_1, &q_lam, scalar);
skew_1 = secp256k1_wnaf_const(wnaf_1, &q_1, WINDOW_A - 1, 128);
skew_lam = secp256k1_wnaf_const(wnaf_lam, &q_lam, WINDOW_A - 1, 128);
} else
{
skew_1 = secp256k1_wnaf_const(wnaf_1, scalar, WINDOW_A - 1, size);
skew_lam = 0;
}
/* split q into q_1 and q_lam (where q = q_1 + q_lam*lambda, and q_1 and q_lam are ~128 bit) */
secp256k1_scalar_split_lambda(&q_1, &q_lam, scalar);
skew_1 = secp256k1_wnaf_const(wnaf_1, &q_1, WINDOW_A - 1, 128);
skew_lam = secp256k1_wnaf_const(wnaf_lam, &q_lam, WINDOW_A - 1, 128);

/* Calculate odd multiples of a.
* All multiples are brought to the same Z 'denominator', which is stored
Expand All @@ -170,28 +162,23 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) {
secp256k1_fe_normalize_weak(&pre_a[i].y);
}
if (size > 128) {
for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) {
secp256k1_ge_mul_lambda(&pre_a_lam[i], &pre_a[i]);
}

for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) {
secp256k1_ge_mul_lambda(&pre_a_lam[i], &pre_a[i]);
}

/* first loop iteration (separated out so we can directly set r, rather
* than having it start at infinity, get doubled several times, then have
* its new value added to it) */
i = wnaf_1[WNAF_SIZE_BITS(rsize, WINDOW_A - 1)];
i = wnaf_1[WNAF_SIZE_BITS(128, WINDOW_A - 1)];
VERIFY_CHECK(i != 0);
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, i, WINDOW_A);
secp256k1_gej_set_ge(r, &tmpa);
if (size > 128) {
i = wnaf_lam[WNAF_SIZE_BITS(rsize, WINDOW_A - 1)];
VERIFY_CHECK(i != 0);
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, i, WINDOW_A);
secp256k1_gej_add_ge(r, r, &tmpa);
}
i = wnaf_lam[WNAF_SIZE_BITS(128, WINDOW_A - 1)];
VERIFY_CHECK(i != 0);
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, i, WINDOW_A);
secp256k1_gej_add_ge(r, r, &tmpa);
/* remaining loop iterations */
for (i = WNAF_SIZE_BITS(rsize, WINDOW_A - 1) - 1; i >= 0; i--) {
for (i = WNAF_SIZE_BITS(128, WINDOW_A - 1) - 1; i >= 0; i--) {
int n;
int j;
for (j = 0; j < WINDOW_A - 1; ++j) {
Expand All @@ -202,12 +189,10 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a, n, WINDOW_A);
VERIFY_CHECK(n != 0);
secp256k1_gej_add_ge(r, r, &tmpa);
if (size > 128) {
n = wnaf_lam[i];
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, n, WINDOW_A);
VERIFY_CHECK(n != 0);
secp256k1_gej_add_ge(r, r, &tmpa);
}
n = wnaf_lam[i];
ECMULT_CONST_TABLE_GET_GE(&tmpa, pre_a_lam, n, WINDOW_A);
VERIFY_CHECK(n != 0);
secp256k1_gej_add_ge(r, r, &tmpa);
}

{
Expand All @@ -218,17 +203,15 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
secp256k1_gej_add_ge(&tmpj, r, &tmpa);
secp256k1_gej_cmov(r, &tmpj, skew_1);

if (size > 128) {
secp256k1_ge_neg(&tmpa, &pre_a_lam[0]);
secp256k1_gej_add_ge(&tmpj, r, &tmpa);
secp256k1_gej_cmov(r, &tmpj, skew_lam);
}
secp256k1_ge_neg(&tmpa, &pre_a_lam[0]);
secp256k1_gej_add_ge(&tmpj, r, &tmpa);
secp256k1_gej_cmov(r, &tmpj, skew_lam);
}

secp256k1_fe_mul(&r->z, &r->z, &Z);
}

static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n, const secp256k1_fe *d, const secp256k1_scalar *q, int bits, int known_on_curve) {
static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n, const secp256k1_fe *d, const secp256k1_scalar *q, int known_on_curve) {

/* This algorithm is a generalization of Peter Dettman's technique for
* avoiding the square root in a random-basepoint x-only multiplication
Expand Down Expand Up @@ -346,7 +329,7 @@ static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n,
#ifdef VERIFY
VERIFY_CHECK(!secp256k1_scalar_is_zero(q));
#endif
secp256k1_ecmult_const(&rj, &p, q, bits);
secp256k1_ecmult_const(&rj, &p, q);
#ifdef VERIFY
VERIFY_CHECK(!secp256k1_gej_is_infinity(&rj));
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/modules/ecdh/main_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se
overflow |= secp256k1_scalar_is_zero(&s);
secp256k1_scalar_cmov(&s, &secp256k1_scalar_one, overflow);

secp256k1_ecmult_const(&res, &pt, &s, 256);
secp256k1_ecmult_const(&res, &pt, &s);
secp256k1_ge_set_gej(&pt, &res);

/* Compute a hash of the point */
Expand Down
30 changes: 15 additions & 15 deletions src/tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4338,9 +4338,9 @@ static void test_ecmult_target(const secp256k1_scalar* target, int mode) {
secp256k1_ecmult(&p2j, &pj, &n2, &zero);
secp256k1_ecmult(&ptj, &pj, target, &zero);
} else {
secp256k1_ecmult_const(&p1j, &p, &n1, 256);
secp256k1_ecmult_const(&p2j, &p, &n2, 256);
secp256k1_ecmult_const(&ptj, &p, target, 256);
secp256k1_ecmult_const(&p1j, &p, &n1);
secp256k1_ecmult_const(&p2j, &p, &n2);
secp256k1_ecmult_const(&ptj, &p, target);
}

/* Add them all up: n1*P + n2*P + target*P = (n1+n2+target)*P = (n1+n1-n1-n2)*P = 0. */
Expand Down Expand Up @@ -4403,7 +4403,7 @@ static void ecmult_const_random_mult(void) {
0xb84e4e1b, 0xfb77e21f, 0x96baae2a, 0x63dec956
);
secp256k1_gej b;
secp256k1_ecmult_const(&b, &a, &xn, 256);
secp256k1_ecmult_const(&b, &a, &xn);

CHECK(secp256k1_ge_is_valid_var(&a));
ge_equals_gej(&expected_b, &b);
Expand All @@ -4419,12 +4419,12 @@ static void ecmult_const_commutativity(void) {
random_scalar_order_test(&a);
random_scalar_order_test(&b);

secp256k1_ecmult_const(&res1, &secp256k1_ge_const_g, &a, 256);
secp256k1_ecmult_const(&res2, &secp256k1_ge_const_g, &b, 256);
secp256k1_ecmult_const(&res1, &secp256k1_ge_const_g, &a);
secp256k1_ecmult_const(&res2, &secp256k1_ge_const_g, &b);
secp256k1_ge_set_gej(&mid1, &res1);
secp256k1_ge_set_gej(&mid2, &res2);
secp256k1_ecmult_const(&res1, &mid1, &b, 256);
secp256k1_ecmult_const(&res2, &mid2, &a, 256);
secp256k1_ecmult_const(&res1, &mid1, &b);
secp256k1_ecmult_const(&res2, &mid2, &a);
secp256k1_ge_set_gej(&mid1, &res1);
secp256k1_ge_set_gej(&mid2, &res2);
ge_equals_ge(&mid1, &mid2);
Expand All @@ -4440,13 +4440,13 @@ static void ecmult_const_mult_zero_one(void) {
secp256k1_scalar_negate(&negone, &one);

random_group_element_test(&point);
secp256k1_ecmult_const(&res1, &point, &zero, 3);
secp256k1_ecmult_const(&res1, &point, &zero);
secp256k1_ge_set_gej(&res2, &res1);
CHECK(secp256k1_ge_is_infinity(&res2));
secp256k1_ecmult_const(&res1, &point, &one, 2);
secp256k1_ecmult_const(&res1, &point, &one);
secp256k1_ge_set_gej(&res2, &res1);
ge_equals_ge(&res2, &point);
secp256k1_ecmult_const(&res1, &point, &negone, 256);
secp256k1_ecmult_const(&res1, &point, &negone);
secp256k1_gej_neg(&res1, &res1);
secp256k1_ge_set_gej(&res2, &res1);
ge_equals_ge(&res2, &point);
Expand Down Expand Up @@ -4476,7 +4476,7 @@ static void ecmult_const_mult_xonly(void) {
n = base.x;
}
/* Perform x-only multiplication. */
res = secp256k1_ecmult_const_xonly(&resx, &n, (i & 1) ? &d : NULL, &q, 256, i & 2);
res = secp256k1_ecmult_const_xonly(&resx, &n, (i & 1) ? &d : NULL, &q, i & 2);
CHECK(res);
/* Perform normal multiplication. */
secp256k1_gej_set_ge(&basej, &base);
Expand Down Expand Up @@ -4509,7 +4509,7 @@ static void ecmult_const_mult_xonly(void) {
} else {
n = x;
}
res = secp256k1_ecmult_const_xonly(&r, &n, (i & 1) ? &d : NULL, &q, 256, 0);
res = secp256k1_ecmult_const_xonly(&r, &n, (i & 1) ? &d : NULL, &q, 0);
CHECK(res == 0);
}
}
Expand All @@ -4534,7 +4534,7 @@ static void ecmult_const_chain_multiply(void) {
for (i = 0; i < 100; ++i) {
secp256k1_ge tmp;
secp256k1_ge_set_gej(&tmp, &point);
secp256k1_ecmult_const(&point, &tmp, &scalar, 256);
secp256k1_ecmult_const(&point, &tmp, &scalar);
}
secp256k1_ge_set_gej(&res, &point);
ge_equals_gej(&res, &expected_point);
Expand Down Expand Up @@ -5432,7 +5432,7 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar
secp256k1_ecmult(&rj3, &infj, &zero, x);
secp256k1_ecmult_multi_var(NULL, scratch, &rj4, x, NULL, NULL, 0);
secp256k1_ecmult_multi_var(NULL, scratch, &rj5, &zero, test_ecmult_accumulate_cb, (void*)x, 1);
secp256k1_ecmult_const(&rj6, &secp256k1_ge_const_g, x, 256);
secp256k1_ecmult_const(&rj6, &secp256k1_ge_const_g, x);
secp256k1_ge_set_gej_var(&r, &rj1);
ge_equals_gej(&r, &rj2);
ge_equals_gej(&r, &rj3);
Expand Down
6 changes: 3 additions & 3 deletions src/tests_exhaustive.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,19 +203,19 @@ static void test_exhaustive_ecmult(const secp256k1_ge *group, const secp256k1_ge
secp256k1_scalar_set_int(&ng, j);

/* Test secp256k1_ecmult_const. */
secp256k1_ecmult_const(&tmp, &group[i], &ng, 256);
secp256k1_ecmult_const(&tmp, &group[i], &ng);
ge_equals_gej(&group[(i * j) % EXHAUSTIVE_TEST_ORDER], &tmp);

if (j != 0) {
/* Test secp256k1_ecmult_const_xonly with all curve X coordinates, and xd=NULL. */
ret = secp256k1_ecmult_const_xonly(&tmpf, &group[i].x, NULL, &ng, 256, 0);
ret = secp256k1_ecmult_const_xonly(&tmpf, &group[i].x, NULL, &ng, 0);
CHECK(ret);
CHECK(secp256k1_fe_equal_var(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));

/* Test secp256k1_ecmult_const_xonly with all curve X coordinates, with random xd. */
random_fe_non_zero(&xd);
secp256k1_fe_mul(&xn, &xd, &group[i].x);
ret = secp256k1_ecmult_const_xonly(&tmpf, &xn, &xd, &ng, 256, 0);
ret = secp256k1_ecmult_const_xonly(&tmpf, &xn, &xd, &ng, 0);
CHECK(ret);
CHECK(secp256k1_fe_equal_var(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));
}
Expand Down

0 comments on commit 9ce9984

Please sign in to comment.