Merge #6420: refactor: several trivial refactorings for test_framework and dead code removing

a34caecdff refactor: replace infinite loop with break to while in forcemninfo (Konstantin Akimov)
4ec385d020 refactor: simplify loop of node connection in test_framework (Konstantin Akimov)
fa4ba4d169 refactor: moved functions do_connect, remove_masternode closer to theirs usages (Konstantin Akimov)
8eb5d852c7 refactor: drop dead constant SPECIALTX_TYPE from quorum commitment (Konstantin Akimov)

Pull request description:

  ## What was done?
  see each commit

  ## How Has This Been Tested?
  Run unit and functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK a34caecdff

Tree-SHA512: d998839c992ab1ff36c88ef10236b2a7811b2edb81a9211a659065bf06bebc2f9174ef3268b1508b489a7b9b29a3f3e0b9fc096ff055cce6c4cacbfaf5877cd2

src/llmq/commitment.h

@@ -34,8 +34,6 @@ namespace llmq
 class CFinalCommitment
 {
 public:
-    static constexpr auto SPECIALTX_TYPE = TRANSACTION_PROVIDER_REGISTER;
-
     static constexpr uint16_t LEGACY_BLS_NON_INDEXED_QUORUM_VERSION = 1;
     static constexpr uint16_t LEGACY_BLS_INDEXED_QUORUM_VERSION = 2;
     static constexpr uint16_t BASIC_BLS_NON_INDEXED_QUORUM_VERSION = 3;

test/functional/feature_dip4_coinbasemerkleroots.py

@@ -45,6 +45,17 @@ class LLMQCoinbaseCommitmentsTest(DashTestFramework):
     def set_test_params(self):
         self.extra_args = [[ f'-testactivationheight=dip0008@{DIP0008_HEIGHT}', "-vbparams=testdummy:999999999999:999999999999" ]] * 4
         self.set_dash_test_params(4, 3, extra_args = self.extra_args)
+
+    def remove_masternode(self, idx):
+        mn = 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"])
+        self.generate(self.nodes[0], 1)
+        self.mninfo.remove(mn)
+
+        self.log.info("Removed masternode %d", idx)
+
     def run_test(self):
         # No IS or Chainlocks in this test
         self.bump_mocktime(1)

test/functional/test_framework/test_framework.py

@@ -1431,16 +1431,6 @@ class DashTestFramework(BitcoinTestFramework):
 
         self.log.info("Prepared MN %d: collateral_txid=%s, collateral_vout=%d, protxHash=%s" % (idx, txid, collateral_vout, proTxHash))
 
-    def remove_masternode(self, idx):
-        mn = 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"])
-        self.generate(self.nodes[0], 1)
-        self.mninfo.remove(mn)
-
-        self.log.info("Removed masternode %d", idx)
-
     def prepare_datadirs(self):
         # stop faucet node so that we can copy the datadir
         self.stop_node(0)
@@ -1461,10 +1451,6 @@ class DashTestFramework(BitcoinTestFramework):
         self.add_nodes(self.mn_count)
         executor = ThreadPoolExecutor(max_workers=20)
 
-        def do_connect(idx):
-            # Connect to the control node only, masternodes should take care of intra-quorum connections themselves
-            self.connect_nodes(self.mninfo[idx].nodeIdx, 0)
-
         jobs = []
 
         # start up nodes in parallel
@@ -1479,9 +1465,9 @@ class DashTestFramework(BitcoinTestFramework):
 
         executor.shutdown()
 
-        # connect nodes
+        # Connect to the control node only, masternodes should take care of intra-quorum connections themselves
         for idx in range(0, self.mn_count):
-            do_connect(idx)
+            self.connect_nodes(self.mninfo[idx].nodeIdx, 0)
 
     def start_masternode(self, mninfo, extra_args=None):
         args = ['-masternodeblsprivkey=%s' % mninfo.keyOperator] + self.extra_args[mninfo.nodeIdx]
@@ -1525,16 +1511,16 @@ class DashTestFramework(BitcoinTestFramework):
 
         # non-masternodes where disconnected from the control node during prepare_datadirs,
         # let's reconnect them back to make sure they receive updates
-        num_simple_nodes = self.num_nodes - self.mn_count - 1
+        num_simple_nodes = self.num_nodes - self.mn_count
-        for i in range(0, num_simple_nodes):
+        for i in range(1, num_simple_nodes):
-            self.connect_nodes(i+1, 0)
+            self.connect_nodes(i, 0)
 
         self.start_masternodes()
 
         self.bump_mocktime(1)
         self.generate(self.nodes[0], 1)
-        for i in range(0, num_simple_nodes):
+        for i in range(1, num_simple_nodes):
-            force_finish_mnsync(self.nodes[i + 1])
+            force_finish_mnsync(self.nodes[i])
 
         # Enable InstantSend (including block filtering) and ChainLocks by default
         self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)

test/functional/test_framework/util.py

@@ -526,9 +526,7 @@ def force_finish_mnsync(node):
     Masternodes won't accept incoming connections while IsSynced is false.
     Force them to switch to this state to speed things up.
     """
-    while True:
+    while not node.mnsync("status")['IsSynced']:
-        if node.mnsync("status")['IsSynced']:
-            break
         node.mnsync("next")
 
 # Transaction/Block functions