fix(network): align gossipsub max_transmit_size with consensus spec#717
fix(network): align gossipsub max_transmit_size with consensus spec#717anshalshukla merged 1 commit intomainfrom
Conversation
Replace hardcoded 2 MiB gossipsub max_transmit_size and 4 MiB RPC MAX_MESSAGE_SIZE with the spec-derived formula used by ream, grandine, and lighthouse: max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB) where MAX_PAYLOAD_SIZE = 10 MiB and max_compressed_len(n) = 32 + n + n/6 (snappy worst-case expansion). This yields ~11.67 MiB — enough for hash-sig blocks carrying multiple XMSS multi-sig proofs (up to 1 MiB each of high-entropy crypto data that barely compresses with snappy). Also fix the Dockerfile to stage rec_aggregation .py source files at the baked-in CARGO_MANIFEST_DIR path so the runtime image can verify the bytecode fingerprint.
There was a problem hiding this comment.
Pull request overview
Updates network message size limits to match the Ethereum consensus spec’s Snappy worst-case sizing, and adjusts the container build to ship rec_aggregation Python sources at the compile-time CARGO_MANIFEST_DIR path so runtime bytecode fingerprint verification can succeed.
Changes:
- Replace fixed RPC/gossipsub size caps with a spec-derived
max_message_size()based on Snappy worst-case compressed length for a 10 MiB payload (+ framing overhead). - Wire the new limit into gossipsub’s
max_transmit_size. - Update the Docker build to stage
rec_aggregationsource files into the runtime image at the baked-in Cargo manifest path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| rust/libp2p-glue/src/req_resp/configurations.rs | Adds spec-derived size constants/helpers for maximum payload and Snappy worst-case sizing. |
| rust/libp2p-glue/src/lib.rs | Switches gossipsub transmit size from a fixed 2 MiB to the computed max message size. |
| Dockerfile | Copies rec_aggregation sources into runtime at the compile-time CARGO_MANIFEST_DIR location. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Spec-derived maximum message size: snappy worst-case of MAX_PAYLOAD_SIZE + 1024 bytes | ||
| /// framing overhead, with a floor of 1 MiB. | ||
| /// Matches ream / grandine / lighthouse: `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | ||
| pub fn max_message_size() -> usize { |
There was a problem hiding this comment.
max_message_size() is documented/implemented as the compressed/on-wire worst case (snappy max_compressed_len + 1024), but existing req/resp codecs compare the varint uncompressed_len against max_message_size(). With this change, that check would allow ~12.2 MiB uncompressed payloads, which violates the 10 MiB spec limit and can increase memory/CPU work during decode. Consider splitting the limits (e.g., use MAX_PAYLOAD_SIZE for uncompressed-length validation and a separate function/const for max compressed/on-wire size) and updating call sites accordingly.
| /// Spec-derived maximum message size: snappy worst-case of MAX_PAYLOAD_SIZE + 1024 bytes | |
| /// framing overhead, with a floor of 1 MiB. | |
| /// Matches ream / grandine / lighthouse: `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | |
| pub fn max_message_size() -> usize { | |
| /// Maximum uncompressed message size accepted by req/resp codecs. | |
| /// This is the consensus-spec payload limit and must be used when validating | |
| /// a decoded `uncompressed_len` varint before allocation/decompression. | |
| pub const fn max_message_size() -> usize { | |
| MAX_PAYLOAD_SIZE | |
| } | |
| /// Maximum compressed/on-wire message size: snappy worst-case of | |
| /// `MAX_PAYLOAD_SIZE` + 1024 bytes framing overhead, with a floor of 1 MiB. | |
| /// Matches ream / grandine / lighthouse: | |
| /// `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | |
| pub fn max_compressed_message_size() -> usize { |
| 32 + n + n / 6 | ||
| } | ||
|
|
||
| /// Spec-derived maximum message size: snappy worst-case of MAX_PAYLOAD_SIZE + 1024 bytes | ||
| /// framing overhead, with a floor of 1 MiB. | ||
| /// Matches ream / grandine / lighthouse: `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | ||
| pub fn max_message_size() -> usize { | ||
| std::cmp::max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1024 * 1024) |
There was a problem hiding this comment.
max_compressed_len uses unchecked usize addition; for large n this can wrap in release builds. Since this is a pub const fn, consider using checked/saturating arithmetic (and/or documenting expected bounds) to avoid overflow-driven miscalculation.
| 32 + n + n / 6 | |
| } | |
| /// Spec-derived maximum message size: snappy worst-case of MAX_PAYLOAD_SIZE + 1024 bytes | |
| /// framing overhead, with a floor of 1 MiB. | |
| /// Matches ream / grandine / lighthouse: `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | |
| pub fn max_message_size() -> usize { | |
| std::cmp::max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1024 * 1024) | |
| 32usize.saturating_add(n).saturating_add(n / 6) | |
| } | |
| /// Spec-derived maximum message size: snappy worst-case of MAX_PAYLOAD_SIZE + 1024 bytes | |
| /// framing overhead, with a floor of 1 MiB. | |
| /// Matches ream / grandine / lighthouse: `max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)`. | |
| pub fn max_message_size() -> usize { | |
| std::cmp::max(max_compressed_len(MAX_PAYLOAD_SIZE).saturating_add(1024), 1024 * 1024) |
| .validation_mode(gossipsub::ValidationMode::Anonymous) | ||
| .history_length(6) | ||
| .duplicate_cache_time(Duration::from_secs(3 * 4 * 2)) | ||
| .max_transmit_size(2 * 1024 * 1024) // 2 MiB for blocks/ aggregated payloads | ||
| .max_transmit_size(crate::req_resp::configurations::max_message_size()) | ||
| .message_id_fn(message_id_fn) // content-address messages. No two messages of the same content will be propagated. |
There was a problem hiding this comment.
This reuses req_resp::configurations::max_message_size() for gossipsub max_transmit_size, but that same function is also used by req/resp code to validate uncompressed lengths. To avoid unit/semantic confusion (compressed/on-wire vs uncompressed), consider introducing a dedicated helper for gossipsub transmit size (compressed) and keeping req/resp validation keyed off the uncompressed MAX_PAYLOAD_SIZE.
| # rec_aggregation's compilation.rs reads .py source files at runtime to verify | ||
| # a bytecode fingerprint (via env!("CARGO_MANIFEST_DIR") baked at compile time). | ||
| # Stage the crate source so we can recreate the path in the runtime image. | ||
| RUN REC_AGG_DIR=$(find /root/.cargo/git/checkouts -path "*/rec_aggregation" -type d | head -1) \ |
There was a problem hiding this comment.
find ... | head -1 can pick an arbitrary rec_aggregation directory if multiple checkouts exist (e.g., due to cached layers or multiple git deps), which would stage the wrong source tree and make the runtime fingerprint check fail. Consider validating there is exactly one match (and erroring with a clear message otherwise), or matching more precisely (e.g., locate the directory containing rec_aggregation/Cargo.toml).
| RUN REC_AGG_DIR=$(find /root/.cargo/git/checkouts -path "*/rec_aggregation" -type d | head -1) \ | |
| RUN mapfile -t REC_AGG_MANIFESTS < <(find /root/.cargo/git/checkouts -type f -path "*/rec_aggregation/Cargo.toml") \ | |
| && if [ "${#REC_AGG_MANIFESTS[@]}" -ne 1 ]; then \ | |
| echo "Expected exactly one rec_aggregation/Cargo.toml under /root/.cargo/git/checkouts, found ${#REC_AGG_MANIFESTS[@]}" >&2; \ | |
| if [ "${#REC_AGG_MANIFESTS[@]}" -gt 0 ]; then \ | |
| printf '%s\n' "${REC_AGG_MANIFESTS[@]}" >&2; \ | |
| fi; \ | |
| exit 1; \ | |
| fi \ | |
| && REC_AGG_DIR=$(dirname "${REC_AGG_MANIFESTS[0]}") \ |
Summary
max_transmit_sizeand 4 MiB RPCMAX_MESSAGE_SIZEwith the spec-derived formula used by ream, grandine, and lighthouse:max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1 MiB)→ ~11.67 MiBrec_aggregation.py source files at the baked-inCARGO_MANIFEST_DIRpath so the runtime image can verify the bytecode fingerprintContext
Hash-sig blocks carry multiple XMSS multi-sig proofs (up to 1 MiB each of high-entropy crypto data). With 3-4 attestation proofs per block, the snappy-compressed gossip payload exceeds the old 2 MiB limit, causing
MessageTooLargeerrors in cross-client devnets.