From 0ff072caa14e4d32f6f4118f15f4f3718cef1d6a 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 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; };