Skip to content

Commit 315275f

Browse files
committed
field: remove secp256k1_fe_equal_var
`fe_equal_var` hits a fast path only when the inputs are unequal, which is uncommon among its callers (public key parsing, ECDSA verify).
1 parent 79abdf0 commit 315275f

File tree

7 files changed

+31
-50
lines changed

7 files changed

+31
-50
lines changed

src/field.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,6 @@ static int secp256k1_fe_is_odd(const secp256k1_fe *a);
176176
*/
177177
static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b);
178178

179-
/** Determine whether two field elements are equal, without constant-time guarantee.
180-
*
181-
* Identical in behavior to secp256k1_fe_equal, but not constant time in either a or b.
182-
*/
183-
static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
184-
185179
/** Compare the values represented by 2 field elements, without constant-time guarantee.
186180
*
187181
* On input, a and b must be valid normalized field elements.

src/field_impl.h

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,6 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp
3131
return secp256k1_fe_normalizes_to_zero(&na);
3232
}
3333

34-
SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b) {
35-
secp256k1_fe na;
36-
#ifdef VERIFY
37-
secp256k1_fe_verify(a);
38-
secp256k1_fe_verify(b);
39-
VERIFY_CHECK(a->magnitude <= 1);
40-
VERIFY_CHECK(b->magnitude <= 31);
41-
#endif
42-
secp256k1_fe_negate(&na, a, 1);
43-
secp256k1_fe_add(&na, b);
44-
return secp256k1_fe_normalizes_to_zero_var(&na);
45-
}
46-
4734
static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k1_fe * SECP256K1_RESTRICT a) {
4835
/** Given that p is congruent to 3 mod 4, we can compute the square root of
4936
* a mod p as the (p+1)/4'th power of a.
@@ -151,7 +138,7 @@ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k
151138
if (!ret) {
152139
secp256k1_fe_negate(&t1, &t1, 1);
153140
secp256k1_fe_normalize_var(&t1);
154-
VERIFY_CHECK(secp256k1_fe_equal_var(&t1, a));
141+
VERIFY_CHECK(secp256k1_fe_equal(&t1, a));
155142
}
156143
#endif
157144
return ret;

src/group_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ static int secp256k1_gej_eq_x_var(const secp256k1_fe *x, const secp256k1_gej *a)
324324
#endif
325325

326326
secp256k1_fe_sqr(&r, &a->z); secp256k1_fe_mul(&r, &r, x);
327-
return secp256k1_fe_equal_var(&r, &a->x);
327+
return secp256k1_fe_equal(&r, &a->x);
328328
}
329329

330330
static void secp256k1_gej_neg(secp256k1_gej *r, const secp256k1_gej *a) {
@@ -353,7 +353,7 @@ static int secp256k1_ge_is_valid_var(const secp256k1_ge *a) {
353353
secp256k1_fe_sqr(&y2, &a->y);
354354
secp256k1_fe_sqr(&x3, &a->x); secp256k1_fe_mul(&x3, &x3, &a->x);
355355
secp256k1_fe_add_int(&x3, SECP256K1_B);
356-
return secp256k1_fe_equal_var(&y2, &x3);
356+
return secp256k1_fe_equal(&y2, &x3);
357357
}
358358

359359
static SECP256K1_INLINE void secp256k1_gej_double(secp256k1_gej *r, const secp256k1_gej *a) {

src/modules/extrakeys/tests_exhaustive_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ static void test_exhaustive_extrakeys(const secp256k1_context *ctx, const secp25
4848

4949
/* Compare the xonly_pubkey bytes against the precomputed group. */
5050
secp256k1_fe_set_b32_mod(&fe, xonly_pubkey_bytes[i - 1]);
51-
CHECK(secp256k1_fe_equal_var(&fe, &group[i].x));
51+
CHECK(secp256k1_fe_equal(&fe, &group[i].x));
5252

