musig: Add hash context tests #1866

pull mllwchrry wants to merge 1 commits into bitcoin-core:master from mllwchrry:musig-hash-ctx-tests changing 1 files +107 −0

mllwchrry commented at 5:44 PM on June 9, 2026: contributor

Adds missing hash context tests for the musig module, covering thepubkey_agg, nonce_gen, and nonce_process functions. Follows the same pattern as the hash context tests for other modules added in #1777.
real-or-random added the label assurance on Jun 10, 2026
real-or-random added the label tweak/refactor on Jun 10, 2026
real-or-random removed the label tweak/refactor on Jun 10, 2026
real-or-random added the label tweak/refactor on Jun 10, 2026
real-or-random commented at 11:34 AM on June 10, 2026: contributor

@furszy Want to review this?

in src/modules/musig/tests_impl.h:625 in e0b1f34ca0

 620 | +
 621 | +    CHECK(secp256k1_ec_pubkey_create(ctx, &pk, sk));
 622 | +    pks[0] = &pk;
 623 | +
 624 | +    /* Default behavior. No ctx-provided SHA256 compression */
 625 | +    CHECK(secp256k1_musig_pubkey_agg(ctx, NULL, &cache_default, pks, 1));

theStack commented at 1:10 PM on August 24, 2026:

nit, for a slightly more precise check, here and for other CHECK instances:

    CHECK(secp256k1_musig_pubkey_agg(ctx, NULL, &cache_default, pks, 1) == 1);

(see also PR #1481 which enforces that)

theStack approved
theStack commented at 1:21 PM on August 24, 2026: contributor

ACK e0b1f34ca0203905779ecf2b518bf9666c40c787

Note that there are three more musig API functions that use the hash context, so adding tests for them would make sense, either here or in a separate PR: _musig_nonce_gen_counter (could probably tested alongside of the existing _musig_nonce_gen) and _musig_partial_{sign,sig_verify}.
furszy commented at 2:54 PM on August 24, 2026: member

ACK e0b1f34ca0203905779ecf2b518bf9666c40c787
mllwchrry force-pushed on Aug 25, 2026
mllwchrry commented at 10:57 AM on August 25, 2026: contributor

Thanks for the reviews! I applied a == 1 check to other CHECKs and added tests for nonce_gen_counter, partial_sign, and partial_sig_verify.
in src/modules/musig/tests_impl.h:810 in 6ccc27b7a3
```
 805 | +    ctx->hash_ctx.fn_sha256_compression = sha256_musig_partial_sig_verify;
 806 | +    CHECK(secp256k1_musig_partial_sig_verify(ctx, &partial_sig, &pubnonce, &pk, &keyagg_cache, &session) == 0);
 807 | +    CHECK(sha256_musig_partial_sig_verify_called);
 808 | +
 809 | +    secp256k1_context_destroy(ctx);
 810 | +}
```
furszy commented at 6:55 PM on August 25, 2026:

These two tests can be merged, they share the same setup. You can just call the first sign function, then set the called flag to false and call the verify one.

And you could apply the same concept for the others as well. Merging sha256_musig_nonce_process within this shouldn't be hard either.
musig: add hash context tests ab6925b8eb
mllwchrry force-pushed on Aug 26, 2026
mllwchrry commented at 1:54 PM on August 26, 2026: contributor

Thanks @furszy! That's a great idea. Consolidated all the flow tests into one musig_ctx_sha256 that builds the session once, keeping nonce_gen_counter separate as it isn't part of the sign/verify flow.

Contributors

Labels

assurance tweak/refactor

Linked (view graph)

#1777 Make SHA256 compression runtime pluggable