From dc27835e4996002e2752ca94eb13e8dca5e150e2 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 1 Aug 2026 17:41:32 -0500 Subject: [PATCH] test: don't expect zero connections after protx operator-key change/revoke feature_protx_version.py intermittently timed out (dash#6702) waiting for getconnectioncount() == 0 on the masternode whose operator key was changed or revoked. The disconnects it waits for do happen: every peer that verified the masternode's MNAUTH drops the connection once it processes the trigger block (CMNAuth::NotifyMasternodeListChanged). But the masternode remains a member of previously formed quorums, so ThreadOpenMasternodeConnections on both sides concurrently re-establishes intra-quorum connections. Under parallel-test load these replacements land before the teardown finishes and the connection count never observably reaches zero, so the wait can only pass by timing luck. Instead of expecting an instantaneous zero, snapshot the node's peer ids right before mining the trigger transaction and wait until none of them remain: peer ids are monotonic, so this deterministically detects that the key change/revocation dropped every pre-existing connection while tolerating concurrent reconnects. The snapshot is asserted non-empty, since waiting for an empty set to disappear would succeed immediately and the test would silently stop asserting anything. The wait is expressed as set disjointness and the "why not zero connections" rationale lives in one place instead of being repeated at both call sites. Reproduced with 15 parallel copies via test_runner.py at a61ade2ef84: 6/15 + 5/15 + 9/15 runs failed before the change (every failure at the line 202/241 waits), 45/45 passed after it. Co-Authored-By: UdjinM6 --- test/functional/feature_protx_version.py | 30 ++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/test/functional/feature_protx_version.py b/test/functional/feature_protx_version.py index 1c3691f8bad0..d89303c923b7 100755 --- a/test/functional/feature_protx_version.py +++ b/test/functional/feature_protx_version.py @@ -58,6 +58,18 @@ def set_test_params(self): ]] * 6 self.set_dash_test_params(6, 5, evo_count=2, extra_args=self.extra_args) + def get_peer_ids(self, node_idx): + return {peer['id'] for peer in self.nodes[node_idx].getpeerinfo()} + + def wait_for_peers_disconnected(self, node_idx, peer_ids): + """Wait until none of `peer_ids` is connected to node `node_idx` anymore. + + Don't wait for getconnectioncount() to hit zero instead: intra-quorum connections are + re-established concurrently, as the masternode remains a member of previously formed + quorums, so zero might never be observable. + """ + assert peer_ids, "no peers to wait for, the disconnect assertion would pass trivially" + self.wait_until(lambda: peer_ids.isdisjoint(self.get_peer_ids(node_idx))) def run_test(self): # Connect all nodes to node1 so that we always have the whole network connected @@ -193,13 +205,15 @@ def test_protx_v24_versioning(self, mn: MasternodeInfo, legacy_mn: MasternodeInf legacy_mn.keyOperator = new_operator['secret'] migrate_result = legacy_mn.update_registrar(node, submit=True, fundsAddr=legacy_mn.fundsAddr) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block + assert legacy_mn.nodeIdx is not None + old_peer_ids = self.get_peer_ids(legacy_mn.nodeIdx) tip = self.generate(node, 1, sync_fun=self.no_op)[0] assert_equal(node.getrawtransaction(migrate_result, 1, tip)['proUpRegTx']['version'], 3) assert_equal(node.protx('info', legacy_mn.proTxHash)['state']['version'], 3) - # Changing the operator key PoSe-bans the masternode, which results in disconnects. Wait for - # them to happen and then reconnect its node back to let sync_all finish correctly. - assert legacy_mn.nodeIdx is not None - self.wait_until(lambda: self.nodes[legacy_mn.nodeIdx].getconnectioncount() == 0) + # Changing the operator key makes every peer drop its existing connections to this + # masternode. Wait for that to happen and then reconnect its node back to let sync_all + # finish correctly. + self.wait_for_peers_disconnected(legacy_mn.nodeIdx, old_peer_ids) self.connect_nodes(legacy_mn.nodeIdx, 0) self.sync_all() @@ -234,11 +248,13 @@ def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo): protx_result = revoke_mn.revoke(self.nodes[0], submit=True, reason=1, fundsAddr=funds_address) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block + old_peer_ids = self.get_peer_ids(node_idx) tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1) - # Revoking a MN results in disconnects. Wait for disconnects to actually happen - # and then reconnect the corresponding node back to let sync_blocks finish correctly. - self.wait_until(lambda: self.nodes[node_idx].getconnectioncount() == 0) + # Revoking a MN makes every peer drop its existing connections to it. Wait for that to + # happen and then reconnect the corresponding node back to let sync_blocks finish + # correctly. + self.wait_for_peers_disconnected(node_idx, old_peer_ids) self.connect_nodes(node_idx, 0) self.sync_all() self.log.info(f"Successfully revoked={revoke_mn.proTxHash}")