Merge #14565: Overhaul importmulti logic

eacff95de Add release notes (Pieter Wuille)
bdacbda25 Overhaul importmulti logic (Pieter Wuille)

Pull request description:

  This is an alternative to #14558 (it will warn when fields are being ignored). In addition:
  * It makes sure no changes to the wallet are made when an error in the input exists.
  * It validates all arguments, and will fail if anything fails to parse.
  * Adds a whole bunch of sanity checks

Tree-SHA512: fdee0b6aca8c643663f0bc295a7c1d69c1960951493b06abf32c58977f3e565f75918dbd0402dde36e508dc746c9310a968a0ebbacccc385a57ac2a68b49c1d0

3 files changed, 281 insertions, 213 deletions

diff --git a/doc/release-notes-14565.md b/doc/release-notes-14565.md
new file mode 100644
index 0000000000..38d76fee46
--- /dev/null
+++ b/doc/release-notes-14565.md
@@ -0,0 +1,5 @@
+Low-level RPC changes
+---------------------
+
+The `importmulti` RPC will now contain a new per-request `warnings` field with strings
+that explain when fields are being ignored or inconsistant, if any.
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 647af3eb86..ed1e2d3940 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -836,9 +836,98 @@ UniValue dumpwallet(const JSONRPCRequest& request)
     return reply;
 }
 
+struct ImportData
+{
+    // Input data
+    std::unique_ptr<CScript> redeemscript; //!< Provided redeemScript; will be moved to `import_scripts` if relevant.
+    std::unique_ptr<CScript> witnessscript; //!< Provided witnessScript; will be moved to `import_scripts` if relevant.
+
+    // Output data
+    std::set<CScript> import_scripts;
+    std::map<CKeyID, bool> used_keys; //!< Import these private keys if available (the value indicates whether if the key is required for solvability)
+};
+
+enum class ScriptContext
+{
+    TOP, //! Top-level scriptPubKey
+    P2SH, //! P2SH redeemScript
+    WITNESS_V0, //! P2WSH witnessScript
+};
+
+// Analyse the provided scriptPubKey, determining which keys and which redeem scripts from the ImportData struct are needed to spend it, and mark them as used.
+// Returns an error string, or the empty string for success.
+static std::string RecurseImportData(const CScript& script, ImportData& import_data, const ScriptContext script_ctx)
+{
+    // Use Solver to obtain script type and parsed pubkeys or hashes:
+    std::vector<std::vector<unsigned char>> solverdata;
+    txnouttype script_type = Solver(script, solverdata);
+
+    switch (script_type) {
+    case TX_PUBKEY: {
+        CPubKey pubkey(solverdata[0].begin(), solverdata[0].end());
+        import_data.used_keys.emplace(pubkey.GetID(), false);
+        return "";
+    }
+    case TX_PUBKEYHASH: {
+        CKeyID id = CKeyID(uint160(solverdata[0]));
+        import_data.used_keys[id] = true;
+        return "";
+    }
+    case TX_SCRIPTHASH: {
+        if (script_ctx == ScriptContext::P2SH) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside another P2SH");
+        if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside a P2WSH");
+        assert(script_ctx == ScriptContext::TOP);
+        CScriptID id = CScriptID(uint160(solverdata[0]));
+        auto subscript = std::move(import_data.redeemscript); // Remove redeemscript from import_data to check for superfluous script later.
+        if (!subscript) return "missing redeemscript";
+        if (CScriptID(*subscript) != id) return "redeemScript does not match the scriptPubKey";
+        import_data.import_scripts.emplace(*subscript);
+        return RecurseImportData(*subscript, import_data, ScriptContext::P2SH);
+    }
+    case TX_MULTISIG: {
+        for (size_t i = 1; i + 1< solverdata.size(); ++i) {
+            CPubKey pubkey(solverdata[i].begin(), solverdata[i].end());
+            import_data.used_keys.emplace(pubkey.GetID(), false);
+        }
+        return "";
+    }
+    case TX_WITNESS_V0_SCRIPTHASH: {
+        if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2WSH inside another P2WSH");
+        uint256 fullid(solverdata[0]);
+        CScriptID id;
+        CRIPEMD160().Write(fullid.begin(), fullid.size()).Finalize(id.begin());
+        auto subscript = std::move(import_data.witnessscript); // Remove redeemscript from import_data to check for superfluous script later.
+        if (!subscript) return "missing witnessscript";
+        if (CScriptID(*subscript) != id) return "witnessScript does not match the scriptPubKey or redeemScript";
+        if (script_ctx == ScriptContext::TOP) {
+            import_data.import_scripts.emplace(script); // Special rule for IsMine: native P2WSH requires the TOP script imported (see script/ismine.cpp)
+        }
+        import_data.import_scripts.emplace(*subscript);
+        return RecurseImportData(*subscript, import_data, ScriptContext::WITNESS_V0);
+    }
+    case TX_WITNESS_V0_KEYHASH: {
+        if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2WPKH inside P2WSH");
+        CKeyID id = CKeyID(uint160(solverdata[0]));
+        import_data.used_keys[id] = true;
+        if (script_ctx == ScriptContext::TOP) {
+            import_data.import_scripts.emplace(script); // Special rule for IsMine: native P2WPKH requires the TOP script imported (see script/ismine.cpp)
+        }
+        return "";
+    }
+    case TX_NULL_DATA:
+        return "unspendable script";
+    case TX_NONSTANDARD:
+    case TX_WITNESS_UNKNOWN:
+    default:
+        return "unrecognized script";
+    }
+}
 
 static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
 {
+    UniValue warnings(UniValue::VARR);
+    UniValue result(UniValue::VOBJ);
+
     try {
         // First ensure scriptPubKey has either a script or JSON with "address" string
         const UniValue& scriptPubKey = data["scriptPubKey"];
@@ -860,18 +949,16 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
         // Generate the script and destination for the scriptPubKey provided
         CScript script;
         CTxDestination dest;
-
         if (!isScript) {
             dest = DecodeDestination(output);
             if (!IsValidDestination(dest)) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address \"" + output + "\"");
             }
             script = GetScriptForDestination(dest);
         } else {
             if (!IsHex(output)) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid scriptPubKey");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid scriptPubKey \"" + output + "\"");
             }
