I don't mind removing the const from the privates in Node, but I'm not a fan of making the publics mutable...
It seems to me we could make them private if we move FindChallenges over.
<details>
<summary>or add getters to the fields</summary>
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index f99c9a8036..181a6614cf 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -495,19 +495,6 @@ struct NoDupCheck {};
//! A node in a miniscript expression.
template<typename Key>
struct Node {
- //! What node type this node is.
- Fragment fragment;
- //! The k parameter (time for OLDER/AFTER, threshold for THRESH(_M))
- uint32_t k = 0;
- //! The keys used by this expression (only for PK_K/PK_H/MULTI)
- std::vector<Key> keys;
- //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160).
- std::vector<unsigned char> data;
- //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH)
- std::vector<Node> subs;
- //! The Script context for this node. Either P2WSH or Tapscript.
- MiniscriptContext m_script_ctx;
-
// Permit 1 level deep recursion since we own instances of our own type.
// NOLINTBEGIN(misc-no-recursion)
~Node()
@@ -541,7 +528,26 @@ struct Node {
return TreeEval<Node>(upfn);
}
+ Fragment GetFragment() const noexcept { return fragment; }
+ uint32_t GetK() const noexcept { return k; }
+ std::span<const Key> GetKeys() const noexcept { return keys; }
+ std::span<const unsigned char> GetData() const noexcept { return data; }
+ std::span<const Node> GetSubs() const noexcept { return subs; }
+
private:
+ //! What node type this node is.
+ Fragment fragment;
+ //! The k parameter (time for OLDER/AFTER, threshold for THRESH(_M))
+ uint32_t k = 0;
+ //! The keys used by this expression (only for PK_K/PK_H/MULTI)
+ std::vector<Key> keys;
+ //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160).
+ std::vector<unsigned char> data;
+ //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH)
+ std::vector<Node> subs;
+ //! The Script context for this node. Either P2WSH or Tapscript.
+ MiniscriptContext m_script_ctx;
+
//! Cached ops counts.
internal::Ops ops;
//! Cached stack size bounds.
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index cee88373c4..8a5a54b0bf 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -121,7 +121,7 @@ enum class ChallengeType {
* For hashes it's just the first 4 bytes of the hash. For pubkeys, it's the last 4 bytes.
*/
uint32_t ChallengeNumber(const CPubKey& pubkey) { return ReadLE32(pubkey.data() + 29); }
-uint32_t ChallengeNumber(const std::vector<unsigned char>& hash) { return ReadLE32(hash.data()); }
+uint32_t ChallengeNumber(std::span<const unsigned char> hash) { return ReadLE32(hash.data()); }
//! A Challenge is a combination of type of leaf condition and its challenge number.
typedef std::pair<ChallengeType, uint32_t> Challenge;
@@ -304,19 +304,19 @@ std::set<Challenge> FindChallenges(const Node& root)
const auto* ref{stack.back()};
stack.pop_back();
- for (const auto& key : ref->keys) {
+ for (const auto& key : ref->GetKeys()) {
chal.emplace(ChallengeType::PK, ChallengeNumber(key));
}
- switch (ref->fragment) {
- case Fragment::OLDER: chal.emplace(ChallengeType::OLDER, ref->k); break;
- case Fragment::AFTER: chal.emplace(ChallengeType::AFTER, ref->k); break;
- case Fragment::SHA256: chal.emplace(ChallengeType::SHA256, ChallengeNumber(ref->data)); break;
- case Fragment::RIPEMD160: chal.emplace(ChallengeType::RIPEMD160, ChallengeNumber(ref->data)); break;
- case Fragment::HASH256: chal.emplace(ChallengeType::HASH256, ChallengeNumber(ref->data)); break;
- case Fragment::HASH160: chal.emplace(ChallengeType::HASH160, ChallengeNumber(ref->data)); break;
+ switch (ref->GetFragment()) {
+ case Fragment::OLDER: chal.emplace(ChallengeType::OLDER, ref->GetK()); break;
+ case Fragment::AFTER: chal.emplace(ChallengeType::AFTER, ref->GetK()); break;
+ case Fragment::SHA256: chal.emplace(ChallengeType::SHA256, ChallengeNumber(ref->GetData())); break;
+ case Fragment::RIPEMD160: chal.emplace(ChallengeType::RIPEMD160, ChallengeNumber(ref->GetData())); break;
+ case Fragment::HASH256: chal.emplace(ChallengeType::HASH256, ChallengeNumber(ref->GetData())); break;
+ case Fragment::HASH160: chal.emplace(ChallengeType::HASH160, ChallengeNumber(ref->GetData())); break;
default: break;
}
- for (const auto& sub : ref->subs) {
+ for (const auto& sub : ref->GetSubs()) {
stack.push_back(&sub);
}
}
</details>
