Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1450: Add group.h ge/gej equality functions
Browse files Browse the repository at this point in the history
04af0ba Replace ge_equals_ge[,j] calls with group.h equality calls (Pieter Wuille)
60525f6 Add unit tests for group.h equality functions (Pieter Wuille)
a47cd97 Add group.h ge/gej equality functions (Pieter Wuille)

Pull request description:

  This pull requests removes the test-only functions `ge_equals_ge` and `ge_equals_gej`, and replaces them with proper group.h functions `secp256k1_ge_eq_var` and `secp256k1_gej_eq_ge_var` (mimicking the existing `secp256k1_gej_eq_var` function).

  This drops some of the arbitrary and undocumented magnitude restristrictions these functions have, makes them properly tested on their own, and makes their semantics cleaner (I'm always left checking whether `ge_equals_ge` does a `CHECK` internally or whether it returns a value...).

ACKs for top commit:
  real-or-random:
    utACK 04af0ba
  stratospher:
    ACK 04af0ba.

Tree-SHA512: 49bc409ffa980144d1305c9389a846af45f0a97bfec19d016929056aa918c6a9f020dbe8549f5318fa8e6a4108621cc3cce60331aa0634f84619a1104d20a62a
  • Loading branch information
real-or-random committed Dec 2, 2023
2 parents 10e6d29 + 04af0ba commit d3e29db
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 73 deletions.
6 changes: 6 additions & 0 deletions src/group.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a
*/
static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const secp256k1_fe *zr);

/** Check two group elements (affine) for equality in variable time. */
static int secp256k1_ge_eq_var(const secp256k1_ge *a, const secp256k1_ge *b);

/** Set a group element (affine) equal to the point at infinity. */
static void secp256k1_ge_set_infinity(secp256k1_ge *r);

Expand All @@ -114,6 +117,9 @@ static void secp256k1_gej_set_ge(secp256k1_gej *r, const secp256k1_ge *a);
/** Check two group elements (jacobian) for equality in variable time. */
static int secp256k1_gej_eq_var(const secp256k1_gej *a, const secp256k1_gej *b);

/** Check two group elements (jacobian and affine) for equality in variable time. */
static int secp256k1_gej_eq_ge_var(const secp256k1_gej *a, const secp256k1_ge *b);

/** Compare the X coordinate of a group element (jacobian).
* The magnitude of the group element's X coordinate must not exceed 31. */
static int secp256k1_gej_eq_x_var(const secp256k1_fe *x, const secp256k1_gej *a);
Expand Down
29 changes: 29 additions & 0 deletions src/group_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,35 @@ static int secp256k1_gej_eq_var(const secp256k1_gej *a, const secp256k1_gej *b)
return secp256k1_gej_is_infinity(&tmp);
}

static int secp256k1_gej_eq_ge_var(const secp256k1_gej *a, const secp256k1_ge *b) {
secp256k1_gej tmp;
SECP256K1_GEJ_VERIFY(a);
SECP256K1_GE_VERIFY(b);

secp256k1_gej_neg(&tmp, a);
secp256k1_gej_add_ge_var(&tmp, &tmp, b, NULL);
return secp256k1_gej_is_infinity(&tmp);
}

static int secp256k1_ge_eq_var(const secp256k1_ge *a, const secp256k1_ge *b) {
secp256k1_fe tmp;
SECP256K1_GE_VERIFY(a);
SECP256K1_GE_VERIFY(b);

if (a->infinity != b->infinity) return 0;
if (a->infinity) return 1;

tmp = a->x;
secp256k1_fe_normalize_weak(&tmp);
if (!secp256k1_fe_equal(&tmp, &b->x)) return 0;

tmp = a->y;
secp256k1_fe_normalize_weak(&tmp);
if (!secp256k1_fe_equal(&tmp, &b->y)) return 0;

return 1;
}

