refactor, key: move CreateMuSig2{Nonce,PartialSig} functions to musig.{h,cpp} module #34225

1. 
   theStack commented at 11:45 pm on January 7, 2026: contributor

   This PR is a follow-up of #29675, see #29675 (review). It moves all MuSig2 functions that currently live in CKey and call secp256k1 musig module API functions (i.e. secp256k1_musig_...) to the musig.{h,cpp} module, as this seems to be a better place. One small drawback of doing this is that the secp256k1_context_signing object needs to be accessed from the outside; this is currently solved by changing the linkage from static to extern to expose the symbol globally [1]. Happy to add an access function if that is preferred instead.

   As the patch is mostly move-only, it can be best reviewed via the git option --color-moved=dimmed-zebra

   [1] note that the same approach is used in the Silent Payments sending PR #28201, though having a TODO comment added, see https://github.com/bitcoin/bitcoin/pull/28201/commits/a4ebd7da5aaaa567fa9c613666513341f2aea241#diff-e5cbd40011e760f175b82d7ec44ad512bdb3d9108068b660642204da9c9187b1R25

2. refactor, key: move `CreateMuSig2Nonce` to `musig.{h,cpp}` module

   Nonce creation is mainly derived by randomness, and the secret
   key merely serves as (optional) additional data for increasing
   misuse-resistance, rather than being a central part that would
   justify an own CKey method, so move it to the musig.cpp module.
   
   Can be reviewed via the git option `--color-moved=dimmed-zebra`.

   138c6339ef
3. 
   DrahtBot commented at 11:45 pm on January 7, 2026: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34225.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	rkrux
   Approach ACK	w0xlt

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

4. refactor, key: move `CreateMuSig2PartialSig` to `musig.{h,cpp}` module

   Compared to `CreateMuSig2Nonce`, creating a partial signature
   has a stronger link to the secret key used, but for consistency
   reasons it still makes sense to move all functionality that call
   the secp256k1 musig API functions to the `musig.{h,cpp}` module
   for consistency.
   
   Can be reviewed via the git option `--color-moved=dimmed-zebra`.

   25709aaa79
5. theStack force-pushed on Jan 8, 2026
6. DrahtBot added the label CI failed on Jan 8, 2026
7. 
   DrahtBot commented at 1:15 am on January 8, 2026: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20800284605/job/59743518329 LLM reason (✨ experimental): Lint failure: a new circular dependency (key -> musig -> key) detected by the Python linter, causing all_python_linters to fail.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. 
   w0xlt commented at 2:03 am on January 8, 2026: contributor

   Approach ACK
9. DrahtBot removed the label CI failed on Jan 8, 2026
10. 
    rkrux commented at 2:43 pm on January 8, 2026: contributor

    Concept ACK 25709aaa790c05e647457bfce64571ca5495ceeb

    I do prefer not seeing the implementation of MuSig2 functions outside the musig module. If I build without the extern keyword, then I see the following warnings but the build doesn’t fail.

     0[ 74%] Linking CXX executable ../bin/bitcoin-wallet
     1ld: warning: duplicate symbol '_secp256k1_context_sign' in:
     2    ../lib/libbitcoin_common.a[29](key.cpp.o)
     3    ../lib/libbitcoin_common.a[32](musig.cpp.o)
     4[ 74%] Built target bitcoin-util
     5[ 74%] Built target bitcoin
     6ld: warning: duplicate symbol '_secp256k1_context_sign' in:
     7    ../lib/libbitcoin_common.a[29](key.cpp.o)
     8    ../lib/libbitcoin_common.a[32](musig.cpp.o)
     9[ 74%] Built target bitcoin-cli
    10[ 74%] Built target bitcoin-tx
    11ld: warning: duplicate symbol '_secp256k1_context_sign' in:
    12    ../lib/libbitcoin_common.a[29](key.cpp.o)
    13    ../lib/libbitcoin_common.a[32](musig.cpp.o)
    14ld: warning: duplicate symbol '_secp256k1_context_sign' in:
    15    ../lib/libbitcoin_common.a[29](key.cpp.o)
    16    ../lib/libbitcoin_common.a[32](musig.cpp.o)
    17[ 75%] Built target bitcoin-wallet
    18[ 75%] Built target bitcoind
    19[ 75%] Built target bitcoin-node
    20[ 75%] Linking CXX executable ../../bin/test_bitcoin
    21ld: warning: ignoring duplicate libraries: '../../lib/libbitcoin_consensus.a', '../secp256k1/lib/libsecp256k1.a'
    22ld: warning: duplicate symbol '_secp256k1_context_sign' in:
    23    ../../lib/libbitcoin_common.a[29](key.cpp.o)
    24    ../../lib/libbitcoin_common.a[32](musig.cpp.o)
    25[101%] Built target test_bitcoin
    

    Wondering if using the accessor function would make it more conspicuous that we must use the variable from the key module instead of creating another one.

Contributors
theStack DrahtBot w0xlt rkrux