Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 12, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
08d18d7
refactor: annotate usage of `MasternodeInfo`
kwvg Jun 9, 2025
3e25b30
refactor: store the funding address in `MasternodeInfo`
kwvg Jun 8, 2025
08e81d8
refactor: store P2P port instead of full address in `MasternodeInfo`
kwvg Jun 9, 2025
07c7319
refactor: use `nodeIdx` instead of `node.index` with `MasternodeInfo`
kwvg Jun 8, 2025
cd7ffcf
refactor: store only the node index in `MasternodeInfo`
kwvg Jun 9, 2025
dfca0ee
refactor: use `MasternodeInfo` in feature_dip3_deterministicmns.py
kwvg Jun 9, 2025
fa56126
refactor: add type annotations for `MasternodeInfo` members
kwvg Jun 9, 2025
f3777c5
refactor: introduce address generation helper, make constructor minimal
kwvg Jun 9, 2025
89b2548
refactor: add helper for transaction burial
kwvg Jun 9, 2025
0532baf
refactor: add helper for finding collateral vout
kwvg Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/functional/feature_asset_locks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
30 changes: 15 additions & 15 deletions test/functional/feature_llmq_connections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def run_test(self):
self.log.info("checking for old intra quorum connections")
total_count = 0
for mn in self.get_quorum_masternodes(q):
count = self.get_mn_connection_count(mn.node)
count = self.get_mn_connection_count(mn.get_node(self))
total_count += count
assert_greater_than_or_equal(count, 2)
assert total_count < 40
Expand All @@ -49,24 +49,24 @@ def run_test(self):
self.log.info("mining one block and waiting for all members to connect to each other")
self.generate(self.nodes[0], 1, sync_fun=self.no_op)
for mn in self.get_quorum_masternodes(q):
self.wait_for_mnauth(mn.node, 4)
self.wait_for_mnauth(mn.get_node(self), 4)

self.log.info("mine a new quorum and verify that all members connect to each other")
q = self.mine_quorum()

self.log.info("checking that all MNs got probed")
for mn in self.get_quorum_masternodes(q):
self.wait_until(lambda: self.get_mn_probe_count(mn.node, q, False) == 4)
self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, False) == 4)
Copy link

@coderabbitai coderabbitai bot Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix lambda function variable capture issue.

The lambda functions are capturing the loop variable mn by reference, which could lead to incorrect behavior when the lambda is executed after the loop iteration completes.

Apply this fix to properly capture the loop variable:

-        for mn in self.get_quorum_masternodes(q):
-            self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, False) == 4)
+        for mn in self.get_quorum_masternodes(q):
+            self.wait_until(lambda mn=mn: self.get_mn_probe_count(mn.get_node(self), q, False) == 4)

Apply the same pattern to lines 64 and 69.

Also applies to: 64-64, 69-69

🧰 Tools
🪛 Ruff (0.11.9)

59-59: Function definition does not bind loop variable mn

(B023)

🤖 Prompt for AI Agents 
In test/functional/feature_llmq_connections.py at lines 59, 64, and 69, the
lambda functions capture the loop variable mn by reference, causing potential
incorrect behavior after the loop ends. To fix this, modify each lambda to
capture the current value of mn as a default argument in the lambda definition,
ensuring the correct mn is used when the lambda executes.


self.log.info("checking that probes age")
self.bump_mocktime(self.MAX_AGE)
for mn in self.get_quorum_masternodes(q):
self.wait_until(lambda: self.get_mn_probe_count(mn.node, q, False) == 0)
self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, False) == 0)

self.log.info("mine a new quorum and re-check probes")
q = self.mine_quorum()
for mn in self.get_quorum_masternodes(q):
self.wait_until(lambda: self.get_mn_probe_count(mn.node, q, True) == 4)
self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, True) == 4)

