From 5626ff8b3ce965eb00ace406a539c11b06ab374f Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Sat, 6 Jan 2024 16:49:16 +0000 Subject: [PATCH] musig: new upstream def of VERIFY_CHECK (empty in non-VERIFY) Remove explicity VERIFY_CHECKs in keyaggcoef_internal since normalization should be checked in the fe_* functions. --- src/modules/musig/keyagg_impl.h | 12 +++++++++--- src/modules/musig/session_impl.h | 32 ++++++++++++++------------------ 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/modules/musig/keyagg_impl.h b/src/modules/musig/keyagg_impl.h index 81338f5da..aff955420 100644 --- a/src/modules/musig/keyagg_impl.h +++ b/src/modules/musig/keyagg_impl.h @@ -131,14 +131,12 @@ static void secp256k1_musig_keyaggcoef_sha256(secp256k1_sha256 *sha) { /* Compute KeyAgg coefficient which is constant 1 for the second pubkey and * otherwise tagged_hash(pk_hash, x) where pk_hash is the hash of public keys. * second_pk is the point at infinity in case there is no second_pk. Assumes - * that pk is not the point at infinity and that the coordinates of pk and + * that pk is not the point at infinity and that the Y-coordinates of pk and * second_pk are normalized. */ static void secp256k1_musig_keyaggcoef_internal(secp256k1_scalar *r, const unsigned char *pk_hash, secp256k1_ge *pk, const secp256k1_ge *second_pk) { secp256k1_sha256 sha; VERIFY_CHECK(!secp256k1_ge_is_infinity(pk)); - VERIFY_CHECK(pk->x.normalized && pk->y.normalized); - VERIFY_CHECK(secp256k1_ge_is_infinity(second_pk) || (second_pk->x.normalized && second_pk->y.normalized)); if (!secp256k1_ge_is_infinity(second_pk) && secp256k1_fe_equal(&pk->x, &second_pk->x) @@ -151,9 +149,13 @@ static void secp256k1_musig_keyaggcoef_internal(secp256k1_scalar *r, const unsig secp256k1_musig_keyaggcoef_sha256(&sha); secp256k1_sha256_write(&sha, pk_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_sha256_write(&sha, buf, sizeof(buf)); secp256k1_sha256_finalize(&sha, buf); secp256k1_scalar_set_b32(r, buf, NULL); @@ -178,9 +180,13 @@ static int secp256k1_musig_pubkey_agg_callback(secp256k1_scalar *sc, secp256k1_g secp256k1_musig_pubkey_agg_ecmult_data *ctx = (secp256k1_musig_pubkey_agg_ecmult_data *) data; int ret; ret = secp256k1_pubkey_load(ctx->ctx, pt, ctx->pks[idx]); +#ifdef VERIFY /* pubkey_load can't fail because the same pks have already been loaded in * `musig_compute_pk_hash` (and we test this). */ VERIFY_CHECK(ret); +#else + (void) ret; +#endif secp256k1_musig_keyaggcoef_internal(sc, ctx->pk_hash, pt, &ctx->second_pk); return 1; } diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 880ecab52..ff87f2fd3 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -174,8 +174,12 @@ int secp256k1_musig_pubnonce_serialize(const secp256k1_context* ctx, unsigned ch int ret; size_t size = 33; ret = secp256k1_eckey_pubkey_serialize(&ge[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 } return 1; } @@ -258,16 +262,6 @@ int secp256k1_musig_partial_sig_parse(const secp256k1_context* ctx, secp256k1_mu return 1; } -/* Normalizes the x-coordinate of the given group element. */ -static int secp256k1_xonly_ge_serialize(unsigned char *output32, secp256k1_ge *ge) { - if (secp256k1_ge_is_infinity(ge)) { - return 0; - } - secp256k1_fe_normalize_var(&ge->x); - secp256k1_fe_get_b32(output32, &ge->x); - return 1; -} - /* Write optional inputs into the hash */ static void secp256k1_nonce_function_musig_helper(secp256k1_sha256 *sha, unsigned int prefix_size, const unsigned char *data, unsigned char len) { unsigned char zero[7] = { 0 }; @@ -364,22 +358,25 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn } if (keyagg_cache != NULL) { - int ret_tmp; if (!secp256k1_keyagg_cache_load(ctx, &cache_i, keyagg_cache)) { return 0; } - ret_tmp = secp256k1_xonly_ge_serialize(aggpk_ser, &cache_i.pk); - /* Serialization can not fail because the loaded point can not be infinity. */ - VERIFY_CHECK(ret_tmp); + /* The loaded point cache_i.pk can not be the point at infinity. */ + secp256k1_fe_get_b32(aggpk_ser, &cache_i.pk.x); aggpk_ser_ptr = aggpk_ser; } if (!secp256k1_pubkey_load(ctx, &pk, pubkey)) { return 0; } pk_serialize_success = secp256k1_eckey_pubkey_serialize(&pk, pk_ser, &pk_ser_len, SECP256K1_EC_COMPRESSED); + +#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_nonce_function_musig(k, session_id32, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32); VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[0])); @@ -460,7 +457,6 @@ static int secp256k1_musig_nonce_process_internal(int *fin_nonce_parity, unsigne secp256k1_ge fin_nonce_pt; secp256k1_gej fin_nonce_ptj; secp256k1_ge aggnonce[2]; - int ret; secp256k1_ge_set_gej(&aggnonce[0], &aggnoncej[0]); secp256k1_ge_set_gej(&aggnonce[1], &aggnoncej[1]); @@ -476,9 +472,9 @@ static int secp256k1_musig_nonce_process_internal(int *fin_nonce_parity, unsigne if (secp256k1_ge_is_infinity(&fin_nonce_pt)) { fin_nonce_pt = secp256k1_ge_const_g; } - ret = secp256k1_xonly_ge_serialize(fin_nonce, &fin_nonce_pt); - /* Can't fail since fin_nonce_pt is not infinity */ - VERIFY_CHECK(ret); + /* fin_nonce_pt is not the point at infinity */ + secp256k1_fe_normalize_var(&fin_nonce_pt.x); + secp256k1_fe_get_b32(fin_nonce, &fin_nonce_pt.x); secp256k1_fe_normalize_var(&fin_nonce_pt.y); *fin_nonce_parity = secp256k1_fe_is_odd(&fin_nonce_pt.y); return 1;