Merge #9761: Use 2 hour grace period for key timestamps in importmulti rescans

e662af3 Use 2 hour grace period for key timestamps in importmulti rescans (Russell Yanofsky)
38d3e9e [qa] Extend import-rescan.py to test imports on pruned nodes. (Russell Yanofsky)
c28583d [qa] Extend import-rescan.py to test specific key timestamps (Russell Yanofsky)
8be0866 [qa] Simplify import-rescan.py (Russell Yanofsky)

2 files changed, 145 insertions, 115 deletions

diff --git a/qa/rpc-tests/import-rescan.py b/qa/rpc-tests/import-rescan.py
index 8f60e63e2e..54cc6d2642 100755
--- a/qa/rpc-tests/import-rescan.py
+++ b/qa/rpc-tests/import-rescan.py
@@ -2,55 +2,105 @@
 # Copyright (c) 2014-2016 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 rescan behavior of importaddress, importpubkey, importprivkey, and
+importmulti RPCs with different types of keys and rescan options.
+
+In the first part of the test, node 0 creates an address for each type of
+import RPC call and sends BTC to it. Then other nodes import the addresses,
+and the test makes listtransactions and getbalance calls to confirm that the
+importing node either did or did not execute rescans picking up the send
+transactions.
+
+In the second part of the test, node 0 sends more BTC to each address, and the
+test makes more listtransactions and getbalance calls to confirm that the
+importing nodes pick up the new transactions regardless of whether rescans
+happened previously.
+"""
+
+from test_framework.authproxy import JSONRPCException
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (start_nodes, connect_nodes, sync_blocks, assert_equal)
+from test_framework.util import (start_nodes, connect_nodes, sync_blocks, assert_equal, set_node_times)
 from decimal import Decimal
 
 import collections
 import enum
 import itertools
-import functools
 
 Call = enum.Enum("Call", "single multi")
 Data = enum.Enum("Data", "address pub priv")
-ImportNode = collections.namedtuple("ImportNode", "rescan")
-
-
-def call_import_rpc(call, data, address, scriptPubKey, pubkey, key, label, node, rescan):
-    """Helper that calls a wallet import RPC on a bitcoin node."""
-    watchonly = data != Data.priv
-    if call == Call.single:
-        if data == Data.address:
-            response = node.importaddress(address, label, rescan)
-        elif data == Data.pub:
-            response = node.importpubkey(pubkey, label, rescan)
-        elif data == Data.priv:
-            response = node.importprivkey(key, label, rescan)
-        assert_equal(response, None)
-    elif call == Call.multi:
-        response = node.importmulti([{
-            "scriptPubKey": {
-                "address": address
-            },
-            "timestamp": "now",
-            "pubkeys": [pubkey] if data == Data.pub else [],
-            "keys": [key] if data == Data.priv else [],
-            "label": label,
-            "watchonly": watchonly
-        }], {"rescan": rescan})
-        assert_equal(response, [{"success": True}])
-    return watchonly
-
-
-# List of RPCs that import a wallet key or address in various ways.
-IMPORT_RPCS = [functools.partial(call_import_rpc, call, data) for call, data in itertools.product(Call, Data)]
-
-# List of bitcoind nodes that will import keys.
-IMPORT_NODES = [
-    ImportNode(rescan=True),
-    ImportNode(rescan=False),
-]
+Rescan = enum.Enum("Rescan", "no yes late_timestamp")
+
+
+class Variant(collections.namedtuple("Variant", "call data rescan prune")):
+    """Helper for importing one key and verifying scanned transactions."""
+
+    def do_import(self, timestamp):
+        """Call one key import RPC."""
+
+        if self.call == Call.single:
+            if self.data == Data.address:
+                response, error = try_rpc(self.node.importaddress, self.address["address"], self.label,
+                                          self.rescan == Rescan.yes)
+            elif self.data == Data.pub:
+                response, error = try_rpc(self.node.importpubkey, self.address["pubkey"], self.label,
+                                          self.rescan == Rescan.yes)
+            elif self.data == Data.priv:
+                response, error = try_rpc(self.node.importprivkey, self.key, self.label, self.rescan == Rescan.yes)
+            assert_equal(response, None)
+            assert_equal(error, {'message': 'Rescan is disabled in pruned mode',
+                                 'code': -4} if self.expect_disabled else None)
+        elif self.call == Call.multi:
+            response = self.node.importmulti([{
+                "scriptPubKey": {
+                    "address": self.address["address"]
+                },
+                "timestamp": timestamp + RESCAN_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0),
+                "pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [],
+                "keys": [self.key] if self.data == Data.priv else [],
+                "label": self.label,
+                "watchonly": self.data != Data.priv
+            }], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)})
+            assert_equal(response, [{"success": True}])
+
+    def check(self, txid=None, amount=None, confirmations=None):
+        """Verify that getbalance/listtransactions return expected values."""
+
+        balance = self.node.getbalance(self.label, 0, True)
+        assert_equal(balance, self.expected_balance)
+
+        txs = self.node.listtransactions(self.label, 10000, 0, True)
+        assert_equal(len(txs), self.expected_txs)
+
+        if txid is not None:
+            tx, = [tx for tx in txs if tx["txid"] == txid]
+            assert_equal(tx["account"], self.label)
+            assert_equal(tx["address"], self.address["address"])
+            assert_equal(tx["amount"], amount)
+            assert_equal(tx["category"], "receive")
+            assert_equal(tx["label"], self.label)
+            assert_equal(tx["txid"], txid)
+            assert_equal(tx["confirmations"], confirmations)
+            assert_equal("trusted" not in tx, True)
+            if self.data != Data.priv:
+                assert_equal(tx["involvesWatchonly"], True)
+            else:
+                assert_equal("involvesWatchonly" not in tx, True)
+
+
+# List of Variants for each way a key or address could be imported.
+IMPORT_VARIANTS = [Variant(*variants) for variants in itertools.product(Call, Data, Rescan, (False, True))]
+
+# List of nodes to import keys to. Half the nodes will have pruning disabled,
+# half will have it enabled. Different nodes will be used for imports that are
+# expected to cause rescans, and imports that are not expected to cause
+# rescans, in order to prevent rescans during later imports picking up
+# transactions associated with earlier imports. This makes it easier to keep
+# track of expected balances and transactions.
+ImportNode = collections.namedtuple("ImportNode", "prune rescan")
+IMPORT_NODES = [ImportNode(*fields) for fields in itertools.product((False, True), repeat=2)]
+
+# Rescans start at the earliest block up to 2 hours before the key timestamp.
+RESCAN_WINDOW = 2 * 60 * 60
 
 
 class ImportRescanTest(BitcoinTestFramework):
@@ -60,6 +110,10 @@ class ImportRescanTest(BitcoinTestFramework):
 
     def setup_network(self):
         extra_args = [["-debug=1"] for _ in range(self.num_nodes)]
+        for i, import_node in enumerate(IMPORT_NODES, 1):
+            if import_node.prune:
+                extra_args[i] += ["-prune=1"]
+
         self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
         for i in range(1, self.num_nodes):
             connect_nodes(self.nodes[i], 0)
@@ -67,89 +121,64 @@ class ImportRescanTest(BitcoinTestFramework):
     def run_test(self):
         # Create one transaction on node 0 with a unique amount and label for
         # each possible type of wallet import RPC.
-        import_rpc_variants = []
-        for i, import_rpc in enumerate(IMPORT_RPCS):
-            label = "label{}".format(i)
-            addr = self.nodes[0].validateaddress(self.nodes[0].getnewaddress(label))
-            key = self.nodes[0].dumpprivkey(addr["address"])
-            amount = 24.9375 - i * .0625
-            txid = self.nodes[0].sendtoaddress(addr["address"], amount)
-            import_rpc = functools.partial(import_rpc, addr["address"], addr["scriptPubKey"], addr["pubkey"], key,
-                                           label)
-            import_rpc_variants.append((import_rpc, label, amount, txid, addr))
-
+        for i, variant in enumerate(IMPORT_VARIANTS):
+            variant.label = "label {} {}".format(i, variant)
+            variant.address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress(variant.label))
+            variant.key = self.nodes[0].dumpprivkey(variant.address["address"])
+            variant.initial_amount = 25 - (i + 1) / 4.0
+            variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
+
+        # Generate a block containing the initial transactions, then another
+        # block further in the future (past the rescan window).
         self.nodes[0].generate(1)
         assert_equal(self.nodes[0].getrawmempool(), [])
+        timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"]
+        set_node_times(self.nodes, timestamp + RESCAN_WINDOW + 1)
+        self.nodes[0].generate(1)
         sync_blocks(self.nodes)
 
-        # For each importing node and variation of wallet import RPC, invoke
-        # the RPC and check the results from getbalance and listtransactions.
-        for node, import_node in zip(self.nodes[1:], IMPORT_NODES):
-            for import_rpc, label, amount, txid, addr in import_rpc_variants:
-                watchonly = import_rpc(node, import_node.rescan)
-
-                balance = node.getbalance(label, 0, True)
-                if import_node.rescan:
-                    assert_equal(balance, amount)
-                else:
-                    assert_equal(balance, 0)
-
-                txs = node.listtransactions(label, 10000, 0, True)
-                if import_node.rescan:
-                    assert_equal(len(txs), 1)
-                    assert_equal(txs[0]["account"], label)
-                    assert_equal(txs[0]["address"], addr["address"])
-                    assert_equal(txs[0]["amount"], amount)
-                    assert_equal(txs[0]["category"], "receive")
-                    assert_equal(txs[0]["label"], label)
-                    assert_equal(txs[0]["txid"], txid)
-                    assert_equal(txs[0]["confirmations"], 1)
-                    assert_equal("trusted" not in txs[0], True)
-                    if watchonly:
-                        assert_equal(txs[0]["involvesWatchonly"], True)
-                    else:
-                        assert_equal("involvesWatchonly" not in txs[0], True)
-                else:
-                    assert_equal(len(txs), 0)
-
-        # Create spends for all the imported addresses.
-        spend_txids = []
+        # For each variation of wallet key import, invoke the import RPC and
+        # check the results from getbalance and listtransactions.
+        for variant in IMPORT_VARIANTS:
+            variant.expect_disabled = variant.rescan == Rescan.yes and variant.prune and variant.call == Call.single
+            expect_rescan = variant.rescan == Rescan.yes and not variant.expect_disabled
+            variant.node = self.nodes[1 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))]
+            variant.do_import(timestamp)
+            if expect_rescan:
+                variant.expected_balance = variant.initial_amount
+                variant.expected_txs = 1
+                variant.check(variant.initial_txid, variant.initial_amount, 2)
+            else:
+                variant.expected_balance = 0
+                variant.expected_txs = 0
+                variant.check()
+
+        # Create new transactions sending to each address.
         fee = self.nodes[0].getnetworkinfo()["relayfee"]
-        for import_rpc, label, amount, txid, addr in import_rpc_variants:
-            raw_tx = self.nodes[0].getrawtransaction(txid)
-            decoded_tx = self.nodes[0].decoderawtransaction(raw_tx)
-            input_vout = next(out["n"] for out in decoded_tx["vout"]
-                              if out["scriptPubKey"]["addresses"] == [addr["address"]])
-            inputs = [{"txid": txid, "vout": input_vout}]
-            outputs = {self.nodes[0].getnewaddress(): Decimal(amount) - fee}
-            raw_spend_tx = self.nodes[0].createrawtransaction(inputs, outputs)
-            signed_spend_tx = self.nodes[0].signrawtransaction(raw_spend_tx)
-            spend_txid = self.nodes[0].sendrawtransaction(signed_spend_tx["hex"])
-            spend_txids.append(spend_txid)
+        for i, variant in enumerate(IMPORT_VARIANTS):
+            variant.sent_amount = 25 - (2 * i + 1) / 8.0
+            variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount)
 
+        # Generate a block containing the new transactions.
         self.nodes[0].generate(1)
         assert_equal(self.nodes[0].getrawmempool(), [])
         sync_blocks(self.nodes)
 
-        # Check the results from getbalance and listtransactions after the spends.
-        for node, import_node in zip(self.nodes[1:], IMPORT_NODES):
-            txs = node.listtransactions("*", 10000, 0, True)
-            for (import_rpc, label, amount, txid, addr), spend_txid in zip(import_rpc_variants, spend_txids):
-                balance = node.getbalance(label, 0, True)
-                spend_tx = [tx for tx in txs if tx["txid"] == spend_txid]
-                if import_node.rescan:
-                    assert_equal(balance, amount)
-                    assert_equal(len(spend_tx), 1)
-                    assert_equal(spend_tx[0]["account"], "")
-                    assert_equal(spend_tx[0]["amount"] + spend_tx[0]["fee"], -amount)
-                    assert_equal(spend_tx[0]["category"], "send")
-                    assert_equal("label" not in spend_tx[0], True)
-                    assert_equal(spend_tx[0]["confirmations"], 1)
-                    assert_equal("trusted" not in spend_tx[0], True)
-                    assert_equal("involvesWatchonly" not in txs[0], True)
-                else:
-                    assert_equal(balance, 0)
-                    assert_equal(spend_tx, [])
+        # Check the latest results from getbalance and listtransactions.
+        for variant in IMPORT_VARIANTS:
+            if not variant.expect_disabled:
+                variant.expected_balance += variant.sent_amount
+                variant.expected_txs += 1
+                variant.check(variant.sent_txid, variant.sent_amount, 1)
+            else:
+                variant.check()
+
+
+def try_rpc(func, *args, **kwargs):
+    try:
+        return func(*args, **kwargs), None
+    except JSONRPCException as e:
+        return None, e.error
 
 
 if __name__ == "__main__":
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 908655d411..30f2414675 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -988,7 +988,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
             "                                                              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"
+            "                                                              0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n"
+            "                                                              creation time of all keys being imported by the importmulti call will be scanned.\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"
@@ -1072,7 +1073,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
     }
 
     if (fRescan && fRunScan && requests.size() && nLowestTimestamp <= chainActive.Tip()->GetBlockTimeMax()) {
-        CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(nLowestTimestamp) : chainActive.Genesis();
+        CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(std::max<int64_t>(nLowestTimestamp - 7200, 0)) : chainActive.Genesis();
 
         if (pindex) {
             pwalletMain->ScanForWalletTransactions(pindex, true);