chore(el): derive reth chainspecs from base-alloy-chains vs. duplicating configs by danyalprout · Pull Request #1592 · base/base

danyalprout · 2026-03-23T02:02:45Z

Description

Part one of deduplicating chain configurations and settings. Creates base-alloy-chains which has the chain configurations defined in Rust.

Upstream consumers will map this generic type into CL/EL specific types via a TryFrom/From impl.

cb-heimdall · 2026-03-23T02:02:50Z

🟡 Heimdall Review Status

Requirement Status More Info

Reviews

🟡 -1/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

vercel · 2026-03-23T02:02:51Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
base	Ignored	Preview	Mar 23, 2026 5:09am

github-actions · 2026-03-23T02:07:04Z

crates/alloy/chains/src/config.rs

+    }
+
+    /// Looks up a chain config by CLI name.
+    pub fn by_name(name: &str) -> Option<&'static Self> {


by_name does a linear scan with contains on each config's cli_names slice. With only 4 configs this is fine, but the &str parameter requires exact equality — consider documenting that this is case-sensitive. No functional issue, just noting for API clarity.

crates/execution/chainspec/src/spec.rs

github-actions · 2026-03-23T02:07:07Z

crates/execution/chainspec/src/spec.rs

-            "base-devnet-0-sepolia-dev-0" => Some(BASE_DEVNET_0_SEPOLIA_DEV_0.clone()),
-            _ => None,
-        }
+        BaseChainConfig::by_name(s).map(|cfg| Arc::new(Self::from(cfg)))


parse_chain now constructs a fresh OpChainSpec (including deserializing the embedded genesis JSON) on every call. The old implementation returned Arc::clone() of LazyLock statics, which was essentially free after first access.

This is called from CLI parsing so it's unlikely to be a hot path, but if anything else calls parse_chain repeatedly (e.g., in tests or validation loops), the repeated deserialization is wasteful. Consider caching the result per chain, or documenting that callers should cache the Arc themselves.

github-actions · 2026-03-23T02:07:29Z

crates/alloy/chains/src/config.rs


    bootnodes: &[],

+    prune_delete_limit: 0,


Behavioral change for dev chain base fee params. The deleted dev.rs used BaseFeeParamsKind::Constant(BaseFeeParams::ethereum()) which is { denominator: 8, elasticity: 2 }. The new From<&BaseChainConfig> impl derives params from these config values, producing BaseFeeParamsKind::Variable([(London, {50, 6}), (Canyon, {250, 6})]) because eip1559_denominator (50) != eip1559_denominator_canyon (250).

This changes the dev chain from Ethereum-style constant base fee params to OP-style variable params. If this is intentional (aligning devnet with production behavior), it's fine — but if the old Ethereum-style params were needed for dev/test compatibility, this is a regression.

This is a correct change

Add base-alloy-chains to udeps ignore list (used in doc-test but undetectable by cargo-udeps). Improve genesis deserialization panic message to include chain_id.

Consolidate chain configuration so adding a new chain only requires touching config.rs. Add cli_names field to BaseChainConfig, derive by_name/by_chain_id lookups from all(), simplify parse_chain(), and remove the 4 LazyLock statics from chainspec.

Add base-alloy-chains to udeps ignore list (used in doc-test but undetectable by cargo-udeps). Improve genesis deserialization panic message to include chain_id.

github-actions · 2026-03-23T05:09:23Z

crates/alloy/chains/src/config.rs


+/// All supported chain names for the CLI.
+pub const SUPPORTED_CHAINS: &[&str] =
+    &["base", "base_sepolia", "base-sepolia", "base-devnet-0-sepolia-dev-0", "dev"];


SUPPORTED_CHAINS duplicates data already embedded in each config's cli_names. The test at the bottom catches drift, but this const could be derived at compile-time or as a function to eliminate the maintenance hazard entirely:

pub fn supported_chains() -> impl Iterator<Item = &'static str> { BaseChainConfig::all().into_iter().flat_map(|cfg| cfg.cli_names.iter().copied()) }

This would also remove the need for the sync test. If the const form is needed for CLI help strings (e.g., clap's possible_values), consider generating it from cli_names via a const fn or LazyLock instead of manually maintaining the list.

github-actions · 2026-03-23T05:15:46Z

Review Summary

This PR consolidates four per-chain LazyLock<Arc<OpChainSpec>> statics (BASE_MAINNET, BASE_SEPOLIA, BASE_DEVNET_0_SEPOLIA_DEV_0, OP_DEV) and their corresponding hardfork statics into a single From<&BaseChainConfig> for OpChainSpec impl, deriving chain specs on demand from BaseChainConfig data. It also moves SUPPORTED_CHAINS into base-alloy-chains and adds cli_names/prune_delete_limit fields to BaseChainConfig.

What looks good

Base fee params are equivalent. Verified that the derived BaseFeeParams from config values match what the old named constructors (BaseFeeParams::optimism(), optimism_canyon(), base_sepolia(), base_sepolia_canyon()) produced. The fork-id and genesis hash tests confirm no regressions for mainnet and sepolia.
Genesis header handling is correct. The genesis_l2_hash.is_zero() check correctly differentiates dev (slow-sealed) from production (pre-known hash) genesis headers.
Test migration is mechanical and correct. All test changes are straightforward replacements of static refs with freshly constructed specs.
Hardfork consolidation is clean. Only BASE_MAINNET_HARDFORKS is retained as a Lazy static (used in the OpChainSpec::from(Genesis) fallback path); the rest are properly derived via BaseChainUpgrades::new(BaseUpgrade::forks_for(cfg)).to_chain_hardforks().

Findings (inline comments posted)

All findings were posted as inline comments on this and prior review passes:

Dev chain base fee behavioral change — The old OP_DEV used BaseFeeParams::ethereum() ({8, 2}), the new path produces OP-style params ({50, 6} / {250, 6}). Should be confirmed intentional.
parse_chain no longer caches — Each call now deserializes genesis JSON. Not a regression for CLI use, but worth noting if called in loops.
SUPPORTED_CHAINS duplication — The const duplicates data from cli_names; a derived function would be more robust (though the sync test mitigates drift).
Panic in From impl — unwrap_or_else with chain ID in the message was adopted (good).
by_name linear scan — Fine at 4 configs, but exact-match / case-sensitivity is undocumented.

No blocking issues found. The main item requiring explicit confirmation is the dev chain base fee parameter change (#1 above).

refcell · 2026-03-23T12:33:12Z

crates/alloy/chains/src/config.rs

+/// All supported chain names for the CLI.
+pub const SUPPORTED_CHAINS: &[&str] =
+    &["base", "base_sepolia", "base-sepolia", "base-devnet-0-sepolia-dev-0", "dev"];


I think these could be placed on BaseChainConfig i.e. BaseChainConfig::ALL

refcell · 2026-03-23T12:35:25Z

crates/execution/chainspec/src/spec.rs

+                hardforks,
+                base_fee_params,
+                prune_delete_limit: cfg.prune_delete_limit,
+                ..Default::default()


Are we sure about this ..Default::default()? I think we ought to list everything out here so it's very explicit about what's made default.

crates/execution/chainspec/src/lib.rs

refcell · 2026-03-23T12:39:02Z

crates/execution/chainspec/src/lib.rs

-
 mod spec;
-pub use spec::{OpChainSpec, OpGenesisInfo, SUPPORTED_CHAINS};
+pub use spec::{OpChainSpec, OpGenesisInfo};


Would be nice to rename this to BaseChainSpec and BaseGenesisInfo while we're in here. If there are too many call sites, fine to leave it for later

refcell · 2026-03-23T12:39:23Z

crates/execution/hardforks/src/lib.rs

+// Re-export base-alloy-upgrades types.
+pub use base_alloy_chains::BaseChainUpgrades;
+pub use chain::{BASE_MAINNET_HARDFORKS, BaseChainUpgradesExt};


rm re-exports

crates/execution/node/src/node.rs

refcell

Couple comments

danyalprout changed the title ~~Danyal/dedup configs 4~~ chore(el): derive reth chainspecs from base-alloy-chains vs. duplicating configs Mar 23, 2026

danyalprout marked this pull request as ready for review March 23, 2026 02:04

github-actions bot reviewed Mar 23, 2026

View reviewed changes

crates/execution/chainspec/src/spec.rs Outdated Show resolved Hide resolved

github-actions bot reviewed Mar 23, 2026

View reviewed changes

danyalprout added a commit that referenced this pull request Mar 23, 2026

fix: address PR #1592 CI and review feedback

3f12f21

Add base-alloy-chains to udeps ignore list (used in doc-test but undetectable by cargo-udeps). Improve genesis deserialization panic message to include chain_id.

danyalprout requested a review from refcell March 23, 2026 03:47

danyalprout added 4 commits March 22, 2026 23:57

more cleanup

8dd4dca

fix: address PR #1592 CI and review feedback

e33edb3

Add base-alloy-chains to udeps ignore list (used in doc-test but undetectable by cargo-udeps). Improve genesis deserialization panic message to include chain_id.

fix format

990da51

danyalprout force-pushed the danyal/dedup-configs-4 branch from fece2c1 to 354facd Compare March 23, 2026 05:07

fix: resolve rebase conflicts and fix imports

57e8e61

github-actions bot reviewed Mar 23, 2026

View reviewed changes

danyalprout force-pushed the danyal/dedup-configs-4 branch from 354facd to 57e8e61 Compare March 23, 2026 05:09

refcell reviewed Mar 23, 2026

View reviewed changes

crates/execution/chainspec/src/lib.rs Show resolved Hide resolved

refcell reviewed Mar 23, 2026

View reviewed changes

crates/execution/node/src/node.rs Show resolved Hide resolved

refcell requested changes Mar 23, 2026

View reviewed changes

github-actions bot added the M-prevent-stale Meta: Not Stale label Mar 24, 2026

Conversation

danyalprout commented Mar 23, 2026

Description

Uh oh!

cb-heimdall commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Heimdall Review Status

Uh oh!

vercel bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

danyalprout Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 23, 2026

Review Summary

What looks good

Findings (inline comments posted)

Uh oh!

refcell Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

refcell Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

refcell Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

refcell Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

refcell left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cb-heimdall commented Mar 23, 2026 •

edited

Loading

vercel bot commented Mar 23, 2026 •

edited

Loading