diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index a4dba8af8d..1fd8aa483d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1032,15 +1032,13 @@ UniValue dumpwallet(const JSONRPCRequest& request) static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { try { - bool success = false; - - // Required fields. + // First ensure scriptPubKey has either a script or JSON with "address" string const UniValue& scriptPubKey = data["scriptPubKey"]; - - // Should have script or JSON with "address". - if (!(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address")) && !(scriptPubKey.getType() == UniValue::VSTR)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid scriptPubKey"); + bool isScript = scriptPubKey.getType() == UniValue::VSTR; + if (!isScript && !(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address"))) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "scriptPubKey must be string with script or JSON with address string"); } + const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str(); // Optional fields. const std::string& strRedeemScript = data.exists("redeemscript") ? data["redeemscript"].get_str() : ""; @@ -1048,11 +1046,7 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con const UniValue& keys = data.exists("keys") ? data["keys"].get_array() : UniValue(); const bool internal = data.exists("internal") ? data["internal"].get_bool() : false; const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false; - const std::string& label = data.exists("label") && !internal ? data["label"].get_str() : ""; - - bool isScript = scriptPubKey.getType() == UniValue::VSTR; - bool isP2SH = strRedeemScript.length() > 0; - const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str(); + const std::string& label = data.exists("label") ? data["label"].get_str() : ""; // Parse the output. // If private keys are disabled, abort if private keys are being imported @@ -1084,35 +1078,32 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con // Watchonly and private keys if (watchOnly && keys.size()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between watchonly and keys"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Watch-only addresses should not include private keys"); } - // Internal + Label + // Internal addresses should not have a label if (internal && data.exists("label")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label"); } - // Keys / PubKeys size check. - if (!isP2SH && (keys.size() > 1 || pubKeys.size() > 1)) { // Address / scriptPubKey - throw JSONRPCError(RPC_INVALID_PARAMETER, "More than private key given for one address"); - } - - // Invalid P2SH redeemScript - if (isP2SH && !IsHex(strRedeemScript)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script"); - } - - // Process. // + CScript scriptpubkey_script = script; + CTxDestination scriptpubkey_dest = dest; // P2SH - if (isP2SH) { + if (!strRedeemScript.empty() && script.IsPayToScriptHash()) { + // Check the redeemScript is valid + if (!IsHex(strRedeemScript)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script: must be hex string"); + } + // Import redeem script. std::vector vData(ParseHex(strRedeemScript)); CScript redeemScript = CScript(vData.begin(), vData.end()); + CScriptID redeem_id(redeemScript); - // Invalid P2SH address - if (!script.IsPayToScriptHash()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid P2SH address / script"); + // 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"); } pwallet->MarkDirty(); @@ -1121,103 +1112,49 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } - CScriptID redeem_id(redeemScript); if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet"); } - CScript redeemDestination = GetScriptForDestination(redeem_id); + // Now set script to the redeemScript so we parse the inner script as P2WSH or P2WPKH below + script = redeemScript; + ExtractDestination(script, dest); + } - if (::IsMine(*pwallet, redeemDestination) == ISMINE_SPENDABLE) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); + // (P2SH-)P2PK/P2PKH + if (dest.type() == typeid(CKeyID)) { + if (keys.size() > 1 || pubKeys.size() > 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "More than one key given for one single-key address"); } - - pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(redeemDestination, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - // add to address book or update label - if (IsValidDestination(dest)) { - pwallet->SetAddressBook(dest, label, "receive"); - } - - // Import private keys. + CPubKey pubkey; if (keys.size()) { - for (size_t i = 0; i < keys.size(); i++) { - const std::string& privkey = keys[i].get_str(); - - CKey key = DecodeSecret(privkey); - - if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - } - - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - - CKeyID vchAddress = pubkey.GetID(); - pwallet->MarkDirty(); - pwallet->SetAddressBook(vchAddress, label, "receive"); - - 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)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); - } - - pwallet->UpdateTimeFirstKey(timestamp); - } + pubkey = DecodeSecret(keys[0].get_str()).GetPubKey(); } - - success = true; - } else { - // Import public keys. - if (pubKeys.size() && keys.size() == 0) { + 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 vData(ParseHex(strPubKey)); - CPubKey pubKey(vData.begin(), vData.end()); - - if (!pubKey.IsFullyValid()) { + std::vector 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 (pubkey.size() > 0) { + if (!pubkey.IsFullyValid()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); } - CTxDestination pubkey_dest = pubKey.GetID(); - - // Consistency check. + // Check the key corresponds to the destination given + CTxDestination pubkey_dest = pubkey.GetID(); if (!(pubkey_dest == dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Key does not match address destination"); } - CScript pubKeyScript = GetScriptForDestination(pubkey_dest); - - if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); - } - - pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(pubKeyScript, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - // add to address book or update label - if (IsValidDestination(pubkey_dest)) { - pwallet->SetAddressBook(pubkey_dest, label, "receive"); - } - - // TODO Is this necessary? - CScript scriptRawPubKey = GetScriptForRawPubKey(pubKey); + // 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"); @@ -1228,74 +1165,62 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con if (!pwallet->AddWatchOnly(scriptRawPubKey, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } - - success = true; - } - - // Import private keys. - if (keys.size()) { - const std::string& strPrivkey = keys[0].get_str(); - - // Checks. - CKey key = DecodeSecret(strPrivkey); - - if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - } - - CPubKey pubKey = key.GetPubKey(); - assert(key.VerifyPubKey(pubKey)); - - CTxDestination pubkey_dest = pubKey.GetID(); - - // Consistency check. - if (!(pubkey_dest == dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed"); - } - - CKeyID vchAddress = pubKey.GetID(); - pwallet->MarkDirty(); - pwallet->SetAddressBook(vchAddress, label, "receive"); - - if (pwallet->HaveKey(vchAddress)) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); - } - - pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp; - - if (!pwallet->AddKeyPubKey(key, pubKey)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); - } - - pwallet->UpdateTimeFirstKey(timestamp); - - success = true; - } - - // Import scriptPubKey only. - if (pubKeys.size() == 0 && keys.size() == 0) { - if (::IsMine(*pwallet, script) == ISMINE_SPENDABLE) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); - } - - pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(script, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - // if not internal add to address book or update label - if (!internal) { - assert(IsValidDestination(dest)); - pwallet->SetAddressBook(dest, label, "receive"); - } - - success = true; } } + // Import the address + if (::IsMine(*pwallet, scriptpubkey_script) == ISMINE_SPENDABLE) { + throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); + } + + 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"); + } + + 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)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); + } + + pwallet->UpdateTimeFirstKey(timestamp); + } + UniValue result = UniValue(UniValue::VOBJ); - result.pushKV("success", UniValue(success)); + result.pushKV("success", UniValue(true)); return result; } catch (const UniValue& e) { UniValue result = UniValue(UniValue::VOBJ); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a1a59fe468..60562418c4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3650,6 +3650,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " \"address\" : \"address\", (string) The dash address validated\n" " \"scriptPubKey\" : \"hex\", (string) The hex-encoded scriptPubKey generated by the address\n" " \"ismine\" : true|false, (boolean) If the address is yours or not\n" + " \"solvable\" : true|false, (boolean) If the address is solvable by the wallet\n" " \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n" " \"isscript\" : true|false, (boolean) If the key is a script\n" " \"ischange\" : true|false, (boolean) If the address was used for change output\n" @@ -3701,6 +3702,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) isminetype mine = IsMine(*pwallet, dest); ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE)); ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY)); + ret.pushKV("solvable", IsSolvable(*pwallet, scriptPubKey)); UniValue detail = DescribeWalletAddress(pwallet, dest); ret.pushKVs(detail); if (pwallet->mapAddressBook.count(dest)) { diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index c6111a9990..3443056683 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2016 The Bitcoin Core developers +# Copyright (c) 2014-2018 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the importmulti RPC.""" @@ -12,7 +12,6 @@ from test_framework.util import ( assert_raises_rpc_error, ) - class ImportMultiTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -90,6 +89,19 @@ class ImportMultiTest(BitcoinTestFramework): assert_equal(address_assert['timestamp'], timestamp) assert_equal(address_assert['ischange'], True) + # ScriptPubKey + internal + label + self.log.info("Should not allow a label to be specified when internal is true") + address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) + result = self.nodes[1].importmulti([{ + "scriptPubKey": address['scriptPubKey'], + "timestamp": "now", + "internal": True, + "label": "Example label" + }]) + assert_equal(result[0]['success'], False) + assert_equal(result[0]['error']['code'], -8) + assert_equal(result[0]['error']['message'], 'Internal addresses should not have a label') + # Nonstandard scriptPubKey + !internal self.log.info("Should not import a nonstandard scriptPubKey without internal flag") nonstandardScriptPubKey = address['scriptPubKey'] + script.CScript([script.OP_NOP]).hex() @@ -199,7 +211,7 @@ class ImportMultiTest(BitcoinTestFramework): }]) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -8) - assert_equal(result[0]['error']['message'], 'Incompatibility found between watchonly and keys') + assert_equal(result[0]['error']['message'], 'Watch-only addresses should not include private keys') address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) @@ -340,7 +352,7 @@ class ImportMultiTest(BitcoinTestFramework): }]) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -8) - assert_equal(result[0]['error']['message'], 'Incompatibility found between watchonly and keys') + assert_equal(result[0]['error']['message'], 'Watch-only addresses should not include private keys') # Address + Public key + !Internal + Wrong pubkey @@ -356,7 +368,7 @@ class ImportMultiTest(BitcoinTestFramework): }]) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -5) - assert_equal(result[0]['error']['message'], 'Consistency check failed') + assert_equal(result[0]['error']['message'], 'Key does not match address destination') address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) @@ -376,7 +388,7 @@ class ImportMultiTest(BitcoinTestFramework): result = self.nodes[1].importmulti(request) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -5) - assert_equal(result[0]['error']['message'], 'Consistency check failed') + assert_equal(result[0]['error']['message'], 'Key does not match address destination') address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) @@ -396,7 +408,7 @@ class ImportMultiTest(BitcoinTestFramework): }]) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -5) - assert_equal(result[0]['error']['message'], 'Consistency check failed') + assert_equal(result[0]['error']['message'], 'Key does not match address destination') address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) @@ -415,7 +427,7 @@ class ImportMultiTest(BitcoinTestFramework): }]) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -5) - assert_equal(result[0]['error']['message'], 'Consistency check failed') + assert_equal(result[0]['error']['message'], 'Key does not match address destination') address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False)