self.log.info("Activating SPORK_21_QUORUM_ALL_CONNECTED")
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
Expand All @@ -84,12 +84,12 @@ def run_test(self):
self.log.info("check that old masternode connections are dropped")
removed = False
for mn in self.mninfo: # type: MasternodeInfo
if len(mn.node.quorum("memberof", mn.proTxHash)) > 0:
if len(mn.get_node(self).quorum("memberof", mn.proTxHash)) > 0:
try:
with mn.node.assert_debug_log(['removing masternodes quorum connections']):
with mn.node.assert_debug_log(['keeping mn quorum connections']):
with mn.get_node(self).assert_debug_log(['removing masternodes quorum connections']):
with mn.get_node(self).assert_debug_log(['keeping mn quorum connections']):
self.mine_cycle_quorum(is_first=False)
mn.node.mockscheduler(60) # we check for old connections via the scheduler every 60 seconds
mn.get_node(self).mockscheduler(60) # we check for old connections via the scheduler every 60 seconds
removed = True
except:
pass # it's ok to not remove connections sometimes
Expand All @@ -100,9 +100,9 @@ def run_test(self):
self.log.info("check that inter-quorum masternode connections are added")
added = False
for mn in self.mninfo: # type: MasternodeInfo
if len(mn.node.quorum("memberof", mn.proTxHash)) > 0:
if len(mn.get_node(self).quorum("memberof", mn.proTxHash)) > 0:
try:
with mn.node.assert_debug_log(['adding mn inter-quorum connections']):
with mn.get_node(self).assert_debug_log(['adding mn inter-quorum connections']):
self.mine_cycle_quorum(is_first=False)
added = True
except:
Expand All @@ -114,17 +114,17 @@ def run_test(self):
def check_reconnects(self, expected_connection_count):
self.log.info("disable and re-enable networking on all masternodes")
for mn in self.mninfo: # type: MasternodeInfo
mn.node.setnetworkactive(False)
mn.get_node(self).setnetworkactive(False)
for mn in self.mninfo: # type: MasternodeInfo
self.wait_until(lambda: len(mn.node.getpeerinfo()) == 0)
self.wait_until(lambda: len(mn.get_node(self).getpeerinfo()) == 0)
Copy link

@coderabbitai coderabbitai bot Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix lambda function variable capture issue.

Similar issue with lambda function capturing loop variable by reference.

Apply this fix:

-        for mn in self.mninfo: # type: MasternodeInfo
-            self.wait_until(lambda: len(mn.get_node(self).getpeerinfo()) == 0)
+        for mn in self.mninfo: # type: MasternodeInfo
+            self.wait_until(lambda mn=mn: len(mn.get_node(self).getpeerinfo()) == 0)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self.wait_until(lambda: len(mn.get_node(self).getpeerinfo()) == 0)
for mn in self.mninfo: # type: MasternodeInfo
self.wait_until(lambda mn=mn: len(mn.get_node(self).getpeerinfo()) == 0)
🧰 Tools
🪛 Ruff (0.11.9)

119-119: Function definition does not bind loop variable mn

(B023)

🤖 Prompt for AI Agents 
In test/functional/feature_llmq_connections.py at line 119, the lambda function
captures the loop variable by reference, causing incorrect behavior. Fix this by
binding the current value of the loop variable as a default argument in the
lambda function to ensure it captures the intended value at each iteration.

for mn in self.mninfo: # type: MasternodeInfo
mn.node.setnetworkactive(True)
mn.get_node(self).setnetworkactive(True)
self.bump_mocktime(60)

self.log.info("verify that all masternodes re-connected")
for q in self.nodes[0].quorum('list')['llmq_test']:
for mn in self.get_quorum_masternodes(q):
self.wait_for_mnauth(mn.node, expected_connection_count)
self.wait_for_mnauth(mn.get_node(self), expected_connection_count)

