diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 0b7cd4a4dc1..b155284964a 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -781,6 +781,8 @@ public: } virtual std::unique_ptr Clone() const = 0; + + bool HasScripts() const override { return true; } }; /** A parsed addr(A) descriptor. */ @@ -1400,6 +1402,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 // //////////////////////////////////////////////////////////////////////////// @@ -2077,6 +2096,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 06e9f2679d3..7cd58b6cd68 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -170,6 +170,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/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 39d85090074..13b3235caae 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1578,6 +1578,26 @@ 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. + // 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/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 4df23873fd8..12ea295466d 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -1018,6 +1018,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(); @@ -1097,6 +1172,7 @@ std::span GetWalletRPCCommands() {"wallet", &abandontransaction}, {"wallet", &abortrescan}, {"wallet", &addmultisigaddress}, + {"wallet", &addhdkey}, {"wallet", &backupwallet}, {"wallet", &bumpfee}, {"wallet", &psbtbumpfee}, diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index ebdf8651e8e..54b08f44095 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -36,6 +36,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 d010d90be38..cc775078eb1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3989,7 +3989,7 @@ util::Result CWallet::AddWalletDescriptor(WalletDescriptor& de // 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; } diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index d7fbc627bab..6b61d0a450d 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_equal, @@ -26,6 +27,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'] @@ -126,6 +181,7 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(keypath[0:14], "m/84h/1h/0h/1/") + self.test_addhdkey() if __name__ == '__main__': WalletHDTest(__file__).main() diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 5b76c196008..2e9496d5c5c 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -63,6 +63,53 @@ 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 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 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) @@ -770,5 +817,9 @@ 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() + self.test_import_unused_key_existing() + self.test_import_unused_noprivs() + if __name__ == '__main__': ImportDescriptorsTest(__file__).main()