From 80c29bc6f14f184cdff28e6bbda2da3821832606 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 22 Dec 2023 15:25:13 -0500 Subject: [PATCH 1/6] descriptor: Add unused(KEY) descriptor unused() descriptors do not have scriptPubKeys. Instead, the wallet uses them to store keys without having any scripts to watch for. --- src/script/descriptor.cpp | 40 ++++++++++++++++++ src/script/descriptor.h | 5 ++- src/test/descriptor_tests.cpp | 61 ++++++++++++++++++++++++++++ src/wallet/test/walletload_tests.cpp | 1 + src/wallet/wallet.cpp | 4 +- 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 19315dd6a30..67770ada78e 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1038,6 +1038,8 @@ public: virtual std::unique_ptr Clone() const = 0; + bool HasScripts() const override { return true; } + // NOLINTNEXTLINE(misc-no-recursion) std::vector Warnings() const override { std::vector all = m_warnings; @@ -1738,6 +1740,23 @@ public: } }; +/** A parsed unused(KEY) descriptor */ +class UnusedDescriptor final : public DescriptorImpl +{ +protected: + std::vector MakeScripts(const std::vector& keys, std::span scripts, FlatSigningProvider& out) const override { return {}; } +public: + UnusedDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "unused") {} + bool IsSingleType() const final { return true; } + bool HasScripts() const override { return false; } + + std::unique_ptr Clone() const override + { + return std::make_unique(m_pubkey_args.at(0)->Clone()); + } +}; + + //////////////////////////////////////////////////////////////////////////// // Parser // //////////////////////////////////////////////////////////////////////////// @@ -2575,6 +2594,27 @@ std::vector> ParseScript(uint32_t& key_exp_index error = "Can only have rawtr at top level"; return {}; } + if (ctx == ParseScriptContext::TOP && Func("unused", expr)) { + // Check for only one expression, should not find commas, brackets, or parentheses + auto arg = Expr(expr); + if (expr.size()) { + error = strprintf("unused(): only one key expected"); + return {}; + } + auto keys = ParsePubkey(key_exp_index, arg, ctx, out, error); + if (keys.empty()) return {}; + for (auto& pubkey : keys) { + if (pubkey->IsRange()) { + error = "unused(): key cannot be ranged"; + return {}; + } + ret.emplace_back(std::make_unique(std::move(pubkey))); + } + return ret; + } else if (Func("unused", expr)) { + error = "Can only have unused at top level"; + return {}; + } if (ctx == ParseScriptContext::TOP && Func("raw", expr)) { std::string str(expr.begin(), expr.end()); if (!IsHex(str)) { diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 3ac748a1746..0f1e799e8f1 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -108,7 +108,7 @@ struct Descriptor { /** Convert the descriptor back to a string, undoing parsing. */ virtual std::string ToString(bool compat_format=false) const = 0; - /** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */ + /** Whether this descriptor will return at most one scriptPubKey or multiple (aka is or is not combo) */ virtual bool IsSingleType() const = 0; /** Whether the given provider has all private keys required by this descriptor. @@ -179,6 +179,9 @@ struct Descriptor { */ virtual void GetPubKeys(std::set& pubkeys, std::set& ext_pubs) const = 0; + /** Whether this descriptor produces any scripts with the Expand functions */ + virtual bool HasScripts() const = 0; + /** Semantic/safety warnings (includes subdescriptors). */ virtual std::vector Warnings() const = 0; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 0ede4dc9dfe..347efd77449 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -1334,4 +1334,65 @@ BOOST_AUTO_TEST_CASE(descriptor_older_warnings) } } +void CheckSingleUnparsable(const std::string& desc, const std::string& expected_error) +{ + FlatSigningProvider keys; + std::string error; + auto parsed = Parse(desc, keys, error); + BOOST_CHECK_MESSAGE(parsed.empty(), desc); + BOOST_CHECK_EQUAL(error, expected_error); +} + +void CheckUnused(const std::string& prv, const std::string& pub) +{ + FlatSigningProvider keys_priv, keys_pub; + std::string error; + + std::unique_ptr parse_priv; + std::unique_ptr parse_pub; + parse_priv = std::move(Parse(prv, keys_priv, error).at(0)); + parse_pub = std::move(Parse(pub, keys_pub, error).at(0)); + BOOST_CHECK_MESSAGE(parse_priv, error); + BOOST_CHECK_MESSAGE(parse_pub, error); + + BOOST_CHECK(parse_priv->GetOutputType() == std::nullopt); + BOOST_CHECK(parse_pub->GetOutputType() == std::nullopt); + + // Check private keys are extracted from the private version but not the public one. + BOOST_CHECK(keys_priv.keys.size()); + BOOST_CHECK(!keys_pub.keys.size()); + + // Check that both versions serialize back to the public version. + std::string pub1 = parse_priv->ToString(); + std::string pub2 = parse_pub->ToString(); + BOOST_CHECK_MESSAGE(EqualDescriptor(pub, pub1), "Private ser: " + pub1 + " Public desc: " + pub); + BOOST_CHECK_MESSAGE(EqualDescriptor(pub, pub2), "Public ser: " + pub2 + " Public desc: " + pub); + + // Check both only have one pubkey + std::set prv_pubkeys; + std::set prv_extpubs; + parse_pub->GetPubKeys(prv_pubkeys, prv_extpubs); + BOOST_CHECK_EQUAL(prv_pubkeys.size() + prv_extpubs.size(), 1); + std::set pub_pubkeys; + std::set pub_extpubs; + parse_pub->GetPubKeys(pub_pubkeys, pub_extpubs); + BOOST_CHECK_EQUAL(pub_pubkeys.size() + pub_extpubs.size(), 1); +} + +// unused() descriptors don't produce scripts, so these need to be tested separately +BOOST_AUTO_TEST_CASE(unused_descriptor_test) +{ + CheckUnparsable("unused(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "unused(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "unused(): only one key expected"); + CheckUnparsable("wsh(unused(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))", "wsh(unused(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Can only have unused at top level"); + CheckUnparsable("unused(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/*)", "unused(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/*)", "unused(): key cannot be ranged"); + CheckUnparsable("unused()", "unused()", "No key provided"); + + // x-only keys cannot be used in unused() + CheckSingleUnparsable("unused(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)", "Pubkey 'a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd' is invalid"); + + CheckUnused("unused(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc)", "unused(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL)"); + CheckUnused("unused(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)", "unused(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)"); + CheckUnused("unused(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/0h/0h/1)", "unused(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/0h/0h/1)"); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 46b0ab16c6d..8648af6b6c2 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -37,6 +37,7 @@ public: std::optional MaxSatisfactionWeight(bool) const override { return {}; } std::optional MaxSatisfactionElems() const override { return {}; } void GetPubKeys(std::set& pubkeys, std::set& ext_pubs) const override {} + bool HasScripts() const override { return true; } std::vector Warnings() const override { return {}; } uint32_t GetMaxKeyExpr() const override { return 0; } size_t GetKeyCount() const override { return 0; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 823e1b48211..f3f6e8f477a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3813,8 +3813,8 @@ util::Result> CWallet::AddWall } // Apply the label if necessary - // Note: we disable labels for ranged descriptors - if (!desc.descriptor->IsRange()) { + // Note: we disable labels for descriptors that are ranged or that don't produce output scripts (i.e. unused()) + if (!desc.descriptor->IsRange() && desc.descriptor->HasScripts()) { auto script_pub_keys = spk_man->GetScriptPubKeys(); if (script_pub_keys.empty()) { return util::Error{_("Could not generate scriptPubKeys (cache is empty)")}; From 82bc280de474f0980c1af9b2532c96bbd065cfbd Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 4 Jan 2024 12:00:16 -0500 Subject: [PATCH 2/6] test: Simple test for importing unused(KEY) --- test/functional/wallet_importdescriptors.py | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index bfb25c63ade..c22bf10310f 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -65,6 +65,29 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(result[0]['error']['code'], error_code) assert_equal(result[0]['error']['message'], error_message) + def test_import_unused_key(self): + self.log.info("Test import of unused(KEY)") + self.nodes[0].createwallet(wallet_name="import_unused", blank=True) + wallet = self.nodes[0].get_wallet_rpc("import_unused") + + assert_equal(len(wallet.gethdkeys()), 0) + + xprv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg" + xpub = "tpubD6NzVbkrYhZ4YNXVQbNhMK1WqguFsUXceaVJKbmno2aZ3B6QfbMeraaYvnBSGpV3vxLyTTK9DYT1yoEck4XUScMzXoQ2U2oSmE2JyMedq3H" + self.test_importdesc({"desc":descsum_create(f"unused({xpub})"), + "timestamp": "now"}, + success=False, + error_code=-4, + error_message='Cannot import descriptor without private keys to a wallet with private keys enabled', + wallet=wallet) + self.test_importdesc({"timestamp": "now", "desc": descsum_create(f"unused({xprv})")}, + success=True, + wallet=wallet) + hdkeys = wallet.gethdkeys() + assert_equal(len(hdkeys), 1) + assert_equal(hdkeys[0]["xpub"], xpub) + wallet.unloadwallet() + def run_test(self): self.log.info('Setting up wallets') self.nodes[0].createwallet(wallet_name='w0', disable_private_keys=False) @@ -822,5 +845,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): ) + self.test_import_unused_key() + if __name__ == '__main__': ImportDescriptorsTest(__file__).main() From f3f8bcbd1df3d66d640885a47fa12308d7b1485c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 22 Dec 2023 17:07:18 -0500 Subject: [PATCH 3/6] wallet: Add addhdkey RPC --- src/wallet/rpc/wallet.cpp | 77 ++++++++++++++++++++++++++++++++++++ test/functional/wallet_hd.py | 54 +++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index b6c9179d19d..8aa15c7ec5c 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -840,6 +840,82 @@ static RPCMethod createwalletdescriptor() }; } +RPCMethod addhdkey() +{ + return RPCMethod{ + "addhdkey", + "Add a BIP 32 HD key to the wallet that can be used with 'createwalletdescriptor'\n", + { + {"hdkey", RPCArg::Type::STR, RPCArg::DefaultHint{"Automatically generated new key"}, "The BIP 32 extended private key to add. If none is provided, a randomly generated one will be added."}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "xpub", "The xpub of the HD key that was added to the wallet"} + }, + }, + RPCExamples{ + HelpExampleCli("addhdkey", "xprv") + HelpExampleRpc("addhdkey", "xprv") + }, + [&](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue + { + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return UniValue::VNULL; + + if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "addhdkey is not available for wallets without private keys"); + } + + EnsureWalletIsUnlocked(*wallet); + + CExtKey hdkey; + if (request.params[0].isNull()) { + CKey seed_key = GenerateRandomKey(); + hdkey.SetSeed(seed_key); + } else { + hdkey = DecodeExtKey(request.params[0].get_str()); + if (!hdkey.key.IsValid()) { + // Check if the user gave us an xpub and give a more descriptive error if so + CExtPubKey xpub = DecodeExtPubKey(request.params[0].get_str()); + if (xpub.pubkey.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Extended public key (xpub) provided, but extended private key (xprv) is required"); + } else { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Could not parse HD key"); + } + } + } + + LOCK(wallet->cs_wallet); + std::string desc_str = "unused(" + EncodeExtKey(hdkey) + ")"; + FlatSigningProvider keys; + std::string error; + std::vector> descs = Parse(desc_str, keys, error, false); + CHECK_NONFATAL(!descs.empty()); + WalletDescriptor w_desc(std::move(descs.at(0)), GetTime(), 0, 0, 0); + if (wallet->GetDescriptorScriptPubKeyMan(w_desc) != nullptr) { + throw JSONRPCError(RPC_WALLET_ERROR, "HD key already exists"); + } + + auto spkm = wallet->AddWalletDescriptor(w_desc, keys, /*label=*/"", /*internal=*/false); + if (!spkm) { + throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(spkm).original); + } + + UniValue response(UniValue::VOBJ); + const DescriptorScriptPubKeyMan& desc_spkm = spkm->get(); + LOCK(desc_spkm.cs_desc_man); + std::set pubkeys; + std::set extpubs; + desc_spkm.GetWalletDescriptor().descriptor->GetPubKeys(pubkeys, extpubs); + CHECK_NONFATAL(pubkeys.size() == 0); + CHECK_NONFATAL(extpubs.size() == 1); + response.pushKV("xpub", EncodeExtPubKey(*extpubs.begin())); + + return response; + }, + }; +} + // addresses RPCMethod getaddressinfo(); RPCMethod getnewaddress(); @@ -907,6 +983,7 @@ std::span GetWalletRPCCommands() {"rawtransactions", &fundrawtransaction}, {"wallet", &abandontransaction}, {"wallet", &abortrescan}, + {"wallet", &addhdkey}, {"wallet", &backupwallet}, {"wallet", &bumpfee}, {"wallet", &psbtbumpfee}, diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index c130c731ce5..e5227e6f496 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -7,10 +7,12 @@ import shutil from test_framework.blocktools import COINBASE_MATURITY +from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, wallet_importprivkey, + assert_raises_rpc_error, ) @@ -25,6 +27,56 @@ class WalletHDTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_addhdkey(self): + self.log.info("Test addhdkey") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("hdkey") + wallet = self.nodes[0].get_wallet_rpc("hdkey") + + assert_equal(len(wallet.gethdkeys()), 1) + + wallet.addhdkey() + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 2) + for x in xpub_info: + if len(x["descriptors"]) == 1 and x["descriptors"][0]["desc"].startswith("unused("): + break + else: + assert False, "Did not find HD key with no descriptors" + + imp_xpub_info = def_wallet.gethdkeys(private=True)[0] + imp_xpub = imp_xpub_info["xpub"] + imp_xprv = imp_xpub_info["xprv"] + + assert_raises_rpc_error(-5, "Extended public key (xpub) provided, but extended private key (xprv) is required", wallet.addhdkey, imp_xpub) + add_res = wallet.addhdkey(imp_xprv) + expected_unused_desc = descsum_create(f"unused({imp_xpub})") + assert_equal(add_res["xpub"], imp_xpub) + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 3) + for x in xpub_info: + if x["xpub"] == imp_xpub: + assert_equal(len(x["descriptors"]), 1) + assert_equal(x["descriptors"][0]["desc"], expected_unused_desc) + break + else: + assert False, "Added HD key was not found in wallet" + + for d in wallet.listdescriptors()["descriptors"]: + if d["desc"] == expected_unused_desc: + assert_equal(d["active"], False) + break + else: + assert False, "Added HD key's descriptor was not found in wallet" + + assert_raises_rpc_error(-4, "HD key already exists", wallet.addhdkey, imp_xprv) + + def test_addhdkey_noprivs(self): + self.log.info("Test addhdkey is not available for wallets without privkeys") + self.nodes[0].createwallet("hdkey_noprivs", disable_private_keys=True) + wallet = self.nodes[0].get_wallet_rpc("hdkey_noprivs") + assert_raises_rpc_error(-4, "addhdkey is not available for wallets without private keys", wallet.addhdkey) + def run_test(self): # Make sure we use hd, keep masterkeyid hd_fingerprint = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress())['hdmasterfingerprint'] @@ -124,6 +176,8 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(keypath[0:14], "m/84h/1h/0h/1/") + self.test_addhdkey() + self.test_addhdkey_noprivs() if __name__ == '__main__': WalletHDTest(__file__).main() From 35bbee63746b567c867bb6d2c3db6f6670286826 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 4 Jan 2024 13:50:35 -0500 Subject: [PATCH 4/6] wallet, rpc: Disallow import of unused() if key already exists --- src/wallet/rpc/backup.cpp | 17 +++++++++++++++++ test/functional/wallet_importdescriptors.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 4b87ba23238..cb417a6e29f 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -265,6 +265,23 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c } } + // If this is an unused(KEY) descriptor, check that the wallet doesn't already have other descriptors with this key + if (!parsed_desc->HasScripts()) { + // Unused descriptors must contain a single key. + // Earlier checks will have enforced that this key is either a private key when private keys are enabled, + // or that this key is a public key when private keys are disabled. + // If we can retrieve the corresponding private key from the wallet, then this key is already in the wallet + // and we should not import it. + std::set pubkeys; + std::set extpubs; + parsed_desc->GetPubKeys(pubkeys, extpubs); + std::transform(extpubs.begin(), extpubs.end(), std::inserter(pubkeys, pubkeys.begin()), [](const CExtPubKey& xpub) { return xpub.pubkey; }); + CHECK_NONFATAL(pubkeys.size() == 1); + if (wallet.GetKey(pubkeys.begin()->GetID())) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import an unused() descriptor when its private key is already in the wallet"); + } + } + WalletDescriptor w_desc(std::move(parsed_desc), timestamp, range_start, range_end, next_index); // Add descriptor to the wallet diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index c22bf10310f..23bb0cf6ca7 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -88,6 +88,22 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(hdkeys[0]["xpub"], xpub) wallet.unloadwallet() + def test_import_unused_key_existing(self): + self.log.info("Test import of unused(KEY) with existing KEY") + self.nodes[0].createwallet(wallet_name="import_existing_unused") + wallet = self.nodes[0].get_wallet_rpc("import_existing_unused") + + hdkeys = wallet.gethdkeys(private=True) + assert_equal(len(hdkeys), 1) + xprv = hdkeys[0]["xprv"] + + self.test_importdesc({"timestamp": "now", "desc": descsum_create(f"unused({xprv})")}, + success=False, + error_code=-4, + error_message="Cannot import an unused() descriptor when its private key is already in the wallet", + wallet=wallet) + wallet.unloadwallet() + def run_test(self): self.log.info('Setting up wallets') self.nodes[0].createwallet(wallet_name='w0', disable_private_keys=False) @@ -846,6 +862,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): self.test_import_unused_key() + self.test_import_unused_key_existing() if __name__ == '__main__': ImportDescriptorsTest(__file__).main() From 89b9a01b4ea6355c1ecacb1d42d85252cfcb2a77 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 4 Jan 2024 14:02:19 -0500 Subject: [PATCH 5/6] wallet, rpc: Disallow importing unused() to wallets without privkeys --- src/wallet/rpc/backup.cpp | 3 +++ test/functional/wallet_importdescriptors.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index cb417a6e29f..c692fdc2ec9 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -267,6 +267,9 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c // If this is an unused(KEY) descriptor, check that the wallet doesn't already have other descriptors with this key if (!parsed_desc->HasScripts()) { + if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import unused() to wallet without private keys enabled"); + } // Unused descriptors must contain a single key. // Earlier checks will have enforced that this key is either a private key when private keys are enabled, // or that this key is a public key when private keys are disabled. diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 23bb0cf6ca7..3e8b05a28d6 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -104,6 +104,20 @@ class ImportDescriptorsTest(BitcoinTestFramework): wallet=wallet) wallet.unloadwallet() + def test_import_unused_noprivs(self): + self.log.info("Test import of unused(KEY) to wallet without privkeys") + self.nodes[0].createwallet(wallet_name="import_unused_noprivs", disable_private_keys=True) + wallet = self.nodes[0].get_wallet_rpc("import_unused_noprivs") + + xpub = "tpubD6NzVbkrYhZ4YNXVQbNhMK1WqguFsUXceaVJKbmno2aZ3B6QfbMeraaYvnBSGpV3vxLyTTK9DYT1yoEck4XUScMzXoQ2U2oSmE2JyMedq3H" + self.test_importdesc({"timestamp": "now", "desc": descsum_create(f"unused({xpub})")}, + success=False, + error_code=-4, + error_message="Cannot import unused() to wallet without private keys enabled", + wallet=wallet) + wallet.unloadwallet() + + def run_test(self): self.log.info('Setting up wallets') self.nodes[0].createwallet(wallet_name='w0', disable_private_keys=False) @@ -863,6 +877,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): self.test_import_unused_key() self.test_import_unused_key_existing() + self.test_import_unused_noprivs() if __name__ == '__main__': ImportDescriptorsTest(__file__).main() From a39cc16b434de7f2d1a7098e4526b7d3e82930be Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 24 Jul 2025 14:04:39 -0700 Subject: [PATCH 6/6] doc: Release note for addhdkey --- doc/release-notes-29136.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doc/release-notes-29136.md diff --git a/doc/release-notes-29136.md b/doc/release-notes-29136.md new file mode 100644 index 00000000000..af074a0b30a --- /dev/null +++ b/doc/release-notes-29136.md @@ -0,0 +1,6 @@ +Wallet +------ +- A new RPC `addhdkey` is added which allows a BIP 32 extended key to be added to the wallet without + needing to import it as part of a separate descriptor. This key will not be used to produce any + output scripts unless it is explicitly imported as part of a separate descriptor independent of + the `addhdkey` RPC.