test: Add various low-level p2p tests#18628
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. Coverage
Updated at: 2020-04-14T02:20:17.491254. |
|
Concept ACK: More coverage is better |
|
|
||
| self.log.info('Check that too large filter is rejected') | ||
| with self.nodes[0].assert_debug_log(['Misbehaving']): | ||
| filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=51, nTweak=0, nFlags=1)) |
There was a problem hiding this comment.
Could also test the other filter size limit, i.e. one with a data size of > 36000 bytes.
There was a problem hiding this comment.
Will leave as-is for now to not invalidate reviews. Can be done as a follow-up
| rejected_parent = CTransaction() | ||
| rejected_parent.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_2_invalid.sha256, 0))) | ||
| rejected_parent.vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=SCRIPT_PUB_KEY_OP_TRUE)) | ||
| rejected_parent.rehash() |
There was a problem hiding this comment.
nit: could also instead use the method calc_sha256() here to be consistent with all other uses above
There was a problem hiding this comment.
I uesed calc_sha256 when later tx.sha256 was accessed and rehash when later tx.hash was accessed
c743718 test: add further BIP37 size limit checks to p2p_filter.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #18628. In addition to the hash-functions limit test introduced with commit fa4c29b, it adds checks for the following size limits as defined in [BIP37](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki): ad message type `filterload`: > The filter itself is simply a bit field of arbitrary byte-aligned size. The maximum size is **36,000 bytes**. ad message type `filteradd`: > The data field must be smaller than or equal to **520 bytes** in size (the maximum size of any potentially matched object). Also introduces new constants for the limits (or reuses the max script size constant in case for the `filteradd` limit). Also fixes #18711 by changing the misbehaviour check on "filteradd without filterset" (introduced with #18544) below to also use the more commonly used `assert_debug_log` method. ACKs for top commit: MarcoFalke: ACK c743718 robot-visions: ACK c743718 jonasschnelli: utACK c743718. Seems to fix it: https://bitcoinbuilds.org/index.php?build=2524 Tree-SHA512: a03e7639263eb36a381922afb4e1d0ed2ae286f2ad2e7bbd922509a043ddf6cfd08747e01d54d29bfb8f54b66908f653974b9c347e4ca4f43332b586778893be
…_filter.py c743718 test: add further BIP37 size limit checks to p2p_filter.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to bitcoin#18628. In addition to the hash-functions limit test introduced with commit bitcoin@fa4c29b, it adds checks for the following size limits as defined in [BIP37](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki): ad message type `filterload`: > The filter itself is simply a bit field of arbitrary byte-aligned size. The maximum size is **36,000 bytes**. ad message type `filteradd`: > The data field must be smaller than or equal to **520 bytes** in size (the maximum size of any potentially matched object). Also introduces new constants for the limits (or reuses the max script size constant in case for the `filteradd` limit). Also fixes bitcoin#18711 by changing the misbehaviour check on "filteradd without filterset" (introduced with bitcoin#18544) below to also use the more commonly used `assert_debug_log` method. ACKs for top commit: MarcoFalke: ACK c743718 robot-visions: ACK c743718 jonasschnelli: utACK c743718. Seems to fix it: https://bitcoinbuilds.org/index.php?build=2524 Tree-SHA512: a03e7639263eb36a381922afb4e1d0ed2ae286f2ad2e7bbd922509a043ddf6cfd08747e01d54d29bfb8f54b66908f653974b9c347e4ca4f43332b586778893be
Summary: This is a backport of Core [[bitcoin/bitcoin#18628 | PR18628]] Test Plan: ninja && test/functional/test_runner.py p2p_* Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Subscribers: majcosta Differential Revision: https://reviews.bitcoinabc.org/D8929
Summary: > This is a follow-up PR to [[bitcoin/bitcoin#18628 | PR18628]] (D8929). In addition to the hash-functions limit test introduced with commit fa4c29b, it adds checks for the following size limits as defined in BIP37: > > ad message type filterload: > >> The filter itself is simply a bit field of arbitrary byte-aligned size. The maximum size is 36,000 bytes. > > ad message type filteradd: >> The data field must be smaller than or equal to 520 bytes in size (the maximum size of any potentially matched object). > > Also introduces new constants for the limits (or reuses the max script size constant in case for the filteradd limit). This is a backport of Core [[bitcoin/bitcoin#18672 | PR18672]] Depends on D8949 Test Plan: `ninja && test/functional/test_runner.py p2p_filter` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D8950
…t framework update) 7b04c0a [Test] Clean duplicate connections creation in mining_pos_coldStaking.py (furszy) 15a799e [Test] MAX_PROTOCOL_MESSAGE_LENGTH PIVXified. (furszy) 0aedf35 [tests] Don't import asyncio to test magic bytes (John Newbery) 9bb5309 Refactor resource exhaustion test (Troy Giorshev) 589a780 Fix "invalid message size" test (Troy Giorshev) 8a0c12b Move size limits to module-global (Troy Giorshev) b3391c5 Remove two unneeded tests (Troy Giorshev) 1404e68 test: Add various low-level p2p tests (furszy) 8aaf7e1 test: Remove fragile assert_memory_usage_stable (MarcoFalke) f68e22c [Test] p2p_invalid_messages.py do not change msg.command in test_command and raise sync_with_ping timeout (furszy) c851c0b Test framework: Wait for verack by default on every new connection (furszy) c02e9a0 [Test] move framework subversion to string (furszy) 3472a39 Replace coroutine with async def in p2p_invalid_messages.py (Elichai Turkel) 33c7b19 net_processing: align msg checksum check properly. (furszy) 7f71b1b Hash P2P messages as they are received instead of at process-time (furszy) 215a638 test: Skip flaky p2p_invalid_messages test on macOS (Fabian Jahr) c11f565 qa: Make swap_magic_bytes in p2p_invalid_messages atomic (MarcoFalke) be979ad test: Fix race in p2p_invalid_messages (MarcoFalke) 6a72f0c qa: Add tests for invalid message headers (MarcoFalke) 51ddd3d Introduce p2p_invalid_messages.py test (furszy) 55a37b5 net: fix missing jump line in "Oversized message from peer.." error. (furszy) 0edfce5 test_node: get_mem_rss fixups (MarcoFalke) 6f21213 tests: add P2PConnection.send_raw_message (James O'Beirne) ae68c6e tests: add utility to assert node memory usage hasn't increased (James O'Beirne) 8469afe test: forward timeouts properly in send_blocks_and_test (James O'Beirne) db28a53 Skip is_closing() check when not available. (Daniel Kraft) be9dacb tests: fixes mininode's P2PConnection sending messages on closing transport (marcoagner) 53599c8 qa: Avoid start/stop of the network thread mid-test (furszy) 688190c [qa] mininode: Expose connection state through is_connected (MarcoFalke) Pull request description: Part of the deep and long net and ser work that I'm doing (and Tor v3 onion addresses support). Friend of #2359. Focused on the end goal of implementing the `p2p_invalid_messages` functional test which validates that invalid msg headers, msg types, oversized payloads and inventory msgs aren't accepted nor can cause a resource exhaustion. And an extra covered scenario, in `p2p_invalid_tx.py`, for the orphan pool overflow. Plus, to get up to the goal, had to work over the functional test framework as well. So.. adapted list: * bitcoin#9045. * bitcoin#13512. * bitcoin#13517. * bitcoin#13715. * bitcoin#13747. * bitcoin#14456. * bitcoin#14522. * bitcoin#14672. * bitcoin#14693. * bitcoin#14812. * bitcoin#15246. * bitcoin#15330. * bitcoin#15697. * bitcoin#16445. * bitcoin#17931. * bitcoin#17469. * bitcoin#18628 (only `p2p_invalid_tx.py` and `p2p_invalid_messages.py`. We don't support the other tests yet). * bitcoin#19177. * bitcoin#19264. ACKs for top commit: random-zebra: utACK 7b04c0a and merging... Tree-SHA512: 48d1f1a6acc24a9abce033fbf1f281ba4392147da39d118a694a09d63c3e0610cc1a29d711b434b16cc0d0e030dd9f1a89843870091b6999b862b9ab18a20679
No description provided.