refactor: behavior-preserving cleanups and unused-API removals

[flyq]

Summary

PR 1/6 of the #170 split. Everything here is behavior-preserving on canonical-block paths, plus the public-API removals that had no consumer left. If a bisect ever lands on this PR, that is a strong signal something in it was not behavior-preserving — which is the whole point of separating it from B–F.

Responds to vincent's review of #170: "Suggest splitting into a genuinely behavior-preserving cleanup plus a handful of small, individually-motivated changes." Every item he listed under "Worth keeping, whatever the split" is in here.

What's in it

Second sources of truth removed

• ChainStore drops the ContractStore supertrait — nothing consumed contracts through it. The forced stubs on the pipeline mock and the trace server's StubBlockStore go with it.
• BlockMeta::from_header / try_from_header replace six hand-rolled header→meta projections: the three tip observations (validator + two in the trace server) default a missing withdrawals_root to zero, and the three anchor-init sites (app.rs, the trace server's main.rs, the validator integration test) go through the strict variant, which returns None so each keeps its own reject error — that path must reject, not default.
• FetcherState::in_flight_blocks deleted: it mirrored task_to_block.values(). Rather than scan those values per completion (O(max_in_flight²), Codex's P2), the one remaining map is keyed by block — in_flight: HashMap<u64, Id> — so recover_gaps and the success/failure paths stay O(1); the task id is only needed to map a panicked task's JoinError back to its block, a scan on that rare path.
• The hardfork reorder loop in ChainSpec::from_genesis is gone, and so is the mega_mainnet_hardforks() ladder: both were copies of mega-evm's MegaHardfork declaration, which the chain-spec tests now read directly as MegaHardfork::VARIANTS. The "unknown fork" append branch was unreachable — into_vec only ever produces those ten.
• BackoffSchedule (stepped from RetryPacing::schedule()) collects the jittered-doubling arithmetic (jitter, max cap, 1 ms floor) that was duplicated between the RPC round-robin loop and the R2 GET loop, comments and all. Both consume it — and BackoffPolicy itself is now a re-export of RetryPacing: the two were field-for-field identical {initial, max} pairs with a hand-written adapter between them (r2_witness.rs::pacing), which goes too.
• The validator's integration mock handlers dedupe onto MockServerState::block_by_number_hex / block_by_hash / invalid_params — which also fixes a drifted bare unwrap() on malformed eth_getHeaderByNumber params.

Dead parameters, accessors, and backdoors

• handle_transient_restart's _reason: String parameter and the upstream string construction that fed it.
• ValidatorFetcher::on_remote_height fn-pointer injection (the only value ever passed was metrics::set_remote_chain_height).
• run_with_signals no longer takes the report endpoint (an Option<String> it only ever .is_some()'d); it reads RpcClient::reports_validation(), since the client is already built from the same flag — two inputs that had to agree became one.
• LightWitnessExecutor::kvs() — no consumer, here or in mega-reth.
• #[cfg(test)] ValidatorDB::set_anchor_block — the roundtrip test now goes through the production reset_to_anchor path instead of a test-only shortcut around the helper layer.
• WitnessExternalEnv::new / from_light_witness share one from_metadata_kvs body (the metadata scan was duplicated verbatim); a doubled doc block on create_evm_env removed.

Reuse

• The blob base-fee fraction comes from revm's BLOB_BASE_FEE_UPDATE_FRACTION_CANCUN instead of a hand-copied consensus constant. Verified equal: revm-primitives defines it as 3_338_477, the local constant was 3338477.
• install_prometheus_exporter hoisted into stateless_common::metrics behind a prometheus-exporter feature both binaries enable; the metrics-exporter-prometheus dep moves from the two bins into common as an optional dep, so mega-reth (which consumes stateless-common and pins the exporter at 0.16) does not build a second copy for a function it never calls.
• The validator's contract-hash collection uses the existing collect_code_hashes instead of open-coding iter_code_hashes().collect::<HashSet<_>>().
• workers.rs → runner.rs (it runs the pipeline; the workers live in core), docs updated.

The invariant that lost its only enforcement

vincent: "Removing mega_mainnet_hardforks() and the explicit reordering loop makes MegaethGenesisHardforks::into_vec's literal the single source of activation order. That is the right direction… But nothing tests the order."

test_mega_hardforks_iterate_in_activation_order now pins into_vec end-to-end through from_genesis against mega-evm's own declaration rather than a local ladder: it schedules every MegaHardfork::VARIANTS entry (genesis field derived from the variant name) and asserts the scheduled MegaETH forks equal VARIANTS, so a variant into_vec lacks, duplicates, or misplaces fails at either end of the ladder — a hand-written expected list or a prefix compare both let a trailing omission through, and new forks are only ever appended. Mutation-checked on the final tree: dropping Rex6 from into_vec fails, swapping Rex5/Rex6 fails, duplicating Rex6 fails. mainnet_genesis_schedules_every_canonical_hardfork iterates VARIANTS the same way, through the same name-keyed ChainHardforks::fork lookup fork selection uses at runtime, which is what let the hand-written ladder go. One correction to the earlier framing: insertion order is not consensus-relevant. mega-evm resolves the active fork through mega_fork_activation, a name-keyed map lookup on ChainHardforks, and nothing outside the tests iterates forks_iter(), so the deleted reorder loop was already inert for consensus; what the tests guard is that the local literal and mega-evm's declaration agree in membership and order, so a fork the executor supports cannot go unscheduled unnoticed.

Deviations from the #170 material

One, forced by main moving 9 PRs since e1b2ede:

1. BackoffSchedule lives in stateless-r2, not stateless-common. feat(stateless-r2): custom-domain R2 target with HTTP/2, keeping the raw S3 endpoint path #187/refactor: dedupe the R2 witness adapters and hoist the jsonrpsee mock scaffolding #192 moved the R2 GET loop into stateless-r2, and stateless-common depends on stateless-r2 — so common is no longer a home both consumers can reach, and stateless-r2 must stay free of upward dependencies (its RetryPacing doc says so explicitly). The schedule therefore sits next to RetryPacing as RetryPacing::schedule(), and BackoffPolicy is RetryPacing re-exported under the name stateless-common's API uses (RetryPacing gained the const fn new that BackoffPolicy had; every existing constructor, struct-literal, and field-access form compiles unchanged). For mega-reth this is an addition to stateless-r2's surface, not a break.

Observable changes (behavior-preserving ≠ byte-identical output)

• The validator's /metrics loses the eth_getTransactionByHash counter series. It was pre-registered but permanently zero — that method is only ever called by the trace server.
• The trace server's exporter-install failure message changes from "Failed to install metrics exporter" to "Failed to install Prometheus exporter" (the validator's wording), since both now share one installer. Startup-only.
• collect_code_hashes returns a sorted, deduplicated Vec where the open-coded HashSet path was unordered. No consumer depended on the order; the cache lookup and the RPC batch are both order-insensitive.

Breaking at the next tag

mega-reth pins stateless-core at v2.0.14, so nothing breaks until the bump — which needs coordination:

• LightWitnessExecutor::kvs() removed.
• BLOB_GASPRICE_UPDATE_FRACTION removed; use revm's BLOB_BASE_FEE_UPDATE_FRACTION_CANCUN (same value, verified — it is a consensus parameter).
• ChainStore: ContractStore supertrait dropped. Downstream payoff: mega-reth's MegaValidatorChainStore can delete the ContractStore impl its own comment calls "intentionally inert".
• stateless_common::BackoffPolicy is now a re-export of stateless_r2::fetch::RetryPacing — source-compatible (same field names, new, and it gains Copy); only a trait impl naming the type would notice, and mega-reth has none. Copy also turns any .clone() on it into a clippy::clone_on_copy warning (this PR dropped two of its own on the retry field); the mega-reth branch carrying the validator crates has no BackoffPolicy reference today, but grep before the pin bump.
• mega_mainnet_hardforks() removed — the refactor: apply simplify-review cleanups across core, common, and the binaries #170 deletion, restored now that its one consumer reads MegaHardfork::VARIANTS. No consumer in mega-reth.

Testing

cargo fmt --all --check, cargo clippy --workspace --all-targets --all-features (0 warnings), cargo sort --check, full workspace suite 478 passed / 0 failed, and cargo test -p stateless-core --no-default-features --lib --no-run (the no-std alias path) all clean.

Notes

PR 1 of 6, stacked: B (EIP-3155 writer deletion) → C (verify/gas_used) → D (blocking work) → E (hot-path allocations) → F (empty witness endpoints). Each targets its predecessor and stands on its own.

[mega-maxwell]

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

🛠️ Review did not finish

Attempted 5918ac59..f43fdfed · updated 2026-09-07T01:27:26+00:00

This round did not publish: MODEL_ACTION_FAILED in phase review_retry. Anything listed below is from the last round that did. Re-run the workflow or push a new commit to try again.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.7%. Comparing base (8ada1c9) to head (f43fdfe).

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3e23b61af

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[vincent-k2026]

Right split of #170: second sources of truth and dead parameters go, hardfork order moves from a runtime reorder to a test-time cross-check. No correctness/security blocker.

What's good

• ChainStore no longer extends ContractStore — the pipeline mock and StubBlockStore stubs go with it, and mega-reth can drop the "intentionally inert" impl on the next pin bump.
• BlockMeta::from_header (tip: missing withdrawals_root → zero) vs try_from_header (anchor: None, each site keeps its own reject error) — defaulting and rejecting are not collapsed into one constructor.
• Fetcher in_flight: HashMap<u64, Id> fixes the Codex P2 O(n²) recover_gaps at the source, without bringing the mirror set back.
• BackoffPolicy as a RetryPacing re-export plus RetryPacing::schedule(): jitter / max / 1 ms floor live in one place; RPC and R2 GET no longer duplicate the arithmetic; stateless-r2 stays free of upward deps.
• mega_mainnet_hardforks() kept as the membership oracle (#190 already consumes it); only the reorder loop goes. Matches main having moved.
• test_mega_hardforks_iterate_in_activation_order is the deleted runtime cross-check, moved to test time. The claimed Rex5/Rex6 swap going red is the right shape.
• set_anchor_block now goes through production reset_to_anchor; the mock eth_getHeaderByNumber parse().unwrap() is parse()?.
• Breaking surface is explicit (kvs(), BLOB_GASPRICE_UPDATE_FRACTION, ChainStore supertrait). mega-reth is pinned at v2.0.14, so nothing breaks until the coordinated bump.

Blocking (correctness / security)

None.

Non-blocking / follow-up

1. The order test filters forks_iter() by ladder names, so an extra fork in into_vec is silently dropped. Omitting it from the ladder fails; adding it only to into_vec does not. assert_eq!(mega_order.len(), expected.len()) (or comparing the MegaETH prefix without a filter) would make that symmetric. Today it is a comment: insert in both places.
2. /metrics loses the permanently-zero eth_getTransactionByHash series, and the exporter-install error string becomes the shared "Prometheus exporter" wording. Disclosed in the PR body; dashboards that keyed on the old name need to know.
3. workers.rs → runner.rs is behavior-preserving and the docs were updated; it is a rename, not an unused-API removal, so greps for the old path will wobble once.
4. install_prometheus_exporter's expect("valid bucket config") is hoisted as-is from the two binaries, not newly introduced. Startup-only, buckets are static — fine.

[flyq]

Thanks — item 1 holds and is fixed; 2–4 need no code change.

1. Order test was one-directional. Confirmed: filter(|name| expected.contains(name)) drops a fork that exists only in into_vec, so that omission passed. Fixed by comparing the prefix instead of filtering — from_genesis concatenates the MegaETH forks ahead of the Optimism ones (chain_spec.rs:167-168), so they are the leading ladder.len() entries, and take(expected.len()) makes the check symmetric (chain_spec.rs:406-419).

Mutation-checked both directions on the same tree:

mutation	filter (before)	prefix (after)
swap Rex5/Rex6 in into_vec	fails	fails
drop Rex5 from the ladder only	passes	fails

The second row is exactly the blind spot you named. The doc on into_vec already says to insert in both places; now the test enforces it.

2. eth_getTransactionByHash series / exporter wording. Agreed, both are in the PR body's "Observable changes" section so the dashboard owners see them at merge time. No code change.

3. workers.rs → runner.rs. Agreed it is a rename rather than a removal; AGENTS.md and README.md both point at the new path, so the wobble is one grep.

4. expect("valid bucket config"). Agreed — hoisted verbatim, startup-only, static buckets.

[RealiCZ]

Two follow-ups on the hardfork section, both inline. Also for the "Breaking at the next tag" list: BackoffPolicy gaining Copy means any .clone() on it in mega-reth becomes a clippy::clone_on_copy warning, which this PR itself had to remove here; worth a grep before the pin bump.

[flyq]

On the BackoffPolicy: Copy note — added to the "Breaking at the next tag" list. This PR dropped two .clone()s of its own on the retry field; the mega-reth branch carrying the validator crates has no BackoffPolicy / rpc_retry reference today, so nothing to change there yet, but the grep is on the pin-bump checklist.

[RealiCZ]

f43fdfe closes both hardfork threads: the order test now pins into_vec against MegaHardfork::VARIANTS (symmetric on a missing, extra, duplicated, or misplaced entry), the membership test iterates VARIANTS through the same name-keyed lookup fork selection uses, the hand-written ladder is gone, and the into_vec doc states what the tests actually guard. The two tests fail in complementary ways on a mega-evm bump that brings a new fork, so the next pin bump cannot leave it unscheduled or unmapped unnoticed.