Merge #6718: test: consolidate masternode info tracking (MasternodeInfo) in functional tests, implement helpers, add type annotations

0532baf refactor: add helper for finding collateral vout (Kittywhiskers Van Gogh)
89b2548 refactor: add helper for transaction burial (Kittywhiskers Van Gogh)
f3777c5 refactor: introduce address generation helper, make constructor minimal (Kittywhiskers Van Gogh)
fa56126 refactor: add type annotations for `MasternodeInfo` members (Kittywhiskers Van Gogh)
dfca0ee refactor: use `MasternodeInfo` in feature_dip3_deterministicmns.py (Kittywhiskers Van Gogh)
cd7ffcf refactor: store only the node index in `MasternodeInfo` (Kittywhiskers Van Gogh)
07c7319 refactor: use `nodeIdx` instead of `node.index` with `MasternodeInfo` (Kittywhiskers Van Gogh)
08e81d8 refactor: store P2P port instead of full address in `MasternodeInfo` (Kittywhiskers Van Gogh)
3e25b30 refactor: store the funding address in `MasternodeInfo` (Kittywhiskers Van Gogh)
08d18d7 refactor: annotate usage of `MasternodeInfo` (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  While working on functional tests for [dash#6666](#6666) (which in turn, relies on the foundation built by [dash#6674](#6674), see `rpc_netinfo.py`, [source](https://github.com/dashpay/dash/blob/c788531d35c48820af951799ff8b67ee2dadb8b3/test/functional/rpc_netinfo.py)), the existing test infrastructure, while well optimized for testing interactions _between_ masternodes, didn't offer enough flexibility for testing _creation_ of masternodes.

  The tests that need to be implemented for extended addresses mimic `feature_dip3_deterministicmns.py` ([source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/feature_dip3_deterministicmns.py)) in objective and while taking cues from it, it was found that instead of the `MasternodeInfo` used in most of the codebase ([source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/test_framework/test_framework.py#L1138-L1153)),  `feature_dip3_deterministicmns.py` implements its own object, `Masternode` ([source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/feature_dip3_deterministicmns.py#L223-L227)).

  In a similar vein, `rpc_netinfo.py`, as implemented in c788531, implemented its own variant, `Node` ([source](https://github.com/dashpay/dash/blob/c788531d35c48820af951799ff8b67ee2dadb8b3/test/functional/rpc_netinfo.py#L31-L194)) to address the testing needs for extended addresses. It became clear that without additional intervention, the test suite would have three different ways of representing masternode information (`MasternodeInfo`, `Masternode`, `Node`) and that it would be more beneficial to consolidate all three approaches together.

  So, taking cue from `Node` (from `rpc_netinfo.py`, not included in `develop` as of this writing, [source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/test_framework/rpc_netinfo.py)), this pull request aims to clean up, consolidate and improve upon `MasternodeInfo` in an attempt to remove the need for `Node`.

  ## Additional Information

  * PEP 526 ("Syntax for Variable Annotations") prohibit annotation of variables defined by a `for` or `with` statement ([source](https://peps.python.org/pep-0526/#where-annotations-aren-t-allowed)), to get around that, the syntax for type comments ([source](https://peps.python.org/pep-0484)) as defined in PEP 484 ("Type Hints") have been used instead.

  * The decision to remove `node` from `MasternodeInfo` despite its convenience was rooted in the observation that there are cases where `nodeIdx` and `node` are assigned separately ([source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/test_framework/test_framework.py#L1480), [source](https://github.com/dashpay/dash/blob/09aa42ef0f20dc94ea63bdc8e7e064d494c9ae8f/test/functional/test_framework/test_framework.py#L1499)), which highlighted the fact that `MasternodeInfo` has no control over the lifetime of the `TestNode` instance as it is ultimately controlled by `BitcoinTestFramework`.

    To avoid potential error and to make it syntactically clear that ownership is vests with `BitcoinTestFramework`, `node` has been removed and replaced with `get_node()`, which alongside some basic sanity checks, is just an alias for `self.nodes[mn.nodeIdx]` but is still less tedious to type or read.

  * The reason why functions like `generate_addresses()` accept a separate `TestNode` ([source](https://github.com/dashpay/dash/blob/c5444b3f7c06aa5b060fd542773df6dd5a9bae1f/test/functional/test_framework/test_framework.py#L1160-L1174)) is due to the practice of using a single node to instantiate all masternodes instead of keeping them as self-contained instances that mine their own blocks and fund their own collaterals (for instance, `DashTestFramework` uses `nodes[0]` to setup all the masternodes even if the masternodes themselves had a different index).

  * The decision to replace `addr` with `nodePort` has been inspired by the need to register masternodes with non-loopback addresses (like in `feature_dip3_deterministicmns.py` or `rpc_netinfo.py`). As a node that is intended to be used will always need the same standardized loopback address, we can safely resort to only storing the port number and save a few `p2p_port()` calls along the way.

  ## Breaking Changes

  None expected. Affects only functional tests.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 0532baf
  UdjinM6:
    utACK 0532baf

Tree-SHA512: c2b5b0f9200c2714159a5e0f6f52174e38eae426c01620cf15e9adb8b8e67bbb078d658edb21b052a1fc9896ba098cffa02248aaa980be5d5cb145e9929c4568

Author: PastaPastaPasta

test/functional/feature_asset_locks.py

@@ -103,7 +103,7 @@ def create_assetunlock(self, index, withdrawal, pubkey=None, fee=tiny_amount):
 
         height = node_wallet.getblockcount()
         self.log.info(f"Creating asset unlock: index={index} {request_id}")
-        quorumHash = mninfo[0].node.quorum("selectquorum", llmq_type_test, request_id)["quorumHash"]
+        quorumHash = mninfo[0].get_node(self).quorum("selectquorum", llmq_type_test, request_id)["quorumHash"]
         self.log.info(f"Used quorum hash: {quorumHash}")
         unlockTx_payload = CAssetUnlockTx(
             version = 1,
@@ -368,7 +368,7 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
         asset_unlock_tx_payload = CAssetUnlockTx()
         asset_unlock_tx_payload.deserialize(BytesIO(asset_unlock_tx.vExtraPayload))
 
-        assert_equal(asset_unlock_tx_payload.quorumHash, int(self.mninfo[0].node.quorum("selectquorum", llmq_type_test, 'e6c7a809d79f78ea85b72d5df7e9bd592aecf151e679d6e976b74f053a7f9056')["quorumHash"], 16))
+        assert_equal(asset_unlock_tx_payload.quorumHash, int(self.mninfo[0].get_node(self).quorum("selectquorum", llmq_type_test, 'e6c7a809d79f78ea85b72d5df7e9bd592aecf151e679d6e976b74f053a7f9056')["quorumHash"], 16))
 
         self.log.info("Test no IS for asset unlock...")
         self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)

test/functional/feature_dip3_deterministicmns.py

@@ -8,15 +8,17 @@
 #
 
 from decimal import Decimal
+from typing import List
 
 from test_framework.blocktools import create_block_with_mnpayments
 from test_framework.messages import tx_from_hex
-from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_framework import (
+    MASTERNODE_COLLATERAL,
+    BitcoinTestFramework,
+    MasternodeInfo,
+)
 from test_framework.util import assert_equal, force_finish_mnsync, p2p_port, softfork_active
 
-class Masternode(object):
-    pass
-
 class DIP3Test(BitcoinTestFramework):
     def set_test_params(self):
         self.num_initial_mn = 11 # Should be >= 11 to make sure quorums are not always the same MNs
@@ -48,19 +50,19 @@ def start_controller_node(self):
 
     def run_test(self):
         self.log.info("funding controller node")
-        while self.nodes[0].getbalance() < (self.num_initial_mn + 3) * 1000:
+        while self.nodes[0].getbalance() < (self.num_initial_mn + 3) * MASTERNODE_COLLATERAL:
             self.generate(self.nodes[0], 10, sync_fun=self.no_op) # generate enough for collaterals
         self.log.info("controller node has {} dash".format(self.nodes[0].getbalance()))
 
         # Make sure we're below block 135 (which activates dip3)
         self.log.info("testing rejection of ProTx before dip3 activation")
         assert self.nodes[0].getblockchaininfo()['blocks'] < 135
 
-        mns = []
+        mns: List[MasternodeInfo] = []
 
         # prepare mn which should still be accepted later when dip3 activates
         self.log.info("creating collateral for mn-before-dip3")
-        before_dip3_mn = self.prepare_mn(self.nodes[0], 1, 'mn-before-dip3')
+        before_dip3_mn: MasternodeInfo = self.prepare_mn(self.nodes[0], 1, 'mn-before-dip3')
         self.create_mn_collateral(self.nodes[0], before_dip3_mn)
         mns.append(before_dip3_mn)
 
@@ -82,7 +84,7 @@ def run_test(self):
 
         self.log.info("registering MNs")
         for i in range(self.num_initial_mn):
-            mn = self.prepare_mn(self.nodes[0], i + 2, "mn-%d" % i)
+            mn: MasternodeInfo = self.prepare_mn(self.nodes[0], i + 2, "mn-%d" % i)
             mns.append(mn)
 
             # start a few MNs before they are registered and a few after they are registered
@@ -93,12 +95,12 @@ def run_test(self):
             # let a few of the protx MNs refer to the existing collaterals
             fund = (i % 2) == 0
             if fund:
-                self.log.info("register_fund %s" % mn.alias)
+                self.log.info(f"register_fund {mn.friendlyName}")
                 self.register_fund_mn(self.nodes[0], mn)
             else:
-                self.log.info("create_collateral %s" % mn.alias)
+                self.log.info(f"create_collateral {mn.friendlyName}")
                 self.create_mn_collateral(self.nodes[0], mn)
-                self.log.info("register %s" % mn.alias)
+                self.log.info(f"register {mn.friendlyName}")
                 self.register_mn(self.nodes[0], mn)
 
             self.generate(self.nodes[0], 1, sync_fun=self.no_op)
@@ -117,7 +119,7 @@ def run_test(self):
         old_tip = self.nodes[0].getblockcount()
         old_listdiff = self.nodes[0].protx("listdiff", 1, old_tip)
         for i in range(spend_mns_count):
-            old_protx_hash = mns[i].protx_hash
+            old_protx_hash = mns[i].proTxHash
             old_collateral_address = mns[i].collateral_address
             old_blockhash = self.nodes[0].getbestblockhash()
             old_rpc_info = self.nodes[0].protx("info", old_protx_hash)
@@ -187,7 +189,7 @@ def run_test(self):
             mn = mns[i]
             # a few of these will actually refer to old ProRegTx internal collaterals,
             # which should work the same as external collaterals
-            new_mn = self.prepare_mn(self.nodes[0], mn.idx, mn.alias)
+            new_mn: MasternodeInfo = self.prepare_mn(self.nodes[0], mn.nodeIdx, mn.friendlyName)
             new_mn.collateral_address = mn.collateral_address
             new_mn.collateral_txid = mn.collateral_txid
             new_mn.collateral_vout = mn.collateral_vout
@@ -196,115 +198,88 @@ def run_test(self):
             mns[i] = new_mn
             self.generate(self.nodes[0], 1)
             self.assert_mnlists(mns)
-            self.log.info("restarting MN %s" % new_mn.alias)
-            self.stop_node(new_mn.idx)
+            self.log.info(f"restarting MN {mn.friendlyName}")
+            self.stop_node(new_mn.nodeIdx)
             self.start_mn(new_mn)
             self.sync_all()
 
         self.log.info("testing masternode status updates")
         # change voting address and see if changes are reflected in `masternode status` rpc output
         mn = mns[0]
         node = self.nodes[0]
-        old_dmnState = mn.node.masternode("status")["dmnState"]
+        old_dmnState = mn.get_node(self).masternode("status")["dmnState"]
         old_voting_address = old_dmnState["votingAddress"]
         new_voting_address = node.getnewaddress()
         assert old_voting_address != new_voting_address
         # also check if funds from payout address are used when no fee source address is specified
         node.sendtoaddress(mn.rewards_address, 0.001)
-        node.protx('update_registrar' if softfork_active(node, 'v19') else 'update_registrar_legacy', mn.protx_hash, "", new_voting_address, "")
+        node.protx('update_registrar' if softfork_active(node, 'v19') else 'update_registrar_legacy', mn.proTxHash, "", new_voting_address, "")
         self.generate(node, 1)
-        new_dmnState = mn.node.masternode("status")["dmnState"]
+        new_dmnState = mn.get_node(self).masternode("status")["dmnState"]
         new_voting_address_from_rpc = new_dmnState["votingAddress"]
         assert new_voting_address_from_rpc == new_voting_address
         # make sure payoutAddress is the same as before
         assert old_dmnState["payoutAddress"] == new_dmnState["payoutAddress"]
 
-    def prepare_mn(self, node, idx, alias):
-        mn = Masternode()
-        mn.idx = idx
-        mn.alias = alias
-        mn.p2p_port = p2p_port(mn.idx)
-        mn.operator_reward = (mn.idx % self.num_initial_mn)
-
-        blsKey = node.bls('generate') if softfork_active(node, 'v19') else node.bls('generate', True)
-        mn.fundsAddr = node.getnewaddress()
-        mn.ownerAddr = node.getnewaddress()
-        mn.operatorAddr = blsKey['public']
-        mn.votingAddr = mn.ownerAddr
-        mn.blsMnkey = blsKey['secret']
-
+    def prepare_mn(self, node, idx, alias) -> MasternodeInfo:
+        mn = MasternodeInfo(evo=False, legacy=(not softfork_active(node, 'v19')))
+        mn.generate_addresses(node)
+        mn.set_params(operator_reward=(idx % self.num_initial_mn), nodePort=p2p_port(idx))
+        mn.set_node(idx, alias)
         return mn
 
-    def create_mn_collateral(self, node, mn):
-        mn.collateral_address = node.getnewaddress()
-        mn.collateral_txid = node.sendtoaddress(mn.collateral_address, 1000)
-        mn.collateral_vout = None
+    def create_mn_collateral(self, node, mn: MasternodeInfo):
+        txid = node.sendtoaddress(mn.collateral_address, mn.get_collateral_value())
         self.generate(node, 1, sync_fun=self.no_op)
-
-        rawtx = node.getrawtransaction(mn.collateral_txid, 1)
-        for txout in rawtx['vout']:
-            if txout['value'] == Decimal(1000):
-                mn.collateral_vout = txout['n']
-                break
-        assert mn.collateral_vout is not None
+        vout = mn.get_collateral_vout(node, txid)
+        mn.set_params(collateral_txid=txid, collateral_vout=vout)
 
     # register a protx MN and also fund it (using collateral inside ProRegTx)
-    def register_fund_mn(self, node, mn):
-        node.sendtoaddress(mn.fundsAddr, 1000.001)
-        mn.collateral_address = node.getnewaddress()
-        mn.rewards_address = node.getnewaddress()
-
-        mn.protx_hash = node.protx('register_fund' if softfork_active(node, 'v19') else 'register_fund_legacy', mn.collateral_address, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr)
-        mn.collateral_txid = mn.protx_hash
-        mn.collateral_vout = None
-
-        rawtx = node.getrawtransaction(mn.collateral_txid, 1)
-        for txout in rawtx['vout']:
-            if txout['value'] == Decimal(1000):
-                mn.collateral_vout = txout['n']
-                break
-        assert mn.collateral_vout is not None
+    def register_fund_mn(self, node, mn: MasternodeInfo):
+        node.sendtoaddress(mn.fundsAddr, mn.get_collateral_value() + 0.001)
+        txid = node.protx('register_fund' if softfork_active(node, 'v19') else 'register_fund_legacy', mn.collateral_address, '127.0.0.1:%d' % mn.nodePort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr)
+        vout = mn.get_collateral_vout(node, txid)
+        mn.set_params(proTxHash=txid, collateral_txid=txid, collateral_vout=vout)
 
     # create a protx MN which refers to an existing collateral
-    def register_mn(self, node, mn):
+    def register_mn(self, node, mn: MasternodeInfo):
         node.sendtoaddress(mn.fundsAddr, 0.001)
-        mn.rewards_address = node.getnewaddress()
-
-        mn.protx_hash = node.protx('register' if softfork_active(node, 'v19') else 'register_legacy', mn.collateral_txid, mn.collateral_vout, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr)
+        proTxHash = node.protx('register' if softfork_active(node, 'v19') else 'register_legacy', mn.collateral_txid, mn.collateral_vout, '127.0.0.1:%d' % mn.nodePort, mn.ownerAddr, mn.pubKeyOperator, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr)
+        mn.set_params(proTxHash=proTxHash)
         self.generate(node, 1, sync_fun=self.no_op)
 
-    def start_mn(self, mn):
-        if len(self.nodes) <= mn.idx:
-            self.add_nodes(mn.idx - len(self.nodes) + 1)
-            assert len(self.nodes) == mn.idx + 1
-        self.start_node(mn.idx, extra_args = self.extra_args + ['-masternodeblsprivkey=%s' % mn.blsMnkey])
-        force_finish_mnsync(self.nodes[mn.idx])
-        mn.node = self.nodes[mn.idx]
-        self.connect_nodes(mn.idx, 0)
+    def start_mn(self, mn: MasternodeInfo):
+        assert mn.nodeIdx is not None, "nodeIdx must be set before starting masternode"
+        if len(self.nodes) <= mn.nodeIdx:
+            self.add_nodes(mn.nodeIdx - len(self.nodes) + 1)
+            assert len(self.nodes) == mn.nodeIdx + 1
+        self.start_node(mn.nodeIdx, extra_args = self.extra_args + ['-masternodeblsprivkey=%s' % mn.keyOperator])
+        force_finish_mnsync(mn.get_node(self))
+        self.connect_nodes(mn.nodeIdx, 0)
         self.sync_all()
 
-    def spend_mn_collateral(self, mn, with_dummy_input_output=False):
-        return self.spend_input(mn.collateral_txid, mn.collateral_vout, 1000, with_dummy_input_output)
+    def spend_mn_collateral(self, mn: MasternodeInfo, with_dummy_input_output=False):
+        return self.spend_input(mn.collateral_txid, mn.collateral_vout, mn.get_collateral_value(), with_dummy_input_output)
 
-    def update_mn_payee(self, mn, payee):
+    def update_mn_payee(self, mn: MasternodeInfo, payee):
         self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001)
-        self.nodes[0].protx('update_registrar' if softfork_active(self.nodes[0], 'v19') else 'update_registrar_legacy', mn.protx_hash, '', '', payee, mn.fundsAddr)
+        self.nodes[0].protx('update_registrar' if softfork_active(self.nodes[0], 'v19') else 'update_registrar_legacy', mn.proTxHash, '', '', payee, mn.fundsAddr)
         self.generate(self.nodes[0], 1)
-        info = self.nodes[0].protx('info', mn.protx_hash)
+        info = self.nodes[0].protx('info', mn.proTxHash)
         assert info['state']['payoutAddress'] == payee
 
-    def test_protx_update_service(self, mn):
+    def test_protx_update_service(self, mn: MasternodeInfo):
         self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001)
-        self.nodes[0].protx('update_service', mn.protx_hash, '127.0.0.2:%d' % mn.p2p_port, mn.blsMnkey, "", mn.fundsAddr)
+        self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.2:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr)
         self.generate(self.nodes[0], 1)
         for node in self.nodes:
-            protx_info = node.protx('info', mn.protx_hash)
+            protx_info = node.protx('info', mn.proTxHash)
             mn_list = node.masternode('list')
-            assert_equal(protx_info['state']['service'], '127.0.0.2:%d' % mn.p2p_port)
-            assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['address'], '127.0.0.2:%d' % mn.p2p_port)
+            assert_equal(protx_info['state']['service'], '127.0.0.2:%d' % mn.nodePort)
+            assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['address'], '127.0.0.2:%d' % mn.nodePort)
 
         # undo
-        self.nodes[0].protx('update_service', mn.protx_hash, '127.0.0.1:%d' % mn.p2p_port, mn.blsMnkey, "", mn.fundsAddr)
+        self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.1:%d' % mn.nodePort, mn.keyOperator, "", mn.fundsAddr)
         self.generate(self.nodes[0], 1, sync_fun=self.no_op)
 
     def assert_mnlists(self, mns):

test/functional/feature_dip3_v19.py

@@ -14,7 +14,10 @@
 from test_framework.p2p import P2PInterface
 from test_framework.messages import CBlock, CBlockHeader, CCbTx, CMerkleBlock, from_hex, hash256, msg_getmnlistd, \
     QuorumId, ser_uint256
-from test_framework.test_framework import DashTestFramework
+from test_framework.test_framework import (
+    DashTestFramework,
+    MasternodeInfo,
+)
 from test_framework.util import (
     assert_equal
 )
@@ -78,16 +81,16 @@ def run_test(self):
         self.log.info("pubkeyoperator should still be shown using legacy scheme")
         assert_equal(pubkeyoperator_list_before, pubkeyoperator_list_after)
 
-        evo_info_0 = self.dynamically_add_masternode(evo=True, rnd=7)
+        evo_info_0: MasternodeInfo = self.dynamically_add_masternode(evo=True, rnd=7)
         assert evo_info_0 is not None
 
         self.log.info("Checking that protxs with duplicate EvoNodes fields are rejected")
-        evo_info_1 = self.dynamically_add_masternode(evo=True, rnd=7, should_be_rejected=True)
+        evo_info_1: MasternodeInfo = self.dynamically_add_masternode(evo=True, rnd=7, should_be_rejected=True)
         assert evo_info_1 is None
         self.dynamically_evo_update_service(evo_info_0, 8)
-        evo_info_2 = self.dynamically_add_masternode(evo=True, rnd=8, should_be_rejected=True)
+        evo_info_2: MasternodeInfo = self.dynamically_add_masternode(evo=True, rnd=8, should_be_rejected=True)
         assert evo_info_2 is None
-        evo_info_3 = self.dynamically_add_masternode(evo=True, rnd=9)
+        evo_info_3: MasternodeInfo = self.dynamically_add_masternode(evo=True, rnd=9)
         assert evo_info_3 is not None
         self.dynamically_evo_update_service(evo_info_0, 9, should_be_rejected=True)
 
@@ -101,7 +104,7 @@ def run_test(self):
         self.log.info("Checking that adding more regular MNs after v19 doesn't break DKGs and IS/CLs")
 
         for i in range(6):
-            new_mn = self.dynamically_add_masternode(evo=False, rnd=(10 + i))
+            new_mn: MasternodeInfo = self.dynamically_add_masternode(evo=False, rnd=(10 + i))
             assert new_mn is not None
 
         # mine more quorums and make sure everything still works
@@ -129,7 +132,7 @@ def test_revoke_protx(self, node_idx, revoke_protx, revoke_keyoperator):
         self.connect_nodes(node_idx, 0)
         self.sync_all()
         self.log.info(f"Successfully revoked={revoke_protx}")
-        for mn in self.mninfo:
+        for mn in self.mninfo: # type: MasternodeInfo
             if mn.proTxHash == revoke_protx:
                 self.mninfo.remove(mn)
                 return

test/functional/feature_dip4_coinbasemerkleroots.py

@@ -14,7 +14,10 @@
 
 from test_framework.messages import CBlock, CBlockHeader, CCbTx, CMerkleBlock, from_hex, hash256, msg_getmnlistd, QuorumId, ser_uint256
 from test_framework.p2p import P2PInterface
-from test_framework.test_framework import DashTestFramework
+from test_framework.test_framework import (
+    DashTestFramework,
+    MasternodeInfo,
+)
 from test_framework.util import assert_equal
 
 DIP0008_HEIGHT = 432
@@ -50,7 +53,7 @@ def set_test_params(self):
         self.delay_v20_and_mn_rr(height=V20_HEIGHT)
 
     def remove_masternode(self, idx):
-        mn = self.mninfo[idx]
+        mn: MasternodeInfo = self.mninfo[idx]
         rawtx = self.nodes[0].createrawtransaction([{"txid": mn.collateral_txid, "vout": mn.collateral_vout}], {self.nodes[0].getnewaddress(): 999.9999})
         rawtx = self.nodes[0].signrawtransactionwithwallet(rawtx)
         self.nodes[0].sendrawtransaction(rawtx["hex"])
@@ -80,7 +83,7 @@ def run_test(self):
         # Register one more MN, but don't start it (that would fail as DashTestFramework doesn't support this atm)
         baseBlockHash = self.nodes[0].getbestblockhash()
         self.prepare_masternode(self.mn_count)
-        new_mn = self.mninfo[self.mn_count]
+        new_mn: MasternodeInfo = self.mninfo[self.mn_count]
 
         # Now test if that MN appears in a diff when the base block is the one just before MN registration
         expectedDeleted = []

test/functional/feature_governance.py

@@ -7,7 +7,10 @@
 import json
 
 from test_framework.messages import uint256_to_string
-from test_framework.test_framework import DashTestFramework
+from test_framework.test_framework import (
+    DashTestFramework,
+    MasternodeInfo,
+)
 from test_framework.governance import have_trigger_for_height, prepare_object
 from test_framework.util import assert_equal, satoshi_round
 
@@ -187,7 +190,7 @@ def run_test(self):
         _, mn_payee_protx = height_protx_list[1]
 
         payee_idx = None
-        for mn in self.mninfo:
+        for mn in self.mninfo: # type: MasternodeInfo
             if mn.proTxHash == mn_payee_protx:
                 payee_idx = mn.nodeIdx
                 break