Repository: bitcoin/bitcoin
File: src/test/miniscript_tests.cpp (C++)

Code before:
        if (key.IsValid()) return key;
        return {};
    }

    template<typename I>
    std::optional<Key> FromPKBytes(I first, I last) const {
        if (!miniscript::IsTapscript(m_script_ctx)) {
            Key key{first, last};
            if (key.IsValid()) return key;
            return {};
        }
        if (last - first != 32) return {};
        XOnlyPubKey xonly_pubkey;
        std::copy(first, last, xonly_pubkey.begin());
        return xonly_pubkey.GetEvenCorrespondingCPubKey();
    }

    template<typename I>
    std::optional<Key> FromPKHBytes(I first, I last) const {
        assert(last - first == 20);
        CKeyID keyid;
        std::copy(first, last, keyid.begin());
        return g_testdata->pkmap.at(keyid);
    }

    std::string ToString(const Key& key, bool* has_priv_key = nullptr) const {
        return HexStr(ToPKBytes(key));
    }

    miniscript::MiniscriptContext MsContext() const {
        return m_script_ctx;
    }
};

/** A class that encapsulates all signing/hash revealing operations. */
struct Satisfier : public KeyConverter {

    Satisfier(miniscript::MiniscriptContext ctx) noexcept : KeyConverter{ctx} {}

    //! Which keys/timelocks/hash preimages are available.
    std::set<Challenge> supported;

    //! Implement simplified CLTV logic: stack value must exactly match an entry in `supported`.
    bool CheckAfter(uint32_t value) const {
        return supported.count(Challenge(ChallengeType::AFTER, value));
    }

    //! Implement simplified CSV logic: stack value must exactly match an entry in `supported`.
    bool CheckOlder(uint32_t value) const {
        return supported.count(Challenge(ChallengeType::OLDER, value));
    }

Review comment (suggestion):
In b32c2b290f8e4dcb7fe54ee4c81714a3785920a7 _descriptor: ToPrivateString() pass if  at least 1 priv key exists_:

To preserve existing behaviour:

```cpp
if (has_priv_key) *has_priv_key = true;
```

It's a bit worrying that the test passes whether I set this to `false` or `true`. But that's a pre-existing issue, as the following change to the original code doesn't break the test either:

```diff
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 0a32727f37..0989f0c50a 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -189,7 +189,7 @@ struct KeyConverter {
     }

     std::optional<std::string> ToString(const Key& key) const {
-        return HexStr(ToPKBytes(key));
+        return {};
     }

     miniscript::MiniscriptContext MsContext() const {
```

cc @darosior 

Code after:
        if (key.IsValid()) return key;
        return {};
    }

    template<typename I>
    std::optional<Key> FromPKBytes(I first, I last) const {
        if (!miniscript::IsTapscript(m_script_ctx)) {
            Key key{first, last};
            if (key.IsValid()) return key;
            return {};
        }
        if (last - first != 32) return {};
        XOnlyPubKey xonly_pubkey;
        std::copy(first, last, xonly_pubkey.begin());
        return xonly_pubkey.GetEvenCorrespondingCPubKey();
    }

    template<typename I>
    std::optional<Key> FromPKHBytes(I first, I last) const {
        assert(last - first == 20);
        CKeyID keyid;
        std::copy(first, last, keyid.begin());
        return g_testdata->pkmap.at(keyid);
    }

    std::optional<std::string> ToString(const Key& key, bool&) const {
        return HexStr(ToPKBytes(key));
    }

    miniscript::MiniscriptContext MsContext() const {
        return m_script_ctx;
    }
};

/** A class that encapsulates all signing/hash revealing operations. */
struct Satisfier : public KeyConverter {

    Satisfier(miniscript::MiniscriptContext ctx) noexcept : KeyConverter{ctx} {}

    //! Which keys/timelocks/hash preimages are available.
    std::set<Challenge> supported;

    //! Implement simplified CLTV logic: stack value must exactly match an entry in `supported`.
    bool CheckAfter(uint32_t value) const {
        return supported.count(Challenge(ChallengeType::AFTER, value));
    }

    //! Implement simplified CSV logic: stack value must exactly match an entry in `supported`.
    bool CheckOlder(uint32_t value) const {
        return supported.count(Challenge(ChallengeType::OLDER, value));
    }