frost/secp256k1-zkp
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin-core/secp256k1#1395: tests: simplify random_fe_non_zero (remove loop limit and unneeded normalize)

Browse Source 
c45b7c4fbbf41b011f138c465a58322a36664fd3 refactor: introduce testutil.h (deduplicate `random_fe_`, `ge_equals_` helpers) (Sebastian Falbesoner)
dc5514144fb9d412aa3845432b053ee06a27da37 tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) (Sebastian Falbesoner)

Pull request description:

  `random_fe_non_zero` contains a loop iteration limit that ensures that we abort if `random_fe` ever yielded zero more than ten times in a row. This construct was first introduced in PR #19 (commit 09ca4f32) for random non-square field elements and was later refactored into the non-zero helper in PR #25 (commit 6d6102fe). The copy-over to the exhaustive tests happened recently in PR #1118 (commit 0f864207).

  This case seems to be practically irrelevant and I'd argue for keeping things simple and removing it (which was already suggested in https://github.com/bitcoin-core/secp256k1/pull/1118#discussion_r1067259954); if there's really a worry that the test's random generator is heavily biased towards certain values or value ranges then there should consequently be checks at other places too (e.g. directly in `random_fe` for 256-bit values that repeatedly overflow, i.e. >= p).

  Also, the _fe_normalize call is not needed and can be removed, as the result of `random_fe` is already normalized.

ACKs for top commit:
  real-or-random:
    utACK c45b7c4fbbf41b011f138c465a58322a36664fd3
  siv2r:
    ACK `c45b7c4` (reviewed the changes and tests for both the commits passed locally).

Tree-SHA512: 4ffa66dd0b8392d7d0083a71e7b0682ad18f9261fd4ce8548c3059b497d3462db97e16114fded9787661ca447a877a27f5b996bd7d47e6f91c4454079d28a8ac
This commit is contained in:
Tim Ruffing 2023-09-14 15:06:37 +02:00
commit ee7aaf213e
No known key found for this signature in database
GPG Key ID: 8C461CCD293F6011
4 changed files with 58 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Makefile.am
View File

@ -46,6 +46,7 @@ noinst_HEADERS += src/precomputed_ecmult.h
noinst_HEADERS += src/precomputed_ecmult_gen.h
noinst_HEADERS += src/assumptions.h
noinst_HEADERS += src/checkmem.h
noinst_HEADERS += src/testutil.h
noinst_HEADERS += src/util.h
noinst_HEADERS += src/int128.h
noinst_HEADERS += src/int128_impl.h

50
src/tests.c
View File

