Usability improvements for z_importkey

- Add height parameter to z_importkey to reduce rescan range
- Change semantics of rescan parameter, so users can explicitly force a rescan
  for existing keys.

Closes #2032

qa/rpc-tests/wallet_1941.py

@@ -31,6 +31,14 @@ class Wallet1941RegressionTest (BitcoinTestFramework):
         connect_nodes_bi(self.nodes,0,1)
         self.sync_all()
 
+    def restart_second_node(self, extra_args=[]):
+        self.nodes[1].stop()
+        bitcoind_processes[1].wait()
+        self.nodes[1] = start_node(1, self.options.tmpdir, extra_args=['-regtestprotectcoinbase','-debug=zrpc'] + extra_args)
+        self.nodes[1].setmocktime(starttime + 9000)
+        connect_nodes_bi(self.nodes, 0, 1)
+        self.sync_all()
+
     def wait_and_assert_operationid_status(self, myopid, in_status='success', in_errormsg=None):
         print('waiting for async operation {}'.format(myopid))
         opids = []
@@ -93,8 +101,16 @@ class Wallet1941RegressionTest (BitcoinTestFramework):
         self.nodes[1].generate(101)
         self.sync_all()
 
-        # Import the key on node 1.
-        self.nodes[1].z_importkey(key)
+        # Import the key on node 1, only scanning the last few blocks.
+        self.nodes[1].z_importkey(key, 'true', self.nodes[1].getblockchaininfo()['blocks'] - 100)
+
+        # Confirm that the balance on node 1 is zero, as we have not
+        # rescanned over the older transactions
+        resp = self.nodes[1].z_getbalance(myzaddr)
+        assert_equal(Decimal(resp), 0)
+
+        # Re-import the key on node 1, scanning from before the transaction.
+        self.nodes[1].z_importkey(key, 'true', self.nodes[1].getblockchaininfo()['blocks'] - 110)
 
         # Confirm that the balance on node 1 is valid now (node 1 must
         # have rescanned)

src/rpcclient.cpp

@@ -111,7 +111,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "z_sendmany", 3},
     { "z_getoperationstatus", 0},
     { "z_getoperationresult", 0},
-    { "z_importkey", 1 }
+    { "z_importkey", 2 },
 };
 
 class CRPCConvertTable

src/test/rpc_wallet_tests.cpp

@@ -500,7 +500,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport)
     BOOST_CHECK_THROW(CallRPC("z_exportkey"), runtime_error);   
 
     // error if too many args
-    BOOST_CHECK_THROW(CallRPC("z_importkey too many args"), runtime_error);
+    BOOST_CHECK_THROW(CallRPC("z_importkey way too many args"), runtime_error);
     BOOST_CHECK_THROW(CallRPC("z_exportkey toomany args"), runtime_error);
 
     // wallet should currently be empty

src/wallet/rpcdump.cpp

@@ -552,19 +552,24 @@ UniValue z_importkey(const UniValue& params, bool fHelp)
     if (!EnsureWalletIsAvailable(fHelp))
         return NullUniValue;
 
-    if (fHelp || params.size() < 1 || params.size() > 2)
+    if (fHelp || params.size() < 1 || params.size() > 3)
         throw runtime_error(
-            "z_importkey \"zkey\" ( rescan )\n"
+            "z_importkey \"zkey\" ( rescan startHeight )\n"
             "\nAdds a zkey (as returned by z_exportkey) to your wallet.\n"
             "\nArguments:\n"
             "1. \"zkey\"             (string, required) The zkey (see z_exportkey)\n"
-            "2. rescan             (boolean, optional, default=true) Rescan the wallet for transactions\n"
+            "2. rescan             (boolean or \"whenkeyisnew\", optional, default=\"whenkeyisnew\") Rescan the wallet for transactions\n"
+            "3. startHeight        (numeric, optional, default=0) Block height to start rescan from\n"
             "\nNote: This call can take minutes to complete if rescan is true.\n"
             "\nExamples:\n"
             "\nExport a zkey\n"
             + HelpExampleCli("z_exportkey", "\"myaddress\"") +
             "\nImport the zkey with rescan\n"
             + HelpExampleCli("z_importkey", "\"mykey\"") +
+            "\nImport the zkey with partial rescan\n"
+            + HelpExampleCli("z_importkey", "\"mykey\", \"whenkeyisnew\", 30000") +
+            "\nRe-import the zkey with longer partial rescan\n"
+            + HelpExampleCli("z_importkey", "\"mykey\", true, 20000") +
             "\nAs a JSON-RPC call\n"
             + HelpExampleRpc("z_importkey", "\"mykey\", false")
         );
@@ -575,8 +580,24 @@ UniValue z_importkey(const UniValue& params, bool fHelp)
 
     // Whether to perform rescan after import
     bool fRescan = true;
-    if (params.size() > 1)
-        fRescan = params[1].get_bool();
+    bool fIgnoreExistingKey = true;
+    if (params.size() > 1) {
+        auto rescan = params[1].get_str();
+        if (rescan.compare("whenkeyisnew") != 0) {
+            fIgnoreExistingKey = false;
+            UniValue jVal;
+            if (!jVal.read(std::string("[")+rescan+std::string("]")) ||
+                !jVal.isArray() || jVal.size()!=1 || !jVal[0].isBool()) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "rescan must be bool or \"whenkeyisnew\"");
+            }
+            fRescan = jVal[0].getBool();
+        }
+    }
+
+    // Height to rescan from
+    int nRescanHeight = 0;
+    if (params.size() > 2)
+        nRescanHeight = params[2].get_int();
 
     string strSecret = params[0].get_str();
     CZCSpendingKey spendingkey(strSecret);
@@ -585,22 +606,25 @@ UniValue z_importkey(const UniValue& params, bool fHelp)
 
     {
         // Don't throw error in case a key is already there
-        if (pwalletMain->HaveSpendingKey(addr))
-            return NullUniValue;
-
-        pwalletMain->MarkDirty();
+        if (pwalletMain->HaveSpendingKey(addr)) {
+            if (fIgnoreExistingKey) {
+                return NullUniValue;
+            }
+        } else {
+            pwalletMain->MarkDirty();
 
-        if (!pwalletMain-> AddZKey(key))
-            throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet");
+            if (!pwalletMain-> AddZKey(key))
+                throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet");
 
-        pwalletMain->mapZKeyMetadata[addr].nCreateTime = 1;
+            pwalletMain->mapZKeyMetadata[addr].nCreateTime = 1;
+        }
 
         // whenever a key is imported, we need to scan the whole chain
         pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value'
 
         // We want to scan for transactions and notes
         if (fRescan) {
-            pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true);
+            pwalletMain->ScanForWalletTransactions(chainActive[nRescanHeight], true);
         }
     }