-
             std::vector<unsigned char> vData(ParseHex(output));
             script = CScript(vData.begin(), vData.end());
             if (!ExtractDestination(script, dest) && !internal) {
@@ -879,203 +966,161 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
             }
         }
 
-        // Watchonly and private keys
-        if (watchOnly && keys.size()) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Watch-only addresses should not include private keys");
-        }
-
-        // Internal addresses should not have a label
-        if (internal && data.exists("label")) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label");
-        }
-
-        // Force users to provide the witness script in its field rather than redeemscript
-        if (!strRedeemScript.empty() && script.IsPayToWitnessScriptHash()) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WSH addresses have an empty redeemscript. Please provide the witnessscript instead.");
-        }
-
-        CScript scriptpubkey_script = script;
-        CTxDestination scriptpubkey_dest = dest;
-        bool allow_p2wpkh = true;
-
-        // P2SH
-        if (!strRedeemScript.empty() && script.IsPayToScriptHash()) {
-            // Check the redeemScript is valid
+        // Parse all arguments
+        ImportData import_data;
+        if (strRedeemScript.size()) {
             if (!IsHex(strRedeemScript)) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script: must be hex string");
-            }
-
-            // Import redeem script.
-            std::vector<unsigned char> vData(ParseHex(strRedeemScript));
-            CScript redeemScript = CScript(vData.begin(), vData.end());
-            CScriptID redeem_id(redeemScript);
-
-            // Check that the redeemScript and scriptPubKey match
-            if (GetScriptForDestination(redeem_id) != script) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The redeemScript does not match the scriptPubKey");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script \"" + strRedeemScript + "\": must be hex string");
             }
-
-            pwallet->MarkDirty();
-
-            if (!pwallet->AddWatchOnly(redeemScript, timestamp)) {
-                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
-            }
-
-            if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) {
-                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet");
-            }
-
-            // Now set script to the redeemScript so we parse the inner script as P2WSH or P2WPKH below
-            script = redeemScript;
-            ExtractDestination(script, dest);
+            auto parsed_redeemscript = ParseHex(strRedeemScript);
+            import_data.redeemscript = MakeUnique<CScript>(parsed_redeemscript.begin(), parsed_redeemscript.end());
         }
-
-        // (P2SH-)P2WSH
-        if (!witness_script_hex.empty() && script.IsPayToWitnessScriptHash()) {
+        if (witness_script_hex.size()) {
             if (!IsHex(witness_script_hex)) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid witness script: must be hex string");
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid witness script \"" + witness_script_hex + "\": must be hex string");
             }
-
-            // Generate the scripts
-            std::vector<unsigned char> witness_script_parsed(ParseHex(witness_script_hex));
-            CScript witness_script = CScript(witness_script_parsed.begin(), witness_script_parsed.end());
-            CScriptID witness_id(witness_script);
-
-            // Check that the witnessScript and scriptPubKey match
-            if (GetScriptForDestination(WitnessV0ScriptHash(witness_script)) != script) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The witnessScript does not match the scriptPubKey or redeemScript");
+            auto parsed_witnessscript = ParseHex(witness_script_hex);
+            import_data.witnessscript = MakeUnique<CScript>(parsed_witnessscript.begin(), parsed_witnessscript.end());
+        }
+        std::map<CKeyID, CPubKey> pubkey_map;
+        for (size_t i = 0; i < pubKeys.size(); ++i) {
+            const auto& str = pubKeys[i].get_str();
+            if (!IsHex(str)) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" must be a hex string");
             }
-
-            // Add the witness script as watch only only if it is not for P2SH-P2WSH
-            if (!scriptpubkey_script.IsPayToScriptHash() && !pwallet->AddWatchOnly(witness_script, timestamp)) {
-                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
+            auto parsed_pubkey = ParseHex(str);
+            CPubKey pubkey(parsed_pubkey.begin(), parsed_pubkey.end());
+            if (!pubkey.IsFullyValid()) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key");
             }
-
-            if (!pwallet->HaveCScript(witness_id) && !pwallet->AddCScript(witness_script)) {
-                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2wsh witnessScript to wallet");
+            pubkey_map.emplace(pubkey.GetID(), pubkey);
+        }
+        std::map<CKeyID, CKey> privkey_map;
+        for (size_t i = 0; i < keys.size(); ++i) {
+            const auto& str = keys[i].get_str();
+            CKey key = DecodeSecret(str);
+            if (!key.IsValid()) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
             }
+            CPubKey pubkey = key.GetPubKey();
+            CKeyID id = pubkey.GetID();
+            if (pubkey_map.count(id)) {
+                pubkey_map.erase(id);
+            }
+            privkey_map.emplace(id, key);
+        }
 
-            // Now set script to the witnessScript so we parse the inner script as P2PK or P2PKH below
-            script = witness_script;
-            ExtractDestination(script, dest);
-            allow_p2wpkh = false; // P2WPKH cannot be embedded in P2WSH
+        // Internal addresses should not have a label
+        if (internal && data.exists("label")) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label");
         }
 
-        // (P2SH-)P2PK/P2PKH/P2WPKH
-        if (dest.type() == typeid(CKeyID) || dest.type() == typeid(WitnessV0KeyHash)) {
-            if (!allow_p2wpkh && dest.type() == typeid(WitnessV0KeyHash)) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WPKH cannot be embedded in P2WSH");
-            }
-            if (keys.size() > 1 || pubKeys.size() > 1) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "More than one key given for one single-key address");
-            }
-            CPubKey pubkey;
-            if (keys.size()) {
-                pubkey = DecodeSecret(keys[0].get_str()).GetPubKey();
+        // Verify and process input data
+        bool have_solving_data = import_data.redeemscript || import_data.witnessscript || pubkey_map.size() || privkey_map.size();
+        if (have_solving_data) {
+            // Match up data in import_data with the scriptPubKey in script.
+            auto error = RecurseImportData(script, import_data, ScriptContext::TOP);
+
+            // Verify whether the watchonly option corresponds to the availability of private keys.
+            bool spendable = std::all_of(import_data.used_keys.begin(), import_data.used_keys.end(), [&](const std::pair<CKeyID, bool>& used_key){ return privkey_map.count(used_key.first) > 0; });
+            if (!watchOnly && !spendable) {
+                warnings.push_back("Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag.");
             }
-            if (pubKeys.size()) {
-                const std::string& strPubKey = pubKeys[0].get_str();
-                if (!IsHex(strPubKey)) {
-                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string");
-                }
-                std::vector<unsigned char> vData(ParseHex(pubKeys[0].get_str()));
-                CPubKey pubkey_temp(vData.begin(), vData.end());
-                if (pubkey.size() && pubkey_temp != pubkey) {
-                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key does not match public key for address");
-                }
-                pubkey = pubkey_temp;
+            if (watchOnly && spendable) {
+                warnings.push_back("All private keys are provided, outputs will be considered spendable. If this is intentional, do not specify the watchonly flag.");
             }
-            if (pubkey.size() > 0) {
-                if (!pubkey.IsFullyValid()) {
-                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
-                }
 
-                // Check the key corresponds to the destination given
-                std::vector<CTxDestination> destinations = GetAllDestinationsForKey(pubkey);
-                if (std::find(destinations.begin(), destinations.end(), dest) == destinations.end()) {
-                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Key does not match address destination");
+            // Check that all required keys for solvability are provided.
+            if (error.empty()) {
+                for (const auto& require_key : import_data.used_keys) {
+                    if (!require_key.second) continue; // Not a required key
+                    if (pubkey_map.count(require_key.first) == 0 && privkey_map.count(require_key.first) == 0) {
+                        error = "some required keys are missing";
+                    }
                 }
+            }
 
-                // This is necessary to force the wallet to import the pubKey
-                CScript scriptRawPubKey = GetScriptForRawPubKey(pubkey);
-
-                if (::IsMine(*pwallet, scriptRawPubKey) == ISMINE_SPENDABLE) {
-                    throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
+            if (!error.empty()) {
+                warnings.push_back("Importing as non-solvable: " + error + ". If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript.");
+                import_data = ImportData();
+                pubkey_map.clear();
+                privkey_map.clear();
+                have_solving_data = false;
+            } else {
+                // RecurseImportData() removes any relevant redeemscript/witnessscript from import_data, so we can use that to discover if a superfluous one was provided.
+                if (import_data.redeemscript) warnings.push_back("Ignoring redeemscript as this is not a P2SH script.");
+                if (import_data.witnessscript) warnings.push_back("Ignoring witnessscript as this is not a (P2SH-)P2WSH script.");
+                for (auto it = privkey_map.begin(); it != privkey_map.end(); ) {
+                    auto oldit = it++;
+                    if (import_data.used_keys.count(oldit->first) == 0) {
+                        warnings.push_back("Ignoring irrelevant private key.");
+                        privkey_map.erase(oldit);
+                    }
                 }
-
-                pwallet->MarkDirty();
-
-                if (!pwallet->AddWatchOnly(scriptRawPubKey, timestamp)) {
-                    throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
+                for (auto it = pubkey_map.begin(); it != pubkey_map.end(); ) {
+                    auto oldit = it++;
+                    auto key_data_it = import_data.used_keys.find(oldit->first);
+                    if (key_data_it == import_data.used_keys.end() || !key_data_it->second) {
+                        warnings.push_back("Ignoring public key \"" + HexStr(oldit->first) + "\" as it doesn't appear inside P2PKH or P2WPKH.");
+                        pubkey_map.erase(oldit);
+                    }
                 }
             }
         }
 
-        // Import the address
-        if (::IsMine(*pwallet, scriptpubkey_script) == ISMINE_SPENDABLE) {
+        // Check whether we have any work to do
+        if (::IsMine(*pwallet, script) & ISMINE_SPENDABLE) {
             throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
         }
 
+        // All good, time to import
         pwallet->MarkDirty();
-
-        if (!pwallet->AddWatchOnly(scriptpubkey_script, timestamp)) {
-            throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
-        }
-
-        if (!watchOnly && !pwallet->HaveCScript(CScriptID(scriptpubkey_script)) && !pwallet->AddCScript(scriptpubkey_script)) {
-            throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet");
-        }
-
-        // if not internal add to address book or update label
-        if (!internal) {
-            assert(IsValidDestination(scriptpubkey_dest));
-            pwallet->SetAddressBook(scriptpubkey_dest, label, "receive");
-        }
-
-        // Import private keys.
-        for (size_t i = 0; i < keys.size(); i++) {
-            const std::string& strPrivkey = keys[i].get_str();
-
-            // Checks.
-            CKey key = DecodeSecret(strPrivkey);
-
-            if (!key.IsValid()) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
+        for (const auto& entry : import_data.import_scripts) {
+            if (!pwallet->HaveCScript(CScriptID(entry)) && !pwallet->AddCScript(entry)) {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding script to wallet");
             }
-
-            CPubKey pubKey = key.GetPubKey();
-            assert(key.VerifyPubKey(pubKey));
-
-            CKeyID vchAddress = pubKey.GetID();
-            pwallet->MarkDirty();
-
-            if (pwallet->HaveKey(vchAddress)) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key");
-            }
-
-            pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
-
-            if (!pwallet->AddKeyPubKey(key, pubKey)) {
+        }
+        for (const auto& entry : privkey_map) {
+            const CKey& key = entry.second;
+            CPubKey pubkey = key.GetPubKey();
+            const CKeyID& id = entry.first;
+            assert(key.VerifyPubKey(pubkey));
+            pwallet->mapKeyMetadata[id].nCreateTime = timestamp;
+            // If the private key is not present in the wallet, insert it.
+            if (!pwallet->HaveKey(id) && !pwallet->AddKeyPubKey(key, pubkey)) {
                 throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
             }
-
             pwallet->UpdateTimeFirstKey(timestamp);
         }
+        for (const auto& entry : pubkey_map) {
+            const CPubKey& pubkey = entry.second;
+            const CKeyID& id = entry.first;
+            CPubKey temp;
+            if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), timestamp)) {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
+            }
+        }
+        if (!have_solving_data || !::IsMine(*pwallet, script)) { // Always call AddWatchOnly for non-solvable watch-only, so that watch timestamp gets updated
+            if (!pwallet->AddWatchOnly(script, timestamp)) {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
+            }
+        }
+        if (!internal) {
+            assert(IsValidDestination(dest));
+            pwallet->SetAddressBook(dest, label, "receive");
+        }
 
-        UniValue result = UniValue(UniValue::VOBJ);
         result.pushKV("success", UniValue(true));
-        return result;
     } catch (const UniValue& e) {
-        UniValue result = UniValue(UniValue::VOBJ);
         result.pushKV("success", UniValue(false));
         result.pushKV("error", e);
-        return result;
     } catch (...) {
-        UniValue result = UniValue(UniValue::VOBJ);
         result.pushKV("success", UniValue(false));
+
         result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields"));
-        return result;
     }
+    if (warnings.size()) result.pushKV("warnings", warnings);
+    return result;
 }
 
 static int64_t GetImportTimestamp(const UniValue& data, int64_t now)
@@ -1122,18 +1167,18 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
                                     },
                                     {"redeemscript", RPCArg::Type::STR, /* opt */ true, /* default_val */ "omitted", "Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey"},
                                     {"witnessscript", RPCArg::Type::STR, /* opt */ true, /* default_val */ "omitted", "Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey"},
-                                    {"pubkeys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving pubkeys that must occur in the output or redeemscript",
+                                    {"pubkeys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving pubkeys to import. They must occur in P2PKH or P2WPKH scripts. They are not required when the private key is also provided (see the \"keys\" argument).",
                                         {
                                             {"pubKey", RPCArg::Type::STR, /* opt */ false, /* default_val */ "", ""},
                                         }
                                     },
-                                    {"keys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript",
+                                    {"keys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.",
                                         {
                                             {"key", RPCArg::Type::STR, /* opt */ false, /* default_val */ "", ""},
                                         }
                                     },
-                                    {"internal", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be treated as not incoming payments aka change"},
-                                    {"watchonly", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty"},
+                                    {"internal", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be treated as not incoming payments (also known as change)"},
+                                    {"watchonly", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be considered watched even when not all private keys are provided."},
                                     {"label", RPCArg::Type::STR, /* opt */ true, /* default_val */ "''", "Label to assign to the address, only allowed with internal=false"},
                                 },
                             },
@@ -1154,7 +1199,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
             HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"<my address>\" }, \"timestamp\":1455191478 }]' '{ \"rescan\": false}'") +
 
             "\nResponse is an array with the same size as the input that has the execution result :\n"
-            "  [{ \"success\": true } , { \"success\": false, \"error\": { \"code\": -1, \"message\": \"Internal Server Error\"} }, ... ]\n");
+            "  [{\"success\": true}, {\"success\": true, \"warnings\": [\"Ignoring irrelevant private key\"]}, {\"success\": false, \"error\": {\"code\": -1, \"message\": \"Internal Server Error\"}}, ...]\n");
 
 
     RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ});
diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py
index a163f7018c..3492075694 100755
--- a/test/functional/wallet_importmulti.py
+++ b/test/functional/wallet_importmulti.py
@@ -119,9 +119,13 @@ class ImportMultiTest(BitcoinTestFramework):
                         CScript([OP_HASH160, witness_script, OP_EQUAL]).hex(),  # p2sh-p2wsh
                         script_to_p2sh_p2wsh(script_code))  # p2sh-p2wsh addr
 
-    def test_importmulti(self, req, success, error_code=None, error_message=None):
+    def test_importmulti(self, req, success, error_code=None, error_message=None, warnings=[]):
         """Run importmulti and assert success"""
         result = self.nodes[1].importmulti([req])
+        observed_warnings = []
+        if 'warnings' in result[0]:
+           observed_warnings = result[0]['warnings']
+        assert_equal("\n".join(sorted(warnings)), "\n".join(sorted(observed_warnings)))
         assert_equal(result[0]['success'], success)
         if error_code is not None:
             assert_equal(result[0]['error']['code'], error_code)
@@ -178,7 +182,7 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now"},
                               False,
                               error_code=-5,
-                              error_message='Invalid address')
+                              error_message='Invalid address \"not valid address\"')
 
         # ScriptPubKey + internal
         self.log.info("Should import a scriptPubKey with internal flag")
@@ -227,7 +231,8 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "pubkeys": [key.pubkey],
                                "internal": False},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
                           iswatchonly=True,
                           ismine=False,
@@ -241,7 +246,8 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "pubkeys": [key.pubkey],
                                "internal": True},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
                           iswatchonly=True,
                           ismine=False,
@@ -284,20 +290,19 @@ class ImportMultiTest(BitcoinTestFramework):
                               error_message='The wallet already contains the private key for this address or script')
 
         # Address + Private key + watchonly
