RFC: Add exhaustive test for 2-of-2 musig [WIP] #1615

pull theStack wants to merge 1 commits into bitcoin-core:master from theStack:add_musig_exhaustive_test changing 5 files +135 −0
  1. theStack commented at 3:51 PM on October 9, 2024: contributor

    This PR attempts to add an exhaustive test for 2-of-2 signature aggregation via MuSig2. Currently, only the combinations of each participant's possible signing keys are fully iterated, and other involved cryptographic elements (generated nonces, signing challenge etc.) are either derived from a counter or statically set. Some invalid conditions that are practically unreachable on the full group order for secp256k1 (due to negligible probablity) can obviously hit for the exhaustive group test orders, so some special treatment is needed here to avoid a crash in the test. Those three conditions are:

    I'm asking for general comments at this point what would make the most sense in an exhaustive musig test, or if it is even worth it to have one at all. If yes, is it a good idea to change function behaviour depending on exhaustive tests being compiled? Are there other solutions that I haven't thought of? It's also a bit strange that some iterations are skipped, so there should probably an extra check at the end (right now if all iterations are skipped, we wouldn't even notice it, apart from the WIP-printf output). I guess the basic idea of the PR is still slightly better than having nothing, but still far away from what we would want ideally.

    Number of full iterations (i.e. nothing is skipped due to any condition described above) depending on the exhaustive test order:

    • EXHAUSTIVE_TEST_ORDER=7 → 18/36 full iterations (50%)
    • EXHAUSTIVE_TEST_ORDER=13 → 99/144 full iterations (68.75%)
    • EXHAUSTIVE_TEST_ORDER=199 → 38453/39204 full iterations (98.08%)
  2. Add exhaustive test for 2-of-2 musig [WIP] f1df2b74cf
  3. real-or-random added the label assurance on Oct 10, 2024
  4. w0xlt commented at 10:44 PM on April 17, 2025: contributor

    The code itself looks good and meets the PR goal. But is there any specific reason for this test or is it just to increase test coverage?

  5. fametrano commented at 7:50 AM on October 21, 2026: contributor

    Concept ACK, but in 13 one-line mutations I tried, this test caught nothing that tests -t=musig misses.

    It could add coverage by looping over all challenges, as the TODO suggests, and over nonces.

    Of six musig mutations it caught the four that break signing and missed the two that change key aggregation, since both signers then use the same wrong key and the signature still verifies. Of seven partial_sig_verify mutations it missed only the one that accepts every partial signature; a loop checking that every wrong value is rejected catches it for about 0.03 s at order 13, but tests -t=musig catches it too, as it does all 13. On master e74a018 the branch applies with one conflict in session_impl.h, and the musig part takes 0.2 s at order 13 and 4.7 s at 199.

  6. apoelstra commented at 8:00 AM on October 21, 2026: contributor

    f1df2b74cfbb4546443bf3051fdccba388eafdcc needs rebase


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-23 08:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me