Skip to content

Commit

Permalink
Merge bitcoin#19252: test: wait for disconnect in disconnect_p2ps + b…
Browse files Browse the repository at this point in the history
…loomfilter test followups

9a40cfc [refactor] use waiting inside disconnect_p2ps (gzhao408)
aeb9fb4 [test] wait for disconnect_p2ps to be reflected in getpeerinfo (gzhao408)
e81942d [test] logging and style followups for bloomfilter tests (gzhao408)

Pull request description:

  Followup to bitcoin#19083 which adds bloomfilter-related tests.

  1. Make test_node `disconnect_p2ps` wait until disconnection is complete to avoid race conditions (and not place the burden on tests) from MarcoFalke's [comment](bitcoin#19083 (comment)). And clean up any redundant `wait_until`s in the functional tests.
  2. Clean up style + logging in p2p_filter.py and p2p_nobloomfilter_messages.py and jonatack's other [comments](bitcoin#19083 (review))

ACKs for top commit:
  jonatack:
    Code review ACK 9a40cfc from re-reviewing the diff and `git range-diff 5cafb46 8386ad5 9a40cfc`
  MarcoFalke:
    ACK 9a40cfc 🐂

Tree-SHA512: 2e14b1c12fc08a355bd5ccad7a2a734a4ccda4bc7dc7bac171cb57359819fc1599d764290729af74832fac3e2be258c5d406c701e78ab6d7262835859b9a7d87
  • Loading branch information
MarcoFalke authored and knst committed Jan 16, 2024
1 parent f7ddb7e commit 1027cce
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
6 changes: 3 additions & 3 deletions test/functional/p2p_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ def test_msg_mempool(self):
self.log.info("Check that a node with bloom filters enabled services p2p mempool messages")
filter_peer = P2PBloomFilter()

self.log.info("Create a tx relevant to the peer before connecting")
self.log.debug("Create a tx relevant to the peer before connecting")
filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0]
txid = self.nodes[0].sendtoaddress(filter_address, 90)

self.log.info("Send a mempool msg after connecting and check that the tx is received")
self.log.debug("Send a mempool msg after connecting and check that the tx is received")
self.nodes[0].add_p2p_connection(filter_peer)
filter_peer.send_and_ping(filter_peer.watch_filter_init)
self.nodes[0].p2p.send_message(msg_mempool())
Expand Down Expand Up @@ -226,8 +226,8 @@ def run_test(self):
self.test_frelay_false(filter_peer_without_nrelay)
self.test_filter(filter_peer_without_nrelay)

self.log.info('Test msg_mempool')
self.test_msg_mempool()


if __name__ == '__main__':
FilterTest().main()
17 changes: 9 additions & 8 deletions test/functional/p2p_nobloomfilter_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test invalid p2p messages for nodes with bloom filters disabled.
Test that, when bloom filters are not enabled, nodes are disconnected if:
Test that, when bloom filters are not enabled, peers are disconnected if:
1. They send a p2p mempool message
2. They send a p2p filterload message
3. They send a p2p filteradd message
Expand All @@ -17,31 +17,32 @@
from test_framework.util import assert_equal


class P2PNobloomfilterMessages(BitcoinTestFramework):
class P2PNoBloomFilterMessages(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 1
self.extra_args = [["-peerbloomfilters=0"]]

def test_message_causes_disconnect(self, message):
# Add a p2p connection that sends a message and check that it disconnects
"""Add a p2p connection that sends a message and check that it disconnects."""
peer = self.nodes[0].add_p2p_connection(P2PInterface())
peer.send_message(message)
peer.wait_for_disconnect()
assert_equal(len(self.nodes[0].getpeerinfo()), 0)
assert_equal(self.nodes[0].getconnectioncount(), 0)

def run_test(self):
self.log.info("Test that node is disconnected if it sends mempool message")
self.log.info("Test that peer is disconnected if it sends mempool message")
self.test_message_causes_disconnect(msg_mempool())

self.log.info("Test that node is disconnected if it sends filterload message")
self.log.info("Test that peer is disconnected if it sends filterload message")
self.test_message_causes_disconnect(msg_filterload())

self.log.info("Test that node is disconnected if it sends filteradd message")
self.log.info("Test that peer is disconnected if it sends filteradd message")
self.test_message_causes_disconnect(msg_filteradd(data=b'\xcc'))

self.log.info("Test that peer is disconnected if it sends a filterclear message")
self.test_message_causes_disconnect(msg_filterclear())


if __name__ == '__main__':
P2PNobloomfilterMessages().main()
P2PNoBloomFilterMessages().main()

0 comments on commit 1027cce

Please sign in to comment.