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

Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants

Browse Source 
f5e815f430 remove secp256k1_eckey_pubkey_serialize function (Sebastian Falbesoner)
0d3659c547 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) (Sebastian Falbesoner)
adb76f82ea use new `_eckey_pubkey_serialize{33,65}` functions in public API (Sebastian Falbesoner)
fc7458ca3e introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions (Sebastian Falbesoner)

Pull request description:

  This PR splits up the pubkey serialization function `secp256k1_eckey_pubkey_serialize` into two variants for the compressed (33 bytes) and uncompressed (65 bytes) public key output format each, where only non-infinity group elements as input are allowed. The motivation is to simplify call-sites significantly, as they currently need to introduce two variables and a VERIFY_CHECKs on the return value and the in/out size parameter within a pre-processor block, typically leading to 8 lines of code. By using the new functions, the code is reduced to a single line of code that just calls the function (see #1773). This is helpful for already existing modules on master (ellswift, musig) and upcoming ones (silentpayments, see #1765).

  One drawback is that the public API function `secp256k1_ec_pubkey_serialize` is now slightly more complex (we now call one of two functions instead of a single one, depending on whether the compressed flag is set or not), but that should hopefully not be a problem.

  The commits are intentionally kept small to ease review, happy to squash them if that is preferred.

  (Kudos to w0xlt for the initial idea (https://github.com/bitcoin-core/secp256k1/pull/1765#pullrequestreview-3462461331) and to real-or-random for the suggestion to split the already existing function (https://github.com/bitcoin-core/secp256k1/issues/1773#issuecomment-3540461718).)

ACKs for top commit:
  real-or-random:
    utACK f5e815f430
  w0xlt:
    ACK f5e815f430

Tree-SHA512: da576bbeae477f31ba76c0001f8df08b51fe5e31d67b422a238348ead3341bf37f0c1509ad9d0a93b63e6d61c152707c85beabd02f4eac3b3bdcff129e0ea750
This commit is contained in:
merge-script
2025-11-27 17:27:08 +01:00
parent b6c2a3cd77 f5e815f430
commit e7f7083b53
7 changed files with 39 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/eckey.h
View File

@@ -15,7 +15,10 @@
#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). */
static void secp256k1_eckey_pubkey_serialize65(secp256k1_ge *elem, unsigned char *pub65);
static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak);
static int secp256k1_eckey_pubkey_tweak_add(secp256k1_ge *key, const secp256k1_scalar *tweak);

29
src/eckey_impl.h
View File

@@ -35,24 +35,23 @@ 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);
static void secp256k1_eckey_pubkey_serialize33(secp256k1_ge *elem, unsigned char *pub33) {
VERIFY_CHECK(!secp256k1_ge_is_infinity(elem));
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;
pub33[0] = secp256k1_fe_is_odd(&elem->y) ? SECP256K1_TAG_PUBKEY_ODD : SECP256K1_TAG_PUBKEY_EVEN;
secp256k1_fe_get_b32(&pub33[1], &elem->x);
}
static void secp256k1_eckey_pubkey_serialize65(secp256k1_ge *elem, unsigned char *pub65) {
VERIFY_CHECK(!secp256k1_ge_is_infinity(elem));
secp256k1_fe_normalize_var(&elem->x);
secp256k1_fe_normalize_var(&elem->y);
pub65[0] = SECP256K1_TAG_PUBKEY_UNCOMPRESSED;
secp256k1_fe_get_b32(&pub65[1], &elem->x);
secp256k1_fe_get_b32(&pub65[33], &elem->y);
}
static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak) {

9
src/modules/ellswift/main_impl.h
View File

@@ -406,19 +406,12 @@ int secp256k1_ellswift_encode(const secp256k1_context *ctx, unsigned char *ell64
if (secp256k1_pubkey_load(ctx, &p, pubkey)) {
secp256k1_fe t;
unsigned char p64[64] = {0};
size_t ser_size;
int ser_ret;
secp256k1_sha256 hash;
/* Set up hasher state; the used RNG is H(pubkey || "\x00"*31 || rnd32 || cnt++), using
* BIP340 tagged hash with tag "secp256k1_ellswift_encode". */
secp256k1_ellswift_sha256_init_encode(&hash);
ser_ret = secp256k1_eckey_pubkey_serialize(&p, p64, &ser_size, 1);
#ifdef VERIFY
VERIFY_CHECK(ser_ret && ser_size == 33);
#else
(void)ser_ret;
#endif
secp256k1_eckey_pubkey_serialize33(&p, p64);
secp256k1_sha256_write(&hash, p64, sizeof(p64));
secp256k1_sha256_write(&hash, rnd32, 32);

9
src/modules/musig/keyagg_impl.h
View File

@@ -124,18 +124,11 @@ static void secp256k1_musig_keyaggcoef_internal(secp256k1_scalar *r, const unsig
} else {
secp256k1_sha256 sha;
unsigned char buf[33];
size_t buflen = sizeof(buf);
int ret;
secp256k1_musig_keyaggcoef_sha256(&sha);
secp256k1_sha256_write(&sha, pks_hash, 32);
ret = secp256k1_eckey_pubkey_serialize(pk, buf, &buflen, 1);
#ifdef VERIFY
/* Serialization does not fail since the pk is not the point at infinity
* (according to this function's precondition). */
VERIFY_CHECK(ret && buflen == sizeof(buf));
#else
(void) ret;
#endif
secp256k1_eckey_pubkey_serialize33(pk, buf);
secp256k1_sha256_write(&sha, buf, sizeof(buf));
secp256k1_sha256_finalize(&sha, buf);
secp256k1_scalar_set_b32(r, buf, NULL);

29
src/modules/musig/session_impl.h
View File

@@ -25,15 +25,8 @@ static void secp256k1_musig_ge_serialize_ext(unsigned char *out33, secp256k1_ge*
if (secp256k1_ge_is_infinity(ge)) {
memset(out33, 0, 33);
} else {
int ret;
size_t size = 33;
ret = secp256k1_eckey_pubkey_serialize(ge, out33, &size, 1);
#ifdef VERIFY
/* Serialize must succeed because the point is not at infinity */
VERIFY_CHECK(ret && size == 33);
#else
(void) ret;
#endif
secp256k1_eckey_pubkey_serialize33(ge, out33);
}
}
@@ -224,15 +217,8 @@ int secp256k1_musig_pubnonce_serialize(const secp256k1_context* ctx, unsigned ch
return 0;
}
for (i = 0; i < 2; i++) {
int ret;
size_t size = 33;
ret = secp256k1_eckey_pubkey_serialize(&ges[i], &out66[33*i], &size, 1);
#ifdef VERIFY
/* serialize must succeed because the point was just loaded */
VERIFY_CHECK(ret && size == 33);
#else
(void) ret;
#endif
secp256k1_eckey_pubkey_serialize33(&ges[i], &out66[33*i]);
}
return 1;
}
@@ -398,11 +384,9 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp
secp256k1_gej nonce_ptj[2];
int i;
unsigned char pk_ser[33];
size_t pk_ser_len = sizeof(pk_ser);
unsigned char aggpk_ser[32];
unsigned char *aggpk_ser_ptr = NULL;
secp256k1_ge pk;
int pk_serialize_success;
int ret = 1;
ARG_CHECK(pubnonce != NULL);
@@ -429,15 +413,8 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp
if (!secp256k1_pubkey_load(ctx, &pk, pubkey)) {
return 0;
}
pk_serialize_success = secp256k1_eckey_pubkey_serialize(&pk, pk_ser, &pk_ser_len, 1);
#ifdef VERIFY
/* A pubkey cannot be the point at infinity */
VERIFY_CHECK(pk_serialize_success);
VERIFY_CHECK(pk_ser_len == sizeof(pk_ser));
#else
(void) pk_serialize_success;
#endif
secp256k1_eckey_pubkey_serialize33(&pk, pk_ser);
secp256k1_nonce_function_musig(k, input_nonce, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32);
VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[0]));

10
src/secp256k1.c
View File

@@ -280,10 +280,14 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o
ARG_CHECK(pubkey != NULL);
ARG_CHECK((flags & SECP256K1_FLAGS_TYPE_MASK) == SECP256K1_FLAGS_TYPE_COMPRESSION);
if (secp256k1_pubkey_load(ctx, &Q, pubkey)) {
ret = secp256k1_eckey_pubkey_serialize(&Q, output, &len, !!(flags & SECP256K1_FLAGS_BIT_COMPRESSION));
if (ret) {
*outputlen = len;
if (flags & SECP256K1_FLAGS_BIT_COMPRESSION) {
secp256k1_eckey_pubkey_serialize33(&Q, output);
*outputlen = 33;
} else {
secp256k1_eckey_pubkey_serialize65(&Q, output);
*outputlen = 65;
}
ret = 1;
}
return ret;
}

23
src/tests.c
View File

@@ -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);
}
}