Skip to content

fix(l1): unschedule Amsterdam on Hoodi - #7455

Merged
ilitteri merged 2 commits into
mainfrom
unschedule-hoodi-amsterdam
Oct 23, 2026
Merged

ilitteri merged 2 commits into
mainfrom
unschedule-hoodi-amsterdam

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

Motivation

eth-clients/hoodi#33, merged 2026-10-22, unschedules the Hoodi Glamsterdam fork planned for 2026-10-26 17:42:48 UTC (epoch 132352, amsterdamTime 1793036568). It removes amsterdamTime from metadata/genesis.json, restoring the files to their state before eth-clients/hoodi#31.

If ethrex keeps the timestamp, it advertises an EIP-2124 fork ID with next = 1793036568 that other clients no longer announce, and on the 26th it activates Amsterdam alone and forks itself off Hoodi.

Description

Reverts #7391:

File Change
cmd/ethrex/networks/hoodi/genesis.json drop amsterdamTime
crates/common/config/networks.rs drop test_hoodi_amsterdam_activation
crates/common/types/fork_id.rs Hoodi rows after BPO2 go back to 0x23aa1351, next = 0
crates/networking/rpc/rpc.rs eth_config test: last is BPO2 again (no builder predeploys)
docs/roadmaps/forks-roadmap.md note that Hoodi was unscheduled; mainnet still TBD

The automatic revert merged rpc.rs by also moving an unrelated handle_get_heap stub. I reapplied only the test hunk, so the diff touches nothing outside the test.

How to Test

cargo test -p ethrex-config
cargo test -p ethrex-common --lib fork_id
cargo test -p ethrex-rpc --lib eth_config

All pass locally: 9 config tests (including test_hoodi_genesis_block_hash), 8 fork_id tests, and the eth_config test.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync. (Not needed: no schema change.)

eth-clients/hoodi#33 removed `amsterdamTime` from Hoodi's genesis.json,
unscheduling the Glamsterdam fork planned for 2026-10-26 (epoch 132352).
Keeping the timestamp would make ethrex advertise a fork ID with
`next = 1793036568` that no other client announces, and activate
Amsterdam on its own on the 26th, splitting it off the network.

This reverts #7391, restoring the Hoodi preset, fork-ID rows and
eth_config expectations to their BPO2-last state.
@MegaRedHand
MegaRedHand requested a review from a team as a code owner October 22, 2026 15:09
Copilot AI balanced review requested due to automatic review settings October 22, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 22, 2026 •

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. Four
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, then #3652 in tests-zkevm@v21.0.1, which removed
public_keys outright without bumping the revision. 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.rs → unsync_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 Oct 22, 2026
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

I'll review this PR which removes the Amsterdam fork activation from Hoodi network configuration.

Summary

