← Watch feed
refactor, key: move `CreateMuSig2PartialSig` to `musig.{h,cpp}` module
Public commit record
78/100 · Adequate
What the developer wrote
Authored by Sebastian Falbesoner
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`.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit is a pure code move: it takes the CreateMuSig2PartialSig function out of the CKey class in key.cpp/key.h and places it in the musig.cpp/musig.h module. The function's logic is unchanged, and the only caller is updated to use the new free-function name. There is no security fix or behavior change.
Recommended action
No security action needed. Treat as routine refactoring during code review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff shows a refactor that relocates CreateMuSig2PartialSig from src/key.cpp/src/key.h to src/musig.cpp/src/musig.h. The implementation is copied almost verbatim, with the only meaningful change being that the secret key is now passed as a const CKey& parameter (our_seckey) instead of being accessed via this. The caller in src/script/sign.cpp is updated from key.CreateMuSig2PartialSig(…) to ::CreateMuSig2PartialSig(*sighash, key, …). The secp256k1_musig.h include is removed from key.cpp and key.h. No cryptographic checks, error handling, or control flow were modified.
Changed components
src/key.cppsrc/key.hsrc/musig.cppsrc/musig.hsrc/script/sign.cppInspect captured patch +90 / −94
diff --git a/src/key.cpp b/src/key.cpp
index c61616f1..cc03df1c 100644
--- a/src/key.cpp
+++ b/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,95 +349,6 @@ KeyPair CKey::ComputeKeyPair(const uint256* merkle_root) const
return KeyPair(*this, merkle_root);
}
-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;
diff --git a/src/key.h b/src/key.h
index e9f34a1a..cd77dcd0 100644
--- a/src/key.h
+++ b/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>
@@ -223,8 +222,6 @@ public:
* Merkle root of the script tree).
*/
KeyPair ComputeKeyPair(const uint256* merkle_root) const;
-
- 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;
diff --git a/src/musig.cpp b/src/musig.cpp
index 73089ea8..9a1b3442 100644
--- a/src/musig.cpp
+++ b/src/musig.cpp
@@ -161,6 +161,94 @@ std::vector<uint8_t> CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256&
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)
{
diff --git a/src/musig.h b/src/musig.h
index e18d2450..b61c2f3f 100644
--- a/src/musig.h
+++ b/src/musig.h
@@ -60,6 +60,7 @@ 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
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 4386e473..e8a0071e 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -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);
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.