Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1358: tests: introduce helper for non-ze…
Browse files Browse the repository at this point in the history
…ro `random_fe_test()` results

5a95a26 tests: introduce helper for non-zero `random_fe_test` results (Sebastian Falbesoner)
304421d tests: refactor: remove duplicate function `random_field_element_test` (Sebastian Falbesoner)

Pull request description:

  There are several instances in the tests where random non-zero field elements are generated by calling `random_fe_test` in a do/while-loop with is-zero condition. This PR deduplicates all these by introducing a `random_fe_non_zero_test` helper. Note that some instances checked the is-zero condition via `secp256k1_fe_normalizes_to_zero_var`, which is unnecessary, as the result of `random_field_element_test` is already normalized (so strictly speaking, this is not a pure refactor, and there could be tiny run-time improvements, though I doubt that's measurable).

  Additionally, the first commit removes the function `random_field_element_test` as it is logically a duplicate of `random_fe_test`.

ACKs for top commit:
  real-or-random:
    ACK 5a95a26

Tree-SHA512: 920404f38ebe8b84bfd52f3354dc17ae6a0fd6355f99b78c9aeb53bf21f7eca5fd4518edc8a422d84f430ae95864661b497de42a3ab7fa9c49515a1df2f1d466
  • Loading branch information
real-or-random committed Jun 27, 2023
2 parents 3aef6ab + 5a95a26 commit 0fa84f8
Showing 1 changed file with 24 additions and 44 deletions.
68 changes: 24 additions & 44 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,6 @@ static void uncounting_illegal_callback_fn(const char* str, void* data) {
(*p)--;
}

static void random_field_element_test(secp256k1_fe *fe) {
do {
unsigned char b32[32];
secp256k1_testrand256_test(b32);
if (secp256k1_fe_set_b32_limit(fe, b32)) {
break;
}
} while(1);
}

static void random_field_element_magnitude(secp256k1_fe *fe) {
secp256k1_fe zero;
int n = secp256k1_testrand_int(9);
Expand All @@ -115,10 +105,26 @@ static void random_field_element_magnitude(secp256k1_fe *fe) {
#endif
}

static void random_fe_test(secp256k1_fe *x) {
unsigned char bin[32];
do {
secp256k1_testrand256_test(bin);
if (secp256k1_fe_set_b32_limit(x, bin)) {
return;
}
} while(1);
}

static void random_fe_non_zero_test(secp256k1_fe *fe) {
do {
random_fe_test(fe);
} while(secp256k1_fe_is_zero(fe));
}

static void random_group_element_test(secp256k1_ge *ge) {
secp256k1_fe fe;
do {
random_field_element_test(&fe);
random_fe_test(&fe);
if (secp256k1_ge_set_xo_var(ge, &fe, secp256k1_testrand_bits(1))) {
secp256k1_fe_normalize(&ge->y);
break;
Expand All @@ -129,12 +135,7 @@ static void random_group_element_test(secp256k1_ge *ge) {

static void random_group_element_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) {
secp256k1_fe z2, z3;
do {
random_field_element_test(&gej->z);
if (!secp256k1_fe_is_zero(&gej->z)) {
break;
}
} while(1);
random_fe_non_zero_test(&gej->z);
secp256k1_fe_sqr(&z2, &gej->z);
secp256k1_fe_mul(&z3, &z2, &gej->z);
secp256k1_fe_mul(&gej->x, &ge->x, &z2);
Expand Down Expand Up @@ -2984,16 +2985,6 @@ static void random_fe(secp256k1_fe *x) {
} while(1);
}

static void random_fe_test(secp256k1_fe *x) {
unsigned char bin[32];
do {
secp256k1_testrand256_test(bin);
if (secp256k1_fe_set_b32_limit(x, bin)) {
return;
}
} while(1);
}

static void random_fe_non_zero(secp256k1_fe *nz) {
int tries = 10;
while (--tries >= 0) {
Expand Down Expand Up @@ -3820,18 +3811,14 @@ static void test_ge(void) {
}

/* Generate random zf, and zfi2 = 1/zf^2, zfi3 = 1/zf^3 */
do {
random_field_element_test(&zf);
} while(secp256k1_fe_is_zero(&zf));
random_fe_non_zero_test(&zf);
random_field_element_magnitude(&zf);
secp256k1_fe_inv_var(&zfi3, &zf);
secp256k1_fe_sqr(&zfi2, &zfi3);
secp256k1_fe_mul(&zfi3, &zfi3, &zfi2);

/* Generate random r */
do {
random_field_element_test(&r);
} while(secp256k1_fe_is_zero(&r));
random_fe_non_zero_test(&r);

for (i1 = 0; i1 < 1 + 4 * runs; i1++) {
int i2;
Expand Down Expand Up @@ -4148,10 +4135,7 @@ static void run_gej(void) {
CHECK(!secp256k1_gej_eq_var(&a, &b));

b = a;
random_field_element_test(&fe);
if (secp256k1_fe_is_zero(&fe)) {
continue;
}
random_fe_non_zero_test(&fe);
secp256k1_gej_rescale(&a, &fe);
CHECK(secp256k1_gej_eq_var(&a, &b));
}
Expand Down Expand Up @@ -4590,9 +4574,7 @@ static void ecmult_const_mult_xonly(void) {
random_scalar_order_test(&q);
/* If i is odd, n=d*base.x for random non-zero d */
if (i & 1) {
do {
random_field_element_test(&d);
} while (secp256k1_fe_normalizes_to_zero_var(&d));
random_fe_non_zero_test(&d);
secp256k1_fe_mul(&n, &base.x, &d);
} else {
n = base.x;
Expand All @@ -4617,13 +4599,11 @@ static void ecmult_const_mult_xonly(void) {
random_scalar_order_test(&q);
/* Generate random X coordinate not on the curve. */
do {
random_field_element_test(&x);
random_fe_test(&x);
} while (secp256k1_ge_x_on_curve_var(&x));
/* If i is odd, n=d*x for random non-zero d. */
if (i & 1) {
do {
random_field_element_test(&d);
} while (secp256k1_fe_normalizes_to_zero_var(&d));
random_fe_non_zero_test(&d);
secp256k1_fe_mul(&n, &x, &d);
} else {
n = x;
Expand Down

0 comments on commit 0fa84f8

Please sign in to comment.