Merge bitcoin/bitcoin#30948: test: Add missing sync_mempools() to fill_mempool()

faf8015 test: Add missing sync_mempools() to fill_mempool() (MarcoFalke)
fa48be6 test: Refactor fill_mempool to extract send_batch helper (MarcoFalke)

Pull request description:

  Not doing the sync will lead to (intermittent) issues, as explained in bitcoin/bitcoin#30922 (comment).

  Fix all issues by doing the sync by default and disable it in places that do not need the sync.

  Fixes #30922

ACKs for top commit:
  mzumsande:
    Tested ACK faf8015
  ismaelsadeeq:
    Tested ACK faf8015
  marcofleon:
    Tested ACK faf8015

Tree-SHA512: 2de62d168cbb6857a9fb8bc12c42a9093fedf5e9beb6f83a32b3fa72a5ba3cf03631055fd25ef553399a27a6fe0d71c44cfe60660a4d31986debd13b8ab00228

Author: fanquake

test/functional/mempool_package_rbf.py

@@ -554,7 +554,7 @@ def test_0fee_package_rbf(self):
         self.generate(node, 1)
 
     def test_child_conflicts_parent_mempool_ancestor(self):
-        fill_mempool(self, self.nodes[0])
+        fill_mempool(self, self.nodes[0], tx_sync_fun=self.no_op)
         # Reset coins since we filled the mempool with current coins
         self.coins = self.wallet.get_utxos(mark_as_spent=False, confirmed_only=True)
 

test/functional/p2p_1p1c_network.py

@@ -49,9 +49,6 @@ def set_test_params(self):
     def raise_network_minfee(self):
         fill_mempool(self, self.nodes[0])
 
-        self.log.debug("Wait for the network to sync mempools")
-        self.sync_mempools()
-
         self.log.debug("Check that all nodes' mempool minimum feerates are above min relay feerate")
         for node in self.nodes:
             assert_equal(node.getmempoolinfo()['minrelaytxfee'], FEERATE_1SAT_VB)

test/functional/p2p_tx_download.py

@@ -250,7 +250,7 @@ def test_spurious_notfound(self):
     def test_rejects_filter_reset(self):
         self.log.info('Check that rejected tx is not requested again')
         node = self.nodes[0]
-        fill_mempool(self, node)
+        fill_mempool(self, node, tx_sync_fun=self.no_op)
         self.wallet.rescan_utxos()
         mempoolminfee = node.getmempoolinfo()['mempoolminfee']
         peer = node.add_p2p_connection(TestP2PConn())

test/functional/test_framework/mempool_util.py

@@ -19,14 +19,11 @@
 )
 
 
-def fill_mempool(test_framework, node):
+def fill_mempool(test_framework, node, *, tx_sync_fun=None):
     """Fill mempool until eviction.
 
     Allows for simpler testing of scenarios with floating mempoolminfee > minrelay
-    Requires -datacarriersize=100000 and
-   -maxmempool=5.
-    It will not ensure mempools become synced as it
-    is based on a single node and assumes -minrelaytxfee
+    Requires -datacarriersize=100000 and -maxmempool=5 and assumes -minrelaytxfee
     is 1 sat/vbyte.
     To avoid unintentional tx dependencies, the mempool filling txs are created with a
     tagged ephemeral miniwallet instance.
@@ -57,18 +54,25 @@ def fill_mempool(test_framework, node):
     tx_to_be_evicted_id = ephemeral_miniwallet.send_self_transfer(
         from_node=node, utxo_to_spend=confirmed_utxos.pop(0), fee_rate=relayfee)["txid"]
 
+    def send_batch(fee):
+        utxos = confirmed_utxos[:tx_batch_size]
+        create_lots_of_big_transactions(ephemeral_miniwallet, node, fee, tx_batch_size, txouts, utxos)
+        del confirmed_utxos[:tx_batch_size]
+
     # Increase the tx fee rate to give the subsequent transactions a higher priority in the mempool
     # The tx has an approx. vsize of 65k, i.e. multiplying the previous fee rate (in sats/kvB)
     # by 130 should result in a fee that corresponds to 2x of that fee rate
     base_fee = relayfee * 130
+    batch_fees = [(i + 1) * base_fee for i in range(num_of_batches)]
 
     test_framework.log.debug("Fill up the mempool with txs with higher fee rate")
-    with node.assert_debug_log(["rolling minimum fee bumped"]):
-        for batch_of_txid in range(num_of_batches):
-            fee = (batch_of_txid + 1) * base_fee
-            utxos = confirmed_utxos[:tx_batch_size]
-            create_lots_of_big_transactions(ephemeral_miniwallet, node, fee, tx_batch_size, txouts, utxos)
-            del confirmed_utxos[:tx_batch_size]
+    for fee in batch_fees[:-3]:
+        send_batch(fee)
+    tx_sync_fun() if tx_sync_fun else test_framework.sync_mempools()  # sync before any eviction
+    assert_equal(node.getmempoolinfo()["mempoolminfee"], Decimal("0.00001000"))
+    for fee in batch_fees[-3:]:
+        send_batch(fee)
+    tx_sync_fun() if tx_sync_fun else test_framework.sync_mempools()  # sync after all evictions
 
     test_framework.log.debug("The tx should be evicted by now")
     # The number of transactions created should be greater than the ones present in the mempool