Repository: bitcoin/bitcoin (language: cpp)

Issue:
importmulti does not allow watchonly imports while wallet is locked
This differs from behaviour of `importaddress` where it works.
```
$ bitcoin-cli importaddress "<address>" "<label>" false
```
vs
```
$ bitcoin-cli importmulti '[{ "scriptPubKey": { "address": "<address>" }, "timestamp":0, "label": "<label>", "watchonly": true }]' '{ "rescan": false }'
error code: -13
error message:
Error: Please enter the wallet passphrase with walletpassphrase first.
```


Discussion / hints:
Should *maybe* be fixed.
The questions for me are:

a) Do users think the imported addresses/pubkeys-sscripts are encrypted when encrypting the wallet (they are not)?

b) should encouraging mixed wallets (watch-onlys & private keys) be avoided?
Yes, I wasn't sure is it a bug or feature of importmulti. In any case, importmulti and importaddress / importpubkey should behave the same way.

> a) Do users think the imported addresses/pubkeys-sscripts are encrypted when encrypting the wallet (they are not)?

Why should they think so? You can view addresses, coins, tx history, generate new addresses without unlocking wallet.

> b) should encouraging mixed wallets (watch-onlys & private keys) be avoided?

It's kinda simple now with multiwallet support. OTOH, I could imagine some people might want to see on the same screen their hot wallet balance together with watch-only cold storage, for example. Mixed wallet allows that. Kinda similar how some banks in Europe now allow to view account balance from your accounts in other banks in the Internet banking website. And I don't see any security risks or other problems this could cause.
Maybe `EnsureWalletIsUnlocked` should be deferred to the point it's needed?

Fix patch:
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 35df151e84f7a..09c74ea2daafe 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1363,7 +1363,18 @@ RPCHelpMan importmulti()
     UniValue response(UniValue::VARR);
     {
         LOCK(pwallet->cs_wallet);
-        EnsureWalletIsUnlocked(*pwallet);
+
+        // Check all requests are watchonly
+        bool is_watchonly{true};
+        for (size_t i = 0; i < requests.size(); ++i) {
+            const UniValue& request = requests[i];
+            if (!request.exists("watchonly") || !request["watchonly"].get_bool()) {
+                is_watchonly = false;
+                break;
+            }
+        }
+        // Wallet does not need to be unlocked if all requests are watchonly
+        if (!is_watchonly) EnsureWalletIsUnlocked(wallet);
 
         // Verify all timestamps are present before importing any keys.
         CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(nLowestTimestamp).mtpTime(now)));


Test patch:
diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py
index 395385149100b..62a1a3341d110 100755
--- a/test/functional/wallet_importmulti.py
+++ b/test/functional/wallet_importmulti.py
@@ -874,6 +874,25 @@ def run_test(self):
             addr = wrpc.getnewaddress('', 'bech32')
             assert_equal(addr, addresses[i])
 
+        # Create wallet with passphrase
+        self.log.info('Test watchonly imports on a wallet with a passphrase, without unlocking')
+        self.nodes[1].createwallet(wallet_name='w1', blank=True, passphrase='pass')
+        wrpc = self.nodes[1].get_wallet_rpc('w1')
+        assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first.",
+                                wrpc.importmulti, [{
+                                    'desc': descsum_create('wpkh(' + pub1 + ')'),
+                                    "timestamp": "now",
+                                }])
+
+        result = wrpc.importmulti(
+            [{
+                'desc': descsum_create('wpkh(' + pub1 + ')'),
+                "timestamp": "now",
+                "watchonly": True,
+            }]
+        )
+        assert result[0]['success']
+
 
 if __name__ == '__main__':
     ImportMultiTest().main()