Require timestamps for importmulti keys

Additionally, accept a "now" timestamp, to allow avoiding rescans for keys
which are known never to have been used.

Note that the behavior when "now" is specified is slightly different than the
previous behavior when no timestamp was specified at all. Previously, when no
timestamp was specified, it would avoid rescanning during the importmulti call,
but set the key's nCreateTime value to 1, which would not prevent future block
reads in later ScanForWalletTransactions calls. With this change, passing a
"now" timestamp will set the key's nCreateTime to the current block time
instead of 1.

Fixes #9491

3 files changed, 64 insertions, 9 deletions

diff --git a/qa/rpc-tests/import-rescan.py b/qa/rpc-tests/import-rescan.py
index e683df26db..8f60e63e2e 100755
--- a/qa/rpc-tests/import-rescan.py
+++ b/qa/rpc-tests/import-rescan.py
@@ -33,6 +33,7 @@ def call_import_rpc(call, data, address, scriptPubKey, pubkey, key, label, node,
             "scriptPubKey": {
                 "address": address
             },
+            "timestamp": "now",
             "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
index e100a3af9d..b4d4b6c5b8 100755
--- a/qa/rpc-tests/importmulti.py
+++ b/qa/rpc-tests/importmulti.py
@@ -52,7 +52,8 @@ class ImportMultiTest (BitcoinTestFramework):
         result = self.nodes[1].importmulti([{
             "scriptPubKey": {
                 "address": address['address']
-            }
+            },
+            "timestamp": "now",
         }])
         assert_equal(result[0]['success'], True)
         address_assert = self.nodes[1].validateaddress(address['address'])
@@ -65,6 +66,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "internal": True
         }])
         assert_equal(result[0]['success'], True)
@@ -76,7 +78,8 @@ class ImportMultiTest (BitcoinTestFramework):
         print("Should not import a scriptPubKey without internal flag")
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
-            "scriptPubKey": address['scriptPubKey']
+            "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
         }])
         assert_equal(result[0]['success'], False)
         assert_equal(result[0]['error']['code'], -8)
@@ -93,6 +96,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": address['address']
             },
+            "timestamp": "now",
             "pubkeys": [ address['pubkey'] ]
         }])
         assert_equal(result[0]['success'], True)
@@ -106,6 +110,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         request = [{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "pubkeys": [ address['pubkey'] ],
             "internal": True
         }]
@@ -120,6 +125,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         request = [{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "pubkeys": [ address['pubkey'] ]
         }]
         result = self.nodes[1].importmulti(request)
@@ -137,6 +143,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": address['address']
             },
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address['address']) ]
         }])
         assert_equal(result[0]['success'], True)
@@ -151,6 +158,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": address['address']
             },
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address['address']) ],
             "watchonly": True
         }])
@@ -166,6 +174,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address['address']) ],
             "internal": True
         }])
@@ -179,6 +188,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address['address']) ]
         }])
         assert_equal(result[0]['success'], False)
@@ -203,7 +213,8 @@ class ImportMultiTest (BitcoinTestFramework):
         result = self.nodes[1].importmulti([{
             "scriptPubKey": {
                 "address": multi_sig_script['address']
-            }
+            },
+            "timestamp": "now",
         }])
         assert_equal(result[0]['success'], True)
         address_assert = self.nodes[1].validateaddress(multi_sig_script['address'])
@@ -229,6 +240,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": multi_sig_script['address']
             },
+            "timestamp": "now",
             "redeemscript": multi_sig_script['redeemScript']
         }])
         assert_equal(result[0]['success'], True)
@@ -253,6 +265,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": multi_sig_script['address']
             },
+            "timestamp": "now",
             "redeemscript": multi_sig_script['redeemScript'],
             "keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])]
         }])
@@ -277,6 +290,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": multi_sig_script['address']
             },
+            "timestamp": "now",
             "redeemscript": multi_sig_script['redeemScript'],
             "keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])],
             "watchonly": True
@@ -294,6 +308,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": address['address']
             },
+            "timestamp": "now",
             "pubkeys": [ address2['pubkey'] ]
         }])
         assert_equal(result[0]['success'], False)
@@ -310,6 +325,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         request = [{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "pubkeys": [ address2['pubkey'] ],
             "internal": True
         }]
@@ -330,6 +346,7 @@ class ImportMultiTest (BitcoinTestFramework):
             "scriptPubKey": {
                 "address": address['address']
             },
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address2['address']) ]
         }])
         assert_equal(result[0]['success'], False)
@@ -346,6 +363,7 @@ class ImportMultiTest (BitcoinTestFramework):
         address2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
             "scriptPubKey": address['scriptPubKey'],
+            "timestamp": "now",
             "keys": [ self.nodes[0].dumpprivkey(address2['address']) ],
             "internal": True
         }])
@@ -356,5 +374,18 @@ class ImportMultiTest (BitcoinTestFramework):
         assert_equal(address_assert['iswatchonly'], False)
         assert_equal(address_assert['ismine'], False)
 
