Skip to content

engine: specify behaviour for an oversized aggregated inclusion list - #864

Open
Marchhill wants to merge 2 commits into
ethereum:mainfrom
Marchhill:engine/aggregated-inclusion-list-bound
Open

Marchhill wants to merge 2 commits into
ethereum:mainfrom
Marchhill:engine/aggregated-inclusion-list-bound

Conversation

@Marchhill

@Marchhill Marchhill commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Motivation

bogota.md bounds inclusion-list size in one direction only. engine_getInclusionListV1 requires the returned list to fit MAX_BYTES_PER_INCLUSION_LIST. For the CL → EL direction there is nothing: engine_newPayloadV6's inclusionListTransactions and PayloadAttributesV5.inclusionListTransactions are Array of DATA with no size constraint and no error condition for an oversized value.

That is already causing divergence. One implementation rejects an oversized aggregate on engine_newPayloadV6 with -32602 while discarding it and building without it on engine_forkchoiceUpdatedV5 — two resolutions of the same unspecified condition in one codebase, because the spec does not say which is correct.

Where the bound comes from

The CL builds the aggregate from the committee's lists: at most one non-equivocating list per member, deduplicated, each bounded on gossip by MAX_TRANSACTIONS_BYTES_PER_INCLUSION_LIST = 2**13 (8,192) with INCLUSION_LIST_COMMITTEE_SIZE = 2**4 (16). So 16 * 8192 = 131,072 bytes bounds any conforming aggregate — but only implicitly, on the consensus side, and never as a constraint on the engine parameter that carries it.

Proposal

Add the derived constant and state the behaviour for exceeding it:

  • engine_newPayloadV6: -32602: Invalid params.
  • engine_forkchoiceUpdatedV5: -38003: Invalid payload attributes. Per point (8.3) of engine_forkchoiceUpdatedV1 the forkchoice update still applies; only the build does not start.

Both codes are already listed for these methods in the OpenRPC schema, and no headings are added, so neither the schema nor the TOC changes.

Open questions

This proposes a resolution rather than recording an agreed one; settled either way is better than open.

  • If no aggregate bound is intended, say so explicitly, or clients will keep inventing bounds and diverging.
  • If -38003 is wrong for engine_forkchoiceUpdatedV5, the alternative is to ignore the value and build without it. That keeps the slot alive but silently produces a payload attesters will not vote for, and gives the caller no signal — defensible, and I will change the PR if preferred.
  • The bound constrains total bytes only, so a large number of zero-length entries satisfies it. Those fail to decode anyway, but only after per-entry allocation. Worth deciding whether the entry count should also be bounded.
  • Measurement is written as the sum of entry byte lengths, fixing that it is the transaction bytes rather than their JSON encoding — the same ambiguity Rename the max IL transaction size constant name consensus-specs#5508 addressed on the consensus side. Rebased onto fix: rename max IL transactions bytes constants #871, so the new constant follows the renamed per-list one as MAX_TRANSACTIONS_BYTES_PER_AGGREGATED_INCLUSION_LIST.

No limit value is changed.

bogota.md bounds the inclusion list in the EL -> CL direction only, via
engine_getInclusionListV1. The inclusionListTransactions parameter of
engine_newPayloadV6 and PayloadAttributesV5 carries the CL-aggregated
list with no stated size constraint and no error condition, leaving
clients to invent their own bound or none.

Add MAX_BYTES_PER_AGGREGATED_INCLUSION_LIST, derived from the committee
size and the per-list byte cap, and state the error returned when the
aggregate exceeds it.
@Marchhill
Marchhill force-pushed the engine/aggregated-inclusion-list-bound branch from f44a10d to f9ec7e6 Compare September 2, 2026 16:31
@jihoonsong

Copy link
Copy Markdown
Member

IIRC, this is not set with the reason that engine APIs are trusted. I'm not strongly against this as long as people want to impose this limit, but it does feel like a leakage to specify IL committee size here. It can be just 2**17, if we were to have this.

@mkalinin

mkalinin commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

For the CL → EL direction there is nothing

I think in this case there shouldn’t be anything, as CL rejects an IL if the IL exceeds the limit. Following the trust assumption between EL and CL, that rejection should be enough to enforce constraints introduced by this PR.

IMO, it makes sense to flag an error when EL cannot process a method call because of an incorrect input. An IL that exceeds the limit shouldn’t cause any difficulties on the EL side.

@jihoonsong

Copy link
Copy Markdown
Member

IMO, it makes sense to flag an error when EL cannot process a method call because of an incorrect input. An IL that exceeds the limit shouldn’t cause any difficulties on the EL side.

Perhaps we can set up a Hive test for this without overloading the spec. Whether it justifies the testing complexity is a different topic though.

This branch has not been deployed

No deployments
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.

3 participants