Skip to content

chore(evm-node): remove stale chain-spec FIXME - #376

Open
mehmetkr-31 wants to merge 1 commit into
circlefin:mainfrom
mehmetkr-31:chore/remove-stale-chainspec-fixme
Open

chore(evm-node): remove stale chain-spec FIXME#376
mehmetkr-31 wants to merge 1 commit into
circlefin:mainfrom
mehmetkr-31:chore/remove-stale-chainspec-fixme

Conversation

@mehmetkr-31

Copy link
Copy Markdown

Summary

Remove the stale FIXME use the ethereum chain spec temporary, we need to define Arc chain spec in crates/evm-node/src/node.rs. ArcChainSpec is defined in crates/execution-config/src/chainspec.rs and node.rs already binds ChainSpec = ArcChainSpec throughout — as it did in the first public commit.

The line after it documented the upstream reth file's ChainSpec bounds and only made sense as the FIXME's continuation, so it goes too; happy to keep it as a standalone porting note if preferred.

Fixes #375.

Test plan

  • Comments-only change; no behavior change
  • cargo fmt --all --check clean

`crates/evm-node/src/node.rs` carries:

    // FIXME use the ethereum chain spec temporary, we need to define Arc chain spec
    // original traits for ChainSpec in this file `Hardforks + EthereumHardforks + EthExecutorSpec`

Arc's chain spec already exists and this file already uses it: `ArcChainSpec`
is defined in `crates/execution-config/src/chainspec.rs` and node.rs binds
`ChainSpec = ArcChainSpec` throughout. That was already true in the first
public commit (a1235c0), where node.rs used `ArcChainSpec` 10 times — so the
FIXME has been stale since it landed.

The second line documented the upstream reth file's ChainSpec bounds and only
made sense as a continuation of the FIXME, so it goes too.

Comments only; no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mehmetkr-31

Copy link
Copy Markdown
Author

@ZhiyuCircle when you have a moment — this is the same shape as #307 (a stale FIXME, comments only, no behaviour change), and its workflow run is waiting on first-time-contributor approval. Thanks!

@osr21

osr21 commented Sep 10, 2026

Copy link
Copy Markdown

Small, low-risk PR — verified both claims directly rather than taking the description at its word.

ArcChainSpec is genuinely bound throughout

crates/evm-node/src/node.rs on main uses ChainSpec = ArcChainSpec in 16 places (type ChainSpec = ArcChainSpec;, plus every NodeTypes<ChainSpec = ArcChainSpec, ...> bound), and ArcChainSpec is a real type defined in crates/execution-config/src/chainspec.rs. The FIXME is unambiguously stale.

The FIXME was stale on day one — even earlier than cited

The issue and PR trace it back to a1235c0 (2026-04-06). It's older than that: the very first open-source commit, 3e2f9f4 ("feat: open source arc-node (#1)", 2026-04-02), already has ArcChainSpec bound 10 times in this same file with the FIXME sitting right above it — 3e2f9f4 is a direct ancestor of a1235c0, four days earlier. So there's no version of the public repo where this comment was ever accurate; it was inherited stale at the open-source cutover, not something that went stale afterward.

cargo fmt --all --check — confirmed clean

Ran rustfmt --edition 2024 --check directly against the PR's version of the file: no diff, exit 0. The deletion leaves a single blank line before use crate::rpc_middleware::{...}, consistent with the blank-line convention already used between import groups elsewhere in the file.

Scope check

Searched the rest of the tree for the same failure mode (a FIXME asserting something no longer true). Only one other FIXME exists outside test code, in crates/types/src/proposal_monitor.rs — that one's a live, still-accurate design note about log placement, unrelated to chain spec and not stale. So this PR isn't missing a sibling case.

Straightforward comments-only change, no behavior difference, matches the pattern already merged in #307.


Disclosure: I'm an external community contributor, not affiliated with Circle, with no write access to this repository. Advisory only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: stale FIXME in evm-node says the Arc chain spec is not defined yet (it is, and is used throughout)

2 participants