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

8ba5f68b1d refactor, key: move `CreateMuSig2PartialSig` to `musig.{h,cpp}` module (Sebastian Falbesoner)
d087f266fc refactor, key: move `CreateMuSig2Nonce` to `musig.{h,cpp}` module (Sebastian Falbesoner)
f36d89f436 key: add `GetSecp256k1SignContext` access function (w0xlt)

Pull request description:

  This PR is a follow-up of #29675, see https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2265077463. 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. For accessing the `secp256k1_context_signing` object from the outside, a new function `GetSecp256k1SignContext` is added in the third commit.

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

ACKs for top commit:
  achow101:
    ACK 8ba5f68b1d
  w0xlt:
    reACK 8ba5f68b1d
  rkrux:
    lgtm ACK 8ba5f68b1d
  furszy:
    ACK 8ba5f68b1d

Tree-SHA512: 95fcaa5d7a09037a0dce0053b8c640a7372a1251a2a3615c565f4dacc5aad5cf0ee8bfc43aa0d0def628465c16330d69f6ea9fcc07bbadc971863248f60d1878

src/key.cpp

@@ -13,7 +13,6 @@
 #include <secp256k1.h>
 #include <secp256k1_ellswift.h>
 #include <secp256k1_extrakeys.h>
-#include <secp256k1_musig.h>
 #include <secp256k1_recovery.h>
 #include <secp256k1_schnorrsig.h>
 
@@ -350,128 +349,6 @@ KeyPair CKey::ComputeKeyPair(const uint256* merkle_root) const
     return KeyPair(*this, merkle_root);
 }
 
-std::vector<uint8_t> CKey::CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256& sighash, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys)
-{
-    // Get the keyagg cache and aggregate pubkey
-    secp256k1_musig_keyagg_cache keyagg_cache;
-    if (!MuSig2AggregatePubkeys(pubkeys, keyagg_cache, aggregate_pubkey)) return {};
-
-    // Parse participant pubkey
-    CPubKey our_pubkey = GetPubKey();
-    secp256k1_pubkey pubkey;
-    if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &pubkey, our_pubkey.data(), our_pubkey.size())) {
-        return {};
-    }
-
-    // Generate randomness for nonce
-    uint256 rand;
-    GetStrongRandBytes(rand);
-
-    // Generate nonce
-    secp256k1_musig_pubnonce pubnonce;
-    if (!secp256k1_musig_nonce_gen(secp256k1_context_sign, secnonce.Get(), &pubnonce, rand.data(), UCharCast(begin()), &pubkey, sighash.data(), &keyagg_cache, nullptr)) {
-        return {};
-    }
-
-    // Serialize pubnonce
-    std::vector<uint8_t> out;
-    out.resize(MUSIG2_PUBNONCE_SIZE);
-    if (!secp256k1_musig_pubnonce_serialize(secp256k1_context_static, out.data(), &pubnonce)) {
-        return {};
-    }
-
-    return out;
-}
-
-std::optional<uint256> CKey::CreateMuSig2PartialSig(const uint256& sighash, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, MuSig2SecNonce& secnonce, const std::vector<std::pair<uint256, bool>>& tweaks)
-{
-    secp256k1_keypair keypair;
-    if (!secp256k1_keypair_create(secp256k1_context_sign, &keypair, UCharCast(begin()))) return std::nullopt;
-
-    // Get the keyagg cache and aggregate pubkey
-    secp256k1_musig_keyagg_cache keyagg_cache;
-    if (!MuSig2AggregatePubkeys(pubkeys, keyagg_cache, aggregate_pubkey)) return std::nullopt;
-
-    // Check that there are enough pubnonces
-    if (pubnonces.size() != pubkeys.size()) return std::nullopt;
-
-    // Parse the pubnonces
-    std::vector<std::pair<secp256k1_pubkey, secp256k1_musig_pubnonce>> signers_data;
-    std::vector<const secp256k1_musig_pubnonce*> pubnonce_ptrs;
-    std::optional<size_t> our_pubkey_idx;
-    CPubKey our_pubkey = GetPubKey();
-    for (const CPubKey& part_pk : pubkeys) {
-        const auto& pn_it = pubnonces.find(part_pk);
-        if (pn_it == pubnonces.end()) return std::nullopt;
-        const std::vector<uint8_t> pubnonce = pn_it->second;
-        if (pubnonce.size() != MUSIG2_PUBNONCE_SIZE) return std::nullopt;
-        if (part_pk == our_pubkey) {
-            our_pubkey_idx = signers_data.size();
-        }
-
-        auto& [secp_pk, secp_pn] = signers_data.emplace_back();
-
-        if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &secp_pk, part_pk.data(), part_pk.size())) {
-            return std::nullopt;
-        }
-
-        if (!secp256k1_musig_pubnonce_parse(secp256k1_context_static, &secp_pn, pubnonce.data())) {
-            return std::nullopt;
-        }
-    }
-    if (our_pubkey_idx == std::nullopt) {
-        return std::nullopt;
-    }
-    pubnonce_ptrs.reserve(signers_data.size());
-    for (auto& [_, pn] : signers_data) {
-        pubnonce_ptrs.push_back(&pn);
-    }
-
-    // Aggregate nonces
-    secp256k1_musig_aggnonce aggnonce;
-    if (!secp256k1_musig_nonce_agg(secp256k1_context_static, &aggnonce, pubnonce_ptrs.data(), pubnonce_ptrs.size())) {
-        return std::nullopt;
-    }
-
-    // Apply tweaks
-    for (const auto& [tweak, xonly] : tweaks) {
-        if (xonly) {
-            if (!secp256k1_musig_pubkey_xonly_tweak_add(secp256k1_context_static, nullptr, &keyagg_cache, tweak.data())) {
-                return std::nullopt;
-            }
-        } else if (!secp256k1_musig_pubkey_ec_tweak_add(secp256k1_context_static, nullptr, &keyagg_cache, tweak.data())) {
-            return std::nullopt;
-        }
-    }
-
-    // Create musig_session
-    secp256k1_musig_session session;
-    if (!secp256k1_musig_nonce_process(secp256k1_context_static, &session, &aggnonce, sighash.data(), &keyagg_cache)) {
-        return std::nullopt;
-    }
-
-    // Create partial signature
-    secp256k1_musig_partial_sig psig;
-    if (!secp256k1_musig_partial_sign(secp256k1_context_static, &psig, secnonce.Get(), &keypair, &keyagg_cache, &session)) {
-        return std::nullopt;
-    }
-    // The secnonce must be deleted after signing to prevent nonce reuse.
-    secnonce.Invalidate();
-
-    // Verify partial signature
-    if (!secp256k1_musig_partial_sig_verify(secp256k1_context_static, &psig, &(signers_data.at(*our_pubkey_idx).second), &(signers_data.at(*our_pubkey_idx).first), &keyagg_cache, &session)) {
-        return std::nullopt;
-    }
-
-    // Serialize
-    uint256 sig;
-    if (!secp256k1_musig_partial_sig_serialize(secp256k1_context_static, sig.data(), &psig)) {
-        return std::nullopt;
-    }
-
-    return sig;
-}
-
 CKey GenerateRandomKey(bool compressed) noexcept
 {
     CKey key;
@@ -568,6 +445,11 @@ bool ECC_InitSanityCheck() {
     return key.VerifyPubKey(pubkey);
 }
 
+secp256k1_context* GetSecp256k1SignContext()
+{
+    return secp256k1_context_sign;
+}
+
 /** Initialize the elliptic curve support. May not be called twice without calling ECC_Stop first. */
 static void ECC_Start() {
     assert(secp256k1_context_sign == nullptr);

src/key.h

@@ -7,7 +7,6 @@
 #ifndef BITCOIN_KEY_H
 #define BITCOIN_KEY_H
 
-#include <musig.h>
 #include <pubkey.h>
 #include <serialize.h>
 #include <support/allocators/secure.h>
@@ -16,6 +15,8 @@
 #include <stdexcept>
 #include <vector>
 
+struct secp256k1_context_struct;
+typedef struct secp256k1_context_struct secp256k1_context;
 
 /**
  * CPrivKey is a serialized private key, with all parameters included
@@ -221,9 +222,6 @@ public:
      *                               Merkle root of the script tree).
      */
     KeyPair ComputeKeyPair(const uint256* merkle_root) const;
-
-    std::vector<uint8_t> CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256& sighash, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys);
-    std::optional<uint256> CreateMuSig2PartialSig(const uint256& hash, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, MuSig2SecNonce& secnonce, const std::vector<std::pair<uint256, bool>>& tweaks);
 };
 
 CKey GenerateRandomKey(bool compressed = true) noexcept;
@@ -315,6 +313,9 @@ private:
 /** Check that required EC support is available at runtime. */
 bool ECC_InitSanityCheck();
 
