Merge bitcoin/bitcoin#30723: lint: Speed up and fix flake8 checks

fafdb7d lint: Speed up flake8 checks (MarcoFalke)
faf17df lint: Document missing py_lint dependency (MarcoFalke)
faebeb8 lint: Remove python whitespace and shadowing lint rules (MarcoFalke)
7777047 lint: Remove python lint rules that are SyntaxError (MarcoFalke)
faaf3e5 test: [refactor] Fix F841 flake8 (MarcoFalke)
444421d test: [refactor] Fix E714 pycodestyle (MarcoFalke)

Pull request description:

  The checks have many issues:

  * Some checks that could in theory hide bugs are not applied -> Fix them and apply them going forward
  * Some checks are redundant Python 2 checks, or of low value -> Remove them
  * The checks are slow -> Speed them up from ~10 seconds to about ~20 milliseconds

ACKs for top commit:
  davidgumberg:
    review and tested reACK bitcoin/bitcoin@fafdb7d
  kevkevinpal:
    ACK [fafdb7d](bitcoin/bitcoin@fafdb7d)
  achow101:
    ACK fafdb7d

Tree-SHA512: a0488b722cfaf7071bd6848cd3be002e0b6c38af80d8b5cbb08613c0b174ef63277289f960db8ac31adb09fe563a4973203b8fb10b83cbcfdc6f0ef39bd04410

Author: achow101

ci/lint/04_install.sh

@@ -48,7 +48,6 @@ fi
 
 ${CI_RETRY_EXE} pip3 install \
   codespell==2.2.6 \
-  flake8==6.1.0 \
   lief==0.13.2 \
   mypy==1.4.1 \
   pyzmq==25.1.0 \

contrib/devtools/clang-format-diff.py

@@ -164,7 +164,7 @@ def main():
                 'Failed to run "%s" - %s"' % (" ".join(command), e.strerror)
             )
 
-        stdout, stderr = p.communicate()
+        stdout, _stderr = p.communicate()
         if p.returncode != 0:
             sys.exit(p.returncode)
 

test/functional/interface_usdt_coinselection.py

@@ -181,7 +181,7 @@ def run_test(self):
         # 5. aps_create_tx_internal (type 4)
         wallet.sendtoaddress(wallet.getnewaddress(), 10)
         events = self.get_tracepoints([1, 2, 3, 1, 4])
-        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
         assert_equal(success, True)
         assert_greater_than(change_pos, -1)
 
@@ -190,7 +190,7 @@ def run_test(self):
         # 1. normal_create_tx_internal (type 2)
         assert_raises_rpc_error(-6, "Insufficient funds", wallet.sendtoaddress, wallet.getnewaddress(), 102 * 50)
         events = self.get_tracepoints([2])
-        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
         assert_equal(success, False)
 
         self.log.info("Explicitly enabling APS results in 2 tracepoints")
@@ -200,7 +200,7 @@ def run_test(self):
         wallet.setwalletflag("avoid_reuse")
         wallet.sendtoaddress(address=wallet.getnewaddress(), amount=10, avoid_reuse=True)
         events = self.get_tracepoints([1, 2])
-        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
         assert_equal(success, True)
         assert_equal(use_aps, None)
 
@@ -213,7 +213,7 @@ def run_test(self):
         # 5. aps_create_tx_internal (type 4)
         wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True, avoid_reuse=False)
         events = self.get_tracepoints([1, 2, 3, 1, 4])
-        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
         assert_equal(success, True)
         assert_equal(change_pos, -1)
 
@@ -223,7 +223,7 @@ def run_test(self):
         # 2. normal_create_tx_internal (type 2)
         wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True)
         events = self.get_tracepoints([1, 2])
-        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events)
         assert_equal(success, True)
         assert_equal(change_pos, -1)
 

test/functional/mempool_package_rbf.py

@@ -189,7 +189,7 @@ def test_package_rbf_additional_fees(self):
         package_hex4, package_txns4 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE)
         node.submitpackage(package_hex4)
         self.assert_mempool_contents(expected=package_txns4)
-        package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE)
+        package_hex5, _package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE)
         pkg_results5 = node.submitpackage(package_hex5)
         assert 'package RBF failed: package feerate is less than or equal to parent feerate' in pkg_results5["package_msg"]
         self.assert_mempool_contents(expected=package_txns4)
@@ -336,16 +336,16 @@ def test_wrong_conflict_cluster_size_linear(self):
         self.assert_mempool_contents(expected=expected_txns)
 
         # Now make conflicting packages for each coin
-        package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
 
         package_result = node.submitpackage(package_hex1)
         assert_equal(f"package RBF failed: {parent_result['tx'].rehash()} has 2 descendants, max 1 allowed", package_result["package_msg"])
 
