Let wallet importmulti RPC accept labels for standard scriptPubKeys

Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.
bitcoin · Nov 28, 2016 · ef25b9a · ef25b9a
1 parent c6fd923
commit ef25b9a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 54 deletions.
diff --git a/qa/rpc-tests/import-rescan.py b/qa/rpc-tests/import-rescan.py
@@ -12,7 +12,7 @@
 import itertools
 import functools
 
-Call = enum.Enum("Call", "single multi")
+Call = enum.Enum("Call", "single multiaddress multiscript")
 Data = enum.Enum("Data", "address pub priv")
 ImportNode = collections.namedtuple("ImportNode", "rescan")
 
@@ -28,11 +28,11 @@ def call_import_rpc(call, data, address, scriptPubKey, pubkey, key, label, node,
         elif data == Data.priv:
             response = node.importprivkey(key, label, rescan)
         assert_equal(response, None)
-    elif call == Call.multi:
+    elif call in (Call.multiaddress, Call.multiscript):
         response = node.importmulti([{
             "scriptPubKey": {
                 "address": address
-            },
+            } if call == Call.multiaddress else scriptPubKey,
             "pubkeys": [pubkey] if data == Data.pub else [],
             "keys": [key] if data == Data.priv else [],
             "label": label,

diff --git a/qa/rpc-tests/importmulti.py b/qa/rpc-tests/importmulti.py
@@ -3,6 +3,7 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+from test_framework import script
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
@@ -72,15 +73,16 @@ def run_test (self):
         assert_equal(address_assert['iswatchonly'], True)
         assert_equal(address_assert['ismine'], False)
 
-        # ScriptPubKey + !internal
-        print("Should not import a scriptPubKey without internal flag")
+        # Nonstandard scriptPubKey + !internal
+        print("Should not import a nonstandard scriptPubKey without internal flag")
+        nonstandardScriptPubKey = address['scriptPubKey'] + bytes_to_hex_str(script.CScript([script.OP_NOP]))
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
-            "scriptPubKey": address['scriptPubKey']
+            "scriptPubKey": nonstandardScriptPubKey,
         }])
         assert_equal(result[0]['success'], False)
         assert_equal(result[0]['error']['code'], -8)
-        assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey')
+        assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
         address_assert = self.nodes[1].validateaddress(address['address'])
         assert_equal(address_assert['iswatchonly'], False)
         assert_equal(address_assert['ismine'], False)
@@ -115,17 +117,17 @@ def run_test (self):
         assert_equal(address_assert['iswatchonly'], True)
         assert_equal(address_assert['ismine'], False)
 
-        # ScriptPubKey + Public key + !internal
-        print("Should not import a scriptPubKey without internal and with public key")
+        # Nonstandard scriptPubKey + Public key + !internal
+        print("Should not import a nonstandard scriptPubKey without internal and with public key")
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         request = [{
-            "scriptPubKey": address['scriptPubKey'],
+            "scriptPubKey": nonstandardScriptPubKey,
             "pubkeys": [ address['pubkey'] ]
         }];
         result = self.nodes[1].importmulti(request)
         assert_equal(result[0]['success'], False)
         assert_equal(result[0]['error']['code'], -8)
-        assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey')
+        assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
         address_assert = self.nodes[1].validateaddress(address['address'])
         assert_equal(address_assert['iswatchonly'], False)
         assert_equal(address_assert['ismine'], False)
@@ -174,16 +176,16 @@ def run_test (self):
         assert_equal(address_assert['iswatchonly'], False)
         assert_equal(address_assert['ismine'], True)
 
-        # ScriptPubKey + Private key + !internal
-        print("Should not import a scriptPubKey without internal and with private key")
+        # Nonstandard scriptPubKey + Private key + !internal
+        print("Should not import a nonstandard scriptPubKey without internal and with private key")
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
-            "scriptPubKey": address['scriptPubKey'],
+            "scriptPubKey": nonstandardScriptPubKey,
             "keys": [ self.nodes[0].dumpprivkey(address['address']) ]
         }])
         assert_equal(result[0]['success'], False)
         assert_equal(result[0]['error']['code'], -8)
