Merge dashpay#6521: test: optimization of functional tests

9bec526 test: speed up feature_maxuploadtarget.py - missing changes from backport bitcoin#18494 (Konstantin Akimov)
4d4a1f5 test: speed up feature_mnehf.py a bit more (Konstantin Akimov)
1b3b546 test: speed up feature_mnehf.py functional test (Konstantin Akimov)
0219735 test: speed up functional test feature_llmq_chainlocks.py (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Functional tests running locally takes too long time; and limitation is not even CPU and RAM for many of them.
  This PR helps to speed some of the functional tests.

  ## What was done?
  Not much has been done, but some of the low-hanging fruit has been picked:
   - add missing changes from backport bitcoin#18494 which makes `feature_maxuploadtarget.py` much faster
   - optimize feature_mnehf.py - no need to restart all nodes that often to be sure they work correctly; replaced some sleeps to `wait_until`
   - optimize feature_llmq_chainlocks.py - change sleeps to `wait_until` with timeout

  ## How Has This Been Tested?

  Before this PR:
  ```
  TEST                       | STATUS    | DURATION

  feature_llmq_chainlocks.py | ✓ Passed  | 230 s
  feature_maxuploadtarget.py | ✓ Passed  | 291 s
  feature_mnehf.py           | ✓ Passed  | 109 s

  ALL                        | ✓ Passed  | 630 s (accumulated)
  ```

  With these optimizations:
  ```
  TEST                       | STATUS    | DURATION

  feature_llmq_chainlocks.py | ✓ Passed  | 123 s
  feature_maxuploadtarget.py | ✓ Passed  | 191 s
  feature_mnehf.py           | ✓ Passed  | 86 s

  ALL                        | ✓ Passed  | 400 s (accumulated)
  ```

  CI finishes 2 minutes faster (for non-tsan):
  https://gitlab.com/dashpay/dash/-/jobs/8739701280
  https://gitlab.com/dashpay/dash/-/jobs/8776172870

  Size of artefacts on CI is slightly smaller (4 MB smaller for ubsan): 759 -> 755MB.

  ## 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 _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 9bec526
  PastaPastaPasta:
    utACK 9bec526

Tree-SHA512: 82bab74266733fc231321210e56c3e9d79504b1259b30ad9cd98e5e063ce48dcd7549dec7e4a0297242bc201708a57eb159a2938ba049eab5d3f616a9ead605d

test/functional/feature_llmq_chainlocks.py

@@ -10,7 +10,6 @@
 
 '''
 
-import time
 from io import BytesIO
 
 from test_framework.messages import CBlock, CCbTx
@@ -84,7 +83,7 @@ def run_test(self):
         previous_block_hash = self.nodes[0].getbestblockhash()
         for _ in range(2):
             block_hash = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]
-            self.wait_for_chainlocked_block_all_nodes(block_hash, expected=False)
+            self.wait_for_chainlocked_block_all_nodes(block_hash, timeout=5, expected=False)
             assert self.nodes[0].getblock(previous_block_hash)["chainlock"]
 
         self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)
@@ -157,12 +156,10 @@ def run_test(self):
         self.nodes[0].invalidateblock(good_tip)
         self.log.info("Now try to reorg the chain")
         self.generate(self.nodes[0], 2, sync_fun=self.no_op)
-        time.sleep(6)
-        assert self.nodes[1].getbestblockhash() == good_tip
+        self.wait_until(lambda: self.nodes[1].getbestblockhash() == good_tip, timeout=6)
         bad_tip = self.generate(self.nodes[0], 2, sync_fun=self.no_op)[-1]
-        time.sleep(6)
-        assert self.nodes[0].getbestblockhash() == bad_tip
-        assert self.nodes[1].getbestblockhash() == good_tip
+        self.wait_until(lambda: self.nodes[1].getbestblockhash() == good_tip and
+                self.nodes[0].getbestblockhash() == bad_tip, timeout=6)
 
         self.log.info("Now let the node which is on the wrong chain reorg back to the locked chain")
         self.nodes[0].reconsiderblock(good_tip)
@@ -185,8 +182,7 @@ def run_test(self):
         self.restart_node(0)
         self.nodes[0].invalidateblock(good_fork)
         self.restart_node(0)
-        time.sleep(1)
-        assert self.nodes[0].getbestblockhash() == good_tip
+        self.wait_until(lambda: self.nodes[0].getbestblockhash() == good_tip, timeout=5)
 
         self.log.info("Isolate a node and let it create some transactions which won't get IS locked")
         force_finish_mnsync(self.nodes[0])
@@ -200,10 +196,15 @@ def run_test(self):
         for txid in txs:
             tx = self.nodes[0].getrawtransaction(txid, 1)
             assert "confirmations" not in tx
-        time.sleep(1)
-        node0_tip_block = self.nodes[0].getblock(node0_tip)
-        assert not node0_tip_block["chainlock"]
-        assert node0_tip_block["previousblockhash"] == good_tip
+
+        def test_cb(self):
+            node0_tip_block = self.nodes[0].getblock(node0_tip)
+            if node0_tip_block["chainlock"]:
+                return False
+            return node0_tip_block["previousblockhash"] == good_tip
+        self.wait_until(lambda: test_cb(self), timeout=5)
+
+
         self.log.info("Disable LLMQ based InstantSend for a very short time (this never gets propagated to other nodes)")
         self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 4070908800)
         self.log.info("Now the TXs should be included")

test/functional/feature_maxuploadtarget.py

@@ -121,7 +121,6 @@ def run_test(self):
         getdata_request.inv = [CInv(MSG_BLOCK, big_new_block)]
         for i in range(200):
             p2p_conns[1].send_and_ping(getdata_request)
-            p2p_conns[1].sync_with_ping()
             assert_equal(p2p_conns[1].block_receive_map[big_new_block], i+1)
 
         self.log.info("Peer 1 able to repeatedly download new block")

test/functional/feature_mnehf.py

@@ -5,7 +5,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 import struct
-import time
 from io import BytesIO
 
 from test_framework.authproxy import JSONRPCException
@@ -25,7 +24,7 @@
 
 class MnehfTest(DashTestFramework):
     def set_test_params(self):
-        extra_args = [["-vbparams=testdummy:0:999999999999:0:4:4:4:5:1", "-persistmempool=0"] for _ in range(4)]
+        extra_args = [["-vbparams=testdummy:0:999999999999:0:4:4:4:5:1", "-persistmempool=0"]] * 4
         self.set_dash_test_params(4, 3, extra_args=extra_args)
 
     def skip_test_if_missing_module(self):
@@ -122,8 +121,8 @@ def run_test(self):
         node = self.nodes[0]
 
         self.set_sporks()
+        self.log.info("Consensus rules assume there're no EHF signal before V20")
         self.activate_v20()
-        self.log.info(f"After v20 activation should be plenty of blocks: {node.getblockcount()}")
 
         self.log.info("Mine a quorum...")
         self.mine_quorum()
@@ -164,17 +163,15 @@ def run_test(self):
             self.generate(node, 1)
 
 
-        self.restart_all_nodes()
-
-        for _ in range(4):
+        for _ in range(4 // 2):
             self.check_fork('started')
-            self.generate(node, 1)
+            self.generate(node, 2)
 
 
-        for i in range(4):
+        for i in range(4 // 2):
             self.check_fork('locked_in')
-            self.generate(node, 1)
-            if i == 7:
+            self.generate(node, 2)
+            if i == 1:
                 self.restart_all_nodes()
 
         self.check_fork('active')
@@ -185,9 +182,8 @@ def run_test(self):
             inode.invalidateblock(ehf_blockhash)
 
         self.log.info("Expecting for fork to be defined in next blocks because no MnEHF tx here")
-        for _ in range(4):
-            self.check_fork('defined')
-            self.generate(node, 1)
+        self.generate(node, 4)
+        self.check_fork('defined')
 
 
         self.log.info("Re-sending MnEHF for new fork")
@@ -198,10 +194,9 @@ def run_test(self):
         assert tx_sent_2 in node.getblock(ehf_blockhash_2)['tx']
 
         self.log.info(f"Generate some more block to jump to `started` status")
-        for _ in range(4):
-            self.generate(node, 1)
+        self.generate(node, 4)
         self.check_fork('started')
-        self.restart_all_nodes()
+        self.restart_node(0)
         self.check_fork('started')
 
 
@@ -226,17 +221,15 @@ def run_test(self):
         self.restart_all_nodes(params=[self.mocktime, self.mocktime + 1000000])
         self.check_fork('defined')
 
+        self.log.info("Wait MNs to sign EHF message")
         self.mine_quorum()
         self.check_fork('defined')
 
-        self.log.info("Waiting a bit to make EHF activating...")
-        self.mine_quorum()
-        for _ in range(4 * 4):
-            time.sleep(1)
+        def check_ehf_activated(self):
             self.bump_mocktime(1)
             self.generate(self.nodes[1], 1)
-        self.check_fork('active')
-
+            return get_bip9_details(self.nodes[0], 'testdummy')['status'] == 'active'
+        self.wait_until(lambda: check_ehf_activated(self))
 
 if __name__ == '__main__':
     MnehfTest().main()