Skip to content

fix: set finite gossipsub decode limits for control messages #9117

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

Open
lodekeeper wants to merge 3 commits into
base: unstable
Choose a base branch
from
+18 −0
Open

fix: set finite gossipsub decode limits for control messages #9117

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39958d2
fix: set finite gossipsub decode limits for control messages
lodekeeper Mar 26, 2026
7a02bc2
fix: raise IHAVE/IWANT decode limits to match GossipsubMaxIHaveLength
lodekeeper Mar 26, 2026
5d43ede
fix: tighten gossipsub decode limit bounds
lodekeeper Mar 27, 2026
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
13 changes: 13 additions & 0 deletions packages/beacon-node/src/network/gossip/gossipsub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,19 @@ export class Eth2Gossipsub {
// This should be large enough to not send IDONTWANT for "small" messages
// See https://github.com/ChainSafe/lodestar/pull/7077#issuecomment-2383679472
idontwantMinDataSize: 16829,
// Protobuf decode limits to bound memory allocation from untrusted RPC messages.
// js-gossipsub defaults all limits to Infinity. Setting finite values provides
// defense-in-depth against resource exhaustion via crafted control messages.
// See: Lighthouse v8.1.3 security patches for analogous rust-libp2p fixes.
decodeRpcLimits: {
maxSubscriptions: 512,
maxMessages: 256,
maxIhaveMessageIDs: 200,
maxIwantMessageIDs: 200,
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 26, 2026

Choose a reason for hiding this comment

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

P1 Badge Raise IHAVE/IWANT decode limits to protocol-compatible values

Capping maxIhaveMessageIDs and maxIwantMessageIDs at 200 is likely too low for valid gossipsub traffic under mainnet load, where honest peers can include far more message IDs in a single IHAVE/IWANT control message. Because this cap is enforced during protobuf decoding, oversized-but-valid control messages are rejected before normal gossip handling, which can drop propagation and create avoidable peer churn. Please set these decode limits to values aligned with the protocol/runtime control-message maxima rather than a low fixed value.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@lodekeeper lodekeeper Mar 26, 2026

Choose a reason for hiding this comment

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

Good catch. GossipsubMaxIHaveLength = 5000 in js-gossipsub means honest peers can include up to 5000 message IDs in IHAVE/IWANT per heartbeat. A decode limit of 200 would silently truncate legitimate messages at the protobuf layer before the runtime caps apply. Raised both to 5000 in 7a02bc2 to align with the protocol constant.

maxIdontwantMessageIDs: 2000,
maxControlMessages: 500,
maxPeerInfos: 100,
},
Comment on lines +191 to +199
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 26, 2026

Choose a reason for hiding this comment

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

medium

For better maintainability and readability, it's good practice to extract these magic numbers into named constants. This makes it easier to understand their purpose and to modify them in the future.

You could define these constants at the top of the file, for example after MAX_OUTBOUND_BUFFER_SIZE, and include the excellent rationale from the PR description as comments for each constant.

Example:

/** Well above the ~140 CL topics (global + attestation subnets + sync subnets + blob/column topics) */
const DECODE_RPC_MAX_SUBSCRIPTIONS = 512;
/** Generous for burst messages (attestations, blob sidecars) */
const DECODE_RPC_MAX_MESSAGES = 256;
// ... and so on for the other limits
Suggested change
decodeRpcLimits: {
maxSubscriptions: 512,
maxMessages: 256,
maxIhaveMessageIDs: 200,
maxIwantMessageIDs: 200,
maxIdontwantMessageIDs: 2000,
maxControlMessages: 500,
maxPeerInfos: 100,
},
decodeRpcLimits: {
maxSubscriptions: DECODE_RPC_MAX_SUBSCRIPTIONS,
maxMessages: DECODE_RPC_MAX_MESSAGES,
maxIhaveMessageIDs: DECODE_RPC_MAX_IHAVE_MESSAGE_IDS,
maxIwantMessageIDs: DECODE_RPC_MAX_IWANT_MESSAGE_IDS,
maxIdontwantMessageIDs: DECODE_RPC_MAX_IDONTWANT_MESSAGE_IDS,
maxControlMessages: DECODE_RPC_MAX_CONTROL_MESSAGES,
maxPeerInfos: DECODE_RPC_MAX_PEER_INFOS,
},

Copy link
Copy Markdown
Contributor Author

@lodekeeper lodekeeper Mar 26, 2026

Choose a reason for hiding this comment

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

For a single 7-value object literal with a descriptive comment block and a rationale table in the PR body, I think the inline values are clear enough. Extracting to file-level constants would roughly double the line count without adding much readability — the values are only referenced in this one place. Keeping as-is.

})(modules.libp2p.services.components) as GossipSubInternal;

if (metrics) {
Expand Down