Skip to content

feat(l1): EIP-7805 (FOCIL) inclusion lists#6734

Closed
ilitteri wants to merge 19 commits into
glamsterdam-devnet-4from
focil
Closed

feat(l1): EIP-7805 (FOCIL) inclusion lists#6734
ilitteri wants to merge 19 commits into
glamsterdam-devnet-4from
focil

Conversation

@ilitteri
Copy link
Copy Markdown
Collaborator

Motivation

Add EIP-7805 (FOCIL — Fork-Choice enforced Inclusion Lists) support so ethrex
can participate in the FOCIL devnet as both an inclusion-list builder and a
validator, exercising the new V5/V6 Engine API surface against the Hegotá fork.

Description

Engine API & payload building

  • New engine_getInclusionListV1, engine_forkchoiceUpdatedV5, and
    engine_newPayloadV6 handlers, plus PayloadAttributesV5
    (inclusionListTransactions) and the IL satisfaction validator.
  • IL-first payload sequencing: IL txs are applied in arrival order before the
    priority-fee mempool fill phase (Decision 5 of the FOCIL design).
  • BuildPayloadArgs::id() now hashes the inclusion list so two V5 FCUs for the
    same slot with different ILs yield distinct payload IDs.
  • V5/V6 receive paths tolerate malformed IL byte strings and drop the 8 KiB IL
    size cap (the cap only bounds engine_getInclusionListV1 responses).
  • FOCIL is treated as a Hegotá-fork feature (no Cargo feature gate); V4/V5
    reject Hegotá-timestamp payloads with UnsupportedFork.

Genesis & config

  • Accept hezeTime/heze_time and bogotaTime/bogota_time as aliases for
    hegota_time.
  • New --mempool.private flag: locally-submitted txs enter the mempool and can
    be built into local blocks but are not propagated to peers.
  • New --il-policy, --il-per-sender-cap, --il-max-bytes builder flags.

Devnet

  • FOCIL kurtosis fixture (fixtures/networks/focil.yaml), 6-node matrix,
    ethereum-package bump for heze support, genesis-generator/dora pins.

Tested end-to-end against the Hive engine-focil suite: 9/9 FOCIL tests pass.

Checklist

  • Updated STORE_SCHEMA_VERSION — N/A, no Store schema changes.

ilitteri added 19 commits May 27, 2026 11:01
…n validator

