Skip to content

Commit 0ff072c

Browse files
achow101achow101
committed
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.
1 parent 99a4ddf commit 0ff072c

File tree

4 files changed

+26
-2
lines changed

4 files changed

+26
-2
lines changed

‎src/script/descriptor.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,7 @@ class AddressDescriptor final : public DescriptorImpl
800800
return OutputTypeFromDestination(m_destination);
801801
}
802802
bool IsSingleType() const final { return true; }
803+
bool IsSingleKey() const final { return false; }
803804
bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; }
804805

805806
std::optional<int64_t> ScriptSize() const override { return GetScriptForDestination(m_destination).size(); }
@@ -827,6 +828,7 @@ class RawDescriptor final : public DescriptorImpl
827828
return OutputTypeFromDestination(dest);
828829
}
829830
bool IsSingleType() const final { return true; }
831+
bool IsSingleKey() const final { return false; }
830832
bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; }
831833

832834
std::optional<int64_t> ScriptSize() const override { return m_script.size(); }
@@ -855,6 +857,7 @@ class PKDescriptor final : public DescriptorImpl
855857
public:
856858
PKDescriptor(std::unique_ptr<PubkeyProvider> prov, bool xonly = false) : DescriptorImpl(Vector(std::move(prov)), "pk"), m_xonly(xonly) {}
857859
bool IsSingleType() const final { return true; }
860+
bool IsSingleKey() const final { return true; }
858861

859862
std::optional<int64_t> ScriptSize() const override {
860863
return 1 + (m_xonly ? 32 : m_pubkey_args[0]->GetSize()) + 1;
@@ -891,6 +894,7 @@ class PKHDescriptor final : public DescriptorImpl
891894
PKHDescriptor(std::unique_ptr<PubkeyProvider> prov) : DescriptorImpl(Vector(std::move(prov)), "pkh") {}
892895
std::optional<OutputType> GetOutputType() const override { return OutputType::LEGACY; }
893896
bool IsSingleType() const final { return true; }
897+
bool IsSingleKey() const final { return true; }
894898

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

@@ -925,6 +929,7 @@ class WPKHDescriptor final : public DescriptorImpl
925929
WPKHDescriptor(std::unique_ptr<PubkeyProvider> prov) : DescriptorImpl(Vector(std::move(prov)), "wpkh") {}
926930
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32; }
927931
bool IsSingleType() const final { return true; }
932+
bool IsSingleKey() const final { return true; }
928933

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

@@ -967,6 +972,7 @@ class ComboDescriptor final : public DescriptorImpl
967972
public:
968973
ComboDescriptor(std::unique_ptr<PubkeyProvider> prov) : DescriptorImpl(Vector(std::move(prov)), "combo") {}
969974
bool IsSingleType() const final { return false; }
975+
bool IsSingleKey() const final { return true; }
970976
std::unique_ptr<DescriptorImpl> Clone() const override
971977
{
972978
return std::make_unique<ComboDescriptor>(m_pubkey_args.at(0)->Clone());
@@ -991,6 +997,7 @@ class MultisigDescriptor final : public DescriptorImpl
991997
public:
992998
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) {}
993999
bool IsSingleType() const final { return true; }
1000+
bool IsSingleKey() const final { return false; }
9941001

9951002
std::optional<int64_t> ScriptSize() const override {
9961003
const auto n_keys = m_pubkey_args.size();
@@ -1042,6 +1049,7 @@ class MultiADescriptor final : public DescriptorImpl
10421049
public:
10431050
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) {}
10441051
bool IsSingleType() const final { return true; }
1052+
bool IsSingleKey() const final { return false; }
10451053

10461054
std::optional<int64_t> ScriptSize() const override {
10471055
const auto n_keys = m_pubkey_args.size();
@@ -1088,6 +1096,7 @@ class SHDescriptor final : public DescriptorImpl
10881096
return OutputType::LEGACY;
10891097
}
10901098
bool IsSingleType() const final { return true; }
1099+
bool IsSingleKey() const final { return m_subdescriptor_args[0]->IsSingleKey(); }
10911100

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

@@ -1129,6 +1138,7 @@ class WSHDescriptor final : public DescriptorImpl
11291138
WSHDescriptor(std::unique_ptr<DescriptorImpl> desc) : DescriptorImpl({}, std::move(desc), "wsh") {}
11301139
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32; }
11311140
bool IsSingleType() const final { return true; }
1141+
bool IsSingleKey() const final { return m_subdescriptor_args[0]->IsSingleKey(); }
11321142

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

@@ -1207,6 +1217,7 @@ class TRDescriptor final : public DescriptorImpl
12071217
}
12081218
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32M; }
12091219
bool IsSingleType() const final { return true; }
1220+
bool IsSingleKey() const final { return false; }
12101221

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

@@ -1334,6 +1345,7 @@ class MiniscriptDescriptor final : public DescriptorImpl
13341345

13351346
bool IsSolvable() const override { return true; }
13361347
bool IsSingleType() const final { return true; }
1348+
bool IsSingleKey() const final { return false; }
13371349

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

@@ -1373,6 +1385,7 @@ class RawTRDescriptor final : public DescriptorImpl
13731385
RawTRDescriptor(std::unique_ptr<PubkeyProvider> output_key) : DescriptorImpl(Vector(std::move(output_key)), "rawtr") {}
13741386
std::optional<OutputType> GetOutputType() const override { return OutputType::BECH32M; }
13751387
bool IsSingleType() const final { return true; }
1388+
bool IsSingleKey() const final { return false; }
13761389

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

‎src/script/descriptor.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ struct Descriptor {
111111
/** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */
112112
virtual bool IsSingleType() const = 0;
113113

114+
/** Whether this descriptor only produces single key scripts (i.e. pk(), pkh(), wpkh(), sh() and wsh() nested of those, and combo())
115+
* TODO: Remove this method once legacy wallets are removed as it is only necessary for importmulti.
116+
*/
117+
virtual bool IsSingleKey() const = 0;
118+
114119
/** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */
115120
virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0;
116121

‎src/wallet/rpc/backup.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,9 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
10911091
std::tie(range_start, range_end) = ParseDescriptorRange(data["range"]);
10921092
}
10931093

1094+
// Only single key descriptors are allowed to be imported to a legacy wallet's keypool
1095+
bool can_keypool = parsed_descs.at(0)->IsSingleKey();
1096+
10941097
const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue();
10951098

10961099
for (size_t j = 0; j < parsed_descs.size(); ++j) {
@@ -1107,8 +1110,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
11071110
std::vector<CScript> scripts_temp;
11081111
parsed_desc->Expand(i, keys, scripts_temp, out_keys);
11091112
std::copy(scripts_temp.begin(), scripts_temp.end(), std::inserter(script_pub_keys, script_pub_keys.end()));
1110-
for (const auto& key_pair : out_keys.pubkeys) {
1111-
ordered_pubkeys.emplace_back(key_pair.first, desc_internal);
1113+
if (can_keypool) {
1114+
for (const auto& key_pair : out_keys.pubkeys) {
1115+
ordered_pubkeys.emplace_back(key_pair.first, desc_internal);
1116+
}
11121117
}
11131118

11141119
for (const auto& x : out_keys.scripts) {

‎src/wallet/test/walletload_tests.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class DummyDescriptor final : public Descriptor {
2626
bool IsRange() const override { return false; }
2727
bool IsSolvable() const override { return false; }
2828
bool IsSingleType() const override { return true; }
29+
bool IsSingleKey() const override { return true; }
2930
bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; }
3031
bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; }
3132
bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; };

0 commit comments

Comments
 (0)