rpc: make importaddress compatible with descriptors wallet

so it's simpler to watch for certain address/hex.

Github-Pull: #27034
Rebased-From: be3ae51ece

src/script/descriptor.cpp

@@ -143,8 +143,6 @@ std::string DescriptorChecksum(const Span<const char>& span)
     return ret;
 }
 
-std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); }
-
 ////////////////////////////////////////////////////////////////////////////
 // Internal representation                                                //
 ////////////////////////////////////////////////////////////////////////////
@@ -1746,6 +1744,8 @@ std::string GetDescriptorChecksum(const std::string& descriptor)
     return ret;
 }
 
+std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); }
+
 std::unique_ptr<Descriptor> InferDescriptor(const CScript& script, const SigningProvider& provider)
 {
     return InferScript(script, ParseScriptContext::TOP, provider);

src/script/descriptor.h

@@ -166,6 +166,11 @@ std::unique_ptr<Descriptor> 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

src/wallet/rpc/backup.cpp

@@ -212,6 +212,8 @@ RPCHelpMan importprivkey()
     };
 }
 
+static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+
 RPCHelpMan importaddress()
 {
     return RPCHelpMan{"importaddress",
@@ -225,7 +227,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\" with \"addr(X)\" 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"},
@@ -246,7 +248,18 @@ RPCHelpMan importaddress()
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
 
-    EnsureLegacyScriptPubKeyMan(*pwallet, true);
+    // Use legacy spkm only if the wallet does not support descriptors.
+    bool use_legacy = !pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS);
+    if (!use_legacy) {
+        // 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");
+        }
+    } else {
+        // In case the wallet is blank
+        EnsureLegacyScriptPubKeyMan(*pwallet, /*also_create=*/true);
+    }
 
     const std::string strLabel{LabelFromValue(request.params[1])};
 
@@ -272,33 +285,55 @@ 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) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
-            }
 
             pwallet->MarkDirty();
 
-            pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
+            if (use_legacy) {
-        } else if (IsHex(request.params[0].get_str())) {
+                if (OutputTypeFromDestination(dest) == OutputType::BECH32M) {
-            std::vector<unsigned char> data(ParseHex(request.params[0].get_str()));
+                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
-            CScript redeem_script(data.begin(), data.end());
+                }
-
+                pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
-            std::set<CScript> scripts = {redeem_script};
+            } else {
-            pwallet->ImportScripts(scripts, /*timestamp=*/0);
+                import_descriptor("addr(" + address + ")", strLabel);
-
-            if (fP2SH) {
-                scripts.insert(GetScriptForDestination(ScriptHash(redeem_script)));
             }
+        } else if (IsHex(request.params[0].get_str())) {
+            const std::string& hex = request.params[0].get_str();
 
-            pwallet->ImportScriptPubKeys(strLabel, scripts, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
+            if (use_legacy) {
+                std::vector<unsigned char> data(ParseHex(hex));
+                CScript redeem_script(data.begin(), data.end());
+                std::set<CScript> scripts = {redeem_script};
+                pwallet->ImportScripts(scripts, /*timestamp=*/0);
+                if (fP2SH) {
+                    scripts.insert(GetScriptForDestination(ScriptHash(redeem_script)));
+                }
+                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");
         }

test/functional/test_framework/test_node.py

@@ -819,30 +819,3 @@ class RPCOverloadWrapper():
         import_res = self.importdescriptors(req)
         if not import_res[0]['success']:
             raise JSONRPCException(import_res[0]['error'])
-
-    def 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)
-        is_hex = False
-        try:
-            int(address ,16)
-            is_hex = True
-            desc = descsum_create('raw(' + address + ')')
-        except Exception:
-            desc = descsum_create('addr(' + address + ')')
-        reqs = [{
-            'desc': desc,
-            'timestamp': 0 if rescan else 'now',
-            'label': label if label else ''
-        }]
-        if is_hex and p2sh:
-            reqs.append({
-                'desc': descsum_create('p2sh(raw(' + address + '))'),
-                'timestamp': 0 if rescan else 'now',
-                'label': label if label else ''
-            })
-        import_res = self.importdescriptors(reqs)
-        for res in import_res:
-            if not res['success']:
-                raise JSONRPCException(res['error'])

test/functional/wallet_basic.py

@@ -55,6 +55,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
@@ -540,6 +558,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]

test/functional/wallet_descriptor.py

@@ -115,7 +115,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())
@@ -123,6 +122,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())

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),
                 )