static int secp256k1_gej_eq_x_var(const secp256k1_fe *x, const secp256k1_gej *a) {
secp256k1_fe r;
SECP256K1_FE_VERIFY(x);
Expand Down
2 changes: 1 addition & 1 deletion src/modules/ellswift/tests_exhaustive_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static void test_exhaustive_ellswift(const secp256k1_context *ctx, const secp256
/* Decode ellswift pubkey and check that it matches the precomputed group element. */
secp256k1_ellswift_decode(ctx, &pub_decoded, ell64);
secp256k1_pubkey_load(ctx, &ge_decoded, &pub_decoded);
ge_equals_ge(&ge_decoded, &group[i]);
CHECK(secp256k1_ge_eq_var(&ge_decoded, &group[i]));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/modules/ellswift/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ void run_ellswift_tests(void) {
secp256k1_ellswift_decode(CTX, &pubkey2, ell64);
secp256k1_pubkey_load(CTX, &g2, &pubkey2);
/* Compare with original. */
ge_equals_ge(&g, &g2);
CHECK(secp256k1_ge_eq_var(&g, &g2));
}
/* Verify the behavior of secp256k1_ellswift_create */
for (i = 0; i < 400 * COUNT; i++) {
Expand All @@ -259,7 +259,7 @@ void run_ellswift_tests(void) {
secp256k1_ellswift_decode(CTX, &pub, ell64);
secp256k1_pubkey_load(CTX, &dec, &pub);
secp256k1_ecmult(&res, NULL, &secp256k1_scalar_zero, &sec);
ge_equals_gej(&dec, &res);
CHECK(secp256k1_gej_eq_ge_var(&res, &dec));
}
/* Verify that secp256k1_ellswift_xdh computes the right shared X coordinate. */
for (i = 0; i < 800 * COUNT; i++) {
Expand Down
73 changes: 42 additions & 31 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -3706,11 +3706,12 @@ static void test_ge(void) {
secp256k1_ge_clear(&ge[0]);
secp256k1_ge_set_gej_var(&ge[0], &gej[0]);
for (i = 0; i < runs; i++) {
int j;
int j, k;
secp256k1_ge g;
random_group_element_test(&g);
if (i >= runs - 2) {
secp256k1_ge_mul_lambda(&g, &ge[1]);
CHECK(!secp256k1_ge_eq_var(&g, &ge[1]));
}
if (i >= runs - 1) {
secp256k1_ge_mul_lambda(&g, &g);
Expand All @@ -3730,6 +3731,16 @@ static void test_ge(void) {
random_gej_y_magnitude(&gej[1 + j + 4 * i]);
random_gej_z_magnitude(&gej[1 + j + 4 * i]);
}

for (j = 0; j < 4; ++j) {
for (k = 0; k < 4; ++k) {
int expect_equal = (j >> 1) == (k >> 1);
CHECK(secp256k1_ge_eq_var(&ge[1 + j + 4 * i], &ge[1 + k + 4 * i]) == expect_equal);
CHECK(secp256k1_gej_eq_var(&gej[1 + j + 4 * i], &gej[1 + k + 4 * i]) == expect_equal);
CHECK(secp256k1_gej_eq_ge_var(&gej[1 + j + 4 * i], &ge[1 + k + 4 * i]) == expect_equal);
CHECK(secp256k1_gej_eq_ge_var(&gej[1 + k + 4 * i], &ge[1 + j + 4 * i]) == expect_equal);
}
}
}

/* Generate random zf, and zfi2 = 1/zf^2, zfi3 = 1/zf^3 */
Expand Down Expand Up @@ -3759,7 +3770,7 @@ static void test_ge(void) {

/* Test gej + ge with Z ratio result (var). */
secp256k1_gej_add_ge_var(&resj, &gej[i1], &ge[i2], secp256k1_gej_is_infinity(&gej[i1]) ? NULL : &zr);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
if (!secp256k1_gej_is_infinity(&gej[i1]) && !secp256k1_gej_is_infinity(&resj)) {
secp256k1_fe zrz; secp256k1_fe_mul(&zrz, &zr, &gej[i1].z);
CHECK(secp256k1_fe_equal(&zrz, &resj.z));
Expand All @@ -3773,31 +3784,31 @@ static void test_ge(void) {
random_ge_x_magnitude(&ge2_zfi);
random_ge_y_magnitude(&ge2_zfi);
secp256k1_gej_add_zinv_var(&resj, &gej[i1], &ge2_zfi, &zf);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
}

/* Test gej + ge (const). */
if (i2 != 0) {
/* secp256k1_gej_add_ge does not support its second argument being infinity. */
secp256k1_gej_add_ge(&resj, &gej[i1], &ge[i2]);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
}

/* Test doubling (var). */
if ((i1 == 0 && i2 == 0) || ((i1 + 3)/4 == (i2 + 3)/4 && ((i1 + 3)%4)/2 == ((i2 + 3)%4)/2)) {
secp256k1_fe zr2;
/* Normal doubling with Z ratio result. */
secp256k1_gej_double_var(&resj, &gej[i1], &zr2);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
/* Check Z ratio. */
secp256k1_fe_mul(&zr2, &zr2, &gej[i1].z);
CHECK(secp256k1_fe_equal(&zr2, &resj.z));
/* Normal doubling. */
secp256k1_gej_double_var(&resj, &gej[i2], NULL);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
/* Constant-time doubling. */
secp256k1_gej_double(&resj, &gej[i2]);
ge_equals_gej(&ref, &resj);
CHECK(secp256k1_gej_eq_ge_var(&resj, &ref));
}

/* Test adding opposites. */
Expand All @@ -3809,12 +3820,12 @@ static void test_ge(void) {
if (i1 == 0) {
CHECK(secp256k1_ge_is_infinity(&ge[i1]));
CHECK(secp256k1_gej_is_infinity(&gej[i1]));
ge_equals_gej(&ref, &gej[i2]);
CHECK(secp256k1_gej_eq_ge_var(&gej[i2], &ref));
}
if (i2 == 0) {
CHECK(secp256k1_ge_is_infinity(&ge[i2]));
CHECK(secp256k1_gej_is_infinity(&gej[i2]));
ge_equals_gej(&ref, &gej[i1]);
CHECK(secp256k1_gej_eq_ge_var(&gej[i1], &ref));
}
}
}
Expand Down Expand Up @@ -3849,7 +3860,7 @@ static void test_ge(void) {
secp256k1_fe s;
random_fe_non_zero(&s);
secp256k1_gej_rescale(&gej[i], &s);
ge_equals_gej(&ge_set_all[i], &gej[i]);
CHECK(secp256k1_gej_eq_ge_var(&gej[i], &ge_set_all[i]));
}
free(ge_set_all);
}
Expand Down Expand Up @@ -3893,7 +3904,7 @@ static void test_ge(void) {
secp256k1_ge_set_all_gej_var(ge, gej, 4 * runs + 1);
/* check result */
for (i = 0; i < 4 * runs + 1; i++) {
ge_equals_gej(&ge[i], &gej[i]);
CHECK(secp256k1_gej_eq_ge_var(&gej[i], &ge[i]));
}

/* Test batch gej -> ge conversion with all infinities. */
Expand Down Expand Up @@ -3992,15 +4003,15 @@ static void test_add_neg_y_diff_x(void) {

secp256k1_gej_add_var(&resj, &aj, &bj, NULL);
secp256k1_ge_set_gej(&res, &resj);
ge_equals_gej(&res, &sumj);
CHECK(secp256k1_gej_eq_ge_var(&sumj, &res));

secp256k1_gej_add_ge(&resj, &aj, &b);
secp256k1_ge_set_gej(&res, &resj);
ge_equals_gej(&res, &sumj);
CHECK(secp256k1_gej_eq_ge_var(&sumj, &res));

secp256k1_gej_add_ge_var(&resj, &aj, &b, NULL);
secp256k1_ge_set_gej(&res, &resj);
ge_equals_gej(&res, &sumj);
CHECK(secp256k1_gej_eq_ge_var(&sumj, &res));
}

static void run_ge(void) {
Expand Down Expand Up @@ -4293,10 +4304,10 @@ static void test_point_times_order(const secp256k1_gej *point) {
CHECK(secp256k1_ge_is_infinity(&res3));
secp256k1_ecmult(&res1, point, &secp256k1_scalar_one, &secp256k1_scalar_zero);
secp256k1_ge_set_gej(&res3, &res1);
ge_equals_gej(&res3, point);
CHECK(secp256k1_gej_eq_ge_var(point, &res3));
secp256k1_ecmult(&res1, point, &secp256k1_scalar_zero, &secp256k1_scalar_one);
secp256k1_ge_set_gej(&res3, &res1);
ge_equals_ge(&res3, &secp256k1_ge_const_g);
CHECK(secp256k1_ge_eq_var(&secp256k1_ge_const_g, &res3));
}

/* These scalars reach large (in absolute value) outputs when fed to secp256k1_scalar_split_lambda.
Expand Down Expand Up @@ -4424,7 +4435,7 @@ static void ecmult_const_random_mult(void) {
secp256k1_ecmult_const(&b, &a, &xn);

CHECK(secp256k1_ge_is_valid_var(&a));
ge_equals_gej(&expected_b, &b);
CHECK(secp256k1_gej_eq_ge_var(&b, &expected_b));
}

static void ecmult_const_commutativity(void) {
Expand All @@ -4445,7 +4456,7 @@ static void ecmult_const_commutativity(void) {
secp256k1_ecmult_const(&res2, &mid2, &a);
secp256k1_ge_set_gej(&mid1, &res1);
secp256k1_ge_set_gej(&mid2, &res2);
ge_equals_ge(&mid1, &mid2);
CHECK(secp256k1_ge_eq_var(&mid1, &mid2));
}

static void ecmult_const_mult_zero_one(void) {
Expand All @@ -4472,13 +4483,13 @@ static void ecmult_const_mult_zero_one(void) {
/* 1*point */
secp256k1_ecmult_const(&res1, &point, &secp256k1_scalar_one);
secp256k1_ge_set_gej(&res2, &res1);
ge_equals_ge(&res2, &point);
CHECK(secp256k1_ge_eq_var(&res2, &point));

/* -1*point */
secp256k1_ecmult_const(&res1, &point, &negone);
secp256k1_gej_neg(&res1, &res1);
secp256k1_ge_set_gej(&res2, &res1);
ge_equals_ge(&res2, &point);
CHECK(secp256k1_ge_eq_var(&res2, &point));
}

static void ecmult_const_check_result(const secp256k1_ge *A, const secp256k1_scalar* q, const secp256k1_gej *res) {
Expand All @@ -4487,7 +4498,7 @@ static void ecmult_const_check_result(const secp256k1_ge *A, const secp256k1_sca
secp256k1_gej_set_ge(&pointj, A);
secp256k1_ecmult(&res2j, &pointj, q, &secp256k1_scalar_zero);
secp256k1_ge_set_gej(&res2, &res2j);
ge_equals_gej(&res2, res);
CHECK(secp256k1_gej_eq_ge_var(res, &res2));
}

static void ecmult_const_edges(void) {
Expand Down Expand Up @@ -4595,7 +4606,7 @@ static void ecmult_const_chain_multiply(void) {
secp256k1_ecmult_const(&point, &tmp, &scalar);
}
secp256k1_ge_set_gej(&res, &point);
ge_equals_gej(&res, &expected_point);
CHECK(secp256k1_gej_eq_ge_var(&expected_point, &res));
}

static void run_ecmult_const_tests(void) {
Expand Down Expand Up @@ -5403,11 +5414,11 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar
secp256k1_ecmult_multi_var(NULL, scratch, &rj5, &secp256k1_scalar_zero, test_ecmult_accumulate_cb, (void*)x, 1);
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);
ge_equals_gej(&r, &rj4);
ge_equals_gej(&r, &rj5);
ge_equals_gej(&r, &rj6);
CHECK(secp256k1_gej_eq_ge_var(&rj2, &r));
CHECK(secp256k1_gej_eq_ge_var(&rj3, &r));
CHECK(secp256k1_gej_eq_ge_var(&rj4, &r));
CHECK(secp256k1_gej_eq_ge_var(&rj5, &r));
CHECK(secp256k1_gej_eq_ge_var(&rj6, &r));
if (secp256k1_ge_is_infinity(&r)) {
/* Store infinity as 0x00 */
const unsigned char zerobyte[1] = {0};
Expand Down Expand Up @@ -5561,7 +5572,7 @@ static void test_ecmult_gen_blind(void) {
CHECK(!gej_xyz_equals_gej(&pgej, &pgej2));
CHECK(!gej_xyz_equals_gej(&i, &CTX->ecmult_gen_ctx.initial));
secp256k1_ge_set_gej(&pge, &pgej);
ge_equals_gej(&pge, &pgej2);
CHECK(secp256k1_gej_eq_ge_var(&pgej2, &pge));
}

static void test_ecmult_gen_blind_reset(void) {
Expand Down Expand Up @@ -5952,7 +5963,7 @@ static void run_ec_pubkey_parse_test(void) {
SECP256K1_CHECKMEM_CHECK(&ge.x, sizeof(ge.x));
SECP256K1_CHECKMEM_CHECK(&ge.y, sizeof(ge.y));
SECP256K1_CHECKMEM_CHECK(&ge.infinity, sizeof(ge.infinity));
ge_equals_ge(&secp256k1_ge_const_g, &ge);
CHECK(secp256k1_ge_eq_var(&ge, &secp256k1_ge_const_g));
/* secp256k1_ec_pubkey_serialize illegal args. */
len = 65;
CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_serialize(CTX, NULL, &len, &pubkey, SECP256K1_EC_UNCOMPRESSED));
Expand Down Expand Up @@ -6521,7 +6532,7 @@ static void test_random_pubkeys(void) {
CHECK(secp256k1_eckey_pubkey_serialize(&elem, in, &size, 0));
CHECK(size == 65);
CHECK(secp256k1_eckey_pubkey_parse(&elem2, in, size));
ge_equals_ge(&elem,&elem2);
CHECK(secp256k1_ge_eq_var(&elem2, &elem));
/* Check that the X9.62 hybrid type is checked. */
in[0] = secp256k1_testrand_bits(1) ? 6 : 7;
res = secp256k1_eckey_pubkey_parse(&elem2, in, size);
Expand All @@ -6533,7 +6544,7 @@ static void test_random_pubkeys(void) {
}
}
if (res) {
ge_equals_ge(&elem,&elem2);
CHECK(secp256k1_ge_eq_var(&elem, &elem2));
CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, 0));
CHECK(secp256k1_memcmp_var(&in[1], &out[1], 64) == 0);
}
Expand Down
Loading

0 comments on commit d3e29db

Please sign in to comment.