@ -23,6 +23,7 @@
#include "../include/secp256k1_preallocated.h"
#include "testrand_impl.h"
#include "checkmem.h"
#include "testutil.h"
#include "util.h"
#include "../contrib/lax_der_parsing.c"
@ -2921,29 +2922,6 @@ static void run_scalar_tests(void) {
/***** FIELD TESTS *****/
static void random_fe(secp256k1_fe *x) {
unsigned char bin[32];
do {
secp256k1_testrand256(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) {
random_fe(nz);
secp256k1_fe_normalize(nz);
if (!secp256k1_fe_is_zero(nz)) {
break;
}
}
/* Infinitesimal probability of spurious failure here */
CHECK(tries >= 0);
}
static void random_fe_non_square(secp256k1_fe *ns) {
secp256k1_fe r;
random_fe_non_zero(ns);
@ -3663,15 +3641,6 @@ static void run_inverse_tests(void)
/***** GROUP TESTS *****/
static void ge_equals_ge(const secp256k1_ge *a, const secp256k1_ge *b) {
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
CHECK(secp256k1_fe_equal(&a->x, &b->x));
CHECK(secp256k1_fe_equal(&a->y, &b->y));
}
/* This compares jacobian points including their Z, not just their geometric meaning. */
static int gej_xyz_equals_gej(const secp256k1_gej *a, const secp256k1_gej *b) {
secp256k1_gej a2;
@ -3694,23 +3663,6 @@ static int gej_xyz_equals_gej(const secp256k1_gej *a, const secp256k1_gej *b) {
return ret;
}
static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
secp256k1_fe z2s;
secp256k1_fe u1, u2, s1, s2;
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
/* Check a.x * b.z^2 == b.x && a.y * b.z^3 == b.y, to avoid inverses. */
secp256k1_fe_sqr(&z2s, &b->z);
secp256k1_fe_mul(&u1, &a->x, &z2s);
u2 = b->x;
secp256k1_fe_mul(&s1, &a->y, &z2s); secp256k1_fe_mul(&s1, &s1, &b->z);
s2 = b->y;
CHECK(secp256k1_fe_equal(&u1, &u2));
CHECK(secp256k1_fe_equal(&s1, &s2));
}
static void test_ge(void) {
int i, i1;
int runs = 6;

52
src/tests_exhaustive.c
View File

@ -28,61 +28,11 @@
#include "testrand_impl.h"
#include "ecmult_compute_table_impl.h"
#include "ecmult_gen_compute_table_impl.h"
#include "testutil.h"
#include "util.h"
static int count = 2;
/** stolen from tests.c */
static void ge_equals_ge(const secp256k1_ge *a, const secp256k1_ge *b) {
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
CHECK(secp256k1_fe_equal(&a->x, &b->x));
CHECK(secp256k1_fe_equal(&a->y, &b->y));
}
static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
secp256k1_fe z2s;
secp256k1_fe u1, u2, s1, s2;
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
/* Check a.x * b.z^2 == b.x && a.y * b.z^3 == b.y, to avoid inverses. */
secp256k1_fe_sqr(&z2s, &b->z);
secp256k1_fe_mul(&u1, &a->x, &z2s);
u2 = b->x;
secp256k1_fe_mul(&s1, &a->y, &z2s); secp256k1_fe_mul(&s1, &s1, &b->z);
s2 = b->y;
CHECK(secp256k1_fe_equal(&u1, &u2));
CHECK(secp256k1_fe_equal(&s1, &s2));
}
static void random_fe(secp256k1_fe *x) {
unsigned char bin[32];
do {
secp256k1_testrand256(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) {
random_fe(nz);
secp256k1_fe_normalize(nz);
if (!secp256k1_fe_is_zero(nz)) {
break;
}
}
/* Infinitesimal probability of spurious failure here */
CHECK(tries >= 0);
}
/** END stolen from tests.c */
static uint32_t num_cores = 1;
static uint32_t this_core = 0;

55
src/testutil.h Normal file
View File

@ -0,0 +1,55 @@
/***********************************************************************
* Distributed under the MIT software license, see the accompanying *
* file COPYING or https://www.opensource.org/licenses/mit-license.php.*
***********************************************************************/
#ifndef SECP256K1_TESTUTIL_H
#define SECP256K1_TESTUTIL_H
#include "field.h"
#include "testrand.h"
#include "util.h"
static void random_fe(secp256k1_fe *x) {
unsigned char bin[32];
do {
secp256k1_testrand256(bin);
if (secp256k1_fe_set_b32_limit(x, bin)) {
return;
}
} while(1);
}
static void random_fe_non_zero(secp256k1_fe *nz) {
do {
random_fe(nz);
} while (secp256k1_fe_is_zero(nz));
}
static void ge_equals_ge(const secp256k1_ge *a, const secp256k1_ge *b) {
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
CHECK(secp256k1_fe_equal(&a->x, &b->x));
CHECK(secp256k1_fe_equal(&a->y, &b->y));
}
static void ge_equals_gej(const secp256k1_ge *a, const secp256k1_gej *b) {
secp256k1_fe z2s;
secp256k1_fe u1, u2, s1, s2;
CHECK(a->infinity == b->infinity);
if (a->infinity) {
return;
}
/* Check a.x * b.z^2 == b.x && a.y * b.z^3 == b.y, to avoid inverses. */
secp256k1_fe_sqr(&z2s, &b->z);
secp256k1_fe_mul(&u1, &a->x, &z2s);
u2 = b->x;
secp256k1_fe_mul(&s1, &a->y, &z2s); secp256k1_fe_mul(&s1, &s1, &b->z);
s2 = b->y;
CHECK(secp256k1_fe_equal(&u1, &u2));
CHECK(secp256k1_fe_equal(&s1, &s2));
}
#endif /* SECP256K1_TESTUTIL_H */