From dc5514144fb9d412aa3845432b053ee06a27da37 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 7 Aug 2023 02:09:58 +0200 Subject: [PATCH] tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) `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; 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. --- src/tests.c | 11 ++--------- src/tests_exhaustive.c | 11 ++--------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/tests.c b/src/tests.c index 5b7c914f8..b4792ebe9 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2967,16 +2967,9 @@ static void random_fe(secp256k1_fe *x) { } static void random_fe_non_zero(secp256k1_fe *nz) { - int tries = 10; - while (--tries >= 0) { + do { random_fe(nz); - secp256k1_fe_normalize(nz); - if (!secp256k1_fe_is_zero(nz)) { - break; - } - } - /* Infinitesimal probability of spurious failure here */ - CHECK(tries >= 0); + } while (secp256k1_fe_is_zero(nz)); } static void random_fe_non_square(secp256k1_fe *ns) { diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index 3af8ec1ee..6cab1a694 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -70,16 +70,9 @@ static void random_fe(secp256k1_fe *x) { } static void random_fe_non_zero(secp256k1_fe *nz) { - int tries = 10; - while (--tries >= 0) { + do { random_fe(nz); - secp256k1_fe_normalize(nz); - if (!secp256k1_fe_is_zero(nz)) { - break; - } - } - /* Infinitesimal probability of spurious failure here */ - CHECK(tries >= 0); + } while (secp256k1_fe_is_zero(nz)); } /** END stolen from tests.c */