-        self.log.info("Should not import an address with private key and with watchonly")
+        self.log.info("Should import an address with private key and with watchonly")
         key = self.get_key()
         address = key.p2pkh_addr
         self.test_importmulti({"scriptPubKey": {"address": address},
                                "timestamp": "now",
                                "keys": [key.privkey],
                                "watchonly": True},
-                              False,
-                              error_code=-8,
-                              error_message='Watch-only addresses should not include private keys')
+                              True,
+                              warnings=["All private keys are provided, outputs will be considered spendable. If this is intentional, do not specify the watchonly flag."])
         self.test_address(address,
                           iswatchonly=False,
-                          ismine=False,
-                          timestamp=None)
+                          ismine=True,
+                          timestamp=timestamp)
 
         # ScriptPubKey + Private key + internal
         self.log.info("Should import a scriptPubKey with internal and with private key")
@@ -358,8 +363,9 @@ class ImportMultiTest(BitcoinTestFramework):
         self.test_importmulti({"scriptPubKey": {"address": multisig.p2sh_addr},
                                "timestamp": "now",
                                "redeemscript": multisig.redeem_script},
-                              True)
-        self.test_address(multisig.p2sh_addr, timestamp=timestamp)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
+        self.test_address(multisig.p2sh_addr, timestamp=timestamp, iswatchonly=True, ismine=False, solvable=True)
 
         p2shunspent = self.nodes[1].listunspent(0, 999999, [multisig.p2sh_addr])[0]
         assert_equal(p2shunspent['spendable'], False)
@@ -377,9 +383,13 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "redeemscript": multisig.redeem_script,
                                "keys": multisig.privkeys[0:2]},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(multisig.p2sh_addr,
-                          timestamp=timestamp)
+                          timestamp=timestamp,
+                          ismine=False,
+                          iswatchonly=True,
+                          solvable=True)
 
         p2shunspent = self.nodes[1].listunspent(0, 999999, [multisig.p2sh_addr])[0]
         assert_equal(p2shunspent['spendable'], False)
@@ -398,28 +408,31 @@ class ImportMultiTest(BitcoinTestFramework):
                                "redeemscript": multisig.redeem_script,
                                "keys": multisig.privkeys[0:2],
                                "watchonly": True},
-                              False,
-                              error_code=-8,
-                              error_message='Watch-only addresses should not include private keys')
+                              True)
+        self.test_address(multisig.p2sh_addr,
+                          iswatchonly=True,
+                          ismine=False,
+                          solvable=True,
+                          timestamp=timestamp)
 
         # Address + Public key + !Internal + Wrong pubkey
