Merge #16465: test: Test p2sh-witness and bech32 in wallet_import_rescan

fa3c6575cac5e3841797980fe60b8368ae579dba lint: Add false positive to python dead code linter (MarcoFalke)
fa25668e1c8982548f1c6f94780709c625811469 test: Test p2sh-witness and bech32 in wallet_import_rescan (MarcoFalke)
fa79af298917d501cee26370fdf9d44d05133d15 test: Replace fragile "rng" with call to random() (MarcoFalke)
fac3dcf7d052586548f2100a0d576618a85741f9 test: Generate one block for each send in wallet_import_rescan (MarcoFalke)

Pull request description:

  This adds test coverage for segwit in the `wallet_import_rescan` test, among other cleanups.

ACKs for top commit:
  jnewbery:
    ACK fa3c6575cac5e3841797980fe60b8368ae579dba

Tree-SHA512: 877741763c62c1bf9d868864a1e3f0699857e8c028e9fcd65c7eeb73600c22cbe97b7b51093737743d9e87bcb991c1fe1086f673e18765aef0fcfe27951402f0

5 files changed, 113 insertions, 28 deletions

diff --git a/.travis/lint_04_install.sh b/.travis/lint_04_install.sh
index 62174620f2..20bff368a5 100755
--- a/.travis/lint_04_install.sh
+++ b/.travis/lint_04_install.sh
@@ -6,9 +6,9 @@
 
 export LC_ALL=C
 
-travis_retry pip install codespell==1.15.0
-travis_retry pip install flake8==3.5.0
-travis_retry pip install vulture==0.29
+travis_retry pip3 install codespell==1.15.0
+travis_retry pip3 install flake8==3.5.0
+travis_retry pip3 install vulture==0.29
 
 SHELLCHECK_VERSION=v0.6.0
 curl -s "https://storage.googleapis.com/shellcheck/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/
diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py
index f36cffe957..194f2f061b 100644
--- a/test/functional/test_framework/address.py
+++ b/test/functional/test_framework/address.py
@@ -4,6 +4,8 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Encode and decode BASE58, P2PKH and P2SH addresses."""
 
+import enum
+
 from .script import hash256, hash160, sha256, CScript, OP_0
 from .util import hex_str_to_bytes
 
@@ -11,6 +13,13 @@ from . import segwit_addr
 
 ADDRESS_BCRT1_UNSPENDABLE = 'bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj'
 
+
+class AddressType(enum.Enum):
+    bech32 = 'bech32'
+    p2sh_segwit = 'p2sh-segwit'
+    legacy = 'legacy'  # P2PKH
+
+
 chars = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz'
 
 
diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index 47c97f62bf..4e20892596 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -20,6 +20,7 @@ happened previously.
 """
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.address import AddressType
 from test_framework.util import (
     connect_nodes,
     assert_equal,
@@ -27,21 +28,30 @@ from test_framework.util import (
 )
 
 import collections
+from decimal import Decimal
 import enum
 import itertools
+import random
 
 Call = enum.Enum("Call", "single multiaddress multiscript")
 Data = enum.Enum("Data", "address pub priv")
 Rescan = enum.Enum("Rescan", "no yes late_timestamp")
 
 
-class Variant(collections.namedtuple("Variant", "call data rescan prune")):
+class Variant(collections.namedtuple("Variant", "call data address_type rescan prune")):
     """Helper for importing one key and verifying scanned transactions."""
 
     def do_import(self, timestamp):
         """Call one key import RPC."""
         rescan = self.rescan == Rescan.yes
 
+        assert_equal(self.address["solvable"], True)
+        assert_equal(self.address["isscript"], self.address_type == AddressType.p2sh_segwit)
+        assert_equal(self.address["iswitness"], self.address_type == AddressType.bech32)
+        if self.address["isscript"]:
+            assert_equal(self.address["embedded"]["isscript"], False)
+            assert_equal(self.address["embedded"]["iswitness"], True)
+
         if self.call == Call.single:
             if self.data == Data.address:
                 response = self.node.importaddress(address=self.address["address"], label=self.label, rescan=rescan)
@@ -52,7 +62,7 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
             assert_equal(response, None)
 
         elif self.call in (Call.multiaddress, Call.multiscript):
-            response = self.node.importmulti([{
+            request = {
                 "scriptPubKey": {
                     "address": self.address["address"]
                 } if self.call == Call.multiaddress else self.address["scriptPubKey"],
@@ -61,13 +71,21 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
                 "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)})
+            }
+            if self.address_type == AddressType.p2sh_segwit and self.data != Data.address:
+                # We need solving data when providing a pubkey or privkey as data
+                request.update({"redeemscript": self.address['embedded']['scriptPubKey']})
+            response = self.node.importmulti(
+                requests=[request],
+                options={"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)},
+            )
             assert_equal(response, [{"success": True}])
 
-    def check(self, txid=None, amount=None, confirmations=None):
+    def check(self, txid=None, amount=None, confirmation_height=None):
         """Verify that listtransactions/listreceivedbyaddress return expected values."""
 
         txs = self.node.listtransactions(label=self.label, count=10000, include_watchonly=True)
+        current_height = self.node.getblockcount()
         assert_equal(len(txs), self.expected_txs)
 
         addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address'])
@@ -82,13 +100,13 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
             assert_equal(tx["category"], "receive")
             assert_equal(tx["label"], self.label)
             assert_equal(tx["txid"], txid)
-            assert_equal(tx["confirmations"], confirmations)
+            assert_equal(tx["confirmations"], 1 + current_height - confirmation_height)
             assert_equal("trusted" not in tx, True)
 
             address, = [ad for ad in addresses if txid in ad["txids"]]
             assert_equal(address["address"], self.address["address"])
             assert_equal(address["amount"], self.expected_balance)
-            assert_equal(address["confirmations"], confirmations)
+            assert_equal(address["confirmations"], 1 + current_height - confirmation_height)
             # Verify the transaction is correctly marked watchonly depending on
             # whether the transaction pays to an imported public key or
             # imported private key. The test setup ensures that transaction
@@ -102,7 +120,7 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
 
 
 # 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))]
+IMPORT_VARIANTS = [Variant(*variants) for variants in itertools.product(Call, Data, AddressType, 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
@@ -116,6 +134,13 @@ IMPORT_NODES = [ImportNode(*fields) for fields in itertools.product((False, True
 # Rescans start at the earliest block up to 2 hours before the key timestamp.
 TIMESTAMP_WINDOW = 2 * 60 * 60
 
+AMOUNT_DUST = 0.00000546
+
+
+def get_rand_amount():
+    r = random.uniform(AMOUNT_DUST, 1)
+    return Decimal(str(round(r, 8)))
+
 
 class ImportRescanTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -125,12 +150,12 @@ class ImportRescanTest(BitcoinTestFramework):
         self.skip_if_no_wallet()
 
     def setup_network(self):
-        extra_args = [["-addresstype=legacy"] for _ in range(self.num_nodes)]
+        self.extra_args = [[]] * self.num_nodes
         for i, import_node in enumerate(IMPORT_NODES, 2):
             if import_node.prune:
-                extra_args[i] += ["-prune=1"]
+                self.extra_args[i] += ["-prune=1"]
 
-        self.add_nodes(self.num_nodes, extra_args=extra_args)
+        self.add_nodes(self.num_nodes, extra_args=self.extra_args)
 
         # Import keys with pruning disabled
         self.start_nodes(extra_args=[[]] * self.num_nodes)
@@ -147,17 +172,23 @@ class ImportRescanTest(BitcoinTestFramework):
         # each possible type of wallet import RPC.
         for i, variant in enumerate(IMPORT_VARIANTS):
             variant.label = "label {} {}".format(i, variant)
-            variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(variant.label))
+            variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(
+                label=variant.label,
+                address_type=variant.address_type.value,
+            ))
             variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
-            variant.initial_amount = 1 - (i + 1) / 64
+            variant.initial_amount = get_rand_amount()
             variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
+            self.nodes[0].generate(1)  # Generate one block for each send
+            variant.confirmation_height = self.nodes[0].getblockcount()
+            variant.timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"]
 
-        # Generate a block containing the initial transactions, then another
-        # block further in the future (past the rescan window).
-        self.nodes[0].generate(1)
+        # Generate a block further in the future (past the rescan window).
         assert_equal(self.nodes[0].getrawmempool(), [])
-        timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"]
-        set_node_times(self.nodes, timestamp + TIMESTAMP_WINDOW + 1)
+        set_node_times(
+            self.nodes,
+            self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] + TIMESTAMP_WINDOW + 1,
+        )
         self.nodes[0].generate(1)
         self.sync_all()
 
@@ -167,11 +198,11 @@ class ImportRescanTest(BitcoinTestFramework):
             self.log.info('Run import for variant {}'.format(variant))
             expect_rescan = variant.rescan == Rescan.yes
             variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))]
-            variant.do_import(timestamp)
+            variant.do_import(variant.timestamp)
             if expect_rescan:
                 variant.expected_balance = variant.initial_amount
                 variant.expected_txs = 1
-                variant.check(variant.initial_txid, variant.initial_amount, 2)
+                variant.check(variant.initial_txid, variant.initial_amount, variant.confirmation_height)
             else:
                 variant.expected_balance = 0
                 variant.expected_txs = 0
@@ -179,11 +210,11 @@ class ImportRescanTest(BitcoinTestFramework):
 
         # Create new transactions sending to each address.
         for i, variant in enumerate(IMPORT_VARIANTS):
-            variant.sent_amount = 1 - (2 * i + 1) / 128
+            variant.sent_amount = get_rand_amount()
             variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount)
+            self.nodes[0].generate(1)  # Generate one block for each send
+            variant.confirmation_height = self.nodes[0].getblockcount()
 
-        # Generate a block containing the new transactions.
-        self.nodes[0].generate(1)
         assert_equal(self.nodes[0].getrawmempool(), [])
         self.sync_all()
 
@@ -192,7 +223,7 @@ class ImportRescanTest(BitcoinTestFramework):
             self.log.info('Run check for variant {}'.format(variant))
             variant.expected_balance += variant.sent_amount
             variant.expected_txs += 1
-            variant.check(variant.sent_txid, variant.sent_amount, 1)
+            variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height)
 
 if __name__ == "__main__":
     ImportRescanTest().main()
diff --git a/test/lint/lint-python-dead-code-whitelist b/test/lint/lint-python-dead-code-whitelist
new file mode 100644
index 0000000000..2522c8fa1c
--- /dev/null
+++ b/test/lint/lint-python-dead-code-whitelist
@@ -0,0 +1,45 @@
+BadInputOutpointIndex  # unused class (test/functional/data/invalid_txs.py)
+_.carbon_path  # unused attribute (contrib/macdeploy/custom_dsstore.py)
+connection_lost  # unused function (test/functional/test_framework/mininode.py)
+connection_made  # unused function (test/functional/test_framework/mininode.py)
+_.converter  # unused attribute (test/functional/test_framework/test_framework.py)
+_.daemon  # unused attribute (test/functional/test_framework/socks5.py)
+data_received  # unused function (test/functional/test_framework/mininode.py)
+DuplicateInput  # unused class (test/functional/data/invalid_txs.py)
+_.filename  # unused attribute (contrib/macdeploy/custom_dsstore.py)
+InvalidOPIFConstruction  # unused class (test/functional/data/invalid_txs.py)
+_.is_compressed  # unused property (test/functional/test_framework/key.py)
+legacy  # unused variable (test/functional/test_framework/address.py)
+msg_generic  # unused class (test/functional/test_framework/messages.py)
+NonexistentInput  # unused class (test/functional/data/invalid_txs.py)
+on_addr  # unused function (test/functional/test_framework/mininode.py)
+on_blocktxn  # unused function (test/functional/test_framework/mininode.py)
+on_block  # unused function (test/functional/test_framework/mininode.py)
+on_cmpctblock  # unused function (test/functional/test_framework/mininode.py)
+on_feefilter  # unused function (test/functional/test_framework/mininode.py)
+on_getaddr  # unused function (test/functional/test_framework/mininode.py)
+on_getblocks  # unused function (test/functional/test_framework/mininode.py)
+on_getblocktxn  # unused function (test/functional/test_framework/mininode.py)
+on_getdata  # unused function (test/functional/test_framework/mininode.py)
+on_getheaders  # unused function (test/functional/test_framework/mininode.py)
+on_headers  # unused function (test/functional/test_framework/mininode.py)
+on_inv  # unused function (test/functional/test_framework/mininode.py)
+on_mempool  # unused function (test/functional/test_framework/mininode.py)
+on_notfound  # unused function (test/functional/test_framework/mininode.py)
+on_ping  # unused function (test/functional/test_framework/mininode.py)
+on_pong  # unused function (test/functional/test_framework/mininode.py)
+on_reject  # unused function (test/functional/test_framework/mininode.py)
+on_sendcmpct  # unused function (test/functional/test_framework/mininode.py)
+on_sendheaders  # unused function (test/functional/test_framework/mininode.py)
+on_tx  # unused function (test/functional/test_framework/mininode.py)
+on_verack  # unused function (test/functional/test_framework/mininode.py)
+on_version  # unused function (test/functional/test_framework/mininode.py)
+_.optionxform  # unused attribute (test/util/bitcoin-util-test.py)
+OutputMissing  # unused class (test/functional/data/invalid_txs.py)
+_.posix_path  # unused attribute (contrib/macdeploy/custom_dsstore.py)
+profile_with_perf  # unused function (test/functional/test_framework/test_node.py)
+SizeTooSmall  # unused class (test/functional/data/invalid_txs.py)
+SpendNegative  # unused class (test/functional/data/invalid_txs.py)
+SpendTooMuch  # unused class (test/functional/data/invalid_txs.py)
+TooManySigops  # unused class (test/functional/data/invalid_txs.py)
+verify_ecdsa  # unused function (test/functional/test_framework/key.py)
diff --git a/test/lint/lint-python-dead-code.sh b/test/lint/lint-python-dead-code.sh
index 588ba428d7..77bf5990a7 100755
--- a/test/lint/lint-python-dead-code.sh
+++ b/test/lint/lint-python-dead-code.sh
@@ -15,5 +15,5 @@ fi
 
 vulture \
     --min-confidence 60 \
-    --ignore-names "argtypes,connection_lost,connection_made,converter,data_received,daemon,errcheck,is_compressed,is_valid,verify_ecdsa,msg_generic,on_*,optionxform,restype,profile_with_perf" \
-    $(git ls-files -- "*.py" ":(exclude)contrib/" ":(exclude)test/functional/data/invalid_txs.py")
+    $(git rev-parse --show-toplevel) \
+    $(dirname "${BASH_SOURCE[0]}")/lint-python-dead-code-whitelist