fix(l1): stop eth_feeHistory aborting when the window is below retained history

[ilitteri]

Motivation

get_range clamps its two bounds independently — the finish is capped by the latest block, the start is raised to the earliest block we still hold (crates/networking/rpc/eth/fee_market.rs:231-234):

let finish_block_num = expected_finish_block_num.min(latest_block_num);
let expected_start_block_num = (finish_block_num + 1).saturating_sub(block_count);
let start_block_num = earliest_block_num.max(expected_start_block_num);

On a snap-synced node the earliest block is the pivot, so a request whose window lies entirely below the pivot yields start > end. The caller then computes (:104-105):

let block_count = (end_block - start_block + 1) as usize;
let mut base_fee_per_gas = vec![0_u64; block_count + 1];

With overflow checks that panics. In release it wraps, and the allocation asks for roughly 1.8e19 elements, which aborts the process. A single unauthenticated eth_feeHistory call is therefore a remote kill on any snap-synced node — that is, on the standard mainnet deployment.

Reproduced against a store whose earliest block is 5:

eth_feeHistory("0x1", "0x1", [])
  → panicked at crates/networking/rpc/eth/fee_market.rs:110:28:
    attempt to subtract with overflow

This will get easier to hit, not harder: history pruning (#6673) makes a non-zero earliest block the normal steady state rather than a snap-sync artifact.

Description

Return the empty fee history the caller is entitled to when the requested window lies entirely below retained history, matching the existing block_count == 0 path immediately above. Partially-available windows are unaffected and still serve the retained part with oldestBlock clamped up.

Tests

Two tests in test/tests/rpc/fee_history_range_tests.rs, against a store with earliest = 5 and earliest = 4 respectively:

• a window entirely below the earliest retained block returns an empty history rather than crashing — this test panics with attempt to subtract with overflow without the fix
• a window partially below it still serves the retained part, with oldestBlock clamped to the earliest retained block, so the fix does not turn a servable request into an empty one

[github-actions]

⚠️ 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.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]

🤖 Kimi Code Review

This PR fixes a critical DoS vulnerability where an unauthenticated eth_feeHistory RPC call could crash the node. The fix is correct and well-tested.

Security Assessment

• Critical vulnerability fixed: The subtraction end_block - start_block + 1 on line 113 (original) would wrap on u64 when start > end, causing vec![0; huge] to abort the process. This is a remote crash exploit.
• Correct mitigation: Returning an empty FeeHistoryResponse when the window is entirely below the earliest retained block is the appropriate behavior per JSON-RPC semantics.

Code Quality

• crates/networking/rpc/eth/fee_market.rs lines 103-110: The defensive check is correctly placed immediately after get_range and before the subtraction. The explanatory comment is excellent—clearly documents the snap-sync edge case.
• Line 109: Using FeeHistoryResponse::default() for the empty case is idiomatic, assuming it produces empty arrays (which the tests confirm).

Test Coverage

• test/tests/rpc/fee_history_range_tests.rs: Good coverage of both the crash scenario (window entirely below pivot) and the clamping behavior (partial overlap).
• Line 28: Consider also asserting that oldestBlock is "0x0" or the earliest available in the empty response case to ensure consistent API behavior.

Minor Suggestions

1. Test completeness: In fee_history_partially_below_earliest_still_serves_the_retained_part, consider verifying the array lengths match the expected 3 retained blocks (plus 1 for baseFeePerGas tail):

   assert_eq!(result["baseFeePerGas"].as_array().unwrap().len(), 4); // 3 blocks + 1
   assert_eq!(result["gasUsedRatio"].as_array().unwrap().len(), 3);

2. Documentation: The comment in fee_market.rs mentions "wraps in release"—technically it's defined behavior for u64 (wrapping), but the panic only occurs on the allocation. Consider clarifying: "which wraps to a very large u64, causing the subsequent vec! allocation to abort."

Verdict: LGTM. The fix correctly prevents the arithmetic underflow/overflow and subsequent memory allocation failure. The tests validate both the security fix and correct functional behavior.

---

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

[github-actions]

🤖 Claude Code Review

This is a small, well-targeted fix. Here's the review:

Review: Stop eth_feeHistory aborting when the window is below retained history

Correctness

