test: check misbehavior more independently in p2p_filter.py #18726

pull robot-visions wants to merge 1 commits into bitcoin:master from robot-visions:p2p-filter changing 1 files +26 −5

robot-visions commented at 6:37 PM on April 21, 2020: contributor
This expands on #18672 in two ways:
- Check positive cases (filterload accepted, filteradd accepted) in addition to the negative cases added in #18672
- Address MarcoFalke 's suggestion to successfully load a filter before testing filteradd
robot-visions cross-referenced this on Apr 21, 2020 from issue test: add further BIP37 size limit checks to p2p_filter.py by theStack
DrahtBot added the label Tests on Apr 21, 2020
DrahtBot commented at 12:43 AM on April 22, 2020: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.
DrahtBot cross-referenced this on Apr 22, 2020 from issue test: enable opening partial p2p connections where useful by jonatack
theStack commented at 11:29 AM on April 22, 2020: contributor

Concept ACK -- nit: now with so many small tests for checking the size limits, you could put them all in a new method of the test?
robot-visions force-pushed on Apr 22, 2020
robot-visions commented at 5:39 PM on April 22, 2020: contributor

Good idea, updated! And thanks for the review.

in test/functional/p2p_filter.py:83 in edf2491c73 outdated

  83 | +        with self.nodes[0].assert_debug_log(['Misbehaving']):
  84 | +            filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS+1))
  85 | +
  86 | +        self.log.info('Check that filter with max hash functions is accepted')
  87 | +        with self.nodes[0].assert_debug_log([], unexpected_msgs=['Misbehaving']):
  88 | +            filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS, nTweak=0, nFlags=1))

theStack commented at 12:43 PM on April 25, 2020:

nit: explicitely setting nTweak and nFlags is not needed here

robot-visions commented at 4:29 PM on April 25, 2020:

Fixed, thanks!

theStack approved
theStack commented at 12:44 PM on April 25, 2020: contributor

ACK https://github.com/bitcoin/bitcoin/pull/18726/commits/edf2491c733c9063c22e2cb8e04b1c3af874d79e
test: check misbehavior more independently in p2p_filter.py cd543d9193
robot-visions force-pushed on Apr 25, 2020
theStack commented at 4:43 PM on April 25, 2020: contributor

re-ACK https://github.com/bitcoin/bitcoin/commit/cd543d9193ac1882c1b4a8a84e3ac7356a8b7ce9
MarcoFalke merged this on Apr 29, 2020
MarcoFalke closed this on Apr 29, 2020
robot-visions deleted the branch on Apr 29, 2020
sidhujag referenced this in commit 44a764d706 on May 2, 2020
Fabcien referenced this in commit a2a9c5790d on Jan 28, 2021
bitcoin locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18672 test: add further BIP37 size limit checks to p2p_filter.py

test: check misbehavior more independently in p2p_filter.py #18726

Conflicts