End-to-end EIP-7805 (Fork-choice enforced Inclusion Lists) on the execution
layer for the Hegotá fork (post-Glamsterdam, per EF Checkpoint #9, April 2026).
All FOCIL code paths are gated behind a new eip-7805 Cargo feature so default
builds remain unaffected.

Phase 1 — Foundation
- Add eip-7805 feature to ethrex-common, ethrex-blockchain, ethrex-rpc, the
  ethrex binary, ethrex-l2, ethrex-l2-rpc, and the test crate.
- Add Fork::Hegota = 26 (unconditional) plus hegota_time field on ChainConfig,
  is_hegota_activated() predicate, and full integration into get_fork,
  get_fork_blob_schedule, next_fork, get_last_scheduled_fork,
  get_activation_timestamp_for_fork, get_blob_schedule_for_fork, gather_forks,
  and display_config. Hegotá inherits Amsterdam's blob schedule.
- Add unconditional pre-Hegotá rejection guards in engine_newPayloadV5 and
  validate_attributes_v4 (-38005: Unsupported fork) so non-FOCIL builds reject
  Hegotá-timestamped payloads on Hegotá-configured chains.

Phase 2 — Types (feature-gated)
- New InclusionList container at crates/common/types/inclusion_list.rs with
  MAX_BYTES_PER_INCLUSION_LIST = 8192 and TryFrom<Vec<Bytes>> validating the
  cap before RLP-decoding.
- New PayloadValidationStatus::InclusionListUnsatisfied variant with explicit
  serde rename "INCLUSION_LIST_UNSATISFIED". PayloadStatus::inclusion_list_unsatisfied()
  returns {status: ..., latestValidHash: null, validationError: null} per spec.
- New PayloadAttributesV5 = V4 + inclusion_list_transactions: Vec<Bytes>
  serialized as inclusionListTransactions.
- New RpcErr::UnknownParent(String) variant (unconditional) mapped to JSON-RPC
  error code -38007.

Phase 3 — Builders & validators (feature-gated)
- New InclusionListBuilder at crates/blockchain/inclusion_list_builder.rs with
  production scoring (age × ln(priority_fee + 1)), priority-fee, and random
  policies. Per-sender cap (default 2), greedy 8 KiB packer, blob and
  PrivilegedL2Transaction exclusion. Defines the IlStateProvider trait used
  across the IL subsystem.
- New InclusionListSatisfactionValidator at crates/blockchain/inclusion_list_validator.rs
  implementing the tracked-state algorithm (no EVM re-execution). Carries the
  StoreIlStateProvider adapter used by the engine handler and block-import
  pipeline.

Phase 4 — Engine API (feature-gated)
- engine_newPayloadV6: parses 5 params including inclusionListTransactions,
  enforces 8 KiB cap at parse time, validates IL signatures, runs Hegotá-fork
  gate, runs V4-equivalent block validation, then runs the satisfaction
  validator. On IlCheckError::Unsatisfied returns
  PayloadStatus::inclusion_list_unsatisfied().
- engine_forkchoiceUpdatedV5: parses PayloadAttributesV5, validates Hegotá
  range, calls build_payload_v5 which decodes IL into BuildPayloadArgs.
- engine_getInclusionListV1: 1-second tokio::time::timeout, builds IL via
  InclusionListBuilder reading from local mempool against parent's state
  via StoreIlStateProvider. Unknown parent returns -38007. Reads
  policy/cap/max_bytes from IlConfig on RpcApiContext.
- engine_exchangeCapabilities advertises the three FOCIL methods iff the
  eip-7805 feature is enabled AND the chain has hegota_time configured
  (FOCIL_CAPABILITIES filtered at runtime).

Phase 5 — Integration (feature-gated)
- BuildPayloadArgs.inclusion_list_transactions field threaded through V1-V5
  FCU constructors and the L2 sequencer.
- BlockValidationContext { inclusion_list } in crates/common/validation.rs
  with empty()/with_inclusion_list() constructors per Decision 9.
- New Blockchain::add_block_pipeline_with_il: runs the standard import
  pipeline, then reads pre/post-state via StoreIlStateProvider, runs the
  validator, returns ChainError::IlUnsatisfied { tx_hash } on failure.
- New InclusionListSatisfactionValidator::refresh_all_from(post_state) for
  bulk tracker initialization (one read per IL sender, equivalent to
  per-tx tracking but cheaper since post-state is already committed).
- IlConfig { policy, per_sender_cap, max_bytes } on RpcApiContext, populated
  from CLI flags --il-policy, --il-per-sender-cap, --il-max-bytes. CLI
  values flow through start_api into engine_getInclusionListV1, which
  clamps max_bytes to the spec ceiling (8192) in non-test builds.

Tests
- ethrex-common --features eip-7805: 60 passed (+8 inclusion-list tests
  covering round-trip, byte-cap rejection, 8 KiB boundary, hash preservation,
  empty list, invalid RLP).
- ethrex-blockchain --features eip-7805: 19 passed (9 builder + 10 validator
  covering all spec scenarios — present/insufficient_gas/invalid_nonce/
  invalid_balance/unsatisfied, idempotence, no-EVM-during-check,
  position-independence, duplicate-tolerance).
- ethrex-rpc --features eip-7805: 81 passed (+ engine_exchangeCapabilities
  filter tests, V4/V5 pre-Hegotá rejection, V5 attribute validation, IL
  parse-time validation, -38007 mapping, INCLUSION_LIST_UNSATISFIED JSON
  serialization).
- Workspace --features eip-7805 lib tests: 475 total green.
- Default-feature build and clippy clean.

Deferred (tracked in openspec/changes/eip-7805-focil-execution-layer):
- 5.1.2/5.1.4: IL-first sequencing in build_payload (would thread the IL
  through Block/PayloadBuildContext/fill_transactions). The V6 import-time
  satisfaction check is the spec-authoritative correctness gate, so this is
  a fee-revenue/latency optimization, not a correctness gap.
- 6.2.1-2: V5→getPayloadV6→V6 end-to-end integration test (depends on a
  fully-populated chain state in the test crate).
- 7.3.1-3: Hegotá devnet pairing (no Hegotá devnets exist in
  ethereum-package yet).

External alignment: method names and parameter shapes match
execution-apis #609; fork name "Hegota" matches EF Checkpoint #9. Test CL
pairing target: ChainSafe/lodestar #7342 (FOCIL).

ROADMAP.md updated with the FOCIL row.
Adds the local-build IL-first ordering path (Phase 5.1) and FOCIL end-to-end
integration tests (Phase 6.2/6.3). Refactors the payload-build pipeline and
start_api to avoid feature-gated function arguments per project preference —
each FOCIL-aware variant is a separate method that delegates to a shared
private helper (no #[cfg] on function args).

Payload-build pipeline
- New Blockchain::initiate_payload_build_with_il, build_payload_loop_with_il,
  build_payload_with_il (all feature-gated entire methods, not arguments).
- Shared private helpers register_payload_build_task, build_payload_loop_inner,
  build_payload_inner take an &[Transaction] (empty slice for non-FOCIL
  callers — byte-equivalent to pre-FOCIL behavior).
- New Blockchain::apply_inclusion_list_transactions runs IL txs in arrival
  order before fill_transactions. Skips blob/PrivilegedL2/invalid txs (each
  is classified as invalid_*/insufficient_gas by the satisfaction algorithm
  on remote validators). Updates payload_size, cumulative_gas, and BAL
  recorder; on apply error, restores the BAL checkpoint.
- BuildPayloadArgs.inclusion_list_transactions field is now unconditional
  (always Option<Vec<Transaction>>) so cross-crate constructors don't have
  to thread feature flags. The IL apply pre-pass only runs when the slice is
  non-empty AND the eip-7805 feature is enabled on ethrex-blockchain.
- engine_forkchoiceUpdatedV5 dispatches to initiate_payload_build_with_il
  when the V5 attributes carry a non-empty IL, otherwise the standard path.

start_api refactor
- start_api keeps its original signature (no feature-gated arg).
- New start_api_with_il_config (feature-gated entire method) accepts an
  IlConfig and wires it into RpcApiContext. Both delegate to a shared
  private start_api_with_context that takes the already-constructed context.
- cmd/ethrex/initializers.rs branches at the call site (cfg attribute on
  the block, not the argument) — eip-7805 path constructs IlConfig from
  CLI flags and calls start_api_with_il_config; non-FOCIL path calls
  start_api unchanged.
- ethrex-l2-rpc, ethrex-l2, and ethrex-test gain an eip-7805 feature that
  propagates to their ethrex-blockchain/ethrex-rpc deps so cargo feature
  unification produces consistent struct shapes across the workspace.

Integration tests (test/tests/blockchain/focil_tests.rs, eip-7805-gated)
- locally_built_block_with_il_satisfies_on_import: V5 build → 2 IL txs are
  sequenced first → import via add_block_pipeline_with_il succeeds.
- externally_built_block_omitting_il_tx_fails_on_import: empty block import
  with non-empty IL context returns ChainError::IlUnsatisfied { tx_hash }
  matching the dropped IL tx.
- il_first_ordering_with_mempool_competition: IL tx is sequenced first even
  when the mempool has competing txs from a different sender; full import
  satisfies on the validator.

Tests
- ethrex-test --features eip-7805 focil_tests → 3 passed.
- cargo test --workspace --features eip-7805 --lib → 475 passed.
- cargo clippy --workspace --features eip-7805 --lib -- -D warnings clean.
- cargo clippy --workspace --lib -- -D warnings clean.
… gate

EIP-7805 (FOCIL) ships in Hegotá. There is no scenario where a mainnet
operator wants to run a Hegotá chain without FOCIL — the EIP is the
fork's defining feature. Treating it as a regular Ethereum fork (like
Cancun, Prague, Osaka, Amsterdam) eliminates the feature-flag plumbing
across 7 Cargo.toml files and ~30 cfg sites, and removes the failure
mode where a non-FOCIL build silently mis-validates Hegotá payloads.

Activation is now governed entirely by `is_hegota_activated(timestamp)`
against the chain config's `hegota_time` field. Pre-Hegotá behavior is
byte-identical because the V5/V6 handlers refuse non-Hegotá timestamps
and the IL pre-pass in payload building is a zero-iteration no-op when
the IL is empty (V1-V4 callers always pass an empty list).

Method-shape changes
--------------------

The split-by-feature `_with_il` variants have been collapsed into the
canonical methods on `Blockchain`:

  build_payload(payload)                     →
    build_payload(payload, &[Transaction])
  build_payload_with_il(...)                 → DELETED (subsumed)
  build_payload_inner(...)                   → DELETED (inlined)

  build_payload_loop(payload, cancel)        →
    build_payload_loop(payload, cancel, Vec<Transaction>)
  build_payload_loop_with_il(...)            → DELETED (subsumed)
  build_payload_loop_inner(...)              → DELETED (inlined)

  initiate_payload_build(payload, id)        →
    initiate_payload_build(payload, id, Vec<Transaction>)
  initiate_payload_build_with_il(...)        → DELETED (subsumed)

  start_api(...)                             →
    start_api(..., IlConfig)  (last positional)
  start_api_with_il_config(...)              → DELETED (subsumed)

V1-V4 FCU callers pass `Vec::new()` / `&[]`; V5 passes the decoded IL
from PayloadAttributesV5. The IL pre-pass in `build_payload` only runs
when the slice is non-empty, preserving pre-FOCIL behavior on every
non-V5 path.

Cargo.toml changes
------------------

Dropped `eip-7805` feature definitions from:
  - crates/common/Cargo.toml
  - crates/blockchain/Cargo.toml
  - crates/networking/rpc/Cargo.toml
  - crates/l2/Cargo.toml
  - crates/l2/networking/rpc/Cargo.toml
  - cmd/ethrex/Cargo.toml
  - test/Cargo.toml

Behavior on pre-Hegotá chains
-----------------------------

A chain with no `hegota_time` set retains byte-identical pre-Hegotá
behavior: `engine_exchangeCapabilities` does not advertise the V5/V6/
`getInclusionListV1` methods (the chain-config check is the gate), and
the V1-V4 paths build payloads identically to before this branch. The
satisfaction validator only runs when both `hegota_time` is configured
AND the block was submitted via V6 with a non-empty IL.

Spec updates
------------

The parent change (`eip-7805-focil-execution-layer`) had its design.md,
proposal.md, and 5 spec files updated in place to reflect the new
activation model:
  - design.md Decision 2 rewritten as "Hegotá-as-fork; activation
    purely via chain config"
  - Decisions 7, 8, 9 updated to drop feature-flag references
  - engine-api-inclusion-list/spec.md: capability advertisement
    requirement now keyed only on `hegota_time` configuration
  - hegota-fork-activation/spec.md: Fork enum scenarios updated;
    pre-Hegotá rejection scenario drops `--features eip-7805`
  - inclusion-list-satisfaction/spec.md: 3-condition activation gate
    reduced to 2 (Hegotá-active timestamp + V6 with non-empty IL)
  - payload-building-with-il/spec.md: "Local-build path is
    feature-gated" requirement deleted; the `BuildPayloadArgs` field
    is unconditional
Pandaops devnet configs (and the upstream FOCIL spec) are migrating the
post-Glamsterdam fork name from "Hegotá" to "Hezé". To stay
forward-compatible without renaming the internal type, accept either
spelling at the genesis JSON parse layer via #[serde(alias = ...)].

Internal type stays Fork::Hegota / hegota_time until upstream picks a
final name; the alias is mechanical and removable.
…time

The ethereum-genesis-generator 6.0.5 (current pandaops focil/heze
devnets) emits the post-Amsterdam fork timestamp under "bogotaTime"
in the EL genesis JSON regardless of which name (heze_fork_epoch,
gloas_fork_epoch, etc.) the kurtosis args use. Without this alias,
ethrex silently drops the field, hegota_time stays None, and FOCIL
never activates on a chain that is otherwise correctly post-Hegotá.

Internal type stays hegota_time; this is purely a serde parsing
alias, removable when upstream stabilizes the name.
Pandaops-supplied participants_matrix (besu + lodestar/lighthouse, count 2)
expanded with ethrex+lodestar / ethrex+lighthouse pairings using
ethrex:local from the focil branch. Fork schedule: gloas at epoch 1,
heze at epoch 2.
The previous pin (e4b3305) predates upstream commit eb8c590
"feat: add heze (#1323)", which is required to parse the
heze_fork_epoch network param used by pandaops focil devnet
configs. Bumping to eac08c0 (current main tip) gets heze plus
49 follow-on commits including ethereum-genesis-generator 6.0.5.
The pandaops-supplied config specified bbusa/dora:focil-fix, but that
tag does not exist on Docker Hub. The closest available tag is :focil
(pushed 2026-05-01). Using that.
So dora is reachable at http://<host>:30000 over Tailscale without an
SSH tunnel — matches the canonical glamsterdam-devnet-0 template.
Additional services land at adjacent ports (30002 spamoor, etc.).
…e / besu × lodestar

Rebuild the participants list as three lodestar-paired EL groups (count 2 each):
plain ethrex, ethrex with --mempool.private, and besu as a reference EL.
Lighthouse pairs dropped — the :focil-tagged lighthouse build is incompatible
with lodestar:focil at the CL gossip layer (peer-count: 1, missing topic
subscriptions); both pairs sat at block 10 in the previous deploy regardless
of which EL they ran. The new matrix gives a clean A/B between regular and
private-mempool ethrex with besu as the third reference point.
…e node

Matches the upstream local-devnet-focil interop registry config verbatim
except for an additional ethrex+lodestar pair running with
--mempool.private so we keep the private-vs-gossiping A/B on the same
chain.

Concrete deltas from previous focil.yaml:

- preset: minimal → mainnet (12s slots, more realistic timing)
- gloas/heze fork epochs: 1/2 → 0/1 (FOCIL active from genesis;
  no pre-Hegota warmup phase to wait through)
- supernode: true on every CL (PeerDAS satisfaction post-fulu)
- ethrex image: ethrex:local → ethpandaops/ethrex:focil
  (public image is published from the focil branch HEAD; matches what
  the upstream interop participants pull)
- additional_services: + tx_fuzz
- spamoor: real spammers (eoatx 100tps, uniswap-swaps 100tps,
  blob-combined 5tps) so ILs actually fill up
- port_publisher.public_port_start: 30000 → 65500 (upstream choice;
  Caddy vhosts on ethrex-office-4 will need to point at the new
  port range after redeploy)
…65500)

