test: refactor: simplify tests by using `_ecmult_gen_ge` helper
What the developer wrote
Authored by Sebastian Falbesoner
test: refactor: simplify tests by using `_ecmult_gen_ge` helper
If the generator point multiplication result in Jacobian coordinates is
immediately converted to affine coordinates after and is not needed for
anything else, we can deduplicate by using the helper introduced in #1861.
Note that in a very strict sense this is not a refactor, as the Jacobian
object is now cleared out which was not done on master, but for the logic
in the tests this shouldn't matter at all.
What changed, and why it matters
This commit is a minor cleanup in the project's internal test code. It replaces a two-step pattern (compute a point in one coordinate format, then convert it to another) with a single helper function that does the same thing. The author explicitly calls it a refactor and notes the only behavioral difference is that an intermediate variable is now cleared, which has no effect on test logic. There is no indication this changes any production cryptography or introduces a security issue.
No security action needed. This is a test-only refactor with no production impact.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors test files (src/tests.c and src/tests_exhaustive.c) to use the secp256k1_ecmult_gen_ge() helper introduced in PR #1861, which directly computes the generator multiplication result in affine coordinates. Previously the tests used secp256k1_ecmult_gen_gej() to produce Jacobian coordinates and then secp256k1_ge_set_gej() to convert them. The change removes intermediate secp256k1_gej variables and the conversion calls. The commit message states that, strictly speaking, this is not a pure refactor because the Jacobian object is now cleared, but this does not matter for the tests. No production library code is modified.
Changed components
src/tests.csrc/tests_exhaustive.cInspect captured patch +6 / −17
diff --git a/src/tests.c b/src/tests.c
index 9084c1d..95f177a 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -241,7 +241,6 @@ static void run_proper_context_tests(int use_prealloc) {
void *my_ctx_prealloc = NULL;
unsigned char seed[32] = {0x17};
- secp256k1_gej pubj;
secp256k1_ge pub;
secp256k1_scalar msg, key, nonce;
secp256k1_scalar sigr, sigs;
@@ -329,8 +328,7 @@ static void run_proper_context_tests(int use_prealloc) {
/*** attempt to use them ***/
testutil_random_scalar_order_test(&msg);
testutil_random_scalar_order_test(&key);
- secp256k1_ecmult_gen_gej(&my_ctx->ecmult_gen_ctx, &pubj, &key);
- secp256k1_ge_set_gej(&pub, &pubj);
+ secp256k1_ecmult_gen_ge(&my_ctx->ecmult_gen_ctx, &pub, &key);
/* obtain a working nonce */
do {
@@ -4304,19 +4302,16 @@ static void test_ec_combine(void) {
const secp256k1_pubkey* d[6];
secp256k1_pubkey sd;
secp256k1_pubkey sd2;
- secp256k1_gej Qj;
secp256k1_ge Q;
int i;
for (i = 1; i <= 6; i++) {
secp256k1_scalar s;
testutil_random_scalar_order_test(&s);
secp256k1_scalar_add(&sum, &sum, &s);
- secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &Qj, &s);
- secp256k1_ge_set_gej(&Q, &Qj);
+ secp256k1_ecmult_gen_ge(&CTX->ecmult_gen_ctx, &Q, &s);
secp256k1_pubkey_save(&data[i - 1], &Q);
d[i - 1] = &data[i - 1];
- secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &Qj, &sum);
- secp256k1_ge_set_gej(&Q, &Qj);
+ secp256k1_ecmult_gen_ge(&CTX->ecmult_gen_ctx, &Q, &sum);
secp256k1_pubkey_save(&sd, &Q);
CHECK(secp256k1_ec_pubkey_combine(CTX, &sd2, d, i) == 1);
CHECK(secp256k1_memcmp_var(&sd, &sd2, sizeof(sd)) == 0);
@@ -6515,7 +6510,6 @@ static void random_sign(secp256k1_scalar *sigr, secp256k1_scalar *sigs, const se
}
static void test_ecdsa_sign_verify(void) {
- secp256k1_gej pubj;
secp256k1_ge pub;
secp256k1_scalar one;
secp256k1_scalar msg, key;
@@ -6524,8 +6518,7 @@ static void test_ecdsa_sign_verify(void) {
int recid;
testutil_random_scalar_order_test(&msg);
testutil_random_scalar_order_test(&key);
- secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &pubj, &key);
- secp256k1_ge_set_gej(&pub, &pubj);
+ secp256k1_ecmult_gen_ge(&CTX->ecmult_gen_ctx, &pub, &key);
getrec = testrand_bits(1);
/* The specific way in which this conditional is written sidesteps a potential bug in clang.
See the commit messages of the commit that introduced this comment for details. */
@@ -7284,7 +7277,6 @@ static void run_ecdsa_edge_cases(void) {
/* Test the case where ECDSA recomputes a point that is infinity. */
{
- secp256k1_gej keyj;
secp256k1_ge key;
secp256k1_scalar msg;
secp256k1_scalar sr, ss;
@@ -7292,8 +7284,7 @@ static void run_ecdsa_edge_cases(void) {
secp256k1_scalar_negate(&ss, &ss);
secp256k1_scalar_inverse(&ss, &ss);
secp256k1_scalar_set_int(&sr, 1);
- secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &keyj, &sr);
- secp256k1_ge_set_gej(&key, &keyj);
+ secp256k1_ecmult_gen_ge(&CTX->ecmult_gen_ctx, &key, &sr);
msg = ss;
CHECK(secp256k1_ecdsa_sig_verify(&sr, &ss, &key, &msg) == 0);
}
diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c
index 888b7ac..80f7156 100644
--- a/src/tests_exhaustive.c
+++ b/src/tests_exhaustive.c
@@ -421,12 +421,10 @@ int main(int argc, char** argv) {
/* Verify against ecmult_gen */
{
secp256k1_scalar scalar_i;
- secp256k1_gej generatedj;
secp256k1_ge generated;
secp256k1_scalar_set_int(&scalar_i, i);
- secp256k1_ecmult_gen_gej(&ctx->ecmult_gen_ctx, &generatedj, &scalar_i);
- secp256k1_ge_set_gej(&generated, &generatedj);
+ secp256k1_ecmult_gen_ge(&ctx->ecmult_gen_ctx, &generated, &scalar_i);
CHECK(!secp256k1_ge_is_infinity(&group[i]));
CHECK(secp256k1_ge_eq_var(&group[i], &generated));
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.