Repository: bitcoin/bitcoin
File: src/script/descriptor.cpp (C++)

Code before:
protected:
    std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, std::span<const CScript> scripts,
                                     FlatSigningProvider& provider) const override
    {
        const auto script_ctx{m_node->GetMsCtx()};
        for (const auto& key : keys) {
            if (miniscript::IsTapscript(script_ctx)) {
                provider.pubkeys.emplace(Hash160(XOnlyPubKey{key}), key);
            } else {
                provider.pubkeys.emplace(key.GetID(), key);
            }
        }
        return Vector(m_node->ToScript(ScriptMaker(keys, script_ctx)));
    }

public:
    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
        : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}

    bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
                        const DescriptorCache* cache = nullptr) const override
    {
        bool has_priv_key{false};
        auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
        if (res) out = *res;
        return type == StringType::PRIVATE ? has_priv_key : res.has_value();
    }

    bool IsSolvable() const override { return true; }
    bool IsSingleType() const final { return true; }

    std::optional<int64_t> ScriptSize() const override { return m_node->ScriptSize(); }

    std::optional<int64_t> MaxSatSize(bool) const override {
        // For Miniscript we always assume high-R ECDSA signatures.
        return m_node->GetWitnessSize();
    }

    std::optional<int64_t> MaxSatisfactionElems() const override {
        return m_node->GetStackSize();
    }

    std::unique_ptr<DescriptorImpl> Clone() const override
    {
        std::vector<std::unique_ptr<PubkeyProvider>> providers;
        providers.reserve(m_pubkey_args.size());
        for (const auto& arg : m_pubkey_args) {
            providers.push_back(arg->Clone());
        }
        return std::make_unique<MiniscriptDescriptor>(std::move(providers), m_node->Clone());
    }

Review comment (bug):
In 4dc37d89de0b8b39b3e5ca802067663cfc3bb529 _descriptor: ToPrivateString() pass if  at least 1 priv key exists_:

The following makes it more clear why for private strings we only look at `has_priv_key` and don't care about whether res has a value:

```cpp
if (type == StringType::PRIVATE) {
    Assume(res.has_value());
    return has_priv_key;
} else {
    return res.has_value();
}
```

Or a bit shorter:

```cpp
return res.has_value() && (type != StringType::PRIVATE || has_priv_key);
```

Though in the edge case of `StringType::PRIVATE` with no `res`, this would return false which is a bit unexpected.

Code after:
protected:
    std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, std::span<const CScript> scripts,
                                     FlatSigningProvider& provider) const override
    {
        const auto script_ctx{m_node->GetMsCtx()};
        for (const auto& key : keys) {
            if (miniscript::IsTapscript(script_ctx)) {
                provider.pubkeys.emplace(Hash160(XOnlyPubKey{key}), key);
            } else {
                provider.pubkeys.emplace(key.GetID(), key);
            }
        }
        return Vector(m_node->ToScript(ScriptMaker(keys, script_ctx)));
    }

public:
    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
        : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}

    bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
                        const DescriptorCache* cache = nullptr) const override
    {
        bool has_priv_key{false};
        auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
        if (res) out = *res;
        if (type == StringType::PRIVATE) {
            Assume(res.has_value());
            return has_priv_key;
        } else {
            return res.has_value();
        }
    }

    bool IsSolvable() const override { return true; }
    bool IsSingleType() const final { return true; }

    std::optional<int64_t> ScriptSize() const override { return m_node->ScriptSize(); }

    std::optional<int64_t> MaxSatSize(bool) const override {
        // For Miniscript we always assume high-R ECDSA signatures.
        return m_node->GetWitnessSize();
    }

    std::optional<int64_t> MaxSatisfactionElems() const override {
        return m_node->GetStackSize();
    }

    std::unique_ptr<DescriptorImpl> Clone() const override
    {
        std::vector<std::unique_ptr<PubkeyProvider>> providers;
        providers.reserve(m_pubkey_args.size());