-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Musig2 tests #32724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Musig2 tests #32724
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32724. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
e3eba1f
to
4b85dbd
Compare
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. |
Mutation testing report for #32724 223 mutants for Unkilled mutants for src/script/descriptor.cpp and src/musig.cppmutation-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"
* 223 MUTANTS *
...
MUTATION SCORE: 90.13%
Surviving mutants:
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.186.cpp
index 8cb78327f7..0f8840c015 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.186.cpp
@@ -1910,7 +1910,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
vec.emplace_back(vec.at(0)->Clone());
}
} else if (vec.size() != length) {
- return false;
+ return true;
}
}
return true;
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.218.cpp
index 8cb78327f7..ca626ff150 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.218.cpp
@@ -2067,7 +2067,7 @@ struct KeyParser {
{
assert(m_out);
Key key = m_keys.size();
- uint32_t exp_index = m_offset + key;
+ uint32_t exp_index = m_offset - key;
auto pk = ParsePubkey(exp_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error);
if (pk.empty()) return {};
m_keys.emplace_back(std::move(pk));
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.22.cpp
index 8cb78327f7..53c1a41269 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.22.cpp
@@ -670,7 +670,7 @@ public:
// Use a dummy signing provider as private keys do not exist for the aggregate pubkey
FlatSigningProvider dummy;
std::optional<CPubKey> pub = m_aggregate_provider->GetPubKey(pos, dummy, out, read_cache, write_cache);
- if (!pub) return std::nullopt;
+ if (1==0) return std::nullopt;
pubout = *pub;
out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
} else {
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.23.cpp
index 8cb78327f7..b47540ebc5 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.23.cpp
@@ -670,7 +670,7 @@ public:
// Use a dummy signing provider as private keys do not exist for the aggregate pubkey
FlatSigningProvider dummy;
std::optional<CPubKey> pub = m_aggregate_provider->GetPubKey(pos, dummy, out, read_cache, write_cache);
- if (!pub) return std::nullopt;
+
pubout = *pub;
out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
} else {
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.221.cpp
index 8cb78327f7..96bc9cd33c 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.221.cpp
@@ -2067,7 +2067,7 @@ struct KeyParser {
{
assert(m_out);
Key key = m_keys.size();
- uint32_t exp_index = m_offset + key;
+ uint32_t exp_index = (m_offset + key) + 1;
auto pk = ParsePubkey(exp_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error);
if (pk.empty()) return {};
m_keys.emplace_back(std::move(pk));
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.209.cpp
index 8cb78327f7..471ea17a83 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.209.cpp
@@ -1940,7 +1940,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
// Final MuSigPubkeyProvider use participant pubkey providers at each multipath position, and the first (and only) path
emplace_final_provider(i, 0);
}
- } else if (paths.size() > 1) {
+ } else if (paths.size() >= 1) {
// All key provider vectors should be length 1. Clone them until they have the same length as paths
if (!clone_providers(paths.size())) {
error = "musig(): Multipath derivation path with multipath participants is disallowed"; // This error is unreachable due to earlier check
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.27.cpp
index 8cb78327f7..1f3b5320c3 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.27.cpp
@@ -677,7 +677,7 @@ public:
if (!Assume(m_ranged_participants)) return std::nullopt;
// Compute aggregate key from derived participants
std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
- if (!aggregate_pubkey) return std::nullopt;
+
pubout = *aggregate_pubkey;
KeyOriginInfo info;
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.26.cpp
index 8cb78327f7..917be613cd 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.26.cpp
@@ -677,7 +677,7 @@ public:
if (!Assume(m_ranged_participants)) return std::nullopt;
// Compute aggregate key from derived participants
std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
- if (!aggregate_pubkey) return std::nullopt;
+ if (1==0) return std::nullopt;
pubout = *aggregate_pubkey;
KeyOriginInfo info;
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.24.cpp
index 8cb78327f7..8a4961d14f 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.24.cpp
@@ -674,7 +674,7 @@ public:
pubout = *pub;
out.aggregate_pubkeys.emplace(m_aggregate_pubkey.value(), pubkeys);
} else {
- if (!Assume(m_ranged_participants)) return std::nullopt;
+
// Compute aggregate key from derived participants
std::optional<CPubKey> aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
if (!aggregate_pubkey) return std::nullopt;
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.18.cpp
index 8cb78327f7..bc2436a7cf 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.18.cpp
@@ -658,7 +658,7 @@ public:
std::vector<CPubKey> pubkeys;
for (const auto& prov : m_participants) {
std::optional<CPubKey> pub = prov->GetPubKey(pos, arg, out, read_cache, write_cache);
- if (!pub) return std::nullopt;
+
pubkeys.emplace_back(*pub);
}
std::sort(pubkeys.begin(), pubkeys.end());
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.31.cpp
index 8cb78327f7..b448f978e8 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.31.cpp
@@ -688,7 +688,7 @@ public:
out.aggregate_pubkeys.emplace(pubout, pubkeys);
}
- if (!Assume(pubout.IsValid())) return std::nullopt;
+
return pubout;
}
bool IsRange() const override { return IsRangedDerivation() || m_ranged_participants; }
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.86.cpp
index 8cb78327f7..73e12ab35e 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.86.cpp
@@ -730,7 +730,7 @@ public:
if (IsRangedDerivation()) {
out += "/*";
}
- if (!any_privkeys) out.clear();
+ if (1==0) out.clear();
return any_privkeys;
}
bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const override
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.99.cpp
index 8cb78327f7..e1cff262a0 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.99.cpp
@@ -741,7 +741,7 @@ public:
if (i) out += ",";
std::string tmp;
if (!pubkey->ToNormalizedString(arg, tmp)) {
- return false;
+ return true;
}
out += tmp;
}
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.117.cpp
index 8cb78327f7..2ef8b39b2d 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.117.cpp
@@ -789,7 +789,7 @@ public:
bool IsBIP32() const override
{
// musig() can only be a BIP 32 key if all participants are bip32 too
- return std::all_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsBIP32(); });
+ return std::any_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsBIP32(); });
}
};
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.165.cpp
index 8cb78327f7..200cabd7a6 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.165.cpp
@@ -1892,7 +1892,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
error = "musig(): Cannot have hardened child derivation";
return {};
}
- bool dummy = false;
+ bool dummy = true;
if (!ParseKeyPath(deriv_split, paths, dummy, error, /*allow_multipath=*/true, /*allow_hardened=*/false)) {
error = "musig(): " + error;
return {};
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.17.cpp
index 8cb78327f7..20f835caf8 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.17.cpp
@@ -658,7 +658,7 @@ public:
std::vector<CPubKey> pubkeys;
for (const auto& prov : m_participants) {
std::optional<CPubKey> pub = prov->GetPubKey(pos, arg, out, read_cache, write_cache);
- if (!pub) return std::nullopt;
+ if (1==0) return std::nullopt;
pubkeys.emplace_back(*pub);
}
std::sort(pubkeys.begin(), pubkeys.end());
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.3.cpp
index 8cb78327f7..93e335fc45 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.3.cpp
@@ -608,7 +608,7 @@ public:
m_participants(std::move(providers)),
m_path(std::move(path)),
m_derive(derive),
- m_ranged_participants(std::any_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsRange(); }))
+ m_ranged_participants(std::all_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsRange(); }))
{
if (!Assume(!(m_ranged_participants && IsRangedDerivation()))) {
throw std::runtime_error("musig(): Cannot have both ranged participants and ranged derivation");
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.12.cpp
index 8cb78327f7..b4a54fb541 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.12.cpp
@@ -648,7 +648,7 @@ public:
extpub.chaincode = MUSIG_CHAINCODE;
extpub.pubkey = m_aggregate_pubkey.value();
- m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/false);
+ m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/true);
} else {
m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/false);
}
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.13.cpp
index 8cb78327f7..72e90a79de 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.13.cpp
@@ -650,7 +650,7 @@ public:
m_aggregate_provider = std::make_unique<BIP32PubkeyProvider>(m_expr_index, extpub, m_path, m_derive, /*apostrophe=*/false);
} else {
- m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/false);
+ m_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, m_aggregate_pubkey.value(), /*xonly=*/true);
}
}
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.177.cpp
index 8cb78327f7..363403a0c2 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.177.cpp
@@ -1906,7 +1906,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
const auto& clone_providers = [&providers](size_t length) -> bool {
for (auto& vec : providers) {
if (vec.size() == 1) {
- for (size_t i = 1; i < length; ++i) {
+ for (size_t i = 1; i < length; --i) {
vec.emplace_back(vec.at(0)->Clone());
}
} else if (vec.size() != length) {
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.175.cpp
index 8cb78327f7..3bee51264c 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.175.cpp
@@ -1906,7 +1906,7 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
const auto& clone_providers = [&providers](size_t length) -> bool {
for (auto& vec : providers) {
if (vec.size() == 1) {
- for (size_t i = 1; i < length; ++i) {
+ for (size_t i = 1; i <= length; ++i) {
vec.emplace_back(vec.at(0)->Clone());
}
} else if (vec.size() != length) {
--------------
diff --git a/src/script/descriptor.cpp b/muts-pr-32724-descriptor-cpp/descriptor.mutant.11.cpp
index 8cb78327f7..9594e83929 100644
--- a/src/script/descriptor.cpp
+++ b/muts-pr-32724-descriptor-cpp/descriptor.mutant.11.cpp
@@ -636,7 +636,7 @@ public:
// Aggregate the pubkey
m_aggregate_pubkey = MuSig2AggregatePubkeys(pubkeys);
- if (!Assume(m_aggregate_pubkey.has_value())) return std::nullopt;
+
// Make our pubkey provider
if (IsRangedDerivation() || !m_path.empty()) {
--------------
```diff
➜ 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"
...
MUTATION SCORE: 80.0%
Surviving mutants:
diff --git a/src/musig.cpp b/muts-pr-32724-musig-cpp/musig.mutant.2.cpp
index b332954312..8d60925399 100644
--- a/src/musig.cpp
+++ b/muts-pr-32724-musig-cpp/musig.mutant.2.cpp
@@ -13,7 +13,7 @@ bool GetMuSig2KeyAggCache(const std::vector<CPubKey>& pubkeys, secp256k1_musig_k
std::vector<const secp256k1_pubkey*> pubkey_ptrs;
for (const CPubKey& pubkey : pubkeys) {
if (!secp256k1_ec_pubkey_parse(secp256k1_context_static, &secp_pubkeys.emplace_back(), pubkey.data(), pubkey.size())) {
- return false;
+ return true;
}
}
pubkey_ptrs.reserve(secp_pubkeys.size());
--------------
diff --git a/src/musig.cpp b/muts-pr-32724-musig-cpp/musig.mutant.5.cpp
index b332954312..219f54aa14 100644
--- a/src/musig.cpp
+++ b/muts-pr-32724-musig-cpp/musig.mutant.5.cpp
@@ -23,7 +23,7 @@ bool GetMuSig2KeyAggCache(const std::vector<CPubKey>& pubkeys, secp256k1_musig_k
// Aggregate the pubkey
if (!secp256k1_musig_pubkey_agg(secp256k1_context_static, nullptr, &keyagg_cache, pubkey_ptrs.data(), pubkey_ptrs.size())) {
- return false;
+ return true;
}
return true;
}
-------------- note: It only generated mutants based on git diff - code touched by this PR. |
Rebased and fixed |
The CI error is related to the wallet migration test. |
This can be moved out of draft? (since the CI failure is unrelated) Concept ACK Just two test vectors? If there's more then it might make sense to put them in a JSON file. |
Yes, they cover BIP 328 test vectors and the mutation detected by @brunoerg . |
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.