+/** Access the secp256k1 context used for signing and MuSig2 nonce generation. */
+secp256k1_context* GetSecp256k1SignContext();
+
 /**
  * RAII class initializing and deinitializing global state for elliptic curve support.
  * Only one instance may be initialized at a time.

src/musig.cpp

@@ -3,6 +3,8 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <musig.h>
+#include <key.h>
+#include <random.h>
 #include <support/allocators/secure.h>
 
 #include <secp256k1_musig.h>
@@ -126,6 +128,128 @@ uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey
     return hasher.GetSHA256();
 }
 
+std::vector<uint8_t> CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256& sighash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys)
+{
+    // Get the keyagg cache and aggregate pubkey
+    secp256k1_musig_keyagg_cache keyagg_cache;
+    if (!MuSig2AggregatePubkeys(pubkeys, keyagg_cache, aggregate_pubkey)) return {};
+
+    // Parse participant pubkey
+    CPubKey our_pubkey = our_seckey.GetPubKey();
+    secp256k1_pubkey pubkey;
+    if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &pubkey, our_pubkey.data(), our_pubkey.size())) {
+        return {};
+    }
+
+    // Generate randomness for nonce
+    uint256 rand;
+    GetStrongRandBytes(rand);
+
+    // Generate nonce
+    secp256k1_musig_pubnonce pubnonce;
+    if (!secp256k1_musig_nonce_gen(GetSecp256k1SignContext(), secnonce.Get(), &pubnonce, rand.data(), UCharCast(our_seckey.begin()), &pubkey, sighash.data(), &keyagg_cache, nullptr)) {
+        return {};
+    }
+
+    // Serialize pubnonce
+    std::vector<uint8_t> out;
+    out.resize(MUSIG2_PUBNONCE_SIZE);
+    if (!secp256k1_musig_pubnonce_serialize(secp256k1_context_static, out.data(), &pubnonce)) {
+        return {};
+    }
+
+    return out;
+}
+
+std::optional<uint256> CreateMuSig2PartialSig(const uint256& sighash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, MuSig2SecNonce& secnonce, const std::vector<std::pair<uint256, bool>>& tweaks)
+{
+    secp256k1_keypair keypair;
+    if (!secp256k1_keypair_create(GetSecp256k1SignContext(), &keypair, UCharCast(our_seckey.begin()))) return std::nullopt;
+
+    // Get the keyagg cache and aggregate pubkey
+    secp256k1_musig_keyagg_cache keyagg_cache;
+    if (!MuSig2AggregatePubkeys(pubkeys, keyagg_cache, aggregate_pubkey)) return std::nullopt;
+
+    // Check that there are enough pubnonces
+    if (pubnonces.size() != pubkeys.size()) return std::nullopt;
+
+    // Parse the pubnonces
+    std::vector<std::pair<secp256k1_pubkey, secp256k1_musig_pubnonce>> signers_data;
+    std::vector<const secp256k1_musig_pubnonce*> pubnonce_ptrs;
+    std::optional<size_t> our_pubkey_idx;
+    CPubKey our_pubkey = our_seckey.GetPubKey();
+    for (const CPubKey& part_pk : pubkeys) {
+        const auto& pn_it = pubnonces.find(part_pk);
+        if (pn_it == pubnonces.end()) return std::nullopt;
+        const std::vector<uint8_t> pubnonce = pn_it->second;
+        if (pubnonce.size() != MUSIG2_PUBNONCE_SIZE) return std::nullopt;
+        if (part_pk == our_pubkey) {
+            our_pubkey_idx = signers_data.size();
+        }
+
+        auto& [secp_pk, secp_pn] = signers_data.emplace_back();
+
+        if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &secp_pk, part_pk.data(), part_pk.size())) {
+            return std::nullopt;
+        }
+
+        if (!secp256k1_musig_pubnonce_parse(secp256k1_context_static, &secp_pn, pubnonce.data())) {
+            return std::nullopt;
+        }
+    }
+    if (our_pubkey_idx == std::nullopt) {
+        return std::nullopt;
+    }
+    pubnonce_ptrs.reserve(signers_data.size());
+    for (auto& [_, pn] : signers_data) {
+        pubnonce_ptrs.push_back(&pn);
+    }
+
+    // Aggregate nonces
+    secp256k1_musig_aggnonce aggnonce;
+    if (!secp256k1_musig_nonce_agg(secp256k1_context_static, &aggnonce, pubnonce_ptrs.data(), pubnonce_ptrs.size())) {
+        return std::nullopt;
+    }
+
+    // Apply tweaks
+    for (const auto& [tweak, xonly] : tweaks) {
+        if (xonly) {
+            if (!secp256k1_musig_pubkey_xonly_tweak_add(secp256k1_context_static, nullptr, &keyagg_cache, tweak.data())) {
+                return std::nullopt;
+            }
+        } else if (!secp256k1_musig_pubkey_ec_tweak_add(secp256k1_context_static, nullptr, &keyagg_cache, tweak.data())) {
+            return std::nullopt;
+        }
+    }
+
+    // Create musig_session
+    secp256k1_musig_session session;
+    if (!secp256k1_musig_nonce_process(secp256k1_context_static, &session, &aggnonce, sighash.data(), &keyagg_cache)) {
+        return std::nullopt;
+    }
+
+    // Create partial signature
+    secp256k1_musig_partial_sig psig;
+    if (!secp256k1_musig_partial_sign(secp256k1_context_static, &psig, secnonce.Get(), &keypair, &keyagg_cache, &session)) {
+        return std::nullopt;
+    }
+    // The secnonce must be deleted after signing to prevent nonce reuse.
+    secnonce.Invalidate();
+
+    // Verify partial signature
+    if (!secp256k1_musig_partial_sig_verify(secp256k1_context_static, &psig, &(signers_data.at(*our_pubkey_idx).second), &(signers_data.at(*our_pubkey_idx).first), &keyagg_cache, &session)) {
+        return std::nullopt;
+    }
+
+    // Serialize
+    uint256 sig;
+    if (!secp256k1_musig_partial_sig_serialize(secp256k1_context_static, sig.data(), &psig)) {
+        return std::nullopt;
+    }
+
+    return sig;
+}
+
 std::optional<std::vector<uint8_t>> CreateMuSig2AggregateSig(const std::vector<CPubKey>& part_pubkeys, const CPubKey& aggregate_pubkey, const std::vector<std::pair<uint256, bool>>& tweaks, const uint256& sighash, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, const std::map<CPubKey, uint256>& partial_sigs)
 {
     if (!part_pubkeys.size()) return std::nullopt;

src/musig.h

@@ -10,6 +10,7 @@
 #include <optional>
 #include <vector>
 
+class CKey;
 struct secp256k1_musig_keyagg_cache;
 class MuSig2SecNonceImpl;
 struct secp256k1_musig_secnonce;
@@ -58,6 +59,8 @@ public:
 
 uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256& sighash);
 
+std::vector<uint8_t> CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256& sighash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys);
+std::optional<uint256> CreateMuSig2PartialSig(const uint256& hash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector<CPubKey>& pubkeys, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, MuSig2SecNonce& secnonce, const std::vector<std::pair<uint256, bool>>& tweaks);
 std::optional<std::vector<uint8_t>> CreateMuSig2AggregateSig(const std::vector<CPubKey>& participants, const CPubKey& aggregate_pubkey, const std::vector<std::pair<uint256, bool>>& tweaks, const uint256& sighash, const std::map<CPubKey, std::vector<uint8_t>>& pubnonces, const std::map<CPubKey, uint256>& partial_sigs);
 
 #endif // BITCOIN_MUSIG_H

src/script/sign.cpp

@@ -119,7 +119,7 @@ std::vector<uint8_t> MutableTransactionSignatureCreator::CreateMuSig2Nonce(const
     if (!sighash.has_value()) return {};
 
     MuSig2SecNonce secnonce;
-    std::vector<uint8_t> out = key.CreateMuSig2Nonce(secnonce, *sighash, aggregate_pubkey, pubkeys);
+    std::vector<uint8_t> out = ::CreateMuSig2Nonce(secnonce, *sighash, key, aggregate_pubkey, pubkeys);
     if (out.empty()) return {};
 
     // Store the secnonce in the SigningProvider
@@ -161,7 +161,7 @@ bool MutableTransactionSignatureCreator::CreateMuSig2PartialSig(const SigningPro
     if (!secnonce || !secnonce->get().IsValid()) return false;
 
     // Compute the sig
-    std::optional<uint256> sig = key.CreateMuSig2PartialSig(*sighash, aggregate_pubkey, pubkeys, pubnonces, *secnonce, tweaks);
+    std::optional<uint256> sig = ::CreateMuSig2PartialSig(*sighash, key, aggregate_pubkey, pubkeys, pubnonces, *secnonce, tweaks);
     if (!sig) return false;
     partial_sig = std::move(*sig);