This PR removes the amsterdamTime fork activation from Hoodi network, updating genesis config, fork ID tests, RPC tests, and documentation. The changes appear to be a straightforward unscheduling of a fork based on upstream changes (eth-clients/hoodi#33).

Detailed Review

1. cmd/ethrex/networks/hoodi/genesis.json (Line 23 removal)

Correct. Removing "amsterdamTime": 1793036568 from Hoodi's genesis config aligns with the upstream change. The field remains in Sepolia's genesis (not touched in this PR), which is correct per the documentation update.

2. crates/common/config/networks.rs (Lines 243-254 removed)

Correct. The removed test test_hoodi_amsterdam_activation would fail after the genesis change, so its removal is necessary. No replacement test needed since the fork is no longer scheduled.

Minor suggestion: Consider adding a test verifying that amsterdam_time is None for Hoodi, to explicitly document and enforce this unscheduling:

#[test]
fn test_hoodi_amsterdam_not_scheduled() {
    let genesis = Network::PublicNetwork(PublicNetwork::Hoodi)
        .get_genesis()
        .unwrap();
    assert_eq!(genesis.config.amsterdam_time, None);
}

This prevents accidental re-scheduling without explicit consideration.

3. crates/common/types/fork_id.rs (Lines 343-369 modified)

Issue: Potential test gap.

The test case modifications look correct for the fork ID changes, but there's a subtle concern. The removed test cases covered:

  • The transition at bpo2Time (1762955544) pointing to amsterdamTime (1793036568)
  • The boundary just before Amsterdam (1793036567)
  • The activation at Amsterdam (1793036568)

The remaining tests now show bpo2Time as the final fork with fork_next: 0. However, verify that the fork_hash 0x23aa1351 at bpo2Time is correct for the post-removal state. The hash should be computed from the fork ID chain without Amsterdam.

The test at line 351 (time 2740434112, far future) now uses 0x23aa1351 instead of 0x3d068b59. This is correct if 0x23aa1351 is the stable final fork hash after bpo2.

Question: Was the fork_hash 0x23aa1351 for bpo2 verified against another client implementation (geth, besu, etc.)? For consensus-critical fork ID values, cross-client verification is essential.

4. crates/networking/rpc/rpc.rs (Lines 2608-2643)

Correct. The RPC test updates properly reflect:

  • activationTime changed from 1793036568 to 1762955544 (bpo2 becomes last fork)
  • forkId changed from 0x3d068b59 to 0x23aa1351
  • Removal of BUILDER_DEPOSIT_CONTRACT_ADDRESS and BUILDER_EXIT_CONTRACT_ADDRESS from systemContracts

Security consideration: The removal of builder contract addresses from the expected RPC output is interesting. Verify that:

  1. These contracts were Amsterdam-specific additions
  2. Their removal from the response doesn't break any EIP-7251 or related validator operations that should still function on Hoodi pre-Amsterdam

If these system contracts were deployed at genesis or earlier forks, they should remain in the response. If they were to be deployed at Amsterdam, their removal is correct.

5. docs/roadmaps/forks-roadmap.md (Lines 104-110)

Correct. Documentation accurately reflects the change, citing the correct upstream reference (eth-clients/hoodi#33).

Additional Checks Needed

  1. Sepolia genesis verification: Confirm cmd/ethrex/networks/sepolia/genesis.json still contains amsterdamTime (should be 1791294816 per docs). This file isn't in the diff, so assumed untouched.

  2. Fork enum completeness: Verify crates/common/types/genesis.rs still contains Amsterdam in the fork enum (mentioned in docs as "ending at Amsterdam"). Removing Hoodi's activation shouldn't require removing the fork variant itself.

  3. Mainnet config: Confirm mainnet genesis has no amsterdamTime as stated.

Conclusion

The PR appears correct and safe with the following recommendations:

  • Consider adding an explicit test for amsterdam_time == None on Hoodi (Item 2)
  • Verify fork_hash values against another client implementation (Item 3)
  • Confirm builder contract address removal rationale (Item 4)

No critical security or consensus issues identified.


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

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Review of PR 7455: unschedule Amsterdam on Hoodi

I read the diff and grepped the repo. I did not build the code or run the tests.

The PR is small and consistent. It removes amsterdamTime (1793036568) from cmd/ethrex/networks/hoodi/genesis.json and updates everything that depended on it.

  • Fork ID tests (crates/common/types/fork_id.rs): The two Amsterdam-era cases are removed. The BPO2 case now expects fork_next: 0. The far-future timestamp case (2740434112) now expects hash 0x23aa1351, which is the BPO2 fork hash. This is correct, since no fork follows BPO2.
  • RPC eth_config test (crates/networking/rpc/rpc.rs): last now reports BPO2's activationTime (1762955544) and fork ID 0x23aa1351. The Amsterdam-only BUILDER_DEPOSIT_CONTRACT_ADDRESS and BUILDER_EXIT_CONTRACT_ADDRESS system contracts are gone from the expected output, which matches Amsterdam being absent.
  • Removed test (crates/common/config/networks.rs): test_hoodi_amsterdam_activation is deleted. Keeping it would fail, so removing it is right.
  • Docs (docs/roadmaps/forks-roadmap.md): The text now says Hoodi's activation was unscheduled, citing chore: unschedule Hoodi Glamsterdam activation eth-clients/hoodi#33, and that Sepolia is the only network still scheduled.
  • Leftover references: A grep for 1793036568 and 3d068b59 finds nothing else in the repo, so no stale fixtures remain.

Minor, non-blocking: I couldn't check eth-clients/hoodi#33 from here. Please confirm that the upstream genesis really dropped amsterdamTime and that BPO2 is the last Hoodi fork. Otherwise ethrex's fork ID would differ from other clients' and break peering on Hoodi.

Looks good to merge.


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

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

Looks good overall. This PR is a coherent rollback of Hoodi’s amsterdamTime, and the config, fork-id expectations, RPC fixture, and docs all move in sync.

  • No correctness issue stood out in the touched logic: removing amsterdamTime from cmd/ethrex/networks/hoodi/genesis.json:22 correctly makes bpo2Time the last scheduled Hoodi fork, and the updated fork-id expectations in crates/common/types/fork_id.rs:341 and crates/common/types/fork_id.rs:350 match that behavior.
  • The RPC expectation update in crates/networking/rpc/rpc.rs:2611 is consistent with the genesis change: the "last" fork now resolves to the BPO2 activation and fork hash rather than Amsterdam.
  • Security/performance impact appears negligible: this is static network metadata plus tests/docs, with no change to EVM execution, gas accounting, trie/storage paths, RLP, or transaction/block validation logic.
  • Readability/maintainability are fine: removing the Hoodi-specific activation test in crates/common/config/networks.rs:213 avoids asserting stale network policy, while keeping the Sepolia Amsterdam test intact in crates/common/config/networks.rs:214.

One small review note:

  • docs/roadmaps/forks-roadmap.md:108 says Hoodi’s activation “was unscheduled,” but the codebase now treats Hoodi as having no amsterdamTime at all. If upstream could later reschedule Amsterdam on Hoodi, consider clarifying the wording to “is currently unscheduled” to avoid the doc reading like a permanent historical fact.

If you want, I can also do a quick pass on whether the updated Hoodi fork hash 0x23aa1351 is derived consistently from the current genesis and ForkId::new implementation.


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

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 0
Total lines removed: 27
Total lines changed: 27

Detailed view
+-----------------------------------------+-------+------+
| File                                    | Lines | Diff |
+-----------------------------------------+-------+------+
| ethrex/crates/common/config/networks.rs | 255   | -7   |
+-----------------------------------------+-------+------+
| ethrex/crates/common/types/fork_id.rs   | 555   | -18  |
+-----------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs     | 2001  | -2   |
+-----------------------------------------+-------+------+

@github-project-automation github-project-automation Bot moved this to In Review in ethrex_l1 Oct 22, 2026
@ilitteri
ilitteri enabled auto-merge October 23, 2026 00:07
@ilitteri
ilitteri added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 43e3b03 Oct 23, 2026
59 checks passed
@ilitteri
ilitteri deleted the unschedule-hoodi-amsterdam branch October 23, 2026 01:09
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Oct 23, 2026
pjdurden pushed a commit to pjdurden/ethrex that referenced this pull request Oct 28, 2026
**Motivation**

Brings the v30.0.1 release branch back into `main` after the release.

**Description**

`release/v30.0.1` (tagged `v30.0.1` at `391a2cdefe`) is `v30.0.0`
(`80b774a959`) plus:

- `00867cd41f` fix(l2): resume block import on a new peer connection
(lambdaclass#7425), squash-merged to main as `70548f1c05`
- `042aabc934` perf(l2): stop re-sending the whole chain to reconnecting
peers (lambdaclass#7426), squash-merged to main as `90dc404abf`
- `54c3e7c7a8` fix(l2): import peer blocks through a single node-wide
importer (lambdaclass#7429), squash-merged to main as `535647e45d`
- `eaa1018c94` perf(l2): commit trie layers by depth when importing peer
blocks (lambdaclass#7430), squash-merged to main as `0a5fd80fb4`
- `5ef1b3f8b3` chore(l1,l2): bump version to 30.0.1
- `f2602cd0f3` fix(l1): unschedule Amsterdam on Hoodi (lambdaclass#7455),
squash-merged to main as `43e3b03b26`
- `391a2cdefe` merge of `main` at `27d6b1b51e` into the branch for rc.3,
with the version-line conflicts resolved to 30.0.1 and the lockfiles
regenerated

After the release, two commits keep the branch in step with `main`:

- `a10123227b` merge of `main` at `3b7395fc6e`, clean
- `d2785af876` records the `ethrex-partial` crate that came in from main
at 30.0.1 in `Cargo.lock`, which the first CI run of this PR flagged

All five PRs are on main already, so the effective change against main
is the 30.0.0 → 30.0.1 version bump (crate manifests, lockfiles and the
`docs/CLI.md` defaults): the branch differs from main only in those 22
files and only in version lines. The `v30.0.1` tag stays on
`391a2cdefe`.

Merging deletes `release/v30.0.1`. Nothing pulls it now: no soak runs on
it.

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync. Not
applicable: schema stays at 4.

---------

Co-authored-by: juanbono <juan.bono@lambdaclass.com>
Co-authored-by: Manuel Iñaki Bilbao <manuel.bilbao@lambdaclass.com>
Co-authored-by: Julian Arce <52429267+JuArce@users.noreply.github.com>
Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
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.

3 participants