-        package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex2)
         assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has both ancestor and descendant, exceeding cluster limit of 2", package_result["package_msg"])
 
-        package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex3)
         assert_equal(f"package RBF failed: {grandchild_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"])
 
@@ -389,15 +389,15 @@ def test_wrong_conflict_cluster_size_parents_child(self):
         self.assert_mempool_contents(expected=expected_txns)
 
         # Now make conflicting packages for each coin
-        package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex1)
         assert_equal(f"package RBF failed: {parent1_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"])
 
-        package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex2)
         assert_equal(f"package RBF failed: {parent2_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"])
 
-        package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex3)
         assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"])
 
@@ -443,15 +443,15 @@ def test_wrong_conflict_cluster_size_parent_children(self):
         self.assert_mempool_contents(expected=expected_txns)
 
         # Now make conflicting packages for each coin
-        package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex1)
         assert_equal(f"package RBF failed: {parent_result['tx'].rehash()} has 2 descendants, max 1 allowed", package_result["package_msg"])
 
-        package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex2)
         assert_equal(f"package RBF failed: {child1_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"])
 
-        package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
+        package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE)
         package_result = node.submitpackage(package_hex3)
         assert_equal(f"package RBF failed: {child2_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"])
 
@@ -519,7 +519,7 @@ def test_insufficient_feerate(self):
 
         # Package 2 feerate is below the feerate of directly conflicted parent, so it fails even though
         # total fees are higher than the original package
-        package_hex2, package_txns2 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE - Decimal("0.00000001"), child_fee=DEFAULT_CHILD_FEE)
+        package_hex2, _package_txns2 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE - Decimal("0.00000001"), child_fee=DEFAULT_CHILD_FEE)
         pkg_results2 = node.submitpackage(package_hex2)
         assert_equal(pkg_results2["package_msg"], 'package RBF failed: insufficient feerate: does not improve feerate diagram')
         self.assert_mempool_contents(expected=package_txns1)

test/functional/mempool_sigoplimit.py

@@ -154,7 +154,7 @@ def create_bare_multisig_tx(utxo_to_spend=None):
             return (tx_utxo, tx)
 
         tx_parent_utxo, tx_parent = create_bare_multisig_tx()
-        tx_child_utxo, tx_child = create_bare_multisig_tx(tx_parent_utxo)
+        _tx_child_utxo, tx_child = create_bare_multisig_tx(tx_parent_utxo)
 
         # Separately, the parent tx is ok
         parent_individual_testres = self.nodes[0].testmempoolaccept([tx_parent.serialize().hex()])[0]

test/functional/p2p_1p1c_network.py

@@ -107,7 +107,7 @@ def create_packages(self):
 
         # 3: 2-parent-1-child package. Both parents are above mempool min feerate. No package submission happens.
         # We require packages to be child-with-unconfirmed-parents and only allow 1-parent-1-child packages.
-        package_hex_3, parent_31, parent_32, child_3 = self.create_package_2p1c(self.wallet)
+        package_hex_3, parent_31, _parent_32, child_3 = self.create_package_2p1c(self.wallet)
 
         # 4: parent + child package where the child spends 2 different outputs from the parent.
         package_hex_4, parent_4, child_4 = self.create_package_2outs(self.wallet)

test/functional/p2p_tx_download.py

@@ -156,9 +156,9 @@ def test_expiry_fallback(self):
         # One of the peers is asked for the tx
         peer2.wait_until(lambda: sum(p.tx_getdata_count for p in [peer1, peer2]) == 1)
         with p2p_lock:
-            peer_expiry, peer_fallback = (peer1, peer2) if peer1.tx_getdata_count == 1 else (peer2, peer1)
+            _peer_expiry, peer_fallback = (peer1, peer2) if peer1.tx_getdata_count == 1 else (peer2, peer1)
             assert_equal(peer_fallback.tx_getdata_count, 0)
-        self.nodes[0].setmocktime(int(time.time()) + GETDATA_TX_INTERVAL + 1)  # Wait for request to peer_expiry to expire
+        self.nodes[0].setmocktime(int(time.time()) + GETDATA_TX_INTERVAL + 1)  # Wait for request to _peer_expiry to expire
         peer_fallback.wait_until(lambda: peer_fallback.tx_getdata_count >= 1, timeout=1)
         self.restart_node(0)  # reset mocktime
 

test/functional/rpc_createmultisig.py

@@ -47,7 +47,7 @@ def create_wallet(self, node, wallet_name):
         return node.get_wallet_rpc(wallet_name)
 
     def run_test(self):
