Skip to content

fix: enforce max size of max_message_size_bytes/2 for proposed blocks#268

Merged
matthias-wright merged 4 commits into
audit-may-2026from
m/p2p-msg-size
Jun 29, 2026
Merged

fix: enforce max size of max_message_size_bytes/2 for proposed blocks#268
matthias-wright merged 4 commits into
audit-may-2026from
m/p2p-msg-size

Conversation

@matthias-wright

Copy link
Copy Markdown
Collaborator

Addresses #252.

Changes:

  • Reject blocks whose size exceeds max_message_size_bytes/2 in handle_verify and handle_certify.
  • Set max_message_size_bytes to 10MB. This ensures that (certificate, block) won't exceed max_message_size_bytes.

@sebastian-osec

Copy link
Copy Markdown

This PR does add block-size validation in application/src/actor.rs: proposed/verified/certified blocks are rejected when block.encode_size() is at least max_message_size_bytes / 2. That is definitely a real guard and reduces the risk.

My remaining concern is that #252 is specifically about the actual single-message resolver/backfill payloads. On this branch, syncer/src/actor.rs still serves block.encode(), (finalization, block).encode(), and (notarization, block).encode() as single responses, and I do not see those actual encoded response sizes checked against max_message_size_bytes.

So the fix seems to rely on the invariant that certificate/resolver/P2P overhead always fits in the remaining half of the message budget. Could we either validate the actual encoded resolver responses directly, or document/prove that this overhead is always bounded below max_message_size_bytes / 2 for the supported validator/config limits? A regression test for a lagging/checkpoint-joining node requesting a near-limit finalized block would also help confirm the original backfill failure mode is fixed.

@matthias-wright

Copy link
Copy Markdown
Collaborator Author

Update(cbd7cef):

  • Added comment with some Math that justifies why the overhead will always be below max_message_size_bytes / 2 for any realistic validator counts

@sebastian-osec

Copy link
Copy Markdown

Thanks, the added explanation makes sense to me.

Given that proposed/verified/certified blocks are now capped at max_message_size_bytes / 2, Request::Block responses should always fit, and the remaining half should cover the finalized/notarized certificate overhead for realistic validator counts. The overhead being one aggregate signature plus a signer bitmap is the key point, since it scales as roughly ceil(N / 8) rather than N * signature_size.

So I think the substantive issue is addressed.

Optional hardening nit: this is still documented as a size proof rather than checked on the actual encoded resolver responses, so if Summit ever supports extremely large validator sets or changes certificate encoding/signature aggregation, this assumption should be revisited. A small regression/unit test around the near-limit (certificate, block).encode_size() case would also make the invariant more explicit.

@matthias-wright

Copy link
Copy Markdown
Collaborator Author

Update(366f72a):

  • Test: assert (certificate, block) backfill response fits the P2P cap

@matthias-wright matthias-wright merged commit 0a56a55 into audit-may-2026 Jun 29, 2026
4 checks passed
@matthias-wright matthias-wright deleted the m/p2p-msg-size branch June 29, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants