Skip to content

fix(network): align gossipsub max_transmit_size with consensus spec #717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anshalshukla merged 1 commit into from
Apr 13, 2026
+32 −7
Merged

fix(network): align gossipsub max_transmit_size with consensus spec #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ RUN --mount=type=cache,target=/root/.cache/zig \
fi && \
zig build -Doptimize=ReleaseSafe -Dgit_version="$GIT_VERSION"

# 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) \
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
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]}") \

Copilot uses AI. Check for mistakes.
&& cp -r "$REC_AGG_DIR" /tmp/rec_aggregation_src \
&& echo "$REC_AGG_DIR" > /tmp/rec_aggregation_path

# Intermediate stage to prepare runtime libraries
FROM ubuntu:24.04 AS runtime-prep
ARG TARGETARCH
Expand All @@ -104,6 +111,13 @@ COPY --from=builder /app/zig-out/ /app/zig-out/
COPY --from=builder /app/resources/ /app/resources/
COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/

# Recreate rec_aggregation .py files at the baked-in CARGO_MANIFEST_DIR path
COPY --from=builder /tmp/rec_aggregation_src /tmp/rec_aggregation_src
COPY --from=builder /tmp/rec_aggregation_path /tmp/rec_aggregation_path
RUN mkdir -p "$(cat /tmp/rec_aggregation_path)" \
&& cp -r /tmp/rec_aggregation_src/* "$(cat /tmp/rec_aggregation_path)/" \
&& rm -rf /tmp/rec_aggregation_src /tmp/rec_aggregation_path

# Create a script to copy the right libraries based on architecture
RUN mkdir -p /runtime-libs && \
case "$TARGETARCH" in \
Expand Down Expand Up @@ -163,6 +177,9 @@ COPY --from=builder /app/zig-out/ /app/zig-out/
# Copy runtime resources
COPY --from=builder /app/resources/ /app/resources/

# Copy rec_aggregation .py files at baked-in CARGO_MANIFEST_DIR path
COPY --from=runtime-prep /root/.cargo/ /root/.cargo/

# Set the zeam binary as the entrypoint with beam parameter by default
ENTRYPOINT ["/app/zig-out/bin/zeam"]

Expand Down
2 changes: 1 addition & 1 deletion rust/libp2p-glue/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ impl Behaviour {
.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.
Comment on lines 1439 to 1443
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
.build()
.unwrap();
Expand Down
20 changes: 14 additions & 6 deletions rust/libp2p-glue/src/req_resp/configurations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,23 @@
/// as we still need rust-libp2p until we fully migrate to zig-libp2p. It needs the custom RPC protocol implementation.
use std::time::Duration;

/// Maximum allowed size for a single RPC payload (compressed).
pub const MAX_MESSAGE_SIZE: usize = 4 * 1024 * 1024; // 4 MiB
/// Maximum uncompressed payload size (10 MiB), per Ethereum consensus spec.
pub const MAX_PAYLOAD_SIZE: usize = 10 * 1024 * 1024;

/// Snappy worst-case compressed length for a payload of size `n`.
pub const fn max_compressed_len(n: usize) -> 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 {
Comment on lines +13 to +16
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/// 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 {

Copilot uses AI. Check for mistakes.
std::cmp::max(max_compressed_len(MAX_PAYLOAD_SIZE) + 1024, 1024 * 1024)
Comment on lines +10 to +17
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)

Copilot uses AI. Check for mistakes.
}

/// Timeout applied to reading requests and responses from a substream.
pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(15);

/// Idle timeout for server-side response streams.
pub const RESPONSE_CHANNEL_IDLE_TIMEOUT: Duration = Duration::from_secs(5 * 60);

pub fn max_message_size() -> usize {
MAX_MESSAGE_SIZE
}
Loading