Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28227: test: check for specific bip157 disconne…
Browse files Browse the repository at this point in the history
…ct reasons, add test coverage

2ab7952 test: add bip157 coverage for (start height > stop height) disconnect (Sebastian Falbesoner)
63e90e1 test: check for specific disconnect reasons in p2p_blockfilters.py (Sebastian Falbesoner)

Pull request description:

  This PR checks for specific disconnect reasons using `assert_debug_log` in the functional test `p2p_blockfilters.py`. With that we ensure that the disconnect happens for the expected reason and also makes it easier to navigate between implementation and test code, i.e. both the questions "do we have test coverage for this disconnect cause?" (from an implementation reader's perspective) and "where is the code handling this disconnect cause?" (from a test reader's perspective) can be answered simply by grep-ping the corresponding debug message.

  Also, based on that, missing coverage for the (start height > stop height) disconnect case is added:
  https://github.com/bitcoin/bitcoin/blob/b7138252ace6d21476964774e094ed1143cd7a1c/src/net_processing.cpp#L3050-L3056

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 2ab7952
  furszy:
    Looks good, code ACK 2ab7952

Tree-SHA512: 0581cb569d5935aaa004a95a6f16eeafe628b9d816ebb89232f2832e377049df878a1e74c369fb46931b94e1a3a5e3f4aaa21a007c0a488f4ad2cda0919c605d
  • Loading branch information
fanquake committed Oct 2, 2023
2 parents 8b44d01 + 2ab7952 commit e3b0528
Showing 1 changed file with 37 additions and 19 deletions.
56 changes: 37 additions & 19 deletions test/functional/p2p_blockfilters.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,38 +211,56 @@ def run_test(self):
]
for request in requests:
peer_1 = self.nodes[1].add_p2p_connection(P2PInterface())
peer_1.send_message(request)
peer_1.wait_for_disconnect()
with self.nodes[1].assert_debug_log(expected_msgs=["requested unsupported block filter type"]):
peer_1.send_message(request)
peer_1.wait_for_disconnect()

self.log.info("Check that invalid requests result in disconnection.")
requests = [
# Requesting too many filters results in disconnection.
msg_getcfilters(
filter_type=FILTER_TYPE_BASIC,
start_height=0,
stop_hash=int(main_block_hash, 16),
(
msg_getcfilters(
filter_type=FILTER_TYPE_BASIC,
start_height=0,
stop_hash=int(main_block_hash, 16),
), "requested too many cfilters/cfheaders"
),
# Requesting too many filter headers results in disconnection.
msg_getcfheaders(
filter_type=FILTER_TYPE_BASIC,
start_height=0,
stop_hash=int(tip_hash, 16),
(
msg_getcfheaders(
filter_type=FILTER_TYPE_BASIC,
start_height=0,
stop_hash=int(tip_hash, 16),
), "requested too many cfilters/cfheaders"
),
# Requesting unknown filter type results in disconnection.
msg_getcfcheckpt(
filter_type=255,
stop_hash=int(main_block_hash, 16),
(
msg_getcfcheckpt(
filter_type=255,
stop_hash=int(main_block_hash, 16),
), "requested unsupported block filter type"
),
# Requesting unknown hash results in disconnection.
msg_getcfcheckpt(
filter_type=FILTER_TYPE_BASIC,
stop_hash=123456789,
(
msg_getcfcheckpt(
filter_type=FILTER_TYPE_BASIC,
stop_hash=123456789,
), "requested invalid block hash"
),
(
# Request with (start block height > stop block height) results in disconnection.
msg_getcfheaders(
filter_type=FILTER_TYPE_BASIC,
start_height=1000,
stop_hash=int(self.nodes[0].getblockhash(999), 16),
), "sent invalid getcfilters/getcfheaders with start height 1000 and stop height 999"
),
]
for request in requests:
for request, expected_log_msg in requests:
peer_0 = self.nodes[0].add_p2p_connection(P2PInterface())
peer_0.send_message(request)
peer_0.wait_for_disconnect()
with self.nodes[0].assert_debug_log(expected_msgs=[expected_log_msg]):
peer_0.send_message(request)
peer_0.wait_for_disconnect()

self.log.info("Test -peerblockfilters without -blockfilterindex raises an error")
self.stop_node(0)
Expand Down

0 comments on commit e3b0528

Please sign in to comment.