ethrex-office-4 has Caddy vhosts and Cloudflare DNS records pinned to
localhost:30000 (dora) and :30002 (spamoor). Adopting the upstream 65500
would break dora.focil.ethrex.xyz / spamoor.focil.ethrex.xyz and
force a Caddy rewire + Let's Encrypt re-issuance on every redeploy.
Note this in the YAML so future merges from the upstream config don't
drag the value back.
ethereum-genesis-generator:6.0.5 (the default the ethereum-package
revision pulls) produces an SSZ genesis state whose latest block root
disagrees with the EL genesis block root when gloas is active at
epoch 0 with mainnet preset. Lodestar:focil v1.42.0 then aborts
during initBeaconState with:

    Error: Genesis block root 0x2525...1342 does not match genesis
    state latest block root 0x42c5...87ea3

Pin to ethpandaops/ethereum-genesis-generator:bbusa-fix-gloas-genesis,
which is literally the patched tag for this case. No EL/CL image
changes.
bbusa-fix-gloas-genesis did not actually fix the SSZ genesis mismatch
lodestar:focil hits when gloas is active at epoch 0 with mainnet
preset. :6.0.6 is the next published genesis-generator tag — try it
before falling back to delayed-gloas workarounds.
Mirrors reth's --txpool.no-local-transactions-propagation. When set,
transactions submitted via this node's eth_sendRawTransaction enter
the mempool and may be included in blocks built locally, but their
hashes are NOT inserted into the broadcast pool — peers never see
them via Transactions / NewPooledTransactionHashes. Transactions
received from peers continue to propagate as before.

Plumbing
--------

- BlockchainOptions gains a `private_mempool: bool` (default false).
- Mempool gets a sibling `add_transaction_no_broadcast` that skips
  the broadcast_pool insertion. Both methods share an internal
  `add_transaction_inner(broadcast: bool)`.
- Blockchain exposes `add_local_transaction_to_pool` and
  `add_local_blob_transaction_to_pool` for the RPC entry point.
  These check `options.private_mempool` and route to the
  no-broadcast variant when set; the existing
  `add_transaction_to_pool` (used by P2P/sync paths) keeps current
  behavior and always broadcasts.
- The eth_sendRawTransaction handler now calls the local variants.
- CLI flag: `--mempool.private` (env: ETHREX_MEMPOOL_PRIVATE),
  matching the existing `--mempool.maxsize` prefix.

Test
----

`add_transaction_no_broadcast_keeps_tx_out_of_broadcast_pool` shows
that a tx routed via the no-broadcast path is queryable from the
mempool but absent from `get_txs_for_broadcast`.
The Hive engine-focil suite catches three FOCIL behaviors we got wrong:

1. **Garbage IL bytes** (`0xdeadbeef`, empty, `0x02c0`) reject the whole
   forkchoiceUpdatedV5 / newPayloadV6 call. Spec says malformed entries
   MUST be tolerated as if they were empty — only real txs in the IL
   impose obligations on the block.

   Fix: in build_payload_v5 and the newPayloadV6 handler, skip-and-log
   entries that fail Transaction::decode_canonical instead of returning
   InvalidPayloadAttributes. Drop the V6 parse-time RLP/sender-recovery
   "sanity check" that was producing the rejection.

