From 2f3260e9c1df942b6f9fbf43839af1fe42f9c94e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 23 Apr 2025 20:18:29 +0700 Subject: [PATCH 1/5] test: move general checks for Masternode from feature_llmq_simplepose.py to test_framework/masternodes.py --- test/functional/feature_llmq_simplepose.py | 23 +++++-------------- test/functional/test_framework/masternodes.py | 17 ++++++++++++++ 2 files changed, 23 insertions(+), 17 deletions(-) create mode 100644 test/functional/test_framework/masternodes.py diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 61d04b243615..1b2578048c79 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -12,10 +12,10 @@ import time +from test_framework.masternodes import check_banned, check_punished from test_framework.test_framework import DashTestFramework from test_framework.util import assert_equal, force_finish_mnsync, p2p_port - class LLMQSimplePoSeTest(DashTestFramework): def set_test_params(self): self.extra_args = [[ f'-testactivationheight=dip0024@9999' ]] * 6 @@ -97,7 +97,7 @@ def test_no_banning(self, expected_connections=None): self.log.info(f"Testing no PoSe banning in normal conditions {i + 1}/3") self.mine_quorum(expected_connections=expected_connections) for mn in self.mninfo: - assert not self.check_punished(mn) and not self.check_banned(mn) + assert not check_punished(self.nodes[0], mn) and not check_banned(self.nodes[0], mn) def mine_quorum_less_checks(self, expected_good_nodes, mninfos_online): # Unlike in mine_quorum we skip most of the checks and only care about @@ -179,7 +179,7 @@ def test_banning(self, invalidate_proc, expected_connections=None): self.reset_probe_timeouts() self.mine_quorum(expected_connections=expected_connections, expected_members=expected_contributors, expected_contributions=expected_contributors, expected_complaints=expected_complaints, expected_commitments=expected_contributors, mninfos_online=mninfos_online, mninfos_valid=mninfos_valid) - if not self.check_banned(mn): + if not check_banned(self.nodes[0], mn): self.log.info("Instant ban still requires 2 missing DKG round. If it is not banned yet, mine 2nd one") self.reset_probe_timeouts() self.mine_quorum(expected_connections=expected_connections, expected_members=expected_contributors, expected_contributions=expected_contributors, expected_complaints=expected_complaints, expected_commitments=expected_contributors, mninfos_online=mninfos_online, mninfos_valid=mninfos_valid) @@ -192,7 +192,7 @@ def test_banning(self, invalidate_proc, expected_connections=None): self.reset_probe_timeouts() self.mine_quorum_less_checks(expected_contributors - 1, mninfos_online) - assert self.check_banned(mn) + assert check_banned(self.nodes[0], mn) if not went_offline: # we do not include PoSe banned mns in quorums, so the next one should have 1 contributor less @@ -201,7 +201,7 @@ def test_banning(self, invalidate_proc, expected_connections=None): def repair_masternodes(self, restart): self.log.info("Repairing all banned and punished masternodes") for mn in self.mninfo: - if self.check_banned(mn) or self.check_punished(mn): + if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn): addr = self.nodes[0].getnewaddress() self.nodes[0].sendtoaddress(addr, 0.1) self.nodes[0].protx('update_service', mn.proTxHash, '127.0.0.1:%d' % p2p_port(mn.node.index), mn.keyOperator, "", addr) @@ -221,7 +221,7 @@ def repair_masternodes(self, restart): # Isolate and re-connect all MNs (otherwise there might be open connections with no MNAUTH for MNs which were banned before) for mn in self.mninfo: - assert not self.check_banned(mn) + assert not check_banned(self.nodes[0], mn) mn.node.setnetworkactive(False) self.wait_until(lambda: mn.node.getconnectioncount() == 0) mn.node.setnetworkactive(True) @@ -234,17 +234,6 @@ def reset_probe_timeouts(self): # Sleep a couple of seconds to let mn sync tick to happen time.sleep(2) - def check_punished(self, mn): - info = self.nodes[0].protx('info', mn.proTxHash) - if info['state']['PoSePenalty'] > 0: - return True - return False - - def check_banned(self, mn): - info = self.nodes[0].protx('info', mn.proTxHash) - if info['state']['PoSeBanHeight'] != -1: - return True - return False if __name__ == '__main__': LLMQSimplePoSeTest().main() diff --git a/test/functional/test_framework/masternodes.py b/test/functional/test_framework/masternodes.py new file mode 100644 index 000000000000..186381932089 --- /dev/null +++ b/test/functional/test_framework/masternodes.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Helpful routines for masternode regression testing.""" + +def check_punished(node, mn): + info = node.protx('info', mn.proTxHash) + if info['state']['PoSePenalty'] > 0: + return True + return False + +def check_banned(node, mn): + info = node.protx('info', mn.proTxHash) + if info['state']['PoSeBanHeight'] != -1: + return True + return False From f72c6f2feb893e5a7debdadb899642a9604fdf14 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 23 Apr 2025 20:26:27 +0700 Subject: [PATCH 2/5] test: correctly pass mninfo_valid to mine_quorum for feature_llmq_dkgerrors.py --- test/functional/feature_llmq_dkgerrors.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/functional/feature_llmq_dkgerrors.py b/test/functional/feature_llmq_dkgerrors.py index 6bc773be21a2..6b844442f98c 100755 --- a/test/functional/feature_llmq_dkgerrors.py +++ b/test/functional/feature_llmq_dkgerrors.py @@ -24,20 +24,22 @@ def run_test(self): qh = self.mine_quorum() self.assert_member_valid(qh, self.mninfo[0].proTxHash, True) + mninfos_valid = self.mninfo.copy()[1:] + self.log.info("Lets omit the contribution") self.mninfo[0].node.quorum('dkgsimerror', 'contribution-omit', '100') - qh = self.mine_quorum(expected_contributions=2) + 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1) + 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=2) + 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)") @@ -46,26 +48,26 @@ def run_test(self): 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=2, expected_justifications=1) + 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=1, expected_justifications=2) + 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2) + 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') - qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2) + 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) def assert_member_valid(self, quorumHash, proTxHash, expectedValid): From 5767344d31bc12bf07877b58ec4f5721a5a94bf4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 23 Apr 2025 20:28:03 +0700 Subject: [PATCH 3/5] test: check for banned or punished masternodes for each quorum in each test It used to be tested only in feature_llmq_simplepose.py The functional test feature_llmq_simplepose.py one of the slowest and mining 3 quorums less make it significantly faster --- test/functional/feature_llmq_simplepose.py | 15 ++++++--------- test/functional/test_framework/test_framework.py | 5 +++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 1b2578048c79..9e4c52abd0f0 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -29,10 +29,7 @@ def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) self.wait_for_sporks_same() - # check if mining quorums with all nodes being online succeeds without punishment/banning - self.test_no_banning() - - # Now lets isolate MNs one by one and verify that punishment/banning happens + # Lets isolate MNs one by one and verify that punishment/banning happens self.test_banning(self.isolate_mn, 2) self.repair_masternodes(False) @@ -43,9 +40,6 @@ def run_test(self): self.reset_probe_timeouts() - # Make sure no banning happens with spork21 enabled - self.test_no_banning() - # Lets restart masternodes with closed ports and verify that they get banned even though they are connected to other MNs (via outbound connections) self.test_banning(self.close_mn_port) self.deaf_mns.clear() @@ -96,8 +90,6 @@ def test_no_banning(self, expected_connections=None): for i in range(3): self.log.info(f"Testing no PoSe banning in normal conditions {i + 1}/3") self.mine_quorum(expected_connections=expected_connections) - for mn in self.mninfo: - assert not check_punished(self.nodes[0], mn) and not check_banned(self.nodes[0], mn) def mine_quorum_less_checks(self, expected_good_nodes, mninfos_online): # Unlike in mine_quorum we skip most of the checks and only care about @@ -161,6 +153,11 @@ def mine_quorum_less_checks(self, expected_good_nodes, mninfos_online): def test_banning(self, invalidate_proc, expected_connections=None): mninfos_online = self.mninfo.copy() mninfos_valid = self.mninfo.copy() + + for mn in mninfos_valid: + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + expected_contributors = len(mninfos_online) for i in range(2): self.log.info(f"Testing PoSe banning due to {invalidate_proc.__name__} {i + 1}/2") diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 53b88e6f0f52..f3a1e2588222 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -26,6 +26,7 @@ from typing import List from .address import ADDRESS_BCRT1_P2SH_OP_TRUE from .authproxy import JSONRPCException +from test_framework.masternodes import check_banned, check_punished from test_framework.blocktools import TIME_GENESIS_BLOCK from . import coverage from .messages import ( @@ -1905,6 +1906,10 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec self.log.info("New quorum: height=%d, quorumHash=%s, quorumIndex=%d, minedBlock=%s" % (quorum_info["height"], new_quorum, quorum_info["quorumIndex"], quorum_info["minedBlock"])) + for mn in mninfos_valid: + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + return new_quorum def mine_cycle_quorum(self, is_first=True): From d82c39c83fa8a0f5795819abe63d81fd7011520e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 23 Apr 2025 21:31:58 +0700 Subject: [PATCH 4/5] test: split feature_llmq_simplepose.py for cases of spork23 and no spork23 --- test/functional/feature_llmq_simplepose.py | 41 ++++++++++++---------- test/functional/test_runner.py | 1 + 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 9e4c52abd0f0..fe7d8452cffb 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -23,7 +23,15 @@ def set_test_params(self): self.set_dash_llmq_test_params(5, 3) # rotating quorums add instability for this functional tests + def add_options(self, parser): + parser.add_argument("--disable-spork23", dest="disable_spork23", default=False, action="store_true", + help="Test with spork21 enabled") + def run_test(self): + if self.options.disable_spork23: + self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 4070908800) + else: + self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 0) self.deaf_mns = [] self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) @@ -35,32 +43,26 @@ def run_test(self): self.repair_masternodes(False) self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) - self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 0) self.wait_for_sporks_same() self.reset_probe_timeouts() - # Lets restart masternodes with closed ports and verify that they get banned even though they are connected to other MNs (via outbound connections) - self.test_banning(self.close_mn_port) - self.deaf_mns.clear() - - self.repair_masternodes(True) - self.reset_probe_timeouts() - - self.test_banning(self.force_old_mn_proto, 3) + if not self.options.disable_spork23: + # Lets restart masternodes with closed ports and verify that they get banned even though they are connected to other MNs (via outbound connections) + self.test_banning(self.close_mn_port) + else: + # With PoSe off there should be no punishing for non-reachable nodes + self.test_no_banning(self.close_mn_port, 3) - # With PoSe off there should be no punishing for non-reachable and outdated nodes - self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 4070908800) - self.wait_for_sporks_same() + self.deaf_mns.clear() self.repair_masternodes(True) - self.force_old_mn_proto(self.mninfo[0]) - self.test_no_banning(3) - self.repair_masternodes(True) - self.close_mn_port(self.mninfo[0]) - self.deaf_mns.clear() - self.test_no_banning(3) + if not self.options.disable_spork23: + self.test_banning(self.force_old_mn_proto, 3) + else: + # With PoSe off there should be no punishing for outdated nodes + self.test_no_banning(self.force_old_mn_proto, 3) def isolate_mn(self, mn): mn.node.setnetworkactive(False) @@ -86,7 +88,8 @@ def force_old_mn_proto(self, mn): self.reset_probe_timeouts() return False, True - def test_no_banning(self, expected_connections=None): + def test_no_banning(self, invalidate_proc, expected_connections=None): + invalidate_proc(self.mninfo[0]) for i in range(3): self.log.info(f"Testing no PoSe banning in normal conditions {i + 1}/3") self.mine_quorum(expected_connections=expected_connections) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 8dcecb2234f4..770618362ea8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -126,6 +126,7 @@ 'feature_llmq_is_retroactive.py', # NOTE: needs dash_hash to pass 'feature_llmq_chainlocks.py', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose.py', # NOTE: needs dash_hash to pass + 'feature_llmq_simplepose.py --disable-spork23', # NOTE: needs dash_hash to pass 'feature_dip3_deterministicmns.py --legacy-wallet', # NOTE: needs dash_hash to pass 'feature_dip3_deterministicmns.py --descriptors', # NOTE: needs dash_hash to pass 'feature_llmq_signing.py', # NOTE: needs dash_hash to pass From 7057df474ee6144b4b25b839af034d880e03073f Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 23 Apr 2025 22:35:37 +0700 Subject: [PATCH 5/5] test: trivial fixes to resolve review comments --- test/functional/feature_llmq_simplepose.py | 6 +++--- test/functional/test_framework/masternodes.py | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index fe7d8452cffb..df12d4bea424 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -18,14 +18,14 @@ class LLMQSimplePoSeTest(DashTestFramework): def set_test_params(self): - self.extra_args = [[ f'-testactivationheight=dip0024@9999' ]] * 6 + # rotating quorums add instability for this functional tests + self.extra_args = [[ '-testactivationheight=dip0024@9999' ]] * 6 self.set_dash_test_params(6, 5) self.set_dash_llmq_test_params(5, 3) - # rotating quorums add instability for this functional tests def add_options(self, parser): parser.add_argument("--disable-spork23", dest="disable_spork23", default=False, action="store_true", - help="Test with spork21 enabled") + help="Test with spork23 disabled") def run_test(self): if self.options.disable_spork23: diff --git a/test/functional/test_framework/masternodes.py b/test/functional/test_framework/masternodes.py index 186381932089..7550c16048b4 100644 --- a/test/functional/test_framework/masternodes.py +++ b/test/functional/test_framework/masternodes.py @@ -6,12 +6,8 @@ def check_punished(node, mn): info = node.protx('info', mn.proTxHash) - if info['state']['PoSePenalty'] > 0: - return True - return False + return info['state']['PoSePenalty'] > 0 def check_banned(node, mn): info = node.protx('info', mn.proTxHash) - if info['state']['PoSeBanHeight'] != -1: - return True - return False + return info['state']['PoSeBanHeight'] != -1