Fix qbft backlog byte limit - #82
Merged
Merged
Conversation
The future-message backlog only counted messages, not bytes. A PRE-PREPARE carries a full block proposal that can run right up to the 10 MiB transport frame, so the 1024-message per-validator cap (sized for ~1KB messages) let a single validator pin multi-GB of proposals before the block ever reached VerifyBlockProposal. Reject any future PRE-PREPARE whose encoded size is over 4 MiB before we retain it. A real block is gas-bound to ~1.9 MiB at the 30M gas limit, so 4 MiB leaves comfortable headroom for legit blocks while killing the oversized-proposal vector. The backlog now stores each entry with its encoded size so we can build a proper byte budget on top of this.
The per-message ceiling stops any single oversized PRE-PREPARE, but on its own it doesn't bound how much the whole backlog can hold: 1024 slots times a few MiB each still adds up to gigabytes per sender. This tracks retained bytes per validator and globally and rejects admission once either budget is hit, so aggregate memory is actually bounded rather than just the worst single message. Budgets are 32 MiB per validator and a dynamic global cap (2x per-validator, floored at 128 MiB, ceilinged at 512 MiB) that mirrors the existing count-cap shape. The byte counters are kept exact across every push, pop, requeue and whole-backlog eviction, since a drift or underflow there would silently wedge the backlog shut and become a self-inflicted liveness bug. Purely local admission policy, so it stays wire- and consensus-compatible with unpatched nodes.
The size ceiling only guarded PRE-PREPARE, but ROUND-CHANGE can embed a full PreparedBlock exactly like a proposal, so it had the same unbounded-payload path into the backlog. PREPARE and COMMIT only carry a fixed-size digest/seal, so they don't need it. Factor the check behind carriesBlockProposal() and apply it to both block-bearing codes. The aggregate byte budget already covered ROUND-CHANGE, so this just closes the single-oversized-message gap for it as well.
The 4 MiB per-message ceiling is sized against the 30M gas limit (max data-carrying block ~1.9 MiB). If someone raises the gas limit later without revisiting the ceiling, legitimate large blocks would start getting dropped from the backlog - a silent liveness bug. Rather than derive the ceiling from live chain state (which would put a consensus-data read on the admission hot path and still need a margin anyway), pin the assumption in a test. It recomputes the max block from the gas limit and TxDataNonZeroGasEIP2028 and asserts the ceiling keeps ~2x headroom, so the two can't drift apart without a red test forcing a reconcile.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.