diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ae9dba6a50..b41f1fb9c9 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -148,8 +148,6 @@ std::string DescriptorChecksum(const Span& span) return ret; } -std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); } - //////////////////////////////////////////////////////////////////////////// // Internal representation // //////////////////////////////////////////////////////////////////////////// @@ -2086,6 +2084,8 @@ std::string GetDescriptorChecksum(const std::string& descriptor) return ret; } +std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); } + std::unique_ptr InferDescriptor(const CScript& script, const SigningProvider& provider) { return InferScript(script, ParseScriptContext::TOP, provider); diff --git a/src/script/descriptor.h b/src/script/descriptor.h index e78a775330..6674de91f9 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -185,6 +185,11 @@ std::unique_ptr Parse(const std::string& descriptor, FlatSigningProv */ std::string GetDescriptorChecksum(const std::string& descriptor); +/** + * Simple wrapper to add the checksum at the end of the descriptor + */ +std::string AddChecksum(const std::string& str); + /** Find a descriptor for the specified `script`, using information from `provider` where possible. * * A non-ranged descriptor which only generates the specified script will be returned in all diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 0d0e86ed24..4d795af747 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -216,6 +216,8 @@ RPCHelpMan importprivkey() }; } +UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); + RPCHelpMan importaddress() { return RPCHelpMan{"importaddress", @@ -229,7 +231,7 @@ RPCHelpMan importaddress() "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" "as change, and not show up in many RPCs.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n" - "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" for descriptor wallets.\n", + "Note: For descriptor wallets, this command will create new descriptor/s, and only works if the wallet has private keys disabled.\n", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, @@ -250,7 +252,18 @@ RPCHelpMan importaddress() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; + // Use legacy spkm only if the wallet does not support descriptors. + bool use_legacy = !pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS); + if (use_legacy) { + // In case the wallet is blank EnsureLegacyScriptPubKeyMan(*pwallet, true); + } else { + // We don't allow mixing watch-only descriptors with spendable ones. + if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import address in wallet with private keys enabled. " + "Create wallet with no private keys to watch specific addresses/scripts"); + } + } const std::string strLabel{LabelFromValue(request.params[1])}; @@ -276,23 +289,41 @@ RPCHelpMan importaddress() if (!request.params[3].isNull()) fP2SH = request.params[3].get_bool(); + // Import descriptor helper function + const auto& import_descriptor = [pwallet](const std::string& desc, const std::string label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + UniValue data(UniValue::VType::VOBJ); + data.pushKV("desc", AddChecksum(desc)); + if (!label.empty()) data.pushKV("label", label); + const UniValue& ret = ProcessDescriptorImport(*pwallet, data, /*timestamp=*/1); + if (ret.exists("error")) throw ret["error"]; + }; + { LOCK(pwallet->cs_wallet); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); + const std::string& address = request.params[0].get_str(); + CTxDestination dest = DecodeDestination(address); if (IsValidDestination(dest)) { if (fP2SH) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); } if (OutputTypeFromDestination(dest) == OutputType::BECH32M) { + if (use_legacy) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets"); } pwallet->MarkDirty(); + if (use_legacy) { pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1); + } else { + import_descriptor("addr(" + address + ")", strLabel); + } } else if (IsHex(request.params[0].get_str())) { - std::vector data(ParseHex(request.params[0].get_str())); + const std::string& hex = request.params[0].get_str(); + + if (use_legacy) { + std::vector data(ParseHex(hex)); CScript redeem_script(data.begin(), data.end()); std::set scripts = {redeem_script}; @@ -303,6 +334,14 @@ RPCHelpMan importaddress() } pwallet->ImportScriptPubKeys(strLabel, scripts, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1); + } else { + // P2SH Not allowed. Can't detect inner P2SH function from a raw hex. + if (fP2SH) throw JSONRPCError(RPC_WALLET_ERROR, "P2SH import feature disabled for descriptors' wallet. " + "Use 'importdescriptors' to specify inner P2SH function"); + + // Import descriptors + import_descriptor("raw(" + hex + ")", strLabel); + } } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 60ca9269a5..88d6632c72 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -979,7 +979,7 @@ class RPCOverloadWrapper(): if not import_res[0]['success']: raise JSONRPCException(import_res[0]['error']) - def importaddress(self, address, label=None, rescan=None, p2sh=None): + def _deleted_importaddress(self, address, label=None, rescan=None, p2sh=None): wallet_info = self.getwalletinfo() if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']): return self.__getattr__('importaddress')(address, label, rescan, p2sh) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index c968e4333a..831fcaa8b0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -61,6 +61,24 @@ class WalletTest(BitcoinTestFramework): def get_vsize(self, txn): return self.nodes[0].decoderawtransaction(txn)['vsize'] + def test_legacy_importaddress(self): + if self.options.descriptors: + return + + addr = self.nodes[1].getnewaddress() + self.nodes[1].sendtoaddress(addr, 10) + self.sync_mempools(self.nodes[0:2]) + + self.log.info("Test 'importaddress' on a blank, private keys disabled, wallet with no descriptors support") + self.nodes[0].createwallet(wallet_name="watch-only-legacy", disable_private_keys=False, descriptors=False, blank=True) + wallet_watch_only = self.nodes[0].get_wallet_rpc("watch-only-legacy") + wallet_watch_only.importaddress(addr) + assert_equal(wallet_watch_only.getaddressinfo(addr)['ismine'], False) + assert_equal(wallet_watch_only.getaddressinfo(addr)['iswatchonly'], True) + assert_equal(wallet_watch_only.getaddressinfo(addr)['solvable'], False) + assert_equal(wallet_watch_only.getbalances()["watchonly"]['untrusted_pending'], 10) + self.nodes[0].unloadwallet("watch-only-legacy") + def run_test(self): # Check that there's no UTXO on none of the nodes @@ -558,6 +576,9 @@ class WalletTest(BitcoinTestFramework): {"address": address_to_import}, {"spendable": True}) + # Test importaddress on a blank, private keys disabled, legacy wallet with no descriptors support + self.test_legacy_importaddress() + # Mine a block from node0 to an address from node1 coinbase_addr = self.nodes[1].getnewaddress() block_hash = self.generatetoaddress(self.nodes[0], 1, coinbase_addr, sync_fun=lambda: self.sync_all(self.nodes[0:3]))[0] diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index 5e0ee97892..54a95078ed 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -153,7 +153,6 @@ class WalletDescriptorTest(BitcoinTestFramework): self.log.info("Test disabled RPCs") assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importprivkey, "cVpF924EspNh8KjYsfhgY96mmxvT6DgdWiTYMtMjuM74hJaU5psW") assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress())["pubkey"]) - assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importaddress, recv_wrpc.getnewaddress()) assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importmulti, []) assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.addmultisigaddress, 1, [recv_wrpc.getnewaddress()]) assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.dumpprivkey, recv_wrpc.getnewaddress()) @@ -161,6 +160,16 @@ class WalletDescriptorTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importwallet, 'wallet.dump') assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.sethdseed) + # Test importaddress + self.log.info("Test watch-only descriptor wallet") + self.nodes[0].createwallet(wallet_name="watch-only-desc", disable_private_keys=True, descriptors=True, blank=True) + wallet_watch_only = self.nodes[0].get_wallet_rpc("watch-only-desc") + wallet_watch_only.importaddress(addr) + assert_equal(wallet_watch_only.getaddressinfo(addr)['ismine'], True) + assert_equal(wallet_watch_only.getaddressinfo(addr)['solvable'], False) + assert_equal(wallet_watch_only.getbalances()["mine"]['untrusted_pending'], 10) + self.nodes[0].unloadwallet("watch-only-desc") + self.log.info("Test encryption") # Get the master fingerprint before encrypt info1 = send_wrpc.getaddressinfo(send_wrpc.getnewaddress()) diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 307e10ae34..71c6286701 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -212,7 +212,7 @@ class WalletLabelsTest(BitcoinTestFramework): ad = BECH32_INVALID[l] assert_raises_rpc_error( -5, - "Address is not valid" if self.options.descriptors else "Invalid Bitcoin address or script", + "Invalid Bitcoin address or script", lambda: wallet_watch_only.importaddress(label=l, rescan=False, address=ad), ) diff --git a/test/functional/wallet_reindex.py b/test/functional/wallet_reindex.py index 6778f76efc..21ed2ebc9f 100755 --- a/test/functional/wallet_reindex.py +++ b/test/functional/wallet_reindex.py @@ -8,6 +8,7 @@ import time 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, @@ -52,7 +53,15 @@ class WalletReindexTest(BitcoinTestFramework): # For a descriptors wallet: Import address with timestamp=now. # For legacy wallet: There is no way of importing a script/address with a custom time. The wallet always imports it with birthtime=1. # In both cases, disable rescan to not detect the transaction. - wallet_watch_only.importaddress(wallet_addr, rescan=False) + if self.options.descriptors: + import_res = wallet_watch_only.importdescriptors([{ + 'desc': descsum_create('addr(' + wallet_addr + ')'), + 'timestamp': 'now', + }]) + assert len(import_res) == 1 + assert import_res[0]['success'] + else: + wallet_watch_only.importaddress(wallet_addr, rescan=False) assert_equal(len(wallet_watch_only.listtransactions()), 0) # Depending on the wallet type, the birth time changes.