Add test coverage for oversized block messages
ClosedPublic
Actions

Authored by roqqit on Jun 17 2024, 21:01.

Details

Reviewers

Fabien
PiRK

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

Summary

Oversized blocks are treated differently from bitcoin core, so extra test coverage near boundaries is warranted.

Test Plan

./test/functional/test_runner.py p2p_invalid_messages

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Owners added a reviewer: Restricted Owners Package.Jun 17 2024, 21:01

roqqit requested review of this revision.Jun 17 2024, 21:01

Fabien requested changes to this revision.Jun 18 2024, 06:40

Fabien added a subscriber: Fabien.

Fabien added inline comments.

test/functional/p2p_invalid_messages.py
315 ↗	(On Diff #48286)	I still don't think you need blockmaxsize, which is only used for mining

This revision now requires changes to proceed.Jun 18 2024, 06:40

The code looks goods. For future maintenance and ease of review, some additional comments would help.

test/functional/p2p_invalid_messages.py
294–295 ↗	(On Diff #48286)
296 ↗	(On Diff #48286)	Maybe add a comment that `size - 5` works for this particular compact size range `0x10000 <= size < 0x100000000`.

PiRK added inline comments.Jun 18 2024, 07:11

test/functional/p2p_invalid_messages.py
315 ↗	(On Diff #48286)	The node won't start if excessiveblocksize < DEFAULT_MAX_BLOCK_SIZE. Here also a comment would be useful for future maintenance.

test/functional/p2p_invalid_messages.py
296 ↗	(On Diff #48286)	I am considering a patch that bumps this arbitrary value to `str_data=b'\xff'` everywhere because I ran into the limit when writing the new tests. I can include the comment in that patch.