The fix is correct and minimal. get_range (fee_market.rs:222-246) clamps start and end independently, so a request window entirely below the retained history (earliest_block_num) produces start_block > end_block. The added guard at fee_market.rs:109-112 catches exactly this case before the unchecked end_block - start_block + 1 subtraction at line 114, which is the actual overflow site. This mirrors the existing block_count == 0 early-return pattern (lines 96-99), so the codebase now handles both "nothing to return" cases the same way — good consistency.

I traced through get_range's arithmetic for both new tests and confirmed the numbers line up:

• Test 1 (earliest=5, request newestBlock=1, count=1): finish=1, start=max(5, 1)=5 → start > end → triggers the new guard, matching the described crash-without-fix scenario.
• Test 2 (earliest=4, request newestBlock=6, count=10): finish=6, start=max(4, 0)=4 → normal path, serving blocks 4..=6 with oldestBlock clamped to 4 — confirms the partial-overlap case is untouched.

Minor observations (non-blocking)

• FeeHistoryResponse::default() returns oldest_block: String::default() (i.e., ""), not a hex-encoded block number like "0x0". This is pre-existing behavior shared with the block_count == 0 path (line 96-99), so this PR isn't introducing a new inconsistency, but it's worth flagging that neither path matches go-ethereum's typical convention of still returning a valid oldestBlock (usually the request's would-be start, clamped) even for an empty result. Not something this PR needs to fix, since it strictly preserves existing empty-response semantics.
• The inline comment at lines 103-108 duplicates a lot of the PR/commit description. It's accurate and helpful for a subtle bug, so it's a reasonable place to leave it, but slightly verbose for the one branch it explains.

Tests

Both new tests are well-constructed, use realistic helper setup (setup_store, add_legacy_tx_blocks, update_earliest_block_number), assert on the actual JSON-RPC response shape, and — per the PR description — the first test reproduces the panic on main and passes only with the fix. The second test guards against a regression where the fix over-corrects and empties out a partially-servable window. Good coverage for the bug's boundary condition.

Security

This directly closes a remote, unauthenticated DoS (panic/abort via a crafted eth_feeHistory call) that becomes increasingly likely as snap-sync and history-pruning make non-zero earliest_block the norm rather than the exception. The fix is narrowly scoped and doesn't change behavior for any previously-working request shape.

Overall: solid, well-tested fix for a real remote-DoS bug. No blocking issues.

---

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

[github-actions]

🤖 Codex Code Review

Findings

1. crates/networking/rpc/eth/fee_market.rs still returns FeeHistoryResponse::default() for the new empty-history case, and that derived default makes oldest_block an empty string via the struct definition. This fixes the underflow/OOM DoS, but it makes a malformed eth_feeHistory payload newly reachable for a normal non-zero request below the retained pivot: oldestBlock is supposed to be a quantity, not "". I’d switch this branch to an explicit empty response with a valid oldestBlock value, and extend the new test to assert that field instead of only checking array lengths.

The guard itself looks correct and important from a security perspective: it closes a remote allocation-abort path in an unauthenticated RPC method. I did not find other correctness or EVM/consensus issues in the diff.

I couldn’t run the Rust tests in this environment because cargo/rustup attempted to write under read-only home directories.

---

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

[github-actions]

Lines of code report

Total lines added: 4
Total lines removed: 0
Total lines changed: 4

Detailed view

+------------------------------------------------+-------+------+
| File                                           | Lines | Diff |
+------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/fee_market.rs | 239   | +4   |
+------------------------------------------------+-------+------+

[klaus993]

/kimi

[klaus993]

/kimi

[github-actions]

🤖 Kimi Code Review

⚠️ Review failed: Kimi API request failed with status 400: invalid temperature: only 0.6 is allowed for this model

---

Automated review by Kimi (Moonshot AI)

[klaus993]

/kimi

[github-actions]

🤖 Kimi Code Review

I'll review this PR focusing on the security-critical fix for the eth_feeHistory RPC endpoint.

Summary

This PR fixes a critical unauthenticated RPC denial-of-service vulnerability where a eth_feeHistory request with a window below the earliest retained block causes a u64 underflow, leading to a massive vec! allocation that aborts the process. The fix is correct and well-tested.

---

Detailed Review

crates/networking/rpc/eth/fee_market.rs (lines 103-112)

The fix itself is correct and necessary. The comment at lines 103-109 accurately describes the vulnerability:

// `get_range` clamps its two bounds independently: the finish is capped by the
// latest block, the start is raised to the earliest block we still hold. On a
// snap-synced node (earliest = pivot) a request whose window lies entirely
// below the pivot therefore yields start > end. The subtraction below would
// wrap, and `vec![0; huge]` aborts the process, so answer the empty history
// the caller is entitled to instead.

Minor suggestion on error handling (line 110-111):

return serde_json::to_value(FeeHistoryResponse::default())
    .map_err(|error| RpcErr::Internal(error.to_string()));

Consider using .map_err(RpcErr::Internal) directly if RpcErr::Internal implements From<String> or takes impl ToString. If to_string() is required, the current code is fine. Not a blocking issue.

Question: Does FeeHistoryResponse::default() produce the correct empty response format? The tests verify baseFeePerGas and gasUsedRatio are empty arrays. Please confirm oldestBlock is also appropriately set (likely 0x0 or omitted) in the default — the RPC spec expects oldestBlock to be present even for empty results.

---

test/tests/rpc/fee_history_range_tests.rs

Test quality is excellent. Two complementary test cases:

1. Lines 14-45: fee_history_below_the_earliest_retained_block_is_empty_not_a_crash — Directly reproduces the vulnerability scenario and verifies no crash, no error, empty result.

2. Lines 47-71: fee_history_partially_below_earliest_still_serves_the_retained_part — Verifies correct clamping behavior when window partially overlaps retained history.

Minor test improvement suggestion (line 21):

store.update_earliest_block_number(5).await.unwrap();

Consider adding a brief comment explaining why 5 specifically: "Blocks 1-4 are now considered unavailable, simulating pivot block 5 after snap-sync." This aids future maintainers.

Test assertion style (lines 26-27, 50-51):

The call_http helper returns a serde_json::Value. The assertions on response.get("error").is_none() are good, but consider also asserting response["jsonrpc"] == "2.0" and response["id"] == 1 to verify well-formed responses, or verify response["error"] is absent more explicitly.

---

test/tests/rpc/mod.rs (line 6)

Correct module inclusion. No issues.

---

Security Assessment

Aspect	Status	Notes
Unauthenticated DoS	Fixed	The start_block > end_block check prevents the u64 underflow
Input validation	Adequate	Bounds are properly clamped; the fix handles the edge case
Resource exhaustion	Fixed	No longer possible to trigger vec![0; ~1.8e19]

Severity if unpatched: Critical — any peer can kill the node with a single RPC request.

---

Recommendations

1. Verify FeeHistoryResponse::default() completeness — Ensure oldestBlock field behavior matches Ethereum JSON-RPC spec for empty history responses.

2. Consider defensive programming in get_range — While the fix at the call site is correct, consider whether get_range itself should guarantee start <= end as a postcondition, or return a Result/Option indicating empty range. This would prevent similar bugs at other call sites.

3. Add test for exact oldestBlock in empty response — The first test verifies baseFeePerGas and gasUsedRatio lengths but doesn't assert oldestBlock value.

---

Conclusion

Approve with minor suggestions. The fix correctly addresses a critical vulnerability. The tests are well-designed and cover both the crash scenario and normal clamping behavior. The code is readable with excellent comments explaining the snap-sync context.

---

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

[klaus993]

/codex

[github-actions]

🤖 Codex Code Review

1. crates/networking/rpc/eth/fee_market.rs:109 returns FeeHistoryResponse::default(). Because the type derives Default and oldest_block is a plain String (crates/networking/rpc/eth/fee_market.rs:32-40), this serializes "oldestBlock": "". That is not a valid QUANTITY for eth_feeHistory, so the new non-crashing path can still break clients that deserialize the response according to the Execution API. This should return an explicit empty response with a valid hex oldestBlock, not the struct default.

2. test/tests/rpc/fee_history_range_tests.rs:35-44 only checks two arrays, so it would miss the invalid oldestBlock above. Add an assertion for result["oldestBlock"] being the expected hex quantity, and ideally cover the rest of the empty-response shape as well.

The start_block > end_block guard itself is the right fix for the underflow / oversized allocation DoS.

I could not run the tests here: local cargo execution is blocked by the sandboxed environment trying to write under /home/runner/.rustup and /home/runner/.cargo and fetch dependencies.

---

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