5353
/* Check the parity against the precomputed group. */
5454
fe = group[i].y;

src/modules/schnorrsig/main_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha
261261

262262
secp256k1_fe_normalize_var(&r.y);
263263
return !secp256k1_fe_is_odd(&r.y) &&
264-
secp256k1_fe_equal_var(&rx, &r.x);
264+
secp256k1_fe_equal(&rx, &r.x);
265265
}
266266

267267
#endif

src/tests.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2967,7 +2967,7 @@ static int check_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) {
29672967
secp256k1_fe an = *a;
29682968
secp256k1_fe bn = *b;
29692969
secp256k1_fe_normalize_weak(&an);
2970-
return secp256k1_fe_equal_var(&an, &bn);
2970+
return secp256k1_fe_equal(&an, &bn);
29712971
}
29722972

29732973
static void run_field_convert(void) {
@@ -2990,9 +2990,9 @@ static void run_field_convert(void) {
29902990
secp256k1_fe_storage fes2;
29912991
/* Check conversions to fe. */
29922992
CHECK(secp256k1_fe_set_b32_limit(&fe2, b32));
2993-
CHECK(secp256k1_fe_equal_var(&fe, &fe2));
2993+
CHECK(secp256k1_fe_equal(&fe, &fe2));
29942994
secp256k1_fe_from_storage(&fe2, &fes);
2995-
CHECK(secp256k1_fe_equal_var(&fe, &fe2));
2995+
CHECK(secp256k1_fe_equal(&fe, &fe2));
29962996
/* Check conversion from fe. */
29972997
secp256k1_fe_get_b32(b322, &fe);
29982998
CHECK(secp256k1_memcmp_var(b322, b32, 32) == 0);
@@ -3149,7 +3149,7 @@ static void run_field_misc(void) {
31493149
CHECK(check_fe_equal(&q, &z));
31503150
/* Test the fe equality and comparison operations. */
31513151
CHECK(secp256k1_fe_cmp_var(&x, &x) == 0);
3152-
CHECK(secp256k1_fe_equal_var(&x, &x));
3152+
CHECK(secp256k1_fe_equal(&x, &x));
31533153
z = x;
31543154
secp256k1_fe_add(&z,&y);
31553155
/* Test fe conditional move; z is not normalized here. */
@@ -3174,7 +3174,7 @@ static void run_field_misc(void) {
31743174
q = z;
31753175
secp256k1_fe_normalize_var(&x);
31763176
secp256k1_fe_normalize_var(&z);
3177-
CHECK(!secp256k1_fe_equal_var(&x, &z));
3177+
CHECK(!secp256k1_fe_equal(&x, &z));
31783178
secp256k1_fe_normalize_var(&q);
31793179
secp256k1_fe_cmov(&q, &z, (i&1));
31803180
#ifdef VERIFY
@@ -3679,8 +3679,8 @@ static void ge_equals_ge(const secp256k1_ge *a, const secp256k1_ge *b) {
36793679
if (a->infinity) {
36803680
return;
36813681
}
3682-
CHECK(secp256k1_fe_equal_var(&a->x, &b->x));
3683-
CHECK(secp256k1_fe_equal_var(&a->y, &b->y));
3682+
CHECK(secp256k1_fe_equal(&a->x, &b->x));
3683+
CHECK(secp256k1_fe_equal(&a->y, &b->y));
36843684
}
36853685

36863686
/* This compares jacobian points including their Z, not just their geometric meaning. */
@@ -3718,8 +3718,8 @@ static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
37183718
u2 = b->x;
37193719
secp256k1_fe_mul(&s1, &a->y, &z2s); secp256k1_fe_mul(&s1, &s1, &b->z);
37203720
s2 = b->y;
3721-
CHECK(secp256k1_fe_equal_var(&u1, &u2));
3722-
CHECK(secp256k1_fe_equal_var(&s1, &s2));
3721+
CHECK(secp256k1_fe_equal(&u1, &u2));
3722+
CHECK(secp256k1_fe_equal(&s1, &s2));
37233723
}
37243724

37253725
static void test_ge(void) {
@@ -3787,7 +3787,7 @@ static void test_ge(void) {
37873787
/* Check Z ratio. */
37883788
if (!secp256k1_gej_is_infinity(&gej[i1]) && !secp256k1_gej_is_infinity(&refj)) {
37893789
secp256k1_fe zrz; secp256k1_fe_mul(&zrz, &zr, &gej[i1].z);
3790-
CHECK(secp256k1_fe_equal_var(&zrz, &refj.z));
3790+
CHECK(secp256k1_fe_equal(&zrz, &refj.z));
37913791
}
37923792
secp256k1_ge_set_gej_var(&ref, &refj);
37933793

@@ -3796,7 +3796,7 @@ static void test_ge(void) {
37963796
ge_equals_gej(&ref, &resj);
37973797
if (!secp256k1_gej_is_infinity(&gej[i1]) && !secp256k1_gej_is_infinity(&resj)) {
37983798
secp256k1_fe zrz; secp256k1_fe_mul(&zrz, &zr, &gej[i1].z);
3799-
CHECK(secp256k1_fe_equal_var(&zrz, &resj.z));
3799+
CHECK(secp256k1_fe_equal(&zrz, &resj.z));
38003800
}
38013801

38023802
/* Test gej + ge (var, with additional Z factor). */
@@ -3825,7 +3825,7 @@ static void test_ge(void) {
38253825
ge_equals_gej(&ref, &resj);
38263826
/* Check Z ratio. */
38273827
secp256k1_fe_mul(&zr2, &zr2, &gej[i1].z);
3828-
CHECK(secp256k1_fe_equal_var(&zr2, &resj.z));
3828+
CHECK(secp256k1_fe_equal(&zr2, &resj.z));
38293829
/* Normal doubling. */
38303830
secp256k1_gej_double_var(&resj, &gej[i2], NULL);
38313831
ge_equals_gej(&ref, &resj);
@@ -3908,7 +3908,7 @@ static void test_ge(void) {
39083908
ret_set_xo = secp256k1_ge_set_xo_var(&q, &r, 0);
39093909
CHECK(ret_on_curve == ret_frac_on_curve);
39103910
CHECK(ret_on_curve == ret_set_xo);
3911-
if (ret_set_xo) CHECK(secp256k1_fe_equal_var(&r, &q.x));
3911+
if (ret_set_xo) CHECK(secp256k1_fe_equal(&r, &q.x));
39123912
}
39133913

39143914
/* Test batch gej -> ge conversion with many infinities. */
@@ -4148,8 +4148,8 @@ static void test_group_decompress(const secp256k1_fe* x) {
41484148
CHECK(!ge_odd.infinity);
41494149

41504150
/* Check that the x coordinates check out. */
4151-
CHECK(secp256k1_fe_equal_var(&ge_even.x, x));
4152-
CHECK(secp256k1_fe_equal_var(&ge_odd.x, x));
4151+
CHECK(secp256k1_fe_equal(&ge_even.x, x));
4152+
CHECK(secp256k1_fe_equal(&ge_odd.x, x));
41534153

41544154
/* Check odd/even Y in ge_odd, ge_even. */
41554155
CHECK(secp256k1_fe_is_odd(&ge_odd.y));
@@ -4207,12 +4207,12 @@ static void test_pre_g_table(const secp256k1_ge_storage * pre_g, size_t n) {
42074207
CHECK(!secp256k1_fe_normalizes_to_zero_var(&dqx) || !secp256k1_fe_normalizes_to_zero_var(&dqy));
42084208

42094209
/* Check that -q is not equal to p */
4210-
CHECK(!secp256k1_fe_equal_var(&dpx, &dqx) || !secp256k1_fe_equal_var(&dpy, &dqy));
4210+
CHECK(!secp256k1_fe_equal(&dpx, &dqx) || !secp256k1_fe_equal(&dpy, &dqy));
42114211

42124212
/* Check that p, -q and gg are colinear */
42134213
secp256k1_fe_mul(&dpx, &dpx, &dqy);
42144214
secp256k1_fe_mul(&dpy, &dpy, &dqx);
4215-
CHECK(secp256k1_fe_equal_var(&dpx, &dpy));
4215+
CHECK(secp256k1_fe_equal(&dpx, &dpy));
42164216

42174217
p = q;
42184218
}
@@ -4431,7 +4431,7 @@ static void run_point_times_order(void) {
44314431
secp256k1_fe_sqr(&x, &x);
44324432
}
44334433
secp256k1_fe_normalize_var(&x);
4434-
CHECK(secp256k1_fe_equal_var(&x, &xr));
4434+
CHECK(secp256k1_fe_equal(&x, &xr));
44354435
}
44364436

44374437
static void ecmult_const_random_mult(void) {

src/tests_exhaustive.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ static void ge_equals_ge(const secp256k1_ge *a, const secp256k1_ge *b) {
3838
if (a->infinity) {
3939
return;
4040
}
41-
CHECK(secp256k1_fe_equal_var(&a->x, &b->x));
42-
CHECK(secp256k1_fe_equal_var(&a->y, &b->y));
41+
CHECK(secp256k1_fe_equal(&a->x, &b->x));
42+
CHECK(secp256k1_fe_equal(&a->y, &b->y));
4343
}
4444

4545
static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
@@ -55,8 +55,8 @@ static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
5555
u2 = b->x;
5656
secp256k1_fe_mul(&s1, &a->y, &z2s); secp256k1_fe_mul(&s1, &s1, &b->z);
5757
s2 = b->y;
58-
CHECK(secp256k1_fe_equal_var(&u1, &u2));
59-
CHECK(secp256k1_fe_equal_var(&s1, &s2));
58+
CHECK(secp256k1_fe_equal(&u1, &u2));
59+
CHECK(secp256k1_fe_equal(&s1, &s2));
6060
}
6161

6262
static void random_fe(secp256k1_fe *x) {
@@ -219,14 +219,14 @@ static void test_exhaustive_ecmult(const secp256k1_ge *group, const secp256k1_ge
219219
/* Test secp256k1_ecmult_const_xonly with all curve X coordinates, and xd=NULL. */
220220
ret = secp256k1_ecmult_const_xonly(&tmpf, &group[i].x, NULL, &ng, 0);
221221
CHECK(ret);
222-
CHECK(secp256k1_fe_equal_var(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));
222+
CHECK(secp256k1_fe_equal(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));
223223

224224
/* Test secp256k1_ecmult_const_xonly with all curve X coordinates, with random xd. */
225225
random_fe_non_zero(&xd);
226226
secp256k1_fe_mul(&xn, &xd, &group[i].x);
227227
ret = secp256k1_ecmult_const_xonly(&tmpf, &xn, &xd, &ng, 0);
228228
CHECK(ret);
229-
CHECK(secp256k1_fe_equal_var(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));
229+
CHECK(secp256k1_fe_equal(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x));
230230
}
231231
}
232232
}
@@ -475,8 +475,8 @@ int main(int argc, char** argv) {
475475

476476
CHECK(group[i].infinity == 0);
477477
CHECK(generated.infinity == 0);
478-
CHECK(secp256k1_fe_equal_var(&generated.x, &group[i].x));
479-
CHECK(secp256k1_fe_equal_var(&generated.y, &group[i].y));
478+
CHECK(secp256k1_fe_equal(&generated.x, &group[i].x));
479+
CHECK(secp256k1_fe_equal(&generated.y, &group[i].y));
480480
}
481481
}
482482

0 commit comments

Comments
 (0)