-        node0, node1, node2 = self.nodes
+        node0, node1, _node2 = self.nodes
         self.wallet = MiniWallet(test_node=node0)
 
         if self.is_wallet_compiled():
@@ -122,7 +122,7 @@ def test_multisig_script_limit(self, wallet_multi):
             assert_raises_rpc_error(-4, "Unsupported multisig script size for legacy wallet. Upgrade to descriptors to overcome this limitation for p2sh-segwit or bech32 scripts", wallet_multi.addmultisigaddress, 16, pubkeys, '', 'bech32')
 
     def do_multisig(self, nkeys, nsigs, output_type, wallet_multi):
-        node0, node1, node2 = self.nodes
+        node0, _node1, node2 = self.nodes
         pub_keys = self.pub[0: nkeys]
         priv_keys = self.priv[0: nkeys]
 

test/functional/test_framework/blocktools.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# Copyright (c) 2015-2022 The Bitcoin Core developers
+# Copyright (c) 2015-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Utilities for manipulating blocks and transactions."""
@@ -74,7 +74,7 @@ def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl
     block.nVersion = version or tmpl.get('version') or VERSIONBITS_LAST_OLD_BLOCK_VERSION
     block.nTime = ntime or tmpl.get('curtime') or int(time.time() + 600)
     block.hashPrevBlock = hashprev or int(tmpl['previousblockhash'], 0x10)
-    if tmpl and not tmpl.get('bits') is None:
+    if tmpl and tmpl.get('bits') is not None:
         block.nBits = struct.unpack('>I', bytes.fromhex(tmpl['bits']))[0]
     else:
         block.nBits = 0x207fffff  # difficulty retargeting is disabled in REGTEST chainparams

test/functional/wallet_upgradewallet.py

@@ -231,7 +231,7 @@ def copy_split_hd():
         assert b'\x07hdchain' in new_kvs
         hd_chain = new_kvs[b'\x07hdchain']
         assert_equal(28, len(hd_chain))
-        hd_chain_version, external_counter, seed_id = struct.unpack('<iI20s', hd_chain)
+        hd_chain_version, _external_counter, seed_id = struct.unpack('<iI20s', hd_chain)
         assert_equal(1, hd_chain_version)
         seed_id = bytearray(seed_id)
         seed_id.reverse()
@@ -258,7 +258,7 @@ def copy_split_hd():
         new_kvs = dump_bdb_kv(node_master_wallet)
         hd_chain = new_kvs[b'\x07hdchain']
         assert_equal(32, len(hd_chain))
-        hd_chain_version, external_counter, seed_id, internal_counter = struct.unpack('<iI20sI', hd_chain)
+        hd_chain_version, _external_counter, seed_id, internal_counter = struct.unpack('<iI20sI', hd_chain)
         assert_equal(2, hd_chain_version)
         assert_equal(0, internal_counter)
         seed_id = bytearray(seed_id)
@@ -284,7 +284,7 @@ def copy_split_hd():
         new_kvs = dump_bdb_kv(node_master_wallet)
         hd_chain = new_kvs[b'\x07hdchain']
         assert_equal(32, len(hd_chain))
-        hd_chain_version, external_counter, seed_id, internal_counter = struct.unpack('<iI20sI', hd_chain)
+        hd_chain_version, _external_counter, seed_id, internal_counter = struct.unpack('<iI20sI', hd_chain)
         assert_equal(2, hd_chain_version)
         assert_equal(2, internal_counter)
         # The next addresses are HD and should be on different HD chains (the one remaining key in each pool should have been flushed)
@@ -301,8 +301,8 @@ def copy_split_hd():
         new_kvs = dump_bdb_kv(node_master_wallet)
         for k, old_v in old_kvs.items():
             if k.startswith(b'\x07keymeta'):
-                new_ver, new_create_time, new_kp_str, new_seed_id, new_fpr, new_path_len, new_path, new_has_key_orig = deser_keymeta(BytesIO(new_kvs[k]))
-                old_ver, old_create_time, old_kp_str, old_seed_id, old_fpr, old_path_len, old_path, old_has_key_orig = deser_keymeta(BytesIO(old_v))
+                new_ver, new_create_time, new_kp_str, new_seed_id, _new_fpr, new_path_len, new_path, new_has_key_orig = deser_keymeta(BytesIO(new_kvs[k]))
+                old_ver, old_create_time, old_kp_str, old_seed_id, _old_fpr, old_path_len, old_path, old_has_key_orig = deser_keymeta(BytesIO(old_v))
                 assert_equal(10, old_ver)
                 if old_kp_str == b"": # imported things that don't have keymeta (i.e. imported coinbase privkeys) won't be upgraded
                     assert_equal(new_kvs[k], old_v)