2. **MAX_BYTES_PER_INCLUSION_LIST = 8192 bytes** is on the RLP-encoded
   list, not on the sum of raw entry lengths. A single 8192-byte entry
   RLPs to ~8198 bytes (string + list headers) which the spec rejects,
   but our `total > 8192` check on raw byte sum accepted it.

   Fix: factor `validate_il_byte_size(&[Bytes])` that RLP-encodes the
   list and checks the actual encoded length. Used by both parse-time
   check in V6 and a new size guard in V5's build_payload_v5.

3. (No code change, but covered by tests.) parse_il_transactions
   continues to accept arbitrary hex byte strings — RLP validity is no
   longer enforced at parse time, only the size cap.

Tests
-----

- `validate_il_byte_size_rejects_single_entry_at_raw_cap` — single
  8192-byte entry with raw sum == cap, RLP > cap → rejected.
- `parse_il_transactions_tolerates_garbage_bytes` — `0xdeadbeef` /
  empty / `0x02c0` round-trip without error.
- Existing V6 / FOCIL integration tests still pass.
The Hive engine-focil suite was updated to remove the upper-cap
rejection from engine_forkchoiceUpdatedV5: per EIP-7805,
MAX_BYTES_PER_INCLUSION_LIST = 8192 constrains what the local builder
PRODUCES via engine_getInclusionListV1, NOT what V5/V6 RECEIVE from the
CL. The "accepts IL larger than MAX_BYTES_PER_INCLUSION_LIST" test sends
a 10 KiB IL and asserts FCU V5 returns a payloadId; the equivalent
rejection path on V6 is also gone.

Also adds a debug log on the IL-first sequencing skip path in
`apply_inclusion_list_transactions` so when a CL-provided IL tx
fails apply_transaction we can see WHICH tx and WHY rather than
silently dropping it. Surfaced while diagnosing why the
newPayloadV6-satisfied test builds a payload that doesn't
contain the IL tx — to be addressed separately once the log
identifies the failure mode.
Two FCU V5 calls for the same slot with different inclusion lists were
returning the SAME payload ID, so the second call's getPayloadV6
retrieved the FIRST call's cached payload (built with the wrong IL).
The IL-first sequencing code itself was correct, but it ran on a stale
build that the second caller couldn't see.

Surfaced by the Hive engine-focil "newPayloadV6 returns VALID" test:
build a payload with IL=[TestAccount[51] tx], assert the built block
contains that tx, then submit newPayloadV6 with the same IL — VALID
expected. Pre-fix, the test consistently retrieved a 3-tx mempool-only
payload from a previous in-flight build and failed with "built payload
does not contain the appendable IL tx".

Fix: include the canonical-encoded IL transactions in the keccak that
produces the payload ID. Distinct ILs → distinct IDs → fresh builds.

After fix, the engine-focil hive suite is 11/11 green.
Rebase carries after replaying FOCIL onto glamsterdam-devnet-4:

