Merge #1265: Remove bits argument from secp256k1_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

Author: real-or-random

src/bench_ecmult.c

@@ -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]);
     }
 }
 

src/ecmult_const.h

@@ -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
@@ -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
 );
 

src/ecmult_const_impl.h

@@ -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;
@@ -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
@@ -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) {
@@ -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);
     }
 
     {
@@ -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
@@ -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

src/modules/ecdh/main_impl.h

@@ -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 */

src/tests.c

@@ -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. */
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);
     }
 }
@@ -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);
@@ -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);

src/tests_exhaustive.c

@@ -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));
             }