From 11767a65d071e73c461f0246145110b8ab161588 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 | 3 ++ src/test/descriptor_tests.cpp | 60 ++++++++++++++++++++++++++++ src/wallet/test/walletload_tests.cpp | 1 + src/wallet/wallet.cpp | 2 +- 5 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index e132a0ba224..e9a91d31bc7 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -782,6 +782,8 @@ public: } virtual std::unique_ptr Clone() const = 0; + + bool HasScripts() const override { return true; } }; /** A parsed addr(A) descriptor. */ @@ -1392,6 +1394,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 // //////////////////////////////////////////////////////////////////////////// @@ -2069,6 +2088,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)) { + 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 {}; + ++key_exp_index; + 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 473649a3144..851392da8b4 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -165,6 +165,9 @@ struct Descriptor { * @param[out] ext_pubs Any extended public keys */ 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; }; /** Parse a `descriptor` string. Included private keys are put in `out`. diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index c3cf5565e6c..aeccb27762b 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -1078,4 +1078,64 @@ BOOST_AUTO_TEST_CASE(descriptor_test) CheckInferDescriptor("4104032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220ac", "pk(04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220)", {}, {{"04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220", ""}}); } +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"); + + // 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 2e43eda582d..be93c5294cc 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -35,6 +35,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; } }; BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9ae0fb5329e..9cf3c65b97e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3984,7 +3984,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat // Note: we disable labels for ranged descriptors if (!desc.descriptor->IsRange()) { auto script_pub_keys = spk_man->GetScriptPubKeys(); - if (script_pub_keys.empty()) { + if (script_pub_keys.empty() && desc.descriptor->HasScripts()) { WalletLogPrintf("Could not generate scriptPubKeys (cache is empty)\n"); return nullptr; } From f02f6cefad6e02d163608c1c80a0923197529787 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 | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index bd50b8cdc02..9b17e8620c4 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -66,6 +66,23 @@ 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({"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, descriptors=True) @@ -773,5 +790,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(w_multipath.getrawchangeaddress(address_type="bech32"), w_multisplit.getrawchangeaddress(address_type="bech32")) assert_equal(sorted(w_multipath.listdescriptors()["descriptors"], key=lambda x: x["desc"]), sorted(w_multisplit.listdescriptors()["descriptors"], key=lambda x: x["desc"])) + self.test_import_unused_key() + if __name__ == '__main__': ImportDescriptorsTest(__file__).main() From 504c5f0df8afb173b1b4a5ecd22e9827e5249456 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 | 76 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 63bd8703989..419fd680ccc 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -1030,6 +1030,81 @@ static RPCHelpMan createwalletdescriptor() }; } +RPCHelpMan addhdkey() +{ + return RPCHelpMan{ + "addhdkey", + "\nAdd 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 RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return UniValue::VNULL; + + if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "addhdkey is not available for non-descriptor wallets"); + } + + 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; + seed_key.MakeNewKey(true); + hdkey.SetSeed(seed_key); + } else { + hdkey = DecodeExtKey(request.params[0].get_str()); + if (!hdkey.key.IsValid()) { + 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, "Unable to add HD key"); + } + + UniValue response(UniValue::VOBJ); + const DescriptorScriptPubKeyMan* desc_spkm = dynamic_cast(spkm); + 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 RPCHelpMan getaddressinfo(); RPCHelpMan getnewaddress(); @@ -1109,6 +1184,7 @@ std::span GetWalletRPCCommands() {"wallet", &abandontransaction}, {"wallet", &abortrescan}, {"wallet", &addmultisigaddress}, + {"wallet", &addhdkey}, {"wallet", &backupwallet}, {"wallet", &bumpfee}, {"wallet", &psbtbumpfee}, From 9e70f515a0fbd3828cf3027a6060b819ab6edd67 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 22 Dec 2023 18:13:27 -0500 Subject: [PATCH 4/6] test: Test for addhdkey --- test/functional/wallet_hd.py | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 78228308101..c41c7cc545a 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -7,6 +7,7 @@ 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_not_equal, @@ -31,6 +32,60 @@ class WalletHDTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_addhdkey(self): + if not self.options.descriptors: + return + + 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, "Could not parse HD key", wallet.addhdkey, imp_xpub) + add_res = wallet.addhdkey(imp_xprv) + expected_void_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_void_desc) + break + else: + assert False, "Added HD key was not found in wallet" + + wallet.listdescriptors() + for d in wallet.listdescriptors()["descriptors"]: + if d["desc"] == expected_void_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") + 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'] @@ -279,6 +334,7 @@ class WalletHDTest(BitcoinTestFramework): info = restore2_rpc.getaddressinfo(addr) assert_equal(info['ismine'], False) + self.test_addhdkey() if __name__ == '__main__': WalletHDTest(__file__).main() From 09e5a684feba9b32fe370648406cf49830d67708 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 4 Jan 2024 13:50:35 -0500 Subject: [PATCH 5/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 ac23b092d40..5449ab26455 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1573,6 +1573,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); // Check if the wallet already contains the descriptor diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 9b17e8620c4..c2534617ec0 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -83,6 +83,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, descriptors=True) @@ -791,6 +807,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(sorted(w_multipath.listdescriptors()["descriptors"], key=lambda x: x["desc"]), sorted(w_multisplit.listdescriptors()["descriptors"], key=lambda x: x["desc"])) self.test_import_unused_key() + self.test_import_unused_key_existing() if __name__ == '__main__': ImportDescriptorsTest(__file__).main() From cbf4dd3e41661b1e837e52d2d5b6f0b5601bb8cc Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 4 Jan 2024 14:02:19 -0500 Subject: [PATCH 6/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 5449ab26455..801baac42d8 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1575,6 +1575,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 c2534617ec0..ea9bd54dd07 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -99,6 +99,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, descriptors=True) @@ -808,6 +822,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()