-        self.log.info("Should not import an address with a wrong public key")
+        self.log.info("Should not import an address with the wrong public key as non-solvable")
         key = self.get_key()
         address = key.p2pkh_addr
         wrong_key = self.get_key().pubkey
         self.test_importmulti({"scriptPubKey": {"address": address},
                                "timestamp": "now",
                                "pubkeys": [wrong_key]},
-                              False,
-                              error_code=-5,
-                              error_message='Key does not match address destination')
+                              True,
+                              warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
-                          iswatchonly=False,
+                          iswatchonly=True,
                           ismine=False,
-                          timestamp=None)
+                          solvable=False,
+                          timestamp=timestamp)
 
         # ScriptPubKey + Public key + internal + Wrong pubkey
-        self.log.info("Should not import a scriptPubKey with internal and with a wrong public key")
+        self.log.info("Should import a scriptPubKey with internal and with a wrong public key as non-solvable")
         key = self.get_key()
         address = key.p2pkh_addr
         wrong_key = self.get_key().pubkey
@@ -427,32 +440,32 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "pubkeys": [wrong_key],
                                "internal": True},
-                              False,
-                              error_code=-5,
-                              error_message='Key does not match address destination')
+                              True,
+                              warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
-                          iswatchonly=False,
+                          iswatchonly=True,
                           ismine=False,
-                          timestamp=None)
+                          solvable=False,
+                          timestamp=timestamp)
 
         # Address + Private key + !watchonly + Wrong private key
-        self.log.info("Should not import an address with a wrong private key")
+        self.log.info("Should import an address with a wrong private key as non-solvable")
         key = self.get_key()
         address = key.p2pkh_addr
         wrong_privkey = self.get_key().privkey
         self.test_importmulti({"scriptPubKey": {"address": address},
                                "timestamp": "now",
                                "keys": [wrong_privkey]},
-                              False,
-                              error_code=-5,
-                              error_message='Key does not match address destination')
+                               True,
+                               warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
-                          iswatchonly=False,
+                          iswatchonly=True,
                           ismine=False,
-                          timestamp=None)
+                          solvable=False,
+                          timestamp=timestamp)
 
         # ScriptPubKey + Private key + internal + Wrong private key
-        self.log.info("Should not import a scriptPubKey with internal and with a wrong private key")
+        self.log.info("Should import a scriptPubKey with internal and with a wrong private key as non-solvable")
         key = self.get_key()
         address = key.p2pkh_addr
         wrong_privkey = self.get_key().privkey
@@ -460,13 +473,13 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "keys": [wrong_privkey],
                                "internal": True},
-                              False,
-                              error_code=-5,
-                              error_message='Key does not match address destination')
+                              True,
+                              warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
-                          iswatchonly=False,
+                          iswatchonly=True,
                           ismine=False,
-                          timestamp=None)
+                          solvable=False,
+                          timestamp=timestamp)
 
         # Importing existing watch only address with new timestamp should replace saved timestamp.
         assert_greater_than(timestamp, watchonly_timestamp)
@@ -516,7 +529,8 @@ class ImportMultiTest(BitcoinTestFramework):
         self.test_importmulti({"scriptPubKey": {"address": address},
                                "timestamp": "now",
                                "pubkeys": [key.pubkey]},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
                           ismine=False,
                           solvable=True)
@@ -571,7 +585,8 @@ class ImportMultiTest(BitcoinTestFramework):
                                "timestamp": "now",
                                "redeemscript": key.p2sh_p2wpkh_redeem_script,
                                "pubkeys": [key.pubkey]},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
                           solvable=True,
                           ismine=False)
@@ -591,14 +606,17 @@ class ImportMultiTest(BitcoinTestFramework):
 
         # P2SH-P2WSH multisig + redeemscript with no private key
         multisig = self.get_multisig()
+        address = multisig.p2sh_p2wsh_addr
         self.log.info("Should import a p2sh-p2wsh with respective redeem script but no private key")
-        self.test_importmulti({"scriptPubKey": {"address": multisig.p2sh_p2wsh_addr},
+        self.test_importmulti({"scriptPubKey": {"address": address},
                                "timestamp": "now",
                                "redeemscript": multisig.p2wsh_script,
                                "witnessscript": multisig.redeem_script},
-                              True)
+                              True,
+                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
         self.test_address(address,
-                          solvable=True)
+                          solvable=True,
+                          ismine=False)
 
 if __name__ == '__main__':
     ImportMultiTest().main()