bal-devnet-7 → glamsterdam-devnet-4 carries:
- PayloadAttributesV4 gained `target_gas_limit` (execution-apis#796).
  The new `From<&PayloadAttributesV5> for PayloadAttributesV4` impl
  initializes it to `None`; V5 falls back to `context.gas_ceil`.
- `validate_attributes_v4` now takes `&ChainConfig` (instead of
  `&RpcApiContext`). Update the unit tests to extract chain_config
  from context.
- `BuildPayloadArgs::id()` now hashes `slot_number` and `gas_ceil`
  (glamsterdam-devnet-4 fix for payload-id collisions across FCUs
  with different target_gas_limit). Combine with FOCIL's inclusion-
  list hashing.
- payload.rs unit-test `base_args` constructor must initialize the
  unconditional `inclusion_list_transactions: None` field.

Carried from the prior bal-devnet-7 rebase (still applies on
glamsterdam-devnet-4 since both branches inherit the EIP-7702 test
files and the unconditional `inclusion_list_transactions` field):
- test/tests/blockchain/eip7702_revert_authority_tests.rs and
  eip7702_zero_transfer_tests.rs: pass `inclusion_list_transactions:
  None` to `BuildPayloadArgs` and `&[]` to `Blockchain::build_payload`.
@ilitteri ilitteri requested review from a team as code owners May 27, 2026 14:28
@github-actions github-actions Bot added the L1 Ethereum client label May 27, 2026
@github-actions
Copy link
Copy Markdown

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues
section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

Why and resolution path

PR #6527 broadened
test-stateless to extract the entire for_amsterdam/ tree from the
zkevm bundle and run all of it under --features stateless; combined with
this branch's bal-devnet-7 semantics that scope produces ~549
GasUsedMismatch / ReceiptsRootMismatch /
BlockAccessListHashMismatch failures.

test-stateless-zkevm filters cargo to the eip8025_optional_proofs
suite, which still validates the stateless harness without the bal-version
mismatch.

Re-broaden by switching test: back to test-stateless in
tooling/ef_tests/blockchain/Makefile once the zkevm bundle is regenerated
against the current bal spec.

@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Overall, this is a high-quality implementation of EIP-7805 (FOCIL). The code is well-structured, thoroughly tested, and follows Ethereum consensus rules correctly. Below are specific observations and minor suggestions.

Issues Found

1. Unused parameter in refresh_all_fromcrates/blockchain/inclusion_list_validator.rs:295

The _crypto parameter is unused (prefixed with underscore) but required by the trait signature. Consider removing it from the signature or documenting why it's retained for API symmetry.

pub fn refresh_all_from(
    &mut self,
    state: &dyn IlStateProvider,
    _crypto: &dyn Crypto, // Unused
) -> Result<(), IlValidatorError> {

2. Outdated comment in build_payload_v5crates/networking/rpc/engine/fork_choice.rs:627

The comment states "does NOT yet thread them into BuildPayloadArgs", but the code actually does populate inclusion_list_transactions. Update the comment to reflect Phase 5.1 completion.

3. Missing CLI validation for il_per_sender_capcmd/ethrex/cli.rs:200

The help text specifies range 1..=64, but the code accepts any usize. Add validation:

pub fn validate(&self) -> Result<(), String> {
    if self.il_per_sender_cap == 0 || self.il_per_sender_cap > 64 {
        return Err("--il-per-sender-cap must be in range 1..=64".to_string());
    }
    // ... validate il_max_bytes <= 8192 in non-test builds if desired
    Ok(())
}

4. Potential panic in order_by_randomcrates/blockchain/inclusion_list_builder.rs:226

The Fisher-Yates shuffle uses now_unix_micros() as a seed. While documented as a debug knob, if the system clock is before Unix epoch (returns Err), the seed becomes 0. This is deterministic but acceptable for the "Random" policy. No action needed unless you want to handle SystemTimeError explicitly.

5. Redundant Clone in IL building — crates/blockchain/payload.rs:517

In build_payload_loop, inclusion_list is cloned for each rebuild iteration. Given the 8 KiB cap, this is cheap, but consider using Arc<[Transaction]> if profiling shows it matters.

6. Error handling in add_block_pipeline_with_ilcrates/blockchain/blockchain.rs:1943

When pre_state_root is None, the code returns Ok(()) with a comment calling it "unreachable". Consider returning ChainError::Internal instead to fail loudly if the invariant is violated:

let Some(pre_state_root) = pre_state_root else {
    return Err(ChainError::Custom(
        "Parent header missing after successful block import".to_string()
    ));
};

7. RLP encoding efficiency — crates/common/types/inclusion_list.rs:47

encoded_byte_len() encodes each transaction twice (once for length, once for actual encoding later). Consider caching the encoded bytes if this becomes hot, though the 8 KiB limit makes this negligible.

Security Observations

  1. IL size enforcement: Correctly hard-capped at 8192 bytes in the builder (cfg!(test) allows override for testing). The receive path in parse_il_transactions intentionally accepts larger ILs per Hive test requirements, which is acceptable since decoding happens later.

  2. Blob exclusion: Properly excludes EIP-4844 transactions from ILs in both builder and validator.

  3. Private mempool: Correctly isolates RPC-submitted transactions from P2P broadcast when --mempool.private is enabled, matching reth's behavior.

  4. Satisfaction check: The validator correctly implements the "no EVM re-execution" rule, using only state comparisons against post-execution state.

Code Quality Positives

  • Excellent test coverage in focil_tests.rs with end-to-end scenarios
  • Clean separation between IlStateProvider trait and Store implementation enables unit testing without heavy storage dependencies
  • Proper use of saturating_add/sub prevents arithmetic overflow in gas accounting
  • The BlockValidationContext struct cleanly separates IL metadata from Block itself

Summary

The implementation is correct and ready for merge after addressing the minor documentation/validation issues noted above. The critical consensus logic (IL satisfaction, fork activation, gas accounting) is sound.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

Lines of code report

Total lines added: 2478
Total lines removed: 0
Total lines changed: 2478

Detailed view
+-------------------------------------------------------+-------+------+
| File                                                  | Lines | Diff |
+-------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                              | 1237  | +40  |
+-------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs                     | 672   | +17  |
+-------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs                  | 382   | +1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                | 2671  | +111 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/error.rs                     | 152   | +3   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/inclusion_list_builder.rs    | 462   | +462 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/inclusion_list_validator.rs  | 436   | +436 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/mempool.rs                   | 490   | +19  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs                   | 1039  | +119 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/genesis.rs                 | 1122  | +101 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/inclusion_list.rs          | 201   | +201 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/mod.rs                     | 33    | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/validation.rs                    | 259   | +16  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/l2/networking/rpc/rpc.rs                | 316   | +1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer.rs          | 325   | +1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/fork_choice.rs    | 978   | +299 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/inclusion_list.rs | 135   | +135 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/mod.rs            | 74    | +11  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/payload.rs        | 1287  | +254 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs                   | 1412  | +74  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/test_utils.rs            | 352   | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/types/fork_choice.rs     | 186   | +129 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/types/payload.rs         | 322   | +25  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/utils.rs                 | 367   | +19  |
+-------------------------------------------------------+-------+------+

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

Findings

  1. High: engine_newPayloadV6 returns INCLUSION_LIST_UNSATISFIED only after the block has already been executed and persisted. The V6 handler first goes through the normal import path (handle_new_payload_v4 -> add_block -> block worker -> add_block_pipeline) and only then runs the IL satisfaction check. That means an IL-unsatisfied block is still stored locally and can later be considered by forkchoice, which is a consensus bug. The new add_block_pipeline_with_il helper exists, but it is not wired into the worker path. See crates/networking/rpc/engine/payload.rs, crates/networking/rpc/engine/payload.rs, crates/networking/rpc/rpc.rs, crates/blockchain/blockchain.rs.

  2. High: --mempool.private transactions still feed engine_getInclusionListV1. The private path only skips broadcast_pool, but the IL builder pulls from transaction_pool via get_all_txs_by_sender(), so RPC-submitted private txs can still leak to the CL and be forced into inclusion lists. That breaks the privacy contract stated by the flag. See crates/blockchain/mempool.rs, crates/blockchain/mempool.rs, crates/blockchain/inclusion_list_builder.rs, crates/networking/rpc/engine/inclusion_list.rs.

  3. Medium: blob IL entries are skipped during local payload construction but still enforced during V6 satisfaction checking. apply_inclusion_list_transactions() explicitly ignores blob txs, yet engine_newPayloadV6 passes all decoded IL txs into the validator, and check() can classify an omitted blob tx as unsatisfied. That makes the builder and validator disagree and lets a malformed/spec-invalid IL spuriously fail an otherwise valid block. See crates/blockchain/payload.rs, crates/networking/rpc/engine/payload.rs, crates/blockchain/inclusion_list_validator.rs.

Aside from those points, the overall FOCIL/private-mempool direction looks coherent and the added tests are useful.

I couldn’t run cargo test here because rustup tried to write under read-only /home/runner/.rustup/tmp.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is the full review:


PR Review: EIP-7805 (FOCIL) Inclusion Lists

Overview

This PR adds EIP-7805 (FOCIL) support across the Engine API, payload builder, block validator, mempool, and genesis layers. The structure is clean: a dedicated IL builder (inclusion_list_builder.rs), a satisfaction validator (inclusion_list_validator.rs), three new Engine API handlers, and a private-mempool flag (--mempool.private). The 9/9 Hive engine-focil pass rate is a strong signal. Test coverage is thorough at both unit and integration level.


Correctness Issues

1. Stale doc comment in build_payload_v5 (fork_choice.rs ~line 2928)

The function-level comment says:

"does NOT yet thread them into BuildPayloadArgs — that wiring lands in Phase 5.1"

But the code immediately below it does exactly that wiring:

inclusion_list_transactions: if decoded_il.is_empty() { None } else { Some(decoded_il.clone()) },

and then passes decoded_il to initiate_payload_build. The entire Phase 5.1 comment is stale and will mislead the next reader.

2. StoreIlStateProvider is defined twice, identically

One definition is public in inclusion_list_validator.rs and another private copy lives in engine/inclusion_list.rs. They have bit-for-bit identical implementations today. This violates DRY and creates a divergence risk. The handler should import the one from inclusion_list_validator.

3. IL satisfaction logic is duplicated between NewPayloadV6Request::handle() and add_block_pipeline_with_il()

The V6 handler builds its own validator, calls refresh_all_from, reads stored header/body, and calls check inline. add_block_pipeline_with_il does the same thing wrapped around the inner pipeline. The two code paths must stay in sync — any change to the satisfaction algorithm needs to land in both places. If the intent is to have a single authoritative path, the V6 handler should go through add_block_pipeline_with_il, or the inline code should be extracted into a shared helper.

4. parse_v5 silently discards attribute parse errors

Err(error) => {
    warn!("Could not parse V5 payload attributes {}", error);
    None
}

V1-V4 parsers propagate deserialization errors to the caller. V5 downgrades a malformed PayloadAttributesV5 to "no attributes" (i.e., a no-op FCU), which masks CL bugs and is inconsistent with every other version. The FOCIL spec tolerance is for malformed IL byte entries within valid attributes, not for malformed attributes themselves.

5. apply_inclusion_list_transactions duplicates fill_transactions validation boilerplate

BAL index setup, checkpoint management, Osaka block-size cap, EIP-155 replay protection — all are copy-pasted from fill_transactions. Any future change to fill_transactions validation logic must be mirrored here manually. Consider extracting a shared apply_single_transaction helper.


Documentation Inconsistency

6. ROADMAP.md says "Behind the eip-7805 Cargo feature"

The PR description explicitly states "FOCIL is treated as a Hegotá-fork feature (no Cargo feature gate)." The ROADMAP entry contradicts that and will confuse readers looking for a feature flag that doesn't exist.


Design / Maintainability

7. Tip is zero for IL transactions in block value accounting

let head = HeadTransaction {
    tx: mtx,
    tip: U256::zero(),
};

The block value (PayloadBuildResult::block_value) that engine_getPayloadV5 returns to the CL will underreport fees from IL transactions. The comment acknowledges this but the impact is silent: the CL uses block value for MEV-boost bid decisions. Depending on the network's MEV landscape, this could cause the builder to undervalue locally-built FOCIL blocks versus external ones.

8. IlConfig is defined in the RPC crate but references blockchain crate types directly

pub policy: ethrex_blockchain::inclusion_list_builder::IlPolicy,

This creates a tight coupling between the RPC crate and a specific internal blockchain module. Moving IlPolicy, DEFAULT_PER_SENDER_CAP, and MAX_BYTES_PER_INCLUSION_LIST to ethrex_common (or a shared config struct) would let the RPC and blockchain crates both depend on common rather than one depending on the other for config types.


Input Validation Gaps

9. --il-per-sender-cap has no runtime bounds check

The help string says "Range 1..=64" but zero is accepted (all senders would contribute zero transactions, producing an empty IL every call) and values >64 are accepted (violating the described contract). A clamp or validation in initializers.rs would prevent misconfiguration.

10. ETHEREUM_PACKAGE_REVISION uses a short commit hash

ETHEREUM_PACKAGE_REVISION := eac08c0

Short hashes become ambiguous as repositories grow. The previous value was a full 40-character SHA. Restoring the full hash makes the pinned revision unambiguous and verifiable without network access.


Minor

11. BlockValidationContext is added but not used by the V6 handler

BlockValidationContext::with_inclusion_list and the BlockValidationContext struct are wired for the add_block_pipeline_with_il path (used in integration tests and as a P2P sync side-channel). The actual V6 handler bypasses this entirely and validates inline. If BlockValidationContext is intended as the long-term interface, it should be the authoritative path for V6 as well. If the inline approach is intended, the struct documentation should clarify that it's for out-of-band IL propagation only (e.g., snap-sync import) and not the V6 handler.

12. filter_and_cap does not account for base fee when checking balance

cost_without_base_fee() is used for balance validation. If base_fee_per_gas is high enough that max_fee_per_gas - base_fee < max_priority_fee_per_gas, the actual cost will be lower than cost_without_base_fee, so this is a conservative overestimate (possibly excluding some valid candidates). That's acceptable, but it also means a transaction with max_fee_per_gas < base_fee could pass the balance check but fail execution. For the IL builder the spec allows conservative selection, so this is fine as-is — worth noting for future refinement.


What Looks Good

  • The satisfaction algorithm correctly classifies present | insufficient_gas | invalid_nonce | invalid_balance without EVM re-execution.
  • The IL is included in BuildPayloadArgs::id() hashing so distinct ILs on the same slot get distinct payload IDs — the Hive regression for this is well-documented.
  • StoreIlStateProvider reads only (nonce, balance) per sender, keeping the hot path cheap.
  • Fork guards on V4/V5 handlers (UnsupportedFork for Hegotá timestamps) are correct and tested.
  • The --mempool.private separation between P2P and local RPC paths is clean and well-tested.
  • The serde aliases for hezeTime/bogotaTime/heze_time/bogota_time are a pragmatic approach to upstream naming instability and are covered by dedicated tests.

Automated review by Claude (Anthropic) · sonnet · custom prompt

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR implements EIP-7805 (FOCIL — Fork-Choice Enforced Inclusion Lists) support across the Engine API, payload builder, and block validator. It introduces engine_getInclusionListV1, engine_forkchoiceUpdatedV5, and engine_newPayloadV6, along with the IL builder (InclusionListBuilder) and satisfaction validator (InclusionListSatisfactionValidator).

  • IL builder: reads the public mempool, applies per-sender nonce/balance filtering and an 8 KiB greedy packer, configurable via new --il-policy, --il-per-sender-cap, and --il-max-bytes flags.
  • IL-first payload sequencing: IL transactions are applied in arrival order before the priority-fee mempool fill phase (apply_inclusion_list_transactions), per Decision 5 of the FOCIL design; V5 FCU handler threads the decoded IL all the way through BuildPayloadArgsbuild_payload.
  • IL satisfaction check in engine_newPayloadV6: after the block passes standard validation, the validator initialises from pre-state, refreshes from post-state, and returns INCLUSION_LIST_UNSATISFIED if a required transaction was omitted while the sender retained nonce/balance/gas.
  • Private mempool (--mempool.private): locally-submitted transactions skip P2P broadcast, and the add_local_transaction_to_pool / add_local_blob_transaction_to_pool split cleanly separates the RPC-submission and P2P-reception paths.
  • Fork guards: V4 rejects Hegotá-timestamp attributes with UnsupportedFork; V5 requires Hegotá activation; NewPayloadV5 / NewPayloadV6 enforce the Amsterdam/Hegotá boundary symmetrically.

Confidence Score: 3/5

Safe to merge after fixing the From<NewPayloadV6Request> IL serialization; all other findings are quality improvements.

The core FOCIL logic — IL builder, satisfaction validator, IL-first payload sequencing, and the engine_newPayloadV6 receive path — is well-structured and well-tested (9/9 Hive engine-focil tests pass). The one concrete defect is in From<NewPayloadV6Request> for RpcRequest, where serde_json::json!(val.inclusion_list_transactions) serializes Vec<bytes::Bytes> as integer arrays rather than the hex strings that parse_il_transactions expects on the receive side. This means any code path that constructs an outgoing V6 request via this From impl (test clients, relays, proxy mode) will produce a malformed engine API call. The build_payload_v5 doccomment also incorrectly states that IL threading is 'not yet' implemented when it fully is, and StoreIlStateProvider is duplicated across two modules, but neither affects runtime correctness.

crates/networking/rpc/engine/payload.rs — From<NewPayloadV6Request> for RpcRequest IL serialization; crates/networking/rpc/engine/fork_choice.rs — build_payload_v5 doccomment.

Important Files Changed

Filename Overview
crates/networking/rpc/engine/payload.rs Adds NewPayloadV6Request (FOCIL / Hegotá) and GetPayloadV6Request; From impl for V6 serializes inclusion_list_transactions as byte arrays instead of hex strings, breaking outgoing requests.
crates/networking/rpc/engine/fork_choice.rs Adds ForkChoiceUpdatedV5 handler with validate_attributes_v5 and build_payload_v5; stale doccomment on build_payload_v5 claims IL is not threaded/used locally, contradicting the actual implementation.
crates/blockchain/inclusion_list_builder.rs New: IL builder with production/priority-fee/random policies, per-sender nonce+balance filtering, and greedy 8 KiB packer; MAX_BYTES_PER_INCLUSION_LIST is an independent duplicate of the constant in ethrex_common.
crates/blockchain/inclusion_list_validator.rs New: IL satisfaction validator with pre/post-state tracker; correctly implements the spec's no-EVM re-execution requirement with well-tested state tracking and idempotent check.
crates/networking/rpc/engine/inclusion_list.rs New: engine_getInclusionListV1 handler with 1-second timeout and 8 KiB cap; contains a private StoreIlStateProvider that duplicates the public one in inclusion_list_validator.
crates/common/types/inclusion_list.rs New: InclusionList container with TryFrom<Vec<Bytes>> (enforces 8 KiB cap before decoding) and round-trip serialization; well-tested edge cases.
crates/blockchain/payload.rs Adds inclusion_list parameter to build_payload/build_payload_loop/initiate_payload_build; implements IL-first sequencing via apply_inclusion_list_transactions with correct skip-on-failure behavior.
crates/blockchain/blockchain.rs Adds add_block_pipeline_with_il, private_mempool option, and add_local_transaction_to_pool / add_local_blob_transaction_to_pool split for private mempool propagation control.
crates/networking/rpc/types/fork_choice.rs Adds PayloadAttributesV5 with inclusion_list_transactions (correctly uses serde_utils::bytes::vec for hex serialization) and a PayloadAttributesV5 → PayloadAttributesV4 conversion.

Sequence Diagram

sequenceDiagram
    participant CL as Consensus Layer
    participant EL as ethrex (Engine API)
    participant Builder as Payload Builder
    participant Validator as IL Satisfaction Validator
    participant Store as Storage

    CL->>EL: engine_getInclusionListV1(parentHash)
    EL->>Store: get_block_header_by_hash(parentHash)
    EL->>Builder: build(mempool, base_fee, StoreIlStateProvider)
    Builder-->>EL: "Vec<Transaction> (8 KiB cap)"
    EL-->>CL: ["0x...", ...] hex-encoded txs

    CL->>EL: engine_forkchoiceUpdatedV5(forkChoiceState, PayloadAttributesV5)
    Note over EL: validate_attributes_v5 (Hegota required)
    EL->>Builder: initiate_payload_build(payload, il_transactions)
    Builder->>Builder: apply_inclusion_list_transactions (IL-first)
    Builder->>Builder: fill_transactions (priority-fee mempool)
    Builder-->>EL: PayloadBuildResult (cached)
    EL-->>CL: "{payloadStatus: VALID, payloadId}"

    CL->>EL: engine_newPayloadV6(payload, blobHashes, beaconRoot, requests, inclusionListTxs)
    EL->>Store: get_block_from_payload + validate
    EL->>Store: add_block (execute + store)
    Store-->>EL: VALID

    EL->>Validator: new(decoded_il, pre_state, crypto)
    EL->>Validator: refresh_all_from(post_state, crypto)
    EL->>Validator: check(il, block_tx_hashes, gas_left, crypto)
    alt IL satisfied
        Validator-->>EL: Ok(())
        EL-->>CL: "{status: VALID}"
    else IL unsatisfied
        Validator-->>EL: Err(IlUnsatisfied)
        EL-->>CL: "{status: INCLUSION_LIST_UNSATISFIED}"
    end
Loading
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
crates/networking/rpc/engine/fork_choice.rs:710-715
**Stale doccomment contradicts the implementation**

The comment says the IL "does NOT yet thread them into `BuildPayloadArgs`" and "the locally-built block does not honor the IL during construction", but both statements are false: `BuildPayloadArgs::inclusion_list_transactions` is set to `Some(decoded_il.clone())` just below, and `initiate_payload_build` passes `decoded_il` all the way to `build_payload``apply_inclusion_list_transactions`, which does the full IL-first sequencing. Any developer reading this comment will incorrectly believe local payload building ignores the IL.

### Issue 2 of 4
crates/networking/rpc/engine/payload.rs:443-457
**`From<NewPayloadV6Request>` serializes IL transactions as byte arrays, not hex strings**

`serde_json::json!(val.inclusion_list_transactions)` on `Vec<bytes::Bytes>` serializes as `[[222, 173, ...], ...]` (integer arrays), not the `["0xdead...", ...]` format that `parse_il_transactions` expects. Any path that uses this `From` impl to build an outgoing request — tests, proxy mode, relay — will produce a call that the receiving EL rejects with `WrongParam("expected hex string")`. Compare with `PayloadAttributesV5` which correctly uses `#[serde(with = "serde_utils::bytes::vec")]`.

```suggestion
impl From<NewPayloadV6Request> for RpcRequest {
    fn from(val: NewPayloadV6Request) -> Self {
        // Serialize inclusion_list_transactions as hex-encoded strings
        // ("0x...") per the engine API wire format, matching how
        // parse_il_transactions on the receive side decodes them.
        let il_hex: Vec<String> = val
            .inclusion_list_transactions
            .iter()
            .map(|b| format!("0x{}", hex::encode(b)))
            .collect();
        RpcRequest {
            method: "engine_newPayloadV6".to_string(),
            params: Some(vec![
                serde_json::json!(val.payload),
                serde_json::json!(val.expected_blob_versioned_hashes),
                serde_json::json!(val.parent_beacon_block_root),
                serde_json::json!(val.execution_requests),
                serde_json::json!(il_hex),
            ]),
            ..Default::default()
        }
    }
}
```

### Issue 3 of 4
crates/networking/rpc/engine/inclusion_list.rs:38-57
**Duplicate `StoreIlStateProvider` — identical struct already exists in `inclusion_list_validator`**

`crates/blockchain/inclusion_list_validator.rs` exposes a public `StoreIlStateProvider` with the same fields and the same `IlStateProvider` impl. The `engine_newPayloadV6` handler already imports and uses that one (`ethrex_blockchain::inclusion_list_validator::StoreIlStateProvider`). Having a second private copy here means the two structs can silently diverge if the `Store` API changes. The `engine_getInclusionListV1` handler could import from `inclusion_list_validator` directly and remove this duplicate.

### Issue 4 of 4
crates/blockchain/inclusion_list_builder.rs:32-34
**`MAX_BYTES_PER_INCLUSION_LIST` is defined twice with independent values**

The same constant (`8192`) is also defined as `pub const MAX_BYTES_PER_INCLUSION_LIST` in `crates/common/types/inclusion_list.rs`, and `engine/inclusion_list.rs` already imports it from there (`use ethrex_common::types::MAX_BYTES_PER_INCLUSION_LIST`). Having a second independent copy here means the two can silently diverge — a change in `ethrex_common` would not affect the builder cap, and vice versa. The builder could simply import and re-export the constant from `ethrex_common`.

Reviews (1): Last reviewed commit: "fixup(l1): rebase EIP-7805 onto glamster..." | Re-trigger Greptile

Comment on lines +710 to +715
/// V5 payload-build hook. Decodes the IL transactions for log/observability
/// but does NOT yet thread them into `BuildPayloadArgs` — that wiring lands
/// in Phase 5.1 (`BuildPayloadArgs::inclusion_list_transactions`). For now,
/// the locally-built block does not honor the IL during construction; the
/// remote-validation path in `engine_newPayloadV6` is the authoritative
/// satisfaction check.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Stale doccomment contradicts the implementation

The comment says the IL "does NOT yet thread them into BuildPayloadArgs" and "the locally-built block does not honor the IL during construction", but both statements are false: BuildPayloadArgs::inclusion_list_transactions is set to Some(decoded_il.clone()) just below, and initiate_payload_build passes decoded_il all the way to build_payloadapply_inclusion_list_transactions, which does the full IL-first sequencing. Any developer reading this comment will incorrectly believe local payload building ignores the IL.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/engine/fork_choice.rs
Line: 710-715

Comment:
**Stale doccomment contradicts the implementation**

The comment says the IL "does NOT yet thread them into `BuildPayloadArgs`" and "the locally-built block does not honor the IL during construction", but both statements are false: `BuildPayloadArgs::inclusion_list_transactions` is set to `Some(decoded_il.clone())` just below, and `initiate_payload_build` passes `decoded_il` all the way to `build_payload``apply_inclusion_list_transactions`, which does the full IL-first sequencing. Any developer reading this comment will incorrectly believe local payload building ignores the IL.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +443 to +457
impl From<NewPayloadV6Request> for RpcRequest {
fn from(val: NewPayloadV6Request) -> Self {
RpcRequest {
method: "engine_newPayloadV6".to_string(),
params: Some(vec![
serde_json::json!(val.payload),
serde_json::json!(val.expected_blob_versioned_hashes),
serde_json::json!(val.parent_beacon_block_root),
serde_json::json!(val.execution_requests),
serde_json::json!(val.inclusion_list_transactions),
]),
..Default::default()
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 From<NewPayloadV6Request> serializes IL transactions as byte arrays, not hex strings

serde_json::json!(val.inclusion_list_transactions) on Vec<bytes::Bytes> serializes as [[222, 173, ...], ...] (integer arrays), not the ["0xdead...", ...] format that parse_il_transactions expects. Any path that uses this From impl to build an outgoing request — tests, proxy mode, relay — will produce a call that the receiving EL rejects with WrongParam("expected hex string"). Compare with PayloadAttributesV5 which correctly uses #[serde(with = "serde_utils::bytes::vec")].

Suggested change
impl From<NewPayloadV6Request> for RpcRequest {
fn from(val: NewPayloadV6Request) -> Self {
RpcRequest {
method: "engine_newPayloadV6".to_string(),
params: Some(vec![
serde_json::json!(val.payload),
serde_json::json!(val.expected_blob_versioned_hashes),
serde_json::json!(val.parent_beacon_block_root),
serde_json::json!(val.execution_requests),
serde_json::json!(val.inclusion_list_transactions),
]),
..Default::default()
}
}
}
impl From<NewPayloadV6Request> for RpcRequest {
fn from(val: NewPayloadV6Request) -> Self {
// Serialize inclusion_list_transactions as hex-encoded strings
// ("0x...") per the engine API wire format, matching how
// parse_il_transactions on the receive side decodes them.
let il_hex: Vec<String> = val
.inclusion_list_transactions
.iter()
.map(|b| format!("0x{}", hex::encode(b)))
.collect();
RpcRequest {
method: "engine_newPayloadV6".to_string(),
params: Some(vec![
serde_json::json!(val.payload),
serde_json::json!(val.expected_blob_versioned_hashes),
serde_json::json!(val.parent_beacon_block_root),
serde_json::json!(val.execution_requests),
serde_json::json!(il_hex),
]),
..Default::default()
}
}
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/engine/payload.rs
Line: 443-457

Comment:
**`From<NewPayloadV6Request>` serializes IL transactions as byte arrays, not hex strings**

`serde_json::json!(val.inclusion_list_transactions)` on `Vec<bytes::Bytes>` serializes as `[[222, 173, ...], ...]` (integer arrays), not the `["0xdead...", ...]` format that `parse_il_transactions` expects. Any path that uses this `From` impl to build an outgoing request — tests, proxy mode, relay — will produce a call that the receiving EL rejects with `WrongParam("expected hex string")`. Compare with `PayloadAttributesV5` which correctly uses `#[serde(with = "serde_utils::bytes::vec")]`.

```suggestion
impl From<NewPayloadV6Request> for RpcRequest {
    fn from(val: NewPayloadV6Request) -> Self {
        // Serialize inclusion_list_transactions as hex-encoded strings
        // ("0x...") per the engine API wire format, matching how
        // parse_il_transactions on the receive side decodes them.
        let il_hex: Vec<String> = val
            .inclusion_list_transactions
            .iter()
            .map(|b| format!("0x{}", hex::encode(b)))
            .collect();
        RpcRequest {
            method: "engine_newPayloadV6".to_string(),
            params: Some(vec![
                serde_json::json!(val.payload),
                serde_json::json!(val.expected_blob_versioned_hashes),
                serde_json::json!(val.parent_beacon_block_root),
                serde_json::json!(val.execution_requests),
                serde_json::json!(il_hex),
            ]),
            ..Default::default()
        }
    }
}
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +38 to +57
struct StoreIlStateProvider<'a> {
store: &'a Store,
state_root: H256,
}

impl<'a> IlStateProvider for StoreIlStateProvider<'a> {
fn get_account(
&self,
address: Address,
) -> Result<Option<AccountStateView>, IlStateProviderError> {
let acct = self
.store
.get_account_state_by_root(self.state_root, address)
.map_err(|e| IlStateProviderError::Read(e.to_string()))?;
Ok(acct.map(|a| AccountStateView {
nonce: a.nonce,
balance: a.balance,
}))
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Duplicate StoreIlStateProvider — identical struct already exists in inclusion_list_validator

crates/blockchain/inclusion_list_validator.rs exposes a public StoreIlStateProvider with the same fields and the same IlStateProvider impl. The engine_newPayloadV6 handler already imports and uses that one (ethrex_blockchain::inclusion_list_validator::StoreIlStateProvider). Having a second private copy here means the two structs can silently diverge if the Store API changes. The engine_getInclusionListV1 handler could import from inclusion_list_validator directly and remove this duplicate.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/engine/inclusion_list.rs
Line: 38-57

Comment:
**Duplicate `StoreIlStateProvider` — identical struct already exists in `inclusion_list_validator`**

`crates/blockchain/inclusion_list_validator.rs` exposes a public `StoreIlStateProvider` with the same fields and the same `IlStateProvider` impl. The `engine_newPayloadV6` handler already imports and uses that one (`ethrex_blockchain::inclusion_list_validator::StoreIlStateProvider`). Having a second private copy here means the two structs can silently diverge if the `Store` API changes. The `engine_getInclusionListV1` handler could import from `inclusion_list_validator` directly and remove this duplicate.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +32 to +34
/// Hard byte cap on the total RLP-encoded size of the returned inclusion list,
/// matching `MAX_BYTES_PER_INCLUSION_LIST` in the execution-apis spec.
pub const MAX_BYTES_PER_INCLUSION_LIST: usize = 8192;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 MAX_BYTES_PER_INCLUSION_LIST is defined twice with independent values

The same constant (8192) is also defined as pub const MAX_BYTES_PER_INCLUSION_LIST in crates/common/types/inclusion_list.rs, and engine/inclusion_list.rs already imports it from there (use ethrex_common::types::MAX_BYTES_PER_INCLUSION_LIST). Having a second independent copy here means the two can silently diverge — a change in ethrex_common would not affect the builder cap, and vice versa. The builder could simply import and re-export the constant from ethrex_common.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/inclusion_list_builder.rs
Line: 32-34

Comment:
**`MAX_BYTES_PER_INCLUSION_LIST` is defined twice with independent values**

The same constant (`8192`) is also defined as `pub const MAX_BYTES_PER_INCLUSION_LIST` in `crates/common/types/inclusion_list.rs`, and `engine/inclusion_list.rs` already imports it from there (`use ethrex_common::types::MAX_BYTES_PER_INCLUSION_LIST`). Having a second independent copy here means the two can silently diverge — a change in `ethrex_common` would not affect the builder cap, and vice versa. The builder could simply import and re-export the constant from `ethrex_common`.

How can I resolve this? If you propose a fix, please make it concise.

@ilitteri ilitteri closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant