Musig2 tests

w0xlt commented at 2:08 am on June 11, 2025: contributor

Built on #31244

This PR adds explicit tests for Bitcoin Core’s MuSig2 interface.

Any issues in musig2.{cpp,h} will likely also be caught by the descriptor tests, but having more detailed tests for the MuSig2 class itself improves test reporting/coverage.

It uses BIP 328 test vectors.

XOnlyPubKey: Add GetCPubKeys

We need to retrieve the even and odd compressed pubkeys for xonly
pubkeys, so add a function to do that. Also reuse it in GetKeyIDs.

5fe4c66462

script/parsing: Allow Const to not skip the found constant 4b135f80d0

util/string: Allow Split to include the separator

When splitting a string, sometimes the separator needs to be included.
Split will now optionally include the separator at the end of the left
side of the splits, i.e. it appears at the end of the splits, except
for the last one.

b89a937225

descriptors: Add PubkeyProvider::IsBIP32() 81fe05d65f

build: Enable secp256k1 musig module fc1e8b63b6

sign: Add GetAggregateParticipantPubkeys to SigningProvider b9c4096beb

Add MuSig2 Keyagg Cache helper functions

secp256k1 provides us secp256k1_musig_keyagg_cache objects which we are
used as part of session info and to get the aggregate pubkey. These
helper functions help us convert to/from the secp256k1 C objects into
the Bitcoin Core C++ objects.

18aa775230

descriptor: Add MuSigPubkeyProvider 5a6bd72993

descriptor: Parse musig() key expressions 54a3c3c4f2

tests: Test musig() parsing 7c9da16cca

doc: Add musig() example af27ffbce3

DrahtBot commented at 2:08 am on June 11, 2025: 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/32724.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin-core/gui/872 (Menu action to export a watchonly wallet by achow101)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#32471 (wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key by Eunovo)
#32332 (refactor: Update XOnlyPubKey::GetKeyIDs() to return a pair of pubkeys by w0xlt)
#31244 (descriptors: MuSig2 by achow101)
#30243 (descriptors: taproot partial descriptors by Eunovo)
#29675 (wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys by achow101)
#26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Jun 11, 2025

DrahtBot commented at 3:08 am on June 11, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/43856471515 LLM reason (✨ experimental): The CI failure is caused by a trailing whitespace error detected in src/test/musig2.cpp.

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.

w0xlt force-pushed on Jun 11, 2025

[test] Add MuSig2 tests (BIP 328 test vectors) 4b85dbd5bc

w0xlt force-pushed on Jun 11, 2025

DrahtBot removed the label CI failed on Jun 11, 2025

rkrux commented at 10:47 am on June 12, 2025: contributor

I feel this PR should be in draft until #31244 is merged as all the commits here are from that PR except the last one.

Though the last commit can be reviewed separately.

brunoerg commented at 6:37 pm on June 12, 2025: contributor

Mutation testing report for #32724

223 mutants for src/script/descriptor.cpp - mutation score: 90.13% 10 mutants for src/musig.cpp - mutation score: 80.0%

  0mutation-core analyze -f="muts-pr-32724-descriptor-cpp" -c="cmake --build build -j 5 && ./build/bin/test_bitcoin --run_test=musig2_tests && ./build/bin/test_bitcoin --run_test=descriptor_tests"
  1* 223 MUTANTS *
  2...
  3MUTATION SCORE: 90.13%
  4
  5Surviving mutants:
  6diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.186.cpp
  7index 8cb78327f7..0f8840c015 100644
  8--- a/src/script/descriptor.cpp
  9+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.186.cpp
 10@@ -1910,7 +1910,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
 11                         vec.emplace_back(vec.at(0)->Clone());
 12                     }
 13                 } else if (vec.size() != length) {
 14-                    return false;
 15+                    return true;
 16                 }
 17             }
 18             return true;
 19
 20--------------
 21diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.218.cpp
 22index 8cb78327f7..ca626ff150 100644
 23--- a/src/script/descriptor.cpp
 24+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.218.cpp
 25@@ -2067,7 +2067,7 @@ struct KeyParser {
 26     {
 27         assert(m_out);
 28         Key key = m_keys.size();
 29-        uint32_t exp_index = m_offset + key;
 30+        uint32_t exp_index = m_offset - key;
 31         auto pk = ParsePubkey(exp_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error);
 32         if (pk.empty()) return {};
 33         m_keys.emplace_back(std::move(pk));
 34
 35--------------
 36diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.22.cpp
 37index 8cb78327f7..53c1a41269 100644
 38--- a/src/script/descriptor.cpp
 39+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.22.cpp
 40@@ -670,7 +670,7 @@ public:
 41             // Use a dummy signing provider as private keys do not exist for the aggregate pubkey
 42             FlatSigningProvider dummy;
 43             std::optional<CPubKey> pub = m_aggregate_provider->GetPubKey(pos, dummy, out, read_cache, write_cache);
 44-            if (!pub) return std::nullopt;
 45+            if (1==0) return std::nullopt;
 46             pubout = *pub;
 47             out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
 48         } else {
 49
 50--------------
 51diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.23.cpp
 52index 8cb78327f7..b47540ebc5 100644
 53--- a/src/script/descriptor.cpp
 54+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.23.cpp
 55@@ -670,7 +670,7 @@ public:
 56             // Use a dummy signing provider as private keys do not exist for the aggregate pubkey
 57             FlatSigningProvider dummy;
 58             std::optional<CPubKey> pub = m_aggregate_provider->GetPubKey(pos, dummy, out, read_cache, write_cache);
 59-            if (!pub) return std::nullopt;
 60+
 61             pubout = *pub;
 62             out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
 63         } else {
 64
 65--------------
 66diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.221.cpp
 67index 8cb78327f7..96bc9cd33c 100644
 68--- a/src/script/descriptor.cpp
 69+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.221.cpp
 70@@ -2067,7 +2067,7 @@ struct KeyParser {
 71     {
 72         assert(m_out);
 73         Key key = m_keys.size();
 74-        uint32_t exp_index = m_offset + key;
 75+        uint32_t exp_index = (m_offset + key) + 1;
 76         auto pk = ParsePubkey(exp_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error);
 77         if (pk.empty()) return {};
 78         m_keys.emplace_back(std::move(pk));
 79
 80--------------
 81diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.209.cpp
 82index 8cb78327f7..471ea17a83 100644
 83--- a/src/script/descriptor.cpp
 84+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.209.cpp
 85@@ -1940,7 +1940,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
 86                 // Final MuSigPubkeyProvider use participant pubkey providers at each multipath position, and the first (and only) path
 87                 emplace_final_provider(i, 0);
 88             }
 89-        } else if (paths.size() > 1) {
 90+        } else if (paths.size() >= 1) {
 91             // All key provider vectors should be length 1. Clone them until they have the same length as paths
 92             if (!clone_providers(paths.size())) {
 93                 error = "musig(): Multipath derivation path with multipath participants is disallowed"; // This error is unreachable due to earlier check
 94
 95--------------
 96diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.27.cpp
 97index 8cb78327f7..1f3b5320c3 100644
 98--- a/src/script/descriptor.cpp
 99+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.27.cpp
100@@ -677,7 +677,7 @@ public:
101             if (!Assume(m_ranged_participants)) return std::nullopt;
102             // Compute aggregate key from derived participants
103             std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
104-            if (!aggregate_pubkey) return std::nullopt;
105+
106             pubout = *aggregate_pubkey;
107
108             KeyOriginInfo info;
109
110--------------
111diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.26.cpp
112index 8cb78327f7..917be613cd 100644
113--- a/src/script/descriptor.cpp
114+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.26.cpp
115@@ -677,7 +677,7 @@ public:
116             if (!Assume(m_ranged_participants)) return std::nullopt;
117             // Compute aggregate key from derived participants
118             std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
119-            if (!aggregate_pubkey) return std::nullopt;
120+            if (1==0) return std::nullopt;
121             pubout = *aggregate_pubkey;
122
123             KeyOriginInfo info;
124
125--------------
126diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.24.cpp
127index 8cb78327f7..8a4961d14f 100644
128--- a/src/script/descriptor.cpp
129+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.24.cpp
130@@ -674,7 +674,7 @@ public:
131             pubout = *pub;
132             out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
133         } else {
134-            if (!Assume(m_ranged_participants)) return std::nullopt;
135+
136             // Compute aggregate key from derived participants
137             std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
138             if (!aggregate_pubkey) return std::nullopt;
139
140--------------
141diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.18.cpp
142index 8cb78327f7..bc2436a7cf 100644
143--- a/src/script/descriptor.cpp
144+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.18.cpp
145@@ -658,7 +658,7 @@ public:
146         std::vector<CPubKey> pubkeys;
147         for (const auto& prov : m_participants) {
148             std::optional<CPubKey> pub = prov->GetPubKey(pos, arg, out, read_cache, write_cache);
149-            if (!pub) return std::nullopt;
150+
151             pubkeys.emplace_back(*pub);
152         }
153         std::sort(pubkeys.begin(), pubkeys.end());
154
155--------------
156diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.31.cpp
157index 8cb78327f7..b448f978e8 100644
158--- a/src/script/descriptor.cpp
159+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.31.cpp
160@@ -688,7 +688,7 @@ public:
161             out.aggregate_pubkeys.emplace(pubout, pubkeys);
162         }
163
164-        if (!Assume(pubout.IsValid())) return std::nullopt;
165+
166         return pubout;
167     }
168     bool IsRange() const override { return IsRangedDerivation() || m_ranged_participants; }
169
170--------------
171diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.86.cpp
172index 8cb78327f7..73e12ab35e 100644
173--- a/src/script/descriptor.cpp
174+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.86.cpp
175@@ -730,7 +730,7 @@ public:
176         if (IsRangedDerivation()) {
177             out += "/*";
178         }
179-        if (!any_privkeys) out.clear();
180+        if (1==0) out.clear();
181         return any_privkeys;
182     }
183     bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const override
184
185--------------
186diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.99.cpp
187index 8cb78327f7..e1cff262a0 100644
188--- a/src/script/descriptor.cpp
189+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.99.cpp
190@@ -741,7 +741,7 @@ public:
191             if (i) out += ",";
192             std::string tmp;
193             if (!pubkey->ToNormalizedString(arg, tmp)) {
194-                return false;
195+                return true;
196             }
197             out += tmp;
198         }
199
200--------------
201diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.117.cpp
202index 8cb78327f7..2ef8b39b2d 100644
203--- a/src/script/descriptor.cpp
204+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.117.cpp
205@@ -789,7 +789,7 @@ public:
206     bool IsBIP32() const override
207     {
208         // musig() can only be a BIP 32 key if all participants are bip32 too
209-        return std::all_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsBIP32(); });
210+        return std::any_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsBIP32(); });
211     }
212 };
213
214
215--------------
216diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.165.cpp
217index 8cb78327f7..200cabd7a6 100644
218--- a/src/script/descriptor.cpp
219+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.165.cpp
220@@ -1892,7 +1892,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
221                 error = "musig(): Cannot have hardened child derivation";
222                 return {};
223             }
224-            bool dummy = false;
225+            bool dummy = true;
226             if (!ParseKeyPath(deriv_split, paths, dummy, error, /*allow_multipath=*/true, /*allow_hardened=*/false)) {
227                 error = "musig(): " + error;
228                 return {};
229
230--------------
231diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.17.cpp
232index 8cb78327f7..20f835caf8 100644
233--- a/src/script/descriptor.cpp
234+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.17.cpp
235@@ -658,7 +658,7 @@ public:
236         std::vector<CPubKey> pubkeys;
237         for (const auto& prov : m_participants) {
238             std::optional<CPubKey> pub = prov->GetPubKey(pos, arg, out, read_cache, write_cache);
239-            if (!pub) return std::nullopt;
240+            if (1==0) return std::nullopt;
241             pubkeys.emplace_back(*pub);
242         }
243         std::sort(pubkeys.begin(), pubkeys.end());
244
245--------------
246diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.3.cpp
247index 8cb78327f7..93e335fc45 100644
248--- a/src/script/descriptor.cpp
249+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.3.cpp
250@@ -608,7 +608,7 @@ public:
251         m_participants(std::move(providers)),
252         m_path(std::move(path)),
253         m_derive(derive),
254-        m_ranged_participants(std::any_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsRange(); }))
255+        m_ranged_participants(std::all_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsRange(); }))
256     {
257         if (!Assume(!(m_ranged_participants && IsRangedDerivation()))) {
258             throw std::runtime_error("musig(): Cannot have both ranged participants and ranged derivation");
259
260--------------
261diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.12.cpp
262index 8cb78327f7..b4a54fb541 100644
263--- a/src/script/descriptor.cpp
264+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.12.cpp
265@@ -648,7 +648,7 @@ public:
266                 extpub.chaincode = MUSIG_CHAINCODE;
267                 extpub.pubkey = m_aggregate_pubkey.value();
268
269-                m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/false);
270+                m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/true);
271             } else {
272                 m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/false);
273             }
274
275--------------
276diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.13.cpp
277index 8cb78327f7..72e90a79de 100644
278--- a/src/script/descriptor.cpp
279+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.13.cpp
280@@ -650,7 +650,7 @@ public:
281
282                 m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/false);
283             } else {
284-                m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/false);
285+                m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/true);
286             }
287         }
288
289
290--------------
291diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.177.cpp
292index 8cb78327f7..363403a0c2 100644
293--- a/src/script/descriptor.cpp
294+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.177.cpp
295@@ -1906,7 +1906,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
296         const auto& clone_providers = [&providers](size_t length) -> bool {
297             for (auto& vec : providers) {
298                 if (vec.size() == 1) {
299-                    for (size_t i = 1; i < length; ++i) {
300+                    for (size_t i = 1; i < length; --i) {
301                         vec.emplace_back(vec.at(0)->Clone());
302                     }
303                 } else if (vec.size() != length) {
304
305--------------
306diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.175.cpp
307index 8cb78327f7..3bee51264c 100644
308--- a/src/script/descriptor.cpp
309+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.175.cpp
310@@ -1906,7 +1906,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
311         const auto& clone_providers = [&providers](size_t length) -> bool {
312             for (auto& vec : providers) {
313                 if (vec.size() == 1) {
314-                    for (size_t i = 1; i < length; ++i) {
315+                    for (size_t i = 1; i <= length; ++i) {
316                         vec.emplace_back(vec.at(0)->Clone());
317                     }
318                 } else if (vec.size() != length) {
319
320--------------
321diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.11.cpp
322index 8cb78327f7..9594e83929 100644
323--- a/src/script/descriptor.cpp
324+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.11.cpp
325@@ -636,7 +636,7 @@ public:
326
327             // Aggregate the pubkey
328             m_aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
329-            if (!Assume(m_aggregate_pubkey.has_value())) return std::nullopt;
330+
331
332             // Make our pubkey provider
333             if (IsRangedDerivation() || !m_path.empty()) {
334
335--------------
336
337```diff
338➜  bitcoin-core-dev git:(pr/32724) ✗ mutation-core analyze -f="muts-pr-32724-musig-cpp" -c="cmake --build build -j 5 && ./build/bin/test_bitcoin --run_test=musig2_tests && ./build/bin/test_bitcoin --run_test=descriptor_tests"
339...
340MUTATION SCORE: 80.0%
341
342Surviving mutants:
343diff --git a/src/musig.cpp b/muts-pr-32724-musig-cpp/musig.mutant.2.cpp
344index b332954312..8d60925399 100644
345--- a/src/musig.cpp
346+++ b/muts-pr-32724-musig-cpp/musig.mutant.2.cpp
347@@ -13,7 +13,7 @@ bool GetMuSig2KeyAggCache(const std::vector<CPubKey>& pubkeys, secp256k1_musig_k
348     std::vector<const secp256k1_pubkey*> pubkey_ptrs;
349     for (const CPubKey& pubkey : pubkeys) {
350         if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &secp_pubkeys.emplace_back(), pubkey.data(), pubkey.size())) {
351-            return false;
352+            return true;
353         }
354     }
355     pubkey_ptrs.reserve(secp_pubkeys.size());
356
357--------------
358diff --git a/src/musig.cpp b/muts-pr-32724-musig-cpp/musig.mutant.5.cpp
359index b332954312..219f54aa14 100644
360--- a/src/musig.cpp
361+++ b/muts-pr-32724-musig-cpp/musig.mutant.5.cpp
362@@ -23,7 +23,7 @@ bool GetMuSig2KeyAggCache(const std::vector<CPubKey>& pubkeys, secp256k1_musig_k
363
364     // Aggregate the pubkey
365     if (!secp256k1_musig_pubkey_agg(secp256k1_context_static, nullptr, &keyagg_cache, pubkey_ptrs.data(), pubkey_ptrs.size())) {
366-        return false;
367+        return true;
368     }
369     return true;
370 }
371--------------

note: It only generated mutants based on git diff - code touched by this PR.

Musig2 tests #32724

Code Coverage & Benchmarks

Reviews

Conflicts