-        assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey')
+        assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
         address_assert = self.nodes[1].validateaddress(address['address'])
         assert_equal(address_assert['iswatchonly'], False)
         assert_equal(address_assert['ismine'], False)

diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -668,17 +668,24 @@ UniValue processImport(const UniValue& data) {
         // Parse the output.
         CScript script;
         CBitcoinAddress address;
+        CTxDestination dest;
 
         if (!isScript) {
             address = CBitcoinAddress(output);
-            script = GetScriptForDestination(address.Get());
+            dest = address.Get();
+            script = GetScriptForDestination(dest);
         } else {
             if (!IsHex(output)) {
                 throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid scriptPubKey");
             }
 
             std::vector<unsigned char> vData(ParseHex(output));
             script = CScript(vData.begin(), vData.end());
+            if (ExtractDestination(script, dest)) {
+                address = CBitcoinAddress(dest);
+            } else if (!internal) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal must be set to true for nonstandard scriptPubKey imports.");
+            }
         }
 
         // Watchonly and private keys
@@ -691,11 +698,6 @@ UniValue processImport(const UniValue& data) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label");
         }
 
-        // Not having Internal + Script
-        if (!internal && isScript) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal must be set for hex scriptPubKey");
-        }
-
         // 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");
@@ -808,23 +810,10 @@ UniValue processImport(const UniValue& data) {
                 CBitcoinAddress pubKeyAddress = CBitcoinAddress(pubKey.GetID());
 
                 // Consistency check.
-                if (!isScript && !(pubKeyAddress.Get() == address.Get())) {
+                if (!(pubKeyAddress.Get() == address.Get())) {
                     throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
                 }
 
-                // Consistency check.
-                if (isScript) {
-                    CBitcoinAddress scriptAddress;
-                    CTxDestination destination;
-
-                    if (ExtractDestination(script, destination)) {
-                        scriptAddress = CBitcoinAddress(destination);
-                        if (!(scriptAddress.Get() == pubKeyAddress.Get())) {
-                            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
-                        }
-                    }
-                }
-
                 CScript pubKeyScript = GetScriptForDestination(pubKeyAddress.Get());
 
                 if (::IsMine(*pwalletMain, pubKeyScript) == ISMINE_SPENDABLE) {
@@ -881,23 +870,10 @@ UniValue processImport(const UniValue& data) {
                 CBitcoinAddress pubKeyAddress = CBitcoinAddress(pubKey.GetID());
 
                 // Consistency check.
-                if (!isScript && !(pubKeyAddress.Get() == address.Get())) {
+                if (!(pubKeyAddress.Get() == address.Get())) {
                     throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
                 }
 
-                // Consistency check.
-                if (isScript) {
-                    CBitcoinAddress scriptAddress;
-                    CTxDestination destination;
-
-                    if (ExtractDestination(script, destination)) {
-                        scriptAddress = CBitcoinAddress(destination);
-                        if (!(scriptAddress.Get() == pubKeyAddress.Get())) {
-                            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
-                        }
-                    }
-                }
-
                 CKeyID vchAddress = pubKey.GetID();
                 pwalletMain->MarkDirty();
                 pwalletMain->SetAddressBook(vchAddress, label, "receive");
@@ -931,11 +907,9 @@ UniValue processImport(const UniValue& data) {
                     throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
                 }
 
-                if (scriptPubKey.getType() == UniValue::VOBJ) {
-                    // add to address book or update label
-                    if (address.IsValid()) {
-                        pwalletMain->SetAddressBook(address.Get(), label, "receive");
-                    }
+                // add to address book or update label
+                if (address.IsValid()) {
+                    pwalletMain->SetAddressBook(address.Get(), label, "receive");
                 }
 
                 success = true;