Skip to content

fix(l1): advertise the earliest block we can actually serve - #7210

Merged
ilitteri merged 1 commit into
mainfrom
feat/eth69-earliest-block
Aug 24, 2026
Merged

fix(l1): advertise the earliest block we can actually serve#7210
ilitteri merged 1 commit into
mainfrom
feat/eth69-earliest-block

Conversation

@ilitteri

Copy link
Copy Markdown
Collaborator

Motivation

Three places hardcoded the advertised earliest block to 0:

Site What it feeds
crates/networking/p2p/rlpx/eth/status.rs:69 shared eth/69+ handshake status (eth/69, eth/71)
crates/networking/p2p/rlpx/eth/eth70/status.rs:108 eth/70's own status struct
crates/networking/p2p/rlpx/eth/update.rs:33 BlockRangeUpdate — the ongoing half of the same advertisement

So every eth/69+ peer was told we hold history from genesis. That is already untrue on any snap-synced node, where the earliest retained block is the pivot, and peers use the advertised range to decide what to request from us — meaning we invite requests we then answer with empty replies.

It gets materially worse with history pruning (#6673): the earliest block moves continuously, so a hardcoded handshake value is wrong within hours of starting, and BlockRangeUpdate exists precisely to keep peers current as the range moves.

Description

Reads the value from the store in all three places.

get_earliest_block_number is async (crates/storage/store.rs:1256) while these constructors were sync — there is no cached sync accessor for it the way get_latest_block_number has one (:1275, backed by latest_block_header). So the constructors and their four handshake call sites in rlpx/connection/server.rs become async. The alternative, caching earliest in memory alongside latest, is a larger change for no benefit on a path that runs once per handshake.

Tests

Three tests in crates/networking/p2p/rlpx/eth/status.rs:

  • the eth/69, eth/70 and eth/71 handshake statuses all advertise a non-zero earliest block set on the store
  • BlockRangeUpdate reads the field from the store rather than hardcoding it, and the result still satisfies validate's earliest <= latest invariant
  • a genesis-synced node still advertises 0, so this is a no-op for full-history nodes

Note the second test deliberately uses earliest = 0 on a genesis-only store: setting a non-zero earliest above the latest block manufactures a range that validate correctly rejects, which is a state a real node cannot reach.

Three places hardcoded earliest_block to 0: the shared eth/69+ handshake status
(rlpx/eth/status.rs), eth/70's own status struct, and BlockRangeUpdate, which is
the ongoing half of the same advertisement. Every eth/69+ peer was therefore told
we hold history from genesis.

That is already untrue on any snap-synced node, where the earliest retained block
is the pivot, and peers use the advertised range to decide what to request. It
becomes materially wrong with history pruning (#6673), where the earliest block
moves continuously and a stale handshake value would be wrong within hours.

Read the value from the store instead. get_earliest_block_number is async while
these constructors were sync, so they and their four handshake call sites become
async; there is no cached sync accessor for it the way there is for the latest
block.
@ilitteri
ilitteri requested a review from a team as a code owner August 24, 2026 20:03
@github-actions

Copy link
Copy Markdown

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.


The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Three
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.


ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rsunsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.


Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

  1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
    reviewers, and add environment: to the two jobs that sign.
  2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

@github-actions github-actions Bot added the L1 Ethereum client label Aug 24, 2026
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

The PR correctly fixes a protocol honesty issue where nodes were unconditionally advertising earliest_block: 0, which is incorrect for snap-synced or pruned nodes. The async conversion is necessary and properly propagated.

Correctness & Safety

  • Line 73 (crates/networking/p2p/rlpx/eth/status.rs): Correctly fetches the actual earliest block from storage instead of hardcoding 0. This prevents peers from requesting historical blocks the node cannot serve, which could lead to peer scoring penalties or bans.
  • Lines 969-971 (crates/networking/p2p/rlpx/connection/server.rs): Properly awaited new async constructors.
  • Line 940 (crates/networking/p2p/rlpx/connection/server.rs): BlockRangeUpdate::new correctly awaited, ensuring the ongoing advertisement matches the handshake value.

Code Quality

  • Test coverage: The added tests in status.rs (lines 130-188) effectively verify:
    1. Non-zero earliest block advertisement (snap-sync case)
    2. BlockRangeUpdate consistency with the status message
    3. Backward compatibility for full-history nodes (earliest = 0)
  • Comments: The explanatory comments about snap-sync and pruning in status.rs and update.rs are valuable for future maintainers.

Minor Observations

  • Consistency: In server.rs, the match arm for eth/68 (line 968) remains synchronous while 69-71 are async. This is acceptable if StatusMessage68 doesn't query the earliest block, but consider making it async for consistency if the structure is similar.
  • Performance: The additional storage lookup is performed during handshake/status updates (cold path), so the async overhead is negligible.

Verdict
The changes are sound, security-conscious, and well-tested. The async migration is properly implemented across all affected call sites.


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

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

  1. Medium: the new earliest_block reads are not taken from the same snapshot as latest_block, so these constructors can now build an impossible advertised range under concurrent sync/backfill. In crates/networking/p2p/rlpx/eth/status.rs:48-60, crates/networking/p2p/rlpx/eth/eth70/status.rs:86-109, and crates/networking/p2p/rlpx/eth/update.rs:25-37, latest_block is read separately from the async frontier read. If snap sync advances the frontier after the head snapshot is taken, we can emit earliest_block > latest_block. BlockRangeUpdate::validate() explicitly rejects that on ingress, but the outbound path sends it unchecked in crates/networking/p2p/rlpx/connection/server.rs:939-942 and server.rs:967-971. I’d at least re-check the invariant before encoding/sending, ideally by deriving both values from a consistent snapshot or retrying when earliest > latest.

  2. Low: the added tests currently normalize an impossible store shape, so they do not protect the real non-genesis path this PR is trying to fix. crates/networking/p2p/rlpx/eth/status.rs:139-147 only loads genesis and then writes earliest_block_number = 12_345; status.rs:156-166 therefore passes even though earliest > latest. The BlockRangeUpdate test in status.rs:175-179 only exercises the zero case, so a regression back to hardcoded 0 would still pass. I’d replace this with a fixture where latest_block >= earliest > 0 and assert both Status and BlockRangeUpdate on that valid snap-shaped range.

No other correctness or EVM/consensus issues stood out in the touched code.

I could not run the Rust tests in this environment: cargo/rustup and dependency fetches are blocked by the read-only home/network restrictions.


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

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Review: PR 7210 — Advertise the earliest block we can actually serve

Summary: Replaces hardcoded earliest_block: 0 with a real read from the store in three places (StatusDataPost68::new, StatusMessage70::new, BlockRangeUpdate::new), threading async through the constructors and their four call sites in server.rs. Small, well-scoped, matches its stated motivation.

What's good

  • Correctly identifies and fixes a real correctness bug: advertising earliest_block: 0 unconditionally is already wrong for snap-synced nodes and would get worse with pruning.
  • The async propagation is complete and consistent — all four call sites (server.rs:940,969-971) and the two StatusDataPost68::new wrappers (eth69/eth71) were updated together; nothing was missed.
  • Good test coverage: verifies the non-zero case across eth/69-71, verifies BlockRangeUpdate reads from the store (not hardcoded), and verifies the genesis-only node still advertises 0 (no-op case). The reasoning in the second test's comment (why earliest = 0 was deliberately chosen to satisfy validate's invariant) shows real care.
  • PeerConnectionError already has #[from] StoreError, so the new ? on get_earliest_block_number().await? composes cleanly with existing error handling.
  • Correctly scoped — no attempt to also implement the pruning mechanism itself (feat(l1): pruning of historical blocks/receipts/txs #6673), just the plumbing to read the real value once it exists.

Findings

  1. Read ordering asymmetry / no self-validation before send (status.rs:48-60 vs update.rs:25-26) — StatusDataPost68::new reads latest_block (cached, line 48) before earliest_block (DB read, line 60), while BlockRangeUpdate::new reads them in the opposite order. Today this is harmless since earliest_block only changes once, at genesis. But once pruning (feat(l1): pruning of historical blocks/receipts/txs #6673) makes earliest_block mutate concurrently with handshakes, a pruning update landing between the two reads could produce earliest_block > latest_block in the outgoing message — and neither constructor calls .validate() on its own output before sending, so a malformed range could go out over the wire (the receiving peer's own validate() would presumably reject it, but that's still a wasted round-trip / potential down-score). Not a bug today, but worth a follow-up note or a debug_assert!(earliest <= latest) before sending, since the pruning PR is already referenced as the immediate next step.

  2. get_earliest_block_number() now hard-fails handshakes instead of degrading to 0 (status.rs:56-60) — previously the constructors never touched the store for this field, so any datadir would produce earliest_block: 0. Now, if ChainDataIndex::EarliestBlockNumber is ever unset (e.g. a datadir populated by some path that bypasses add_initial_state, or a legacy/corrupted store), every eth/69+ handshake fails with StoreError::MissingEarliestBlockNumber instead of just advertising 0. Worth confirming there's no such path in practice (it looks like add_initial_state_inner always sets it on first genesis load, and the field is a long-standing chain-data index, not new — so this is likely a non-issue, but it's a behavior change worth being explicit about in the PR description).

Both points are low-severity / forward-looking rather than blocking — the change as written is correct for the current (non-pruned) system.


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

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 46
Total lines removed: 0
Total lines changed: 46

Detailed view
+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/status.rs | 151   | +45  |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/update.rs | 69    | +1   |
+-------------------------------------------------+-------+------+

@github-project-automation github-project-automation Bot moved this to In Review in ethrex_l1 Aug 24, 2026
@ilitteri
ilitteri added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit 7ad398f Aug 24, 2026
61 checks passed
@ilitteri
ilitteri deleted the feat/eth69-earliest-block branch August 24, 2026 21:33
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Aug 24, 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.

2 participants