Skip to content

Commit

Permalink
Merge bitcoin#19564: test: p2p_feefilter improvements (logging, refac…
Browse files Browse the repository at this point in the history
…toring, speedup)

9e78943 test: speedup p2p_feefilter.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
fe3f0cc test: use wait_until for invs matching in p2p_feefilter.py (Sebastian Falbesoner)
6d94192 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](bitcoin@12410b1)) 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. bitcoin#17121, bitcoin#17124, bitcoin#17340, bitcoin#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 bitcoin@9e78943
  jonatack:
    re-ACK 9e78943 per `git range-diff 3ab2582 ea74a3c 9e78943`

Tree-SHA512: fe21c1c5413df9165fea916b5d5f609d3ba33e7b5c3364b38eb824fcc55d9e6abddf27116cbc0b325913d451a73c44542040fb916aec9c46f805c6e12f6f10cf
  • Loading branch information
MarcoFalke authored and sidhujag committed Aug 16, 2020
1 parent d5dd38f commit 9f7e4e2
Showing 1 changed file with 19 additions and 23 deletions.
42 changes: 19 additions & 23 deletions test/functional/p2p_feefilter.py
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -48,6 +37,10 @@ def on_inv(self, message):
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 = []
Expand All @@ -61,7 +54,12 @@ def set_test_params(self):
# 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()
Expand All @@ -88,24 +86,22 @@ def test_feefilter(self):

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
Expand All @@ -119,13 +115,13 @@ def test_feefilter(self):
# 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()


Expand Down

0 comments on commit 9f7e4e2

Please sign in to comment.