Merge #19564: test: p2p_feefilter improvements (logging, refactoring, speedup)

9e7894357e296dbe0be775e1527654729dbb71b0 test: speedup p2p_feefilter.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
fe3f0cc44ec6301a86de48cdc3883377932e44eb test: use wait_until for invs matching in p2p_feefilter.py (Sebastian Falbesoner)
6d941923c318ce9cb2380d3e41ffb760636656a2 test: add logging for p2p_feefilter.py (Sebastian Falbesoner)

Pull request description:

  This PR gives some love to the functional test `p2p_feefilter.py` by introducing the following changes:
  * add missing log messages for the `test_feefilter` subtest (the main one)
  * deduplicate code by using the utility function `wait_until` (already using the [recently introduced version](https://github.com/bitcoin/bitcoin/commit/12410b1feb80189061eb4a2b43421e53cbb758a8)) instead of a manual condition checking loop with `time.sleep`
  * improve naming of the function `matchAllInvs` (more expressive name, snake_case) and moving it from global namespace to the connection class `FeefilterConn`
  * speeding up the test significantly by the good ol' method of activating immediate tx relay (as seen on e.g. https://github.com/bitcoin/bitcoin/pull/17121, https://github.com/bitcoin/bitcoin/pull/17124, https://github.com/bitcoin/bitcoin/pull/17340, https://github.com/bitcoin/bitcoin/pull/17362, ...):
  ```
  master branch:
  $ time ./p2p_feefilter.py
  ...
  real    0m39.367s
  user    0m1.227s
  sys 0m0.571s

  PR branch:
  $ time ./p2p_feefilter.py
  ...
  real    0m9.386s
  user    0m1.120s
  sys 0m0.577s
  ```

ACKs for top commit:
  instagibbs:
    code review ACK https://github.com/bitcoin/bitcoin/pull/19564/commits/9e7894357e296dbe0be775e1527654729dbb71b0
  jonatack:
    re-ACK 9e78943 per `git range-diff 3ab2582 ea74a3c 9e78943`

Tree-SHA512: fe21c1c5413df9165fea916b5d5f609d3ba33e7b5c3364b38eb824fcc55d9e6abddf27116cbc0b325913d451a73c44542040fb916aec9c46f805c6e12f6f10cf

1 files changed, 19 insertions, 23 deletions

diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py
index 0b51d8f4bb..3a9b8dfbd7 100755
--- a/test/functional/p2p_feefilter.py
+++ b/test/functional/p2p_feefilter.py
@@ -5,7 +5,6 @@
 """Test processing of feefilter messages."""
 
 from decimal import Decimal
-import time
 
 from test_framework.messages import MSG_TX, MSG_WTX, msg_feefilter
 from test_framework.mininode import mininode_lock, P2PInterface
@@ -17,16 +16,6 @@ def hashToHex(hash):
     return format(hash, '064x')
 
 
-# Wait up to 60 secs to see if the testnode has received all the expected invs
-def allInvsMatch(invsExpected, testnode):
-    for _ in range(60):
-        with mininode_lock:
-            if (sorted(invsExpected) == sorted(testnode.txinvs)):
-                return True
-        time.sleep(1)
-    return False
-
-
 class FeefilterConn(P2PInterface):
     feefilter_received = False
 
@@ -48,6 +37,10 @@ class TestP2PConn(P2PInterface):
             if (i.type == MSG_TX) or (i.type == MSG_WTX):
                 self.txinvs.append(hashToHex(i.hash))
 
+    def wait_for_invs_to_match(self, invs_expected):
+        invs_expected.sort()
+        self.wait_until(lambda: invs_expected == sorted(self.txinvs), timeout=60)
+
     def clear_invs(self):
         with mininode_lock:
             self.txinvs = []
@@ -61,7 +54,12 @@ class FeeFilterTest(BitcoinTestFramework):
         # mempool and wallet feerate calculation based on GetFee
         # rounding down 3 places, leading to stranded transactions.
         # See issue #16499
-        self.extra_args = [["-minrelaytxfee=0.00000100", "-mintxfee=0.00000100"]] * self.num_nodes
+        # grant noban permission to all peers to speed up tx relay / mempool sync
+        self.extra_args = [[
+            "-minrelaytxfee=0.00000100",
+            "-mintxfee=0.00000100",
+            "-whitelist=noban@127.0.0.1",
+        ]] * self.num_nodes
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -88,24 +86,22 @@ class FeeFilterTest(BitcoinTestFramework):
 
         conn = self.nodes[0].add_p2p_connection(TestP2PConn())
 
-        # Test that invs are received by test connection for all txs at
-        # feerate of .2 sat/byte
+        self.log.info("Test txs paying 0.2 sat/byte are received by test connection")
         node1.settxfee(Decimal("0.00000200"))
         txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)]
-        assert allInvsMatch(txids, conn)
+        conn.wait_for_invs_to_match(txids)
         conn.clear_invs()
 
-        # Set a filter of .15 sat/byte on test connection
+        # Set a fee filter of 0.15 sat/byte on test connection
         conn.send_and_ping(msg_feefilter(150))
 
-        # Test that txs are still being received by test connection (paying .15 sat/byte)
+        self.log.info("Test txs paying 0.15 sat/byte are received by test connection")
         node1.settxfee(Decimal("0.00000150"))
         txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)]
-        assert allInvsMatch(txids, conn)
+        conn.wait_for_invs_to_match(txids)
         conn.clear_invs()
 
-        # Change tx fee rate to .1 sat/byte and test they are no longer received
-        # by the test connection
+        self.log.info("Test txs paying 0.1 sat/byte are no longer received by test connection")
         node1.settxfee(Decimal("0.00000100"))
         [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)]
         self.sync_mempools()  # must be sure node 0 has received all txs
@@ -119,13 +115,13 @@ class FeeFilterTest(BitcoinTestFramework):
         # as well.
         node0.settxfee(Decimal("0.00020000"))
         txids = [node0.sendtoaddress(node0.getnewaddress(), 1)]
-        assert allInvsMatch(txids, conn)
+        conn.wait_for_invs_to_match(txids)
         conn.clear_invs()
 
-        # Remove fee filter and check that txs are received again
+        self.log.info("Remove fee filter and check txs are received again")
         conn.send_and_ping(msg_feefilter(0))
         txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)]
-        assert allInvsMatch(txids, conn)
+        conn.wait_for_invs_to_match(txids)
         conn.clear_invs()