This fixes a subtle bug: if max block size * 2 is less than max protocol message size, messages could be rejected.
This should not be an issue unless or until MAX_PROTOCOL_MESSAGE_LENGTH is made much larger.
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rABCb138ee23c41b: Fix message size check for max block sizes much smaller than max protocol…
ninja check check-functional
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- oversized-fix
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 29326 Build 58188: Build Diff build-diff · build-without-wallet · lint-circular-dependencies · build-clang-tidy · build-clang · build-debug Build 58187: arc lint + arc unit
Event Timeline
Comment Actions
It's a nice code readability improvement and test coverage increase.
But I don't see the bug, if any. The test passes without the changes. It's normal for the block message to be larger than other messages in the protocol, hence the special case. But if the protocol size is increased more than twice the block size, then there is no reason to allow for a larger block message anyway (that would be a rejected block).
| test/functional/p2p_invalid_messages.py | ||
|---|---|---|
| 300 | ||
| 315 | blockmaxsize is for mining only, not needed here | |
| 324 | Dito | |
Comment Actions
Rebased on D16345 and introduce test coverage that breaks without the fix. It may look a bit strange at first blush to test non-oversized messages in this test, but checking boundaries is clearly a good idea here.
| test/functional/p2p_invalid_messages.py | ||
|---|---|---|
| 295 ↗ | (On Diff #48290) | This is always False in all the tests, so this branch is never reached |
| 299 ↗ | (On Diff #48290) | I'm actually surprised it works, it means that the message you send (full of garbage) do not cause a disconnect ? |
| 352 ↗ | (On Diff #48290) | That's the default |