Skip to content

Commit

Permalink
musig: new upstream def of VERIFY_CHECK (empty in non-VERIFY)
Browse files Browse the repository at this point in the history
Remove explicity VERIFY_CHECKs in keyaggcoef_internal since normalization should
be checked in the fe_* functions.
  • Loading branch information
jonasnick authored and real-or-random committed Jan 23, 2024
1 parent 764ff37 commit 097b960
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 21 deletions.
12 changes: 9 additions & 3 deletions src/modules/musig/keyagg_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down
32 changes: 14 additions & 18 deletions src/modules/musig/session_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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]));
Expand Down Expand Up @@ -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]);
Expand All @@ -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;
Expand Down

0 comments on commit 097b960

Please sign in to comment.