+        # Bad or missing timestamps
+        print("Should throw on invalid or missing timestamp values")
+        assert_raises_message(JSONRPCException, 'Missing required timestamp field for key',
+            self.nodes[1].importmulti, [{
+                "scriptPubKey": address['scriptPubKey'],
+            }])
+        assert_raises_message(JSONRPCException, 'Expected number or "now" timestamp value for key. got type string',
+            self.nodes[1].importmulti, [{
+                "scriptPubKey": address['scriptPubKey'],
+                "timestamp": "",
+            }])
+
+
 if __name__ == '__main__':
     ImportMultiTest ().main ()
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 7d4ed70ed9..4674c95bfe 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -640,7 +640,8 @@ UniValue dumpwallet(const JSONRPCRequest& request)
 }
 
 
-UniValue processImport(const UniValue& data) {
+UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
+{
     try {
         bool success = false;
 
@@ -659,7 +660,6 @@ UniValue processImport(const UniValue& data) {
         const bool& internal = data.exists("internal") ? data["internal"].get_bool() : false;
         const bool& watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
         const string& label = data.exists("label") && !internal ? data["label"].get_str() : "";
-        const int64_t& timestamp = data.exists("timestamp") && data["timestamp"].get_int64() > 1 ? data["timestamp"].get_int64() : 1;
 
         bool isScript = scriptPubKey.getType() == UniValue::VSTR;
         bool isP2SH = strRedeemScript.length() > 0;
@@ -958,6 +958,20 @@ UniValue processImport(const UniValue& data) {
     }
 }
 
+int64_t GetImportTimestamp(const UniValue& data, int64_t now)
+{
+    if (data.exists("timestamp")) {
+        const UniValue& timestamp = data["timestamp"];
+        if (timestamp.isNum()) {
+            return timestamp.get_int64();
+        } else if (timestamp.isStr() && timestamp.get_str() == "now") {
+            return now;
+        }
+        throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected number or \"now\" timestamp value for key. got type %s", uvTypeName(timestamp.type())));
+    }
+    throw JSONRPCError(RPC_TYPE_ERROR, "Missing required timestamp field for key");
+}
+
 UniValue importmulti(const JSONRPCRequest& mainRequest)
 {
     // clang-format off
@@ -970,13 +984,17 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
             "  [     (array of json objects)\n"
             "    {\n"
             "      \"scriptPubKey\": \"<script>\" | { \"address\":\"<address>\" }, (string / json, required) Type of scriptPubKey (string for script, json for address)\n"
+            "      \"timestamp\": timestamp | \"now\"                        , (integer / string, required) Creation time of the key in seconds since epoch (Jan 1 1970 GMT),\n"
+            "                                                              or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n"
+            "                                                              key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n"
+            "                                                              \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n"
+            "                                                              0 can be specified to scan the entire blockchain.\n"
             "      \"redeemscript\": \"<script>\"                            , (string, optional) Allowed only if the scriptPubKey is a P2SH address or a P2SH scriptPubKey\n"
             "      \"pubkeys\": [\"<pubKey>\", ... ]                         , (array, optional) Array of strings giving pubkeys that must occur in the output or redeemscript\n"
             "      \"keys\": [\"<key>\", ... ]                               , (array, optional) Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript\n"
             "      \"internal\": <true>                                    , (boolean, optional, default: false) Stating whether matching outputs should be be treated as not incoming payments\n"
             "      \"watchonly\": <true>                                   , (boolean, optional, default: false) Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty\n"
             "      \"label\": <label>                                      , (string, optional, default: '') Label to assign to the address (aka account name, for now), only allowed with internal=false\n"
-            "      \"timestamp\": 1454686740,                                (integer, optional, default now) Timestamp\n"
             "    }\n"
             "  ,...\n"
             "  ]\n"
@@ -1015,6 +1033,12 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
     LOCK2(cs_main, pwalletMain->cs_wallet);
     EnsureWalletIsUnlocked();
 
+    // Verify all timestamps are present before importing any keys.
+    const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetBlockTime() : 0;
+    for (const UniValue& data : requests.getValues()) {
+        GetImportTimestamp(data, now);
+    }
+
     bool fRunScan = false;
     const int64_t minimumTimestamp = 1;
     int64_t nLowestTimestamp = 0;
@@ -1028,7 +1052,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
     UniValue response(UniValue::VARR);
 
     BOOST_FOREACH (const UniValue& data, requests.getValues()) {
-        const UniValue result = processImport(data);
+        const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
+        const UniValue result = ProcessImport(data, timestamp);
         response.push_back(result);
 
         if (!fRescan) {
@@ -1041,8 +1066,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
         }
 
         // Get the lowest timestamp.
-        const int64_t& timestamp = data.exists("timestamp") && data["timestamp"].get_int64() > minimumTimestamp ? data["timestamp"].get_int64() : minimumTimestamp;
-
         if (timestamp < nLowestTimestamp) {
             nLowestTimestamp = timestamp;
         }