recovery: check exhaustive API results
What the developer wrote
Authored by Lőrinc
recovery: check exhaustive API results
The exhaustive recovery tests decoded and verified signatures without requiring recoverable signing or conversion to succeed.
Check those documented success results before consuming the outputs, matching the ordinary exhaustive signing test.
What changed, and why it matters
This commit tightens a test suite for the secp256k1 cryptographic library. It adds checks to ensure that recoverable signing and signature conversion succeed before using their outputs. It does not change the library's actual signing or verification code, so it does not fix a live security bug in production software. It is a defensive improvement to testing.
No urgent action. Treat as normal test-quality improvement. Reviewers may optionally verify that the documented success preconditions for the two API calls are indeed always met in the exhaustive test parameters.
Security signals we found
defensive test-hardening only
no change to cryptographic implementation
no change to signature parsing, verification, or recovery logic
CHECK macros added to enforce documented API success in tests
Evidence from the diff
The change is confined to src/modules/recovery/tests_exhaustive_impl.h, an exhaustive test harness. It wraps three calls to secp256k1_ecdsa_sign_recoverable and secp256k1_ecdsa_recoverable_signature_convert with CHECK(… == 1) so the test fails if those documented-to-succeed API calls return failure. The production implementation is unchanged; the patch only makes the test enforce documented API success results, matching the ordinary exhaustive signing test. No runtime behavior of the library is altered.
Changed components
src/modules/recovery/tests_exhaustive_impl.hInspect captured patch +3 / −3
diff --git a/src/modules/recovery/tests_exhaustive_impl.h b/src/modules/recovery/tests_exhaustive_impl.h
index 6bbc02b..650cd9d 100644
--- a/src/modules/recovery/tests_exhaustive_impl.h
+++ b/src/modules/recovery/tests_exhaustive_impl.h
@@ -33,7 +33,7 @@ static void test_exhaustive_recovery_sign(const secp256k1_context *ctx, const se
secp256k1_scalar_get_b32(sk32, &sk);
secp256k1_scalar_get_b32(msg32, &msg);
- secp256k1_ecdsa_sign_recoverable(ctx, &rsig, msg32, sk32, secp256k1_nonce_function_smallint, &k);
+ CHECK(secp256k1_ecdsa_sign_recoverable(ctx, &rsig, msg32, sk32, secp256k1_nonce_function_smallint, &k) == 1);
/* Check directly */
secp256k1_ecdsa_recoverable_signature_load(ctx, &r, &s, &recid, &rsig);
@@ -59,7 +59,7 @@ static void test_exhaustive_recovery_sign(const secp256k1_context *ctx, const se
CHECK(recid == expected_recid);
/* Convert to a standard sig then check */
- secp256k1_ecdsa_recoverable_signature_convert(ctx, &sig, &rsig);
+ CHECK(secp256k1_ecdsa_recoverable_signature_convert(ctx, &sig, &rsig) == 1);
secp256k1_ecdsa_signature_load(ctx, &r, &s, &sig);
/* Note that we compute expected_r *after* signing -- this is important
* because our nonce-computing function function might change k during
@@ -129,7 +129,7 @@ static void test_exhaustive_recovery_verify(const secp256k1_context *ctx, const
/* Verify by converting to a standard signature and calling verify */
secp256k1_ecdsa_recoverable_signature_save(&rsig, &r_s, &s_s, recid);
- secp256k1_ecdsa_recoverable_signature_convert(ctx, &sig, &rsig);
+ CHECK(secp256k1_ecdsa_recoverable_signature_convert(ctx, &sig, &rsig) == 1);
memcpy(&nonconst_ge, &group[sk_s], sizeof(nonconst_ge));
secp256k1_pubkey_save(&pk, &nonconst_ge);
CHECK(should_verify ==
Why this scored 17/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.