Skip to content

Commit

Permalink
wallet, rpc: Only allow keypool import from single key descriptors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
achow101 committed Nov 7, 2024
1 parent c9e67e2 commit f06600f
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
13 changes: 13 additions & 0 deletions src/script/descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int64_t> ScriptSize() const override { return GetScriptForDestination(m_destination).size(); }
Expand Down Expand Up @@ -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<int64_t> ScriptSize() const override { return m_script.size(); }
Expand Down Expand Up @@ -862,6 +864,7 @@ class PKDescriptor final : public DescriptorImpl
public:
PKDescriptor(std::unique_ptr<PubkeyProvider> 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<int64_t> ScriptSize() const override {
return 1 + (m_xonly ? 32 : m_pubkey_args[0]->GetSize()) + 1;
Expand Down Expand Up @@ -898,6 +901,7 @@ class PKHDescriptor final : public DescriptorImpl
PKHDescriptor(std::unique_ptr<PubkeyProvider> prov) : DescriptorImpl(Vector(std::move(prov)), "pkh") {}
std::optional<OutputType> GetOutputType() const override { return OutputType::LEGACY; }
bool IsSingleType() const final { return true; }
bool IsSingleKey() const final { return true; }

std::optional<int64_t> ScriptSize() const override { return 1 + 1 + 1 + 20 + 1 + 1; }

Expand Down Expand Up @@ -932,6 +936,7 @@ class WPKHDescriptor final : public DescriptorImpl
WPKHDescriptor(std::unique_ptr<PubkeyProvider> prov) : DescriptorImpl(Vector(std::move(prov)), "wpkh") {}
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32; }
bool IsSingleType() const final { return true; }
bool IsSingleKey() const final { return true; }

std::optional<int64_t> ScriptSize() const override { return 1 + 1 + 20; }

Expand Down Expand Up @@ -978,6 +983,7 @@ class ComboDescriptor final : public DescriptorImpl
{
return std::make_unique<ComboDescriptor>(m_pubkey_args.at(0)->Clone());
}
bool IsSingleKey() const final { return true; }
};

/** A parsed multi(...) or sortedmulti(...) descriptor */
Expand All @@ -998,6 +1004,7 @@ class MultisigDescriptor final : public DescriptorImpl
public:
MultisigDescriptor(int threshold, std::vector<std::unique_ptr<PubkeyProvider>> 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<int64_t> ScriptSize() const override {
const auto n_keys = m_pubkey_args.size();
Expand Down Expand Up @@ -1049,6 +1056,7 @@ class MultiADescriptor final : public DescriptorImpl
public:
MultiADescriptor(int threshold, std::vector<std::unique_ptr<PubkeyProvider>> 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<int64_t> ScriptSize() const override {
const auto n_keys = m_pubkey_args.size();
Expand Down Expand Up @@ -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<int64_t> ScriptSize() const override { return 1 + 1 + 20 + 1; }

Expand Down Expand Up @@ -1136,6 +1145,7 @@ class WSHDescriptor final : public DescriptorImpl
WSHDescriptor(std::unique_ptr<DescriptorImpl> desc) : DescriptorImpl({}, std::move(desc), "wsh") {}
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32; }
bool IsSingleType() const final { return true; }
bool IsSingleKey() const final { return m_subdescriptor_args[0]->IsSingleKey(); }

std::optional<int64_t> ScriptSize() const override { return 1 + 1 + 32; }

Expand Down Expand Up @@ -1214,6 +1224,7 @@ class TRDescriptor final : public DescriptorImpl
}
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32M; }
bool IsSingleType() const final { return true; }
bool IsSingleKey() const final { return false; }

std::optional<int64_t> ScriptSize() const override { return 1 + 1 + 32; }

Expand Down Expand Up @@ -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<int64_t> ScriptSize() const override { return m_node->ScriptSize(); }

Expand Down Expand Up @@ -1380,6 +1392,7 @@ class RawTRDescriptor final : public DescriptorImpl
RawTRDescriptor(std::unique_ptr<PubkeyProvider> output_key) : DescriptorImpl(Vector(std::move(output_key)), "rawtr") {}
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32M; }
bool IsSingleType() const final { return true; }
bool IsSingleKey() const final { return false; }

std::optional<int64_t> ScriptSize() const override { return 1 + 1 + 32; }

Expand Down
3 changes: 3 additions & 0 deletions src/script/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 6 additions & 2 deletions src/wallet/rpc/backup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,8 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
std::tie(range_start, range_end) = ParseDescriptorRange(data["range"]);
}

bool can_keypool = parsed_descs.at(0)->IsSingleKey();

const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue();

for (size_t j = 0; j < parsed_descs.size(); ++j) {
Expand All @@ -1107,8 +1109,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
std::vector<CScript> 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) {
Expand Down
1 change: 1 addition & 0 deletions src/wallet/test/walletload_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; };
Expand Down

0 comments on commit f06600f

Please sign in to comment.