From f06600f0a18f0f34be10f97cc266bde777dd4436 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Jan 2024 19:00:31 -0500 Subject: [PATCH] wallet, rpc: Only allow keypool import from single key descriptors Instead of relying on implicit assumptions about whether pubkeys show up or now after expanding a descriptor, just explicitly allow only single key descriptors to import keys into a legacy wallet's keypool. --- src/script/descriptor.cpp | 13 +++++++++++++ src/script/descriptor.h | 3 +++ src/wallet/rpc/backup.cpp | 8 ++++++-- src/wallet/test/walletload_tests.cpp | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 5026470edcfb5..cf074953e2e5b 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -807,6 +807,7 @@ class AddressDescriptor final : public DescriptorImpl 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(); } @@ -834,6 +835,7 @@ class RawDescriptor final : public DescriptorImpl 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(); } @@ -862,6 +864,7 @@ class PKDescriptor final : public DescriptorImpl 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; @@ -898,6 +901,7 @@ class PKHDescriptor final : public DescriptorImpl 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; } @@ -932,6 +936,7 @@ class WPKHDescriptor final : public DescriptorImpl 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; } @@ -978,6 +983,7 @@ class ComboDescriptor final : public DescriptorImpl { return std::make_unique(m_pubkey_args.at(0)->Clone()); } + bool IsSingleKey() const final { return true; } }; /** A parsed multi(...) or sortedmulti(...) descriptor */ @@ -998,6 +1004,7 @@ class MultisigDescriptor final : public DescriptorImpl 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(); @@ -1049,6 +1056,7 @@ class MultiADescriptor final : public DescriptorImpl 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(); @@ -1095,6 +1103,7 @@ class SHDescriptor final : public DescriptorImpl 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; } @@ -1136,6 +1145,7 @@ class WSHDescriptor final : public DescriptorImpl 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; } @@ -1214,6 +1224,7 @@ class TRDescriptor final : public DescriptorImpl } 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; } @@ -1341,6 +1352,7 @@ class MiniscriptDescriptor final : public DescriptorImpl 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(); } @@ -1380,6 +1392,7 @@ class RawTRDescriptor final : public DescriptorImpl 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 473649a314491..d5d0215d1613d 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -111,6 +111,9 @@ 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() */ + 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 ea7c349d0b9f8..7106d84beb99b 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1091,6 +1091,8 @@ 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 +1109,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 2e43eda582d39..2e8801d2a8714 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -26,6 +26,7 @@ class DummyDescriptor final : public Descriptor { 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; };