From f27fd1d5e754fc9b919d9c9f6e47a6eb8c9e2af7 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Wed, 12 May 2021 20:37:41 +0000 Subject: [PATCH] musig: improve test coverage of pubkey_combine --- src/modules/musig/main_impl.h | 9 ++++++--- src/modules/musig/tests_impl.h | 20 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/modules/musig/main_impl.h b/src/modules/musig/main_impl.h index 57d9da16..fc769348 100644 --- a/src/modules/musig/main_impl.h +++ b/src/modules/musig/main_impl.h @@ -81,9 +81,11 @@ typedef struct { /* Callback for batch EC multiplication to compute ell_0*P0 + ell_1*P1 + ... */ static int secp256k1_musig_pubkey_combine_callback(secp256k1_scalar *sc, secp256k1_ge *pt, size_t idx, void *data) { secp256k1_musig_pubkey_combine_ecmult_data *ctx = (secp256k1_musig_pubkey_combine_ecmult_data *) data; - if (!secp256k1_xonly_pubkey_load(ctx->ctx, pt, ctx->pks[idx])) { - return 0; - } + int ret; + ret = secp256k1_xonly_pubkey_load(ctx->ctx, pt, ctx->pks[idx]); + /* pubkey_load can't fail because the same pks have already been loaded (and + * we test this) */ + VERIFY_CHECK(ret); secp256k1_musig_keyaggcoef_internal(sc, ctx->ell, &pt->x, &ctx->second_pk_x); return 1; } @@ -130,6 +132,7 @@ int secp256k1_musig_pubkey_combine(const secp256k1_context* ctx, secp256k1_scrat return 0; } if (!secp256k1_ecmult_multi_var(&ctx->error_callback, &ctx->ecmult_ctx, scratch, &pkj, NULL, secp256k1_musig_pubkey_combine_callback, (void *) &ecmult_data, n_pubkeys)) { + /* The current implementation of ecmult_multi_var makes this code unreachable with tests. */ return 0; } secp256k1_ge_set_gej(&pkp, &pkj); diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h index ff20d15b..eb7527fe 100644 --- a/src/modules/musig/tests_impl.h +++ b/src/modules/musig/tests_impl.h @@ -102,11 +102,15 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { secp256k1_musig_pre_session pre_session_uninitialized; secp256k1_xonly_pubkey pk[2]; const secp256k1_xonly_pubkey *pk_ptr[2]; + secp256k1_xonly_pubkey invalid_pk; + const secp256k1_xonly_pubkey *invalid_pk_ptr2[2]; + const secp256k1_xonly_pubkey *invalid_pk_ptr3[3]; unsigned char tweak[32]; unsigned char sec_adaptor[32]; unsigned char sec_adaptor1[32]; secp256k1_pubkey adaptor; + int i; /** setup **/ secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); @@ -127,6 +131,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { * structs. */ memset(&pre_session_uninitialized, 0, sizeof(pre_session_uninitialized)); memset(&session_uninitialized, 0, sizeof(session_uninitialized)); + memset(&invalid_pk, 0, sizeof(invalid_pk)); secp256k1_testrand256(session_id[0]); secp256k1_testrand256(session_id[1]); @@ -142,6 +147,13 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(secp256k1_xonly_pubkey_create(&pk[1], sk[1]) == 1); CHECK(secp256k1_ec_pubkey_create(ctx, &adaptor, sec_adaptor) == 1); + for (i = 0; i < 2; i++) { + invalid_pk_ptr2[i] = &invalid_pk; + invalid_pk_ptr3[i] = &pk[i]; + } + /* invalid_pk_ptr3 has two valid, one invalid pk, which is important to test + * musig_pubkeys_combine */ + invalid_pk_ptr3[2] = &invalid_pk; /** main test body **/ @@ -167,10 +179,14 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(ecount == 3); CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, NULL, 2) == 0); CHECK(ecount == 4); - CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, pk_ptr, 0) == 0); + CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, invalid_pk_ptr2, 2) == 0); CHECK(ecount == 5); - CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, NULL, 0) == 0); + CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, invalid_pk_ptr3, 3) == 0); CHECK(ecount == 6); + CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, pk_ptr, 0) == 0); + CHECK(ecount == 7); + CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, NULL, 0) == 0); + CHECK(ecount == 8); CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, pk_ptr, 2) == 1); CHECK(secp256k1_musig_pubkey_combine(vrfy, scratch, &combined_pk, &pre_session, pk_ptr, 2) == 1);