From 0ff072caa14e4d32f6f4118f15f4f3718cef1d6a Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Jan 2024 19:00:31 -0500 Subject: [PATCH 1/5] wallet, rpc: Only allow keypool import from single key descriptors Legacy wallets should only import keys to the keypool if they came in a single key descriptor. Instead of relying on assumptions about the descriptor based on how many pubkeys show up after expanding the descriptor, explicitly mark descriptors as being single key type and use that for the check. --- src/script/descriptor.cpp | 13 +++++++++++++ src/script/descriptor.h | 5 +++++ src/wallet/rpc/backup.cpp | 9 +++++++-- src/wallet/test/walletload_tests.cpp | 1 + 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index e132a0ba224..099defe3ae8 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -800,6 +800,7 @@ public: return OutputTypeFromDestination(m_destination); } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; } std::optional ScriptSize() const override { return GetScriptForDestination(m_destination).size(); } @@ -827,6 +828,7 @@ public: return OutputTypeFromDestination(dest); } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; } std::optional ScriptSize() const override { return m_script.size(); } @@ -855,6 +857,7 @@ protected: public: PKDescriptor(std::unique_ptr prov, bool xonly = false) : DescriptorImpl(Vector(std::move(prov)), "pk"), m_xonly(xonly) {} bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return true; } std::optional ScriptSize() const override { return 1 + (m_xonly ? 32 : m_pubkey_args[0]->GetSize()) + 1; @@ -891,6 +894,7 @@ public: PKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "pkh") {} std::optional GetOutputType() const override { return OutputType::LEGACY; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return true; } std::optional ScriptSize() const override { return 1 + 1 + 1 + 20 + 1 + 1; } @@ -925,6 +929,7 @@ public: WPKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "wpkh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return true; } std::optional ScriptSize() const override { return 1 + 1 + 20; } @@ -967,6 +972,7 @@ protected: public: ComboDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "combo") {} bool IsSingleType() const final { return false; } + bool IsSingleKey() const final { return true; } std::unique_ptr Clone() const override { return std::make_unique(m_pubkey_args.at(0)->Clone()); @@ -991,6 +997,7 @@ protected: public: MultisigDescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), sorted ? "sortedmulti" : "multi"), m_threshold(threshold), m_sorted(sorted) {} bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } std::optional ScriptSize() const override { const auto n_keys = m_pubkey_args.size(); @@ -1042,6 +1049,7 @@ protected: public: MultiADescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), sorted ? "sortedmulti_a" : "multi_a"), m_threshold(threshold), m_sorted(sorted) {} bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } std::optional ScriptSize() const override { const auto n_keys = m_pubkey_args.size(); @@ -1088,6 +1096,7 @@ public: return OutputType::LEGACY; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return m_subdescriptor_args[0]->IsSingleKey(); } std::optional ScriptSize() const override { return 1 + 1 + 20 + 1; } @@ -1129,6 +1138,7 @@ public: WSHDescriptor(std::unique_ptr desc) : DescriptorImpl({}, std::move(desc), "wsh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return m_subdescriptor_args[0]->IsSingleKey(); } std::optional ScriptSize() const override { return 1 + 1 + 32; } @@ -1207,6 +1217,7 @@ public: } std::optional GetOutputType() const override { return OutputType::BECH32M; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } std::optional ScriptSize() const override { return 1 + 1 + 32; } @@ -1334,6 +1345,7 @@ public: bool IsSolvable() const override { return true; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } std::optional ScriptSize() const override { return m_node->ScriptSize(); } @@ -1373,6 +1385,7 @@ public: RawTRDescriptor(std::unique_ptr output_key) : DescriptorImpl(Vector(std::move(output_key)), "rawtr") {} std::optional GetOutputType() const override { return OutputType::BECH32M; } bool IsSingleType() const final { return true; } + bool IsSingleKey() const final { return false; } std::optional ScriptSize() const override { return 1 + 1 + 32; } diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 473649a3144..06e9f2679d3 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -111,6 +111,11 @@ struct Descriptor { /** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */ virtual bool IsSingleType() const = 0; + /** Whether this descriptor only produces single key scripts (i.e. pk(), pkh(), wpkh(), sh() and wsh() nested of those, and combo()) + * TODO: Remove this method once legacy wallets are removed as it is only necessary for importmulti. + */ + virtual bool IsSingleKey() const = 0; + /** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */ virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0; diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index ac23b092d40..e12453458c4 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1091,6 +1091,9 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::mapIsSingleKey(); + const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue(); for (size_t j = 0; j < parsed_descs.size(); ++j) { @@ -1107,8 +1110,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map scripts_temp; parsed_desc->Expand(i, keys, scripts_temp, out_keys); std::copy(scripts_temp.begin(), scripts_temp.end(), std::inserter(script_pub_keys, script_pub_keys.end())); - for (const auto& key_pair : out_keys.pubkeys) { - ordered_pubkeys.emplace_back(key_pair.first, desc_internal); + if (can_keypool) { + for (const auto& key_pair : out_keys.pubkeys) { + ordered_pubkeys.emplace_back(key_pair.first, desc_internal); + } } for (const auto& x : out_keys.scripts) { diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 2e43eda582d..2e8801d2a87 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -26,6 +26,7 @@ public: bool IsRange() const override { return false; } bool IsSolvable() const override { return false; } bool IsSingleType() const override { return true; } + bool IsSingleKey() const override { return true; } bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; } bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; } bool Expand(int pos, const SigningProvider& provider, std::vector& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; }; From 6268bde0af0aa9cfb3df08d6b9b67fdffa2a1a12 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 10 Apr 2025 14:22:20 -0700 Subject: [PATCH 2/5] descriptor: Remove unused parent_info from BIP32PUbKeyProvider::GetPubKey --- src/script/descriptor.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 099defe3ae8..a0caf4149cb 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -396,16 +396,12 @@ public: size_t GetSize() const override { return 33; } bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key_out, KeyOriginInfo& final_info_out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override { - // Info of parent of the to be derived pubkey - KeyOriginInfo parent_info; + KeyOriginInfo info; CKeyID keyid = m_root_extkey.pubkey.GetID(); - std::copy(keyid.begin(), keyid.begin() + sizeof(parent_info.fingerprint), parent_info.fingerprint); - parent_info.path = m_path; - - // Info of the derived key itself which is copied out upon successful completion - KeyOriginInfo final_info_out_tmp = parent_info; - if (m_derive == DeriveType::UNHARDENED) final_info_out_tmp.path.push_back((uint32_t)pos); - if (m_derive == DeriveType::HARDENED) final_info_out_tmp.path.push_back(((uint32_t)pos) | 0x80000000L); + std::copy(keyid.begin(), keyid.begin() + sizeof(info.fingerprint), info.fingerprint); + info.path = m_path; + if (m_derive == DeriveType::UNHARDENED) info.path.push_back((uint32_t)pos); + if (m_derive == DeriveType::HARDENED) info.path.push_back(((uint32_t)pos) | 0x80000000L); // Derive keys or fetch them from cache CExtPubKey final_extkey = m_root_extkey; @@ -441,7 +437,7 @@ public: } if (!der) return false; - final_info_out = final_info_out_tmp; + final_info_out = info; key_out = final_extkey.pubkey; if (write_cache) { From 25a3b9b0f52e61e0189d6e7e727a0ffd2b1e39fa Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Jan 2024 17:07:50 -0500 Subject: [PATCH 3/5] descriptors: Have GetPubKey fill origins directly Instead of having ExpandHelper fill in the origins in the FlatSigningProvider output, have GetPubKey do it by itself. This reduces the extra variables needed in order to track and set origins in ExpandHelper. Also changes GetPubKey to return a std::optional rather than using a bool and output parameters. --- src/script/descriptor.cpp | 72 ++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index a0caf4149cb..3abd85dd7ff 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -174,22 +174,20 @@ public: * Used by the Miniscript descriptors to check for duplicate keys in the script. */ bool operator<(PubkeyProvider& other) const { - CPubKey a, b; - SigningProvider dummy; - KeyOriginInfo dummy_info; + FlatSigningProvider dummy; - GetPubKey(0, dummy, a, dummy_info); - other.GetPubKey(0, dummy, b, dummy_info); + std::optional a = GetPubKey(0, dummy, dummy); + std::optional b = other.GetPubKey(0, dummy, dummy); return a < b; } - /** Derive a public key. + /** Derive a public key and put it into out. * read_cache is the cache to read keys from (if not nullptr) * write_cache is the cache to write keys to (if not nullptr) * Caches are not exclusive but this is not tested. Currently we use them exclusively */ - virtual bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const = 0; + virtual std::optional GetPubKey(int pos, const SigningProvider& arg, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const = 0; /** Whether this represent multiple public keys at different positions. */ virtual bool IsRange() const = 0; @@ -240,12 +238,15 @@ class OriginPubkeyProvider final : public PubkeyProvider public: OriginPubkeyProvider(uint32_t exp_index, KeyOriginInfo info, std::unique_ptr provider, bool apostrophe) : PubkeyProvider(exp_index), m_origin(std::move(info)), m_provider(std::move(provider)), m_apostrophe(apostrophe) {} - bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override + std::optional GetPubKey(int pos, const SigningProvider& arg, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override { - if (!m_provider->GetPubKey(pos, arg, key, info, read_cache, write_cache)) return false; - std::copy(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint), info.fingerprint); - info.path.insert(info.path.begin(), m_origin.path.begin(), m_origin.path.end()); - return true; + std::optional pub = m_provider->GetPubKey(pos, arg, out, read_cache, write_cache); + if (!pub) return std::nullopt; + auto& [pubkey, suborigin] = out.origins[pub->GetID()]; + Assert(pubkey == *pub); // m_provider must have a valid origin by this point. + std::copy(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint), suborigin.fingerprint); + suborigin.path.insert(suborigin.path.begin(), m_origin.path.begin(), m_origin.path.end()); + return pub; } bool IsRange() const override { return m_provider->IsRange(); } size_t GetSize() const override { return m_provider->GetSize(); } @@ -298,13 +299,13 @@ class ConstPubkeyProvider final : public PubkeyProvider public: ConstPubkeyProvider(uint32_t exp_index, const CPubKey& pubkey, bool xonly) : PubkeyProvider(exp_index), m_pubkey(pubkey), m_xonly(xonly) {} - bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override + std::optional GetPubKey(int pos, const SigningProvider&, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override { - key = m_pubkey; - info.path.clear(); + KeyOriginInfo info; CKeyID keyid = m_pubkey.GetID(); std::copy(keyid.begin(), keyid.begin() + sizeof(info.fingerprint), info.fingerprint); - return true; + out.origins.emplace(keyid, std::make_pair(m_pubkey, info)); + return m_pubkey; } bool IsRange() const override { return false; } size_t GetSize() const override { return m_pubkey.size(); } @@ -394,7 +395,7 @@ public: BIP32PubkeyProvider(uint32_t exp_index, const CExtPubKey& extkey, KeyPath path, DeriveType derive, bool apostrophe) : PubkeyProvider(exp_index), m_root_extkey(extkey), m_path(std::move(path)), m_derive(derive), m_apostrophe(apostrophe) {} bool IsRange() const override { return m_derive != DeriveType::NO; } size_t GetSize() const override { return 33; } - bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key_out, KeyOriginInfo& final_info_out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override + std::optional GetPubKey(int pos, const SigningProvider& arg, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override { KeyOriginInfo info; CKeyID keyid = m_root_extkey.pubkey.GetID(); @@ -410,16 +411,16 @@ public: bool der = true; if (read_cache) { if (!read_cache->GetCachedDerivedExtPubKey(m_expr_index, pos, final_extkey)) { - if (m_derive == DeriveType::HARDENED) return false; + if (m_derive == DeriveType::HARDENED) return std::nullopt; // Try to get the derivation parent - if (!read_cache->GetCachedParentExtPubKey(m_expr_index, parent_extkey)) return false; + if (!read_cache->GetCachedParentExtPubKey(m_expr_index, parent_extkey)) return std::nullopt; final_extkey = parent_extkey; if (m_derive == DeriveType::UNHARDENED) der = parent_extkey.Derive(final_extkey, pos); } } else if (IsHardened()) { CExtKey xprv; CExtKey lh_xprv; - if (!GetDerivedExtKey(arg, xprv, lh_xprv)) return false; + if (!GetDerivedExtKey(arg, xprv, lh_xprv)) return std::nullopt; parent_extkey = xprv.Neuter(); if (m_derive == DeriveType::UNHARDENED) der = xprv.Derive(xprv, pos); if (m_derive == DeriveType::HARDENED) der = xprv.Derive(xprv, pos | 0x80000000UL); @@ -429,16 +430,15 @@ public: } } else { for (auto entry : m_path) { - if (!parent_extkey.Derive(parent_extkey, entry)) return false; + if (!parent_extkey.Derive(parent_extkey, entry)) return std::nullopt; } final_extkey = parent_extkey; if (m_derive == DeriveType::UNHARDENED) der = parent_extkey.Derive(final_extkey, pos); assert(m_derive != DeriveType::HARDENED); } - if (!der) return false; + if (!der) return std::nullopt; - final_info_out = info; - key_out = final_extkey.pubkey; + out.origins.emplace(final_extkey.pubkey.GetID(), std::make_pair(final_extkey.pubkey, info)); if (write_cache) { // Only cache parent if there is any unhardened derivation @@ -448,12 +448,12 @@ public: if (last_hardened_extkey.pubkey.IsValid()) { write_cache->CacheLastHardenedExtPubKey(m_expr_index, last_hardened_extkey); } - } else if (final_info_out.path.size() > 0) { + } else if (info.path.size() > 0) { write_cache->CacheDerivedExtPubKey(m_expr_index, pos, final_extkey); } } - return true; + return final_extkey.pubkey; } std::string ToString(StringType type, bool normalized) const { @@ -696,16 +696,17 @@ public: // NOLINTNEXTLINE(misc-no-recursion) bool ExpandHelper(int pos, const SigningProvider& arg, const DescriptorCache* read_cache, std::vector& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache) const { - std::vector> entries; - entries.reserve(m_pubkey_args.size()); + FlatSigningProvider subprovider; + std::vector pubkeys; + pubkeys.reserve(m_pubkey_args.size()); - // Construct temporary data in `entries`, `subscripts`, and `subprovider` to avoid producing output in case of failure. + // Construct temporary data in `pubkeys`, `subscripts`, and `subprovider` to avoid producing output in case of failure. for (const auto& p : m_pubkey_args) { - entries.emplace_back(); - if (!p->GetPubKey(pos, arg, entries.back().first, entries.back().second, read_cache, write_cache)) return false; + std::optional pubkey = p->GetPubKey(pos, arg, subprovider, read_cache, write_cache); + if (!pubkey) return false; + pubkeys.push_back(pubkey.value()); } std::vector subscripts; - FlatSigningProvider subprovider; for (const auto& subarg : m_subdescriptor_args) { std::vector outscripts; if (!subarg->ExpandHelper(pos, arg, read_cache, outscripts, subprovider, write_cache)) return false; @@ -714,13 +715,6 @@ public: } out.Merge(std::move(subprovider)); - std::vector pubkeys; - pubkeys.reserve(entries.size()); - for (auto& entry : entries) { - pubkeys.push_back(entry.first); - out.origins.emplace(entry.first.GetID(), std::make_pair(CPubKey(entry.first), std::move(entry.second))); - } - output_scripts = MakeScripts(pubkeys, std::span{subscripts}, out); return true; } From 4b0303197e40a556bea2d9df76d1407981c361e6 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Jan 2024 17:23:08 -0500 Subject: [PATCH 4/5] descriptors: Move FlatSigningProvider pubkey filling to GetPubKey Instead of MakeScripts inconsistently filling the output FlatSigningProvider with the pubkeys involved, just do it in GetPubKey. --- src/script/descriptor.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 3abd85dd7ff..e6fc1feb2b0 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -242,6 +242,7 @@ public: { std::optional pub = m_provider->GetPubKey(pos, arg, out, read_cache, write_cache); if (!pub) return std::nullopt; + Assert(out.pubkeys.contains(pub->GetID())); auto& [pubkey, suborigin] = out.origins[pub->GetID()]; Assert(pubkey == *pub); // m_provider must have a valid origin by this point. std::copy(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint), suborigin.fingerprint); @@ -305,6 +306,7 @@ public: CKeyID keyid = m_pubkey.GetID(); std::copy(keyid.begin(), keyid.begin() + sizeof(info.fingerprint), info.fingerprint); out.origins.emplace(keyid, std::make_pair(m_pubkey, info)); + out.pubkeys.emplace(keyid, m_pubkey); return m_pubkey; } bool IsRange() const override { return false; } @@ -439,6 +441,7 @@ public: if (!der) return std::nullopt; out.origins.emplace(final_extkey.pubkey.GetID(), std::make_pair(final_extkey.pubkey, info)); + out.pubkeys.emplace(final_extkey.pubkey.GetID(), final_extkey.pubkey); if (write_cache) { // Only cache parent if there is any unhardened derivation @@ -874,10 +877,9 @@ public: class PKHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, std::span, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector& keys, std::span, FlatSigningProvider&) const override { CKeyID id = keys[0].GetID(); - out.pubkeys.emplace(id, keys[0]); return Vector(GetScriptForDestination(PKHash(id))); } public: @@ -909,10 +911,9 @@ public: class WPKHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, std::span, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector& keys, std::span, FlatSigningProvider&) const override { CKeyID id = keys[0].GetID(); - out.pubkeys.emplace(id, keys[0]); return Vector(GetScriptForDestination(WitnessV0KeyHash(id))); } public: @@ -948,7 +949,6 @@ protected: { std::vector ret; CKeyID id = keys[0].GetID(); - out.pubkeys.emplace(id, keys[0]); ret.emplace_back(GetScriptForRawPubKey(keys[0])); // P2PK ret.emplace_back(GetScriptForDestination(PKHash(id))); // P2PKH if (keys[0].IsCompressed()) { @@ -1175,7 +1175,6 @@ protected: builder.Finalize(xpk); WitnessV1Taproot output = builder.GetOutput(); out.tr_trees[output] = builder; - out.pubkeys.emplace(keys[0].GetID(), keys[0]); return Vector(GetScriptForDestination(output)); } bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const override From acee5c59e68f31acba111bc4d1892b08243ea5e0 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 Feb 2024 17:06:01 -0500 Subject: [PATCH 5/5] descriptors: Have GetPrivKey fill keys directly Instead of GetPrivKey returning a key and having the caller fill the FlatSigningProvider, have GetPrivKey take the FlatSigningProvider and fill it by itself. This will be necessary for descriptors such as musig() where there are private keys that need to be added to the FlatSigningProvider but do not directly appear in any resulting scripts. GetPrivKey is now changed to void as the caller no longer cares whether it succeeds or fails. --- src/script/descriptor.cpp | 44 ++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index e6fc1feb2b0..0b7cd4a4dc1 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -211,8 +211,8 @@ public: */ virtual bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const = 0; - /** Derive a private key, if private data is available in arg. */ - virtual bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const = 0; + /** Derive a private key, if private data is available in arg and put it into out. */ + virtual void GetPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const = 0; /** Return the non-extended public key for this PubkeyProvider, if it has one. */ virtual std::optional GetRootPubKey() const = 0; @@ -274,9 +274,9 @@ public: } return true; } - bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override + void GetPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const override { - return m_provider->GetPrivKey(pos, arg, key); + m_provider->GetPrivKey(pos, arg, out); } std::optional GetRootPubKey() const override { @@ -298,6 +298,14 @@ class ConstPubkeyProvider final : public PubkeyProvider CPubKey m_pubkey; bool m_xonly; + std::optional GetPrivKey(const SigningProvider& arg) const + { + CKey key; + if (!(m_xonly ? arg.GetKeyByXOnly(XOnlyPubKey(m_pubkey), key) : + arg.GetKey(m_pubkey.GetID(), key))) return std::nullopt; + return key; + } + public: ConstPubkeyProvider(uint32_t exp_index, const CPubKey& pubkey, bool xonly) : PubkeyProvider(exp_index), m_pubkey(pubkey), m_xonly(xonly) {} std::optional GetPubKey(int pos, const SigningProvider&, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override @@ -314,9 +322,9 @@ public: std::string ToString(StringType type) const override { return m_xonly ? HexStr(m_pubkey).substr(2) : HexStr(m_pubkey); } bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override { - CKey key; - if (!GetPrivKey(/*pos=*/0, arg, key)) return false; - ret = EncodeSecret(key); + std::optional key = GetPrivKey(arg); + if (!key) return false; + ret = EncodeSecret(*key); return true; } bool ToNormalizedString(const SigningProvider& arg, std::string& ret, const DescriptorCache* cache) const override @@ -324,10 +332,11 @@ public: ret = ToString(StringType::PUBLIC); return true; } - bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override + void GetPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const override { - return m_xonly ? arg.GetKeyByXOnly(XOnlyPubKey(m_pubkey), key) : - arg.GetKey(m_pubkey.GetID(), key); + std::optional key = GetPrivKey(arg); + if (!key) return; + out.keys.emplace(key->GetPubKey().GetID(), *key); } std::optional GetRootPubKey() const override { @@ -542,15 +551,14 @@ public: } return true; } - bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override + void GetPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const override { CExtKey extkey; CExtKey dummy; - if (!GetDerivedExtKey(arg, extkey, dummy)) return false; - if (m_derive == DeriveType::UNHARDENED && !extkey.Derive(extkey, pos)) return false; - if (m_derive == DeriveType::HARDENED && !extkey.Derive(extkey, pos | 0x80000000UL)) return false; - key = extkey.key; - return true; + if (!GetDerivedExtKey(arg, extkey, dummy)) return; + if (m_derive == DeriveType::UNHARDENED && !extkey.Derive(extkey, pos)) return; + if (m_derive == DeriveType::HARDENED && !extkey.Derive(extkey, pos | 0x80000000UL)) return; + out.keys.emplace(extkey.key.GetPubKey().GetID(), extkey.key); } std::optional GetRootPubKey() const override { @@ -736,9 +744,7 @@ public: void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const final { for (const auto& p : m_pubkey_args) { - CKey key; - if (!p->GetPrivKey(pos, provider, key)) continue; - out.keys.emplace(key.GetPubKey().GetID(), key); + p->GetPrivKey(pos, provider, out); } for (const auto& arg : m_subdescriptor_args) { arg->ExpandPrivate(pos, provider, out);