remove secp256k1_eckey_pubkey_serialize function
What changed, and why it matters
This commit removes an unused internal helper function for serializing public keys and updates the test code to use two newer, more specific helper functions instead. There is no indication of a security bug being fixed; it appears to be a routine code cleanup.
No security action required; treat as normal refactoring. Reviewers may verify that all former callers of the removed function were updated and that no production code depended on its infinity-check behavior.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit deletes secp256k1_eckey_pubkey_serialize() from src/eckey.h and src/eckey_impl.h, replacing its remaining uses in src/tests.c with secp256k1_eckey_pubkey_serialize33() and secp256k1_eckey_pubkey_serialize65(). These newer functions lack the infinity-point failure path of the old function, but the test contexts already guarantee the serialized point is not infinity (e.g., successful parse results, non-infinity ecmult outputs). The change is a refactoring/cleanup with no security-relevant diff evidence.
Changed components
src/eckey.hsrc/eckey_impl.hsrc/tests.cInspect captured patch +9 / −35
diff --git a/src/eckey.h b/src/eckey.h
index 3ebe896..c2bbc47 100644
--- a/src/eckey.h
+++ b/src/eckey.h
@@ -15,7 +15,6 @@
#include "ecmult_gen.h"
static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size);
-static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed);
/** Serialize a group element (that is not allowed to be infinity) to a compressed public key (33 bytes). */
static void secp256k1_eckey_pubkey_serialize33(secp256k1_ge *elem, unsigned char *pub33);
/** Serialize a group element (that is not allowed to be infinity) to an uncompressed public key (65 bytes). */
diff --git a/src/eckey_impl.h b/src/eckey_impl.h
index 4f64760..48745e8 100644
--- a/src/eckey_impl.h
+++ b/src/eckey_impl.h
@@ -35,26 +35,6 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char
}
}
-static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed) {
- VERIFY_CHECK(compressed == 0 || compressed == 1);
-
- if (secp256k1_ge_is_infinity(elem)) {
- return 0;
- }
- secp256k1_fe_normalize_var(&elem->x);
- secp256k1_fe_normalize_var(&elem->y);
- secp256k1_fe_get_b32(&pub[1], &elem->x);
- if (compressed) {
- *size = 33;
- pub[0] = secp256k1_fe_is_odd(&elem->y) ? SECP256K1_TAG_PUBKEY_ODD : SECP256K1_TAG_PUBKEY_EVEN;
- } else {
- *size = 65;
- pub[0] = SECP256K1_TAG_PUBKEY_UNCOMPRESSED;
- secp256k1_fe_get_b32(&pub[33], &elem->y);
- }
- return 1;
-}
-
static void secp256k1_eckey_pubkey_serialize33(secp256k1_ge *elem, unsigned char *pub33) {
VERIFY_CHECK(!secp256k1_ge_is_infinity(elem));
diff --git a/src/tests.c b/src/tests.c
index 0ccdbee..4029de5 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -4315,8 +4315,6 @@ static void test_point_times_order(const secp256k1_gej *point) {
secp256k1_scalar nx;
secp256k1_gej res1, res2;
secp256k1_ge res3;
- unsigned char pub[65];
- size_t psize = 65;
testutil_random_scalar_order_test(&x);
secp256k1_scalar_negate(&nx, &x);
secp256k1_ecmult(&res1, point, &x, &x); /* calc res1 = x * point + x * G; */
@@ -4326,9 +4324,6 @@ static void test_point_times_order(const secp256k1_gej *point) {
secp256k1_ge_set_gej(&res3, &res1);
CHECK(secp256k1_ge_is_infinity(&res3));
CHECK(secp256k1_ge_is_valid_var(&res3) == 0);
- CHECK(secp256k1_eckey_pubkey_serialize(&res3, pub, &psize, 0) == 0);
- psize = 65;
- CHECK(secp256k1_eckey_pubkey_serialize(&res3, pub, &psize, 1) == 0);
/* check zero/one edge cases */
secp256k1_ecmult(&res1, point, &secp256k1_scalar_zero, &secp256k1_scalar_zero);
secp256k1_ge_set_gej(&res3, &res1);
@@ -5435,7 +5430,6 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar
secp256k1_gej rj1, rj2, rj3, rj4, rj5, rj6, gj, infj;
secp256k1_ge r;
unsigned char bytes[65];
- size_t size = 65;
secp256k1_gej_set_ge(&gj, &secp256k1_ge_const_g);
secp256k1_gej_set_infinity(&infj);
secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &rj1, x);
@@ -5456,9 +5450,8 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar
secp256k1_sha256_write(acc, zerobyte, 1);
} else {
/* Store other points using their uncompressed serialization. */
- secp256k1_eckey_pubkey_serialize(&r, bytes, &size, 0);
- CHECK(size == 65);
- secp256k1_sha256_write(acc, bytes, size);
+ secp256k1_eckey_pubkey_serialize65(&r, bytes);
+ secp256k1_sha256_write(acc, bytes, sizeof(bytes));
}
}
@@ -6543,16 +6536,18 @@ static void test_random_pubkeys(void) {
size_t size = len;
firstb = in[0];
/* If the pubkey can be parsed, it should round-trip... */
- CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, len == 33));
- CHECK(size == len);
+ if (len == 33) {
+ secp256k1_eckey_pubkey_serialize33(&elem, out);
+ } else {
+ secp256k1_eckey_pubkey_serialize65(&elem, out);
+ }
CHECK(secp256k1_memcmp_var(&in[1], &out[1], len-1) == 0);
/* ... except for the type of hybrid inputs. */
if ((in[0] != 6) && (in[0] != 7)) {
CHECK(in[0] == out[0]);
}
size = 65;
- CHECK(secp256k1_eckey_pubkey_serialize(&elem, in, &size, 0));
- CHECK(size == 65);
+ secp256k1_eckey_pubkey_serialize65(&elem, in);
CHECK(secp256k1_eckey_pubkey_parse(&elem2, in, size));
CHECK(secp256k1_ge_eq_var(&elem2, &elem));
/* Check that the X9.62 hybrid type is checked. */
@@ -6567,7 +6562,7 @@ static void test_random_pubkeys(void) {
}
if (res) {
CHECK(secp256k1_ge_eq_var(&elem, &elem2));
- CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, 0));
+ secp256k1_eckey_pubkey_serialize65(&elem, out);
CHECK(secp256k1_memcmp_var(&in[1], &out[1], 64) == 0);
}
}
Why this scored 14/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.