# Also re-connect non-masternode connections
for i in range(1, len(self.nodes)):
Expand Down
6 changes: 3 additions & 3 deletions test/functional/feature_llmq_data_recovery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def restart_mn(self, mn: MasternodeInfo, reindex=False, qvvec_sync=None, qdata_r
args.append('-llmq-qvvec-sync=%s:%d' % (llmq_type_strings[llmq_sync[0]], llmq_sync[1]))
if reindex:
args.append('-reindex')
bb_hash = mn.node.getbestblockhash()
bb_hash = mn.get_node(self).getbestblockhash()
self.restart_node(mn.nodeIdx, args)
self.wait_until(lambda: mn.node.getbestblockhash() == bb_hash)
self.wait_until(lambda: mn.get_node(self).getbestblockhash() == bb_hash)
else:
self.restart_node(mn.nodeIdx, args)
force_finish_mnsync(mn.node)
force_finish_mnsync(mn.get_node(self))
self.connect_nodes(mn.nodeIdx, 0)
if qdata_recovery_enabled:
# trigger recovery threads and wait for them to start
Expand Down
26 changes: 13 additions & 13 deletions test/functional/feature_llmq_dkgerrors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,46 +27,46 @@ def run_test(self):
mninfos_valid = self.mninfo.copy()[1:]

self.log.info("Lets omit the contribution")
self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-omit', '100')
qh = self.mine_quorum(expected_contributions=2, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)

self.log.info("Lets lie in the contribution but provide a correct justification")
self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'contribution-lie', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-omit', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-lie', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)

self.log.info("Lets lie in the contribution and then omit the justification")
self.mninfo[0].node.quorum('dkgsimerror', 'justify-omit', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'justify-omit', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)

self.log.info("Heal some damage (don't get PoSe banned)")
self.heal_masternodes(33)

self.log.info("Lets lie in the contribution and then also lie in the justification")
self.mninfo[0].node.quorum('dkgsimerror', 'justify-omit', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'justify-lie', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'justify-omit', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'justify-lie', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, False)

self.log.info("Lets lie about another MN")
self.mninfo[0].node.quorum('dkgsimerror', 'contribution-lie', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'justify-lie', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'complain-lie', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'contribution-lie', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'justify-lie', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'complain-lie', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=1, expected_justifications=2, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)

self.log.info("Lets omit 1 premature commitments")
self.mninfo[0].node.quorum('dkgsimerror', 'complain-lie', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'commit-omit', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'complain-lie', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'commit-omit', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)

self.log.info("Lets lie in 1 premature commitments")
self.mninfo[0].node.quorum('dkgsimerror', 'commit-omit', '0')
self.mninfo[0].node.quorum('dkgsimerror', 'commit-lie', '100')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'commit-omit', '0')
self.mninfo[0].get_node(self).quorum('dkgsimerror', 'commit-lie', '100')
qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2, mninfos_valid=mninfos_valid)
self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)

Expand Down
6 changes: 3 additions & 3 deletions test/functional/feature_llmq_rotation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def run_test(self):

# At this point, we want to wait for CLs just before the self.mine_cycle_quorum to diversify the CLs in CbTx.
# Although because here a new quorum cycle is starting, and we don't want to mine them now, mine 8 blocks (to skip all DKG phases)
nodes = [self.nodes[0]] + [mn.node for mn in self.mninfo.copy()]
nodes = [self.nodes[0]] + [mn.get_node(self) for mn in self.mninfo.copy()]
self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes))
self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash())

Expand Down Expand Up @@ -193,7 +193,7 @@ def run_test(self):
quorumList = self.test_getmnlistdiff_quorums(b_1, b_2, quorumList, expectedDeleted, expectedNew)

mninfos_online = self.mninfo.copy()
nodes = [self.nodes[0]] + [mn.node for mn in mninfos_online]
nodes = [self.nodes[0]] + [mn.get_node(self) for mn in mninfos_online]
self.sync_blocks(nodes)
quorum_list = self.nodes[0].quorum("list", llmq_type)
quorum_blockhash = self.nodes[0].getbestblockhash()
Expand Down Expand Up @@ -398,7 +398,7 @@ def test_quorum_listextended(self, quorum_info, llmq_type_name):
def move_to_next_cycle(self):
cycle_length = 24
mninfos_online = self.mninfo.copy()
nodes = [self.nodes[0]] + [mn.node for mn in mninfos_online]
nodes = [self.nodes[0]] + [mn.get_node(self) for mn in mninfos_online]
cur_block = self.nodes[0].getblockcount()

# move forward to next DKG
Expand Down
Loading