Merge bitcoin/bitcoin#23300: test: Implicitly sync after generate*, unless opted out

facc352648e4fe1ed9b406400b6e4a9d51f30349 test: Implicitly sync after generate*, unless opted out (MarcoFalke)

Pull request description:

  The most frequent failure in functional tests are intermittent races. Fixing such bugs is cumbersome because it involves:
  * Noticing the failure
  * Fetching and reading the log to determine the test case that failed
  * Adding a `self.sync_all()` where it was forgotten
  * Spamming out a pr and waiting for review, which is already sparse

  Also, writing a linter to catch those is not possible, nor is review effective in finding these bugs prior to merge.

  Fix all future intermittent races caused by a missing sync_block call by calling `sync_all` implicitly after each `generate*`, unless opted out. This ensures that the code is race-free (with regards to blocks) when the tests pass once, instead of our current approach where the code can never be guaranteed to be race-free.

  There are some scripted-diff cleanups (see https://github.com/bitcoin/bitcoin/pull/22567), but they will be submitted in a follow-up to reduce the conflicts in this pull.

ACKs for top commit:
  lsilva01:
    tACK facc352 on Ubuntu 20.04
  brunoerg:
    tACK facc352648e4fe1ed9b406400b6e4a9d51f30349 on MacOS 11.6

Tree-SHA512: 046a40a066b4a3bd28a3077bd654fa8887442dd1f0ec6fd11671865809ef02376f126eb667a1320ebd67b6e372c78c00dbf8bd25d86ed86f1d9a25363103ed97

2 files changed, 15 insertions, 8 deletions

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index ec3561b1f2..84d94b34f7 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -413,7 +413,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             # To ensure that all nodes are out of IBD, the most recent block
             # must have a timestamp not too old (see IsInitialBlockDownload()).
             self.log.debug('Generate a block with current time')
-            block_hash = self.generate(self.nodes[0], 1)[0]
+            block_hash = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]
             block = self.nodes[0].getblock(blockhash=block_hash, verbosity=0)
             for n in self.nodes:
                 n.submitblock(block)
@@ -627,20 +627,27 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         self.connect_nodes(1, 2)
         self.sync_all()
 
-    def generate(self, generator, *args, **kwargs):
+    def no_op(self):
+        pass
+
+    def generate(self, generator, *args, sync_fun=None, **kwargs):
         blocks = generator.generate(*args, invalid_call=False, **kwargs)
+        sync_fun() if sync_fun else self.sync_all()
         return blocks
 
-    def generateblock(self, generator, *args, **kwargs):
+    def generateblock(self, generator, *args, sync_fun=None, **kwargs):
         blocks = generator.generateblock(*args, invalid_call=False, **kwargs)
+        sync_fun() if sync_fun else self.sync_all()
         return blocks
 
-    def generatetoaddress(self, generator, *args, **kwargs):
+    def generatetoaddress(self, generator, *args, sync_fun=None, **kwargs):
         blocks = generator.generatetoaddress(*args, invalid_call=False, **kwargs)
+        sync_fun() if sync_fun else self.sync_all()
         return blocks
 
-    def generatetodescriptor(self, generator, *args, **kwargs):
+    def generatetodescriptor(self, generator, *args, sync_fun=None, **kwargs):
         blocks = generator.generatetodescriptor(*args, invalid_call=False, **kwargs)
+        sync_fun() if sync_fun else self.sync_all()
         return blocks
 
     def sync_blocks(self, nodes=None, wait=1, timeout=60):
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 0678e2e6fe..ca1ffd48de 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -463,10 +463,10 @@ def find_output(node, txid, amount, *, blockhash=None):
 
 # Helper to create at least "count" utxos
 # Pass in a fee that is sufficient for relay and mining new transactions.
-def create_confirmed_utxos(test_framework, fee, node, count):
+def create_confirmed_utxos(test_framework, fee, node, count, **kwargs):
     to_generate = int(0.5 * count) + 101
     while to_generate > 0:
-        test_framework.generate(node, min(25, to_generate))
+        test_framework.generate(node, min(25, to_generate), **kwargs)
         to_generate -= 25
     utxos = node.listunspent()
     iterations = count - len(utxos)
@@ -487,7 +487,7 @@ def create_confirmed_utxos(test_framework, fee, node, count):
         node.sendrawtransaction(signed_tx)
 
     while (node.getmempoolinfo()['size'] > 0):
-        test_framework.generate(node, 1)
+        test_framework.generate(node, 1, **kwargs)
 
     utxos = node.listunspent()
     assert len(utxos) >= count