Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules

2f73e5281d group: Avoid using infinity field directly in other modules (Tim Ruffing)

Pull request description:

  Minor refactoring to make the abstraction cleaner

ACKs for top commit:
  hebasto:
    ACK 2f73e5281d, I have reviewed the code and it looks OK.
  theStack:
    ACK 2f73e5281d

Tree-SHA512: eae5ad1ce81f491adb48ab1cbf04211f8d43e41255abcacc958fa3dcb1de5021707d56ed1b009a6f3f6c45cd8f20c1f2677891690a3c0a467fc7e064af2512a8

src/ecmult_const_impl.h

@@ -87,17 +87,15 @@ static void secp256k1_ecmult_const_odd_multiples_table_globalz(secp256k1_ge *pre
     secp256k1_fe neg_y; \
     VERIFY_CHECK((n) < (1U << ECMULT_CONST_GROUP_SIZE)); \
     VERIFY_CHECK(index < (1U << (ECMULT_CONST_GROUP_SIZE - 1))); \
-    /* Unconditionally set r->x = (pre)[m].x. r->y = (pre)[m].y. because it's either the correct one
+    /* Unconditionally set r->x = (pre)[m].x and r->y = (pre)[m].y because it's either the correct one
      * or will get replaced in the later iterations, this is needed to make sure `r` is initialized. */ \
-    (r)->x = (pre)[m].x; \
+    secp256k1_ge_set_xy((r), &(pre)[m].x, &(pre)[m].y); \
-    (r)->y = (pre)[m].y; \
     for (m = 1; m < ECMULT_CONST_TABLE_SIZE; m++) { \
         /* This loop is used to avoid secret data in array indices. See
          * the comment in ecmult_gen_impl.h for rationale. */ \
         secp256k1_fe_cmov(&(r)->x, &(pre)[m].x, m == index); \
         secp256k1_fe_cmov(&(r)->y, &(pre)[m].y, m == index); \
     } \
-    (r)->infinity = 0; \
     secp256k1_fe_negate(&neg_y, &(r)->y, 1); \
     secp256k1_fe_cmov(&(r)->y, &neg_y, negative); \
 } while(0)
@@ -375,11 +373,14 @@ static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n,
 
     SECP256K1_FE_VERIFY_MAGNITUDE(&g, 2);
 
-    /* Compute base point P = (n*g, g^2), the effective affine version of (n*g, g^2, v), which has
+    /* Compute base point P = (n*g, g^2), the effective affine version of
-     * corresponding affine X coordinate n/d. */
+     * (n*g, g^2, v), which has corresponding affine X coordinate n/d. */
-    secp256k1_fe_mul(&p.x, &g, n);
+    {
-    secp256k1_fe_sqr(&p.y, &g);
+        secp256k1_fe x, y;
-    p.infinity = 0;
+        secp256k1_fe_mul(&x, &g, n);
+        secp256k1_fe_sqr(&y, &g);
+        secp256k1_ge_set_xy(&p, &x, &y);
+    }
 
     /* Perform x-only EC multiplication of P with q. */
     VERIFY_CHECK(!secp256k1_scalar_is_zero(q));

src/ecmult_impl.h

@@ -75,7 +75,7 @@ static void secp256k1_ecmult_odd_multiples_table(int n, secp256k1_ge *pre_a, sec
     secp256k1_ge d_ge;
     int i;
 
-    VERIFY_CHECK(!a->infinity);
+    VERIFY_CHECK(!secp256k1_gej_is_infinity(a));
 
     secp256k1_gej_double_var(&d, a, NULL);
 
@@ -356,7 +356,7 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state *
         }
     }
 
-    if (!r->infinity) {
+    if (!secp256k1_gej_is_infinity(r)) {
         secp256k1_fe_mul(&r->z, &r->z, &Z);
     }
 }

src/tests.c

@@ -4152,8 +4152,8 @@ static void test_group_decompress(const secp256k1_fe* x) {
         secp256k1_fe_normalize_var(&ge_even.y);
 
         /* No infinity allowed. */
-        CHECK(!ge_even.infinity);
+        CHECK(!secp256k1_ge_is_infinity(&ge_even));
-        CHECK(!ge_odd.infinity);
+        CHECK(!secp256k1_ge_is_infinity(&ge_odd));
 
         /* Check that the x coordinates check out. */
         CHECK(secp256k1_fe_equal(&ge_even.x, x));

src/tests_exhaustive.c

@@ -103,9 +103,11 @@ static void test_exhaustive_addition(const secp256k1_ge *group, const secp256k1_
             secp256k1_gej_add_ge_var(&tmp, &groupj[i], &group[j], NULL);
             CHECK(secp256k1_gej_eq_ge_var(&tmp, &group[(i + j) % EXHAUSTIVE_TEST_ORDER]));
             /* add_zinv_var */
-            zless_gej.infinity = groupj[j].infinity;
+            if (secp256k1_gej_is_infinity(&groupj[j])) {
-            zless_gej.x = groupj[j].x;
+                secp256k1_ge_set_infinity(&zless_gej);
-            zless_gej.y = groupj[j].y;
+            } else {
+                secp256k1_ge_set_xy(&zless_gej, &groupj[j].x, &groupj[j].y);
+            }
             secp256k1_gej_add_zinv_var(&tmp, &groupj[i], &zless_gej, &fe_inv);
             CHECK(secp256k1_gej_eq_ge_var(&tmp, &group[(i + j) % EXHAUSTIVE_TEST_ORDER]));
         }
@@ -422,10 +424,8 @@ int main(int argc, char** argv) {
                 secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &generatedj, &scalar_i);
                 secp256k1_ge_set_gej(&generated, &generatedj);
 
-                CHECK(group[i].infinity == 0);
+                CHECK(!secp256k1_ge_is_infinity(&group[i]));
-                CHECK(generated.infinity == 0);
+                CHECK(secp256k1_ge_eq_var(&group[i], &generated));
-                CHECK(secp256k1_fe_equal(&generated.x, &group[i].x));
-                CHECK(secp256k1_fe_equal(&generated.y, &group[i].y));
             }
         }
 

src/testutil.h

@@ -96,17 +96,13 @@ static void testutil_random_ge_test(secp256k1_ge *ge) {
             break;
         }
     } while(1);
-    ge->infinity = 0;
 }
 
 static void testutil_random_ge_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) {
-    secp256k1_fe z2, z3;
+    secp256k1_fe z;
-    testutil_random_fe_non_zero_test(&gej->z);
+    testutil_random_fe_non_zero_test(&z);
-    secp256k1_fe_sqr(&z2, &gej->z);
+    secp256k1_gej_set_ge(gej, ge);
-    secp256k1_fe_mul(&z3, &z2, &gej->z);
+    secp256k1_gej_rescale(gej, &z);
-    secp256k1_fe_mul(&gej->x, &ge->x, &z2);
-    secp256k1_fe_mul(&gej->y, &ge->y, &z3);
-    gej->infinity = ge->infinity;
 }
 
 static void testutil_random_gej_test(secp256k1_gej *gej) {