Merge #1614: Add _ge_set_all_gej and use it in musig for own public nonces

64228a6 musig: Use _ge_set_all_gej for own public nonces (Tim Ruffing)
300aab1 tests: Improve _ge_set_all_gej(_var) tests (Tim Ruffing)
365f274 group: Simplify secp256k1_ge_set_all_gej (Tim Ruffing)
d3082dd group: Add constant-time secp256k1_ge_set_all_gej (Tim Ruffing)

Pull request description:

  As suggested in #1479 (comment)

ACKs for top commit:
  theStack:
    re-ACK 64228a6
  sipa:
    ACK 64228a6

Tree-SHA512: f62a95e44dc09bb55a64da0640ad323e7ef5acc262d3c2aea6787eae0918769ea97da466b7d602e59693e4fb85c5ec9a67fdfba8b890624467855b6d1e1596c0

Author: real-or-random

src/group.h

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

src/group_impl.h

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

src/modules/musig/session_impl.h

@@ -395,6 +395,7 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
 static 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);
@@ -445,13 +446,14 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp
     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[i]));
+        secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj[i], &k[i]);
         secp256k1_scalar_clear(&k[i]);
         secp256k1_gej_clear(&nonce_ptj);
     }
+    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[i]));
+    }
     /* None of the nonce_pts will be infinity because k != 0 with overwhelming
      * probability */
     secp256k1_musig_pubnonce_save(pubnonce, nonce_pts);

src/tests.c

@@ -3821,14 +3821,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);
     }