Merge bitcoin/bitcoin#23420: test: Correct MyPy typing for subtest decorator

467fe57 test: Correct MyPy typing for subtest decorator (Pavel Safronov)

Pull request description:

  This is the part of the effort to make python typing correct bitcoin/bitcoin#19389

  The typing of the `subtest` decorator within `p2p_segwit.py` test file was incorrect.

  Since `subtest` function is defined as a member of the class, it expects `self` as a first argument, and it is not provided. Hence the typing errors (that are currently suppressed by `type: ignore`).

  ```
  (venv) vagrant@ubuntu-focal:/vagrant/test/functional$ mypy p2p_segwit.py
  p2p_segwit.py:298: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:327: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:358: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:447: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:519: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:561: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:659: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:670: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:737: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:826: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:866: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:941: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:977: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1052: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1089: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1136: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1220: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1312: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1406: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1440: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1543: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1729: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1782: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1881: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:1983: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  p2p_segwit.py:2027: error: Argument 1 to "subtest" has incompatible type "Callable[[SegWitTest], Any]"; expected "SegWitTest"
  Found 26 errors in 1 file (checked 1 source file)
  ```

  However, the tests are passing, because there is no `self` argument passed when it is called as a decorator.

  There is also suppressed pylint error `# noqa: N805` pointing to the same issue.
  ```
  N805 first argument of a method should be named 'self'
  ```

  So the solution is to move the `subtest` definition outside the class, so the `self` argument is no longer required.

  After doing so, both mypy and unittests are successfully passing:

  ```
  (venv) vagrant@ubuntu-focal:/vagrant/test/functional$ mypy p2p_segwit.py
  Success: no issues found in 1 source file
  ```

  ```
  (venv) vagrant@ubuntu-focal:/vagrant/test/functional$ ./test_runner.py p2p_segwit
  Temporary test directory at /tmp/test_runner__🏃_20211103_011449
  Running Unit Tests for Test Framework Modules
  ..........
  ----------------------------------------------------------------------
  Ran 10 tests in 0.546s

  OK
  Remaining jobs: [p2p_segwit.py]
  1/1 - p2p_segwit.py passed, Duration: 81 s

  TEST          | STATUS    | DURATION

  p2p_segwit.py | ✓ Passed  | 81 s

  ALL           | ✓ Passed  | 81 s (accumulated)
  Runtime: 81 s
  ```
  ```

ACKs for top commit:
  fanquake:
    ACK 467fe57

Tree-SHA512: e4c3e2d284f47a6bfbf4af22d4021123cdd9c2ea16ec90a91b466ad1a5af615bb4e15959e6cf56c788701d7e7cbda91a8ffc4347965095c3384eae3d28f261af

Author: fanquake

test/functional/p2p_segwit.py

@@ -101,6 +101,23 @@ def __init__(self, sha256, n, value):
         self.n = n
         self.nValue = value
 
+
+def subtest(func):
+    """Wraps the subtests for logging and state assertions."""
+    def func_wrapper(self, *args, **kwargs):
+        self.log.info("Subtest: {} (Segwit active = {})".format(func.__name__, self.segwit_active))
+        # Assert segwit status is as expected
+        assert_equal(softfork_active(self.nodes[0], 'segwit'), self.segwit_active)
+        func(self, *args, **kwargs)
+        # Each subtest should leave some utxos for the next subtest
+        assert self.utxo
+        self.sync_blocks()
+        # Assert segwit status is as expected at end of subtest
+        assert_equal(softfork_active(self.nodes[0], 'segwit'), self.segwit_active)
+
+    return func_wrapper
+
+
 def sign_p2pk_witness_input(script, tx_to, in_idx, hashtype, value, key):
     """Add signature for a P2PK witness script."""
     tx_hash = SegwitV0SignatureHash(script, tx_to, in_idx, hashtype, value)
@@ -280,22 +297,7 @@ def run_test(self):
 
     # Individual tests
 
-    def subtest(func):  # noqa: N805
-        """Wraps the subtests for logging and state assertions."""
-        def func_wrapper(self, *args, **kwargs):
-            self.log.info("Subtest: {} (Segwit active = {})".format(func.__name__, self.segwit_active))
-            # Assert segwit status is as expected
-            assert_equal(softfork_active(self.nodes[0], 'segwit'), self.segwit_active)
-            func(self, *args, **kwargs)
-            # Each subtest should leave some utxos for the next subtest
-            assert self.utxo
-            self.sync_blocks()
-            # Assert segwit status is as expected at end of subtest
-            assert_equal(softfork_active(self.nodes[0], 'segwit'), self.segwit_active)
-
-        return func_wrapper
-
-    @subtest  # type: ignore
+    @subtest
     def test_non_witness_transaction(self):
         """See if sending a regular transaction works, and create a utxo to use in later tests."""
         # Mine a block with an anyone-can-spend coinbase,
@@ -324,7 +326,7 @@ def test_non_witness_transaction(self):
         self.utxo.append(UTXO(tx.sha256, 0, 49 * 100000000))
         self.generate(self.nodes[0], 1)
 
-    @subtest  # type: ignore
+    @subtest
     def test_unnecessary_witness_before_segwit_activation(self):
         """Verify that blocks with witnesses are rejected before activation."""
 
@@ -355,7 +357,7 @@ def test_unnecessary_witness_before_segwit_activation(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx.sha256, 0, tx.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_block_relay(self):
         """Test that block requests to NODE_WITNESS peer are with MSG_WITNESS_FLAG.
 
@@ -444,7 +446,7 @@ def test_block_relay(self):
             self.old_node.announce_tx_and_wait_for_getdata(block4.vtx[0])
             assert block4.sha256 not in self.old_node.getdataset
 
-    @subtest  # type: ignore
+    @subtest
     def test_v0_outputs_arent_spendable(self):
         """Test that v0 outputs aren't spendable before segwit activation.
 
@@ -516,7 +518,7 @@ def test_v0_outputs_arent_spendable(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(txid, 2, value))
 
-    @subtest  # type: ignore
+    @subtest
     def test_witness_tx_relay_before_segwit_activation(self):
 
         # Generate a transaction that doesn't require a witness, but send it
@@ -558,7 +560,7 @@ def test_witness_tx_relay_before_segwit_activation(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx_hash, 0, tx_value))
 
-    @subtest  # type: ignore
+    @subtest
     def test_standardness_v0(self):
         """Test V0 txout standardness.
 
@@ -656,7 +658,7 @@ def test_standardness_v0(self):
         self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
         assert_equal(len(self.nodes[1].getrawmempool()), 0)
 
-    @subtest  # type: ignore
+    @subtest
     def advance_to_segwit_active(self):
         """Mine enough blocks to activate segwit."""
         assert not softfork_active(self.nodes[0], 'segwit')
@@ -667,7 +669,7 @@ def advance_to_segwit_active(self):
         assert softfork_active(self.nodes[0], 'segwit')
         self.segwit_active = True
 
-    @subtest  # type: ignore
+    @subtest
     def test_p2sh_witness(self):
         """Test P2SH wrapped witness programs."""
 
@@ -734,7 +736,7 @@ def test_p2sh_witness(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(spend_tx.sha256, 0, spend_tx.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_witness_commitments(self):
         """Test witness commitments.
 
@@ -823,7 +825,7 @@ def test_witness_commitments(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_block_malleability(self):
 
         # Make sure that a block that has too big a virtual size
@@ -863,7 +865,7 @@ def test_block_malleability(self):
         block.vtx[0].wit.vtxinwit[0].scriptWitness.stack = [ser_uint256(0)]
         test_witness_block(self.nodes[0], self.test_node, block, accepted=True)
 
-    @subtest  # type: ignore
+    @subtest
     def test_witness_block_size(self):
         # TODO: Test that non-witness carrying blocks can't exceed 1MB
         # Skipping this test for now; this is covered in feature_block.py
@@ -938,7 +940,7 @@ def test_witness_block_size(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(block.vtx[-1].sha256, 0, block.vtx[-1].vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_submit_block(self):
         """Test that submitblock adds the nonce automatically when possible."""
         block = self.build_next_block()
@@ -974,7 +976,7 @@ def test_submit_block(self):
         # Tip should not advance!
         assert self.nodes[0].getbestblockhash() != block_2.hash
 
-    @subtest  # type: ignore
+    @subtest
     def test_extra_witness_data(self):
         """Test extra witness data in a transaction."""
 
@@ -1049,7 +1051,7 @@ def test_extra_witness_data(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_max_witness_push_length(self):
         """Test that witness stack can only allow up to 520 byte pushes."""
 
@@ -1086,7 +1088,7 @@ def test_max_witness_push_length(self):
         self.utxo.pop()
         self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_max_witness_script_length(self):
         """Test that witness outputs greater than 10kB can't be spent."""
 
@@ -1133,7 +1135,7 @@ def test_max_witness_script_length(self):
         self.utxo.pop()
         self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_witness_input_length(self):
         """Test that vin length must match vtxinwit length."""
 
@@ -1217,7 +1219,7 @@ def serialize_with_witness(self):
         self.utxo.pop()
         self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_tx_relay_after_segwit_activation(self):
         """Test transaction relay after segwit activation.
 
@@ -1309,7 +1311,7 @@ def test_tx_relay_after_segwit_activation(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_segwit_versions(self):
         """Test validity of future segwit version transactions.
 
@@ -1403,7 +1405,7 @@ def test_segwit_versions(self):
         # Add utxo to our list
         self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_premature_coinbase_witness_spend(self):
 
         block = self.build_next_block()
@@ -1437,7 +1439,7 @@ def test_premature_coinbase_witness_spend(self):
         test_witness_block(self.nodes[0], self.test_node, block2, accepted=True)
         self.sync_blocks()
 
-    @subtest  # type: ignore
+    @subtest
     def test_uncompressed_pubkey(self):
         """Test uncompressed pubkey validity in segwit transactions.
 
@@ -1540,7 +1542,7 @@ def test_uncompressed_pubkey(self):
         test_witness_block(self.nodes[0], self.test_node, block, accepted=True)
         self.utxo.append(UTXO(tx5.sha256, 0, tx5.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_signature_version_1(self):
 
         key = ECKey()
@@ -1726,7 +1728,7 @@ def test_signature_version_1(self):
         for i in range(len(tx.vout)):
             self.utxo.append(UTXO(tx.sha256, i, tx.vout[i].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_non_standard_witness_blinding(self):
         """Test behavior of unnecessary witnesses in transactions does not blind the node for the transaction"""
 
@@ -1779,7 +1781,7 @@ def test_non_standard_witness_blinding(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_non_standard_witness(self):
         """Test detection of non-standard P2WSH witness"""
         pad = chr(1).encode('latin-1')
@@ -1878,7 +1880,7 @@ def test_non_standard_witness(self):
 
         self.utxo.pop(0)
 
-    @subtest  # type: ignore
+    @subtest
     def test_witness_sigops(self):
         """Test sigop counting is correct inside witnesses."""
 
@@ -1980,7 +1982,7 @@ def test_witness_sigops(self):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
 
-    @subtest  # type: ignore
+    @subtest
     def test_superfluous_witness(self):
         # Serialization of tx that puts witness flag to 3 always
         def serialize_with_bogus_witness(tx):
@@ -2024,7 +2026,7 @@ def serialize(self):
         with self.nodes[0].assert_debug_log(['Unknown transaction optional data']):
             self.test_node.send_and_ping(msg_bogus_tx(tx))
 
-    @subtest  # type: ignore
+    @subtest
     def test_wtxid_relay(self):
         # Use brand new nodes to avoid contamination from earlier tests
         self.wtx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=P2P_SERVICES)