feat: update the exposed hardfork API to match Hardhat's definition - #1645
feat: update the exposed hardfork API to match Hardhat's definition#1645anaPerezGhiglia wants to merge 11 commits into
Conversation
🦋 Changeset detectedLatest commit: da7670b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1645 +/- ##
==========================================
+ Coverage 79.97% 80.05% +0.08%
==========================================
Files 462 462
Lines 80509 80500 -9
Branches 80509 80500 -9
==========================================
+ Hits 64384 64445 +61
+ Misses 13896 13841 -55
+ Partials 2229 2214 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR aligns EDR’s exposed L1 and OP hardfork APIs with Hardhat-compatible names and enums.
Changes:
- Renames
SpecIdtoL1Hardforkand removes obsolete forks/constants. - Switches hardfork strings to camelCase.
- Updates N-API conversions, tests, and pre-hardfork validation.
Blocking findings remain:
- Critical (1 vote): Checked-in Hardhat 2/3 patches still use outdated hardfork conversion layers and
SpecId. - Critical (1 vote): Removing legacy exports breaks the patched Hardhat 2 converter.
- Critical (2 votes): The OP chain config generator still capitalizes names before parsing, causing valid activations to be rejected.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crates/eips/1559/src/lib.rs |
Updates hardfork test inputs. |
crates/edr_op/src/hardfork.rs |
Applies camelCase OP hardfork names. |
crates/edr_napi/test/provider.ts |
Migrates provider tests to L1Hardfork. |
crates/edr_napi/test/logs.ts |
Updates log provider configuration. |
crates/edr_napi/test/hardforks.ts |
Tests updated hardfork names and enums. |
crates/edr_napi/test/gasReport.ts |
Updates gas report configuration. |
crates/edr_napi/src/chains/op.rs |
Updates OP N-API conversions. |
crates/edr_napi/src/chains/l1.rs |
Adds the new L1 N-API enum and conversions. |
crates/edr_napi/index.js |
Updates generated JavaScript exports. |
crates/edr_napi/index.d.ts |
Updates TypeScript declarations. |
crates/edr_chain_l1/src/hardfork.rs |
Removes pre-Byzantium forks and renames strings. |
crates/edr_chain_l1/src/chains.rs |
Removes unsupported chain activations. |
crates/chain/config/src/lib.rs |
Adds oldest-hardfork lookup. |
crates/blockchain/fork/src/lib.rs |
Validates forks against supported history. |
.changeset/wise-falcons-relate.md |
Documents string API changes. |
.changeset/mighty-poems-tickle.md |
Documents enum and fork behavior changes. |
Suppressed comments (1)
crates/edr_napi/src/chains/l1.rs:103
- The public type is now an L1 hardfork enum, but the doc comment carried over from
SpecIdstill calls it an “Ethereum spec” identifier. This wording is also propagated toindex.d.ts, so consumers see an inaccurate description of the new API; update it to say “Identifier for an Ethereum L1 hardfork.”
// N-API projection of [`edr_chain_l1::Hardfork`], which only exists to
// generate the TS enum; string conversions delegate to the domain type.
#[napi]
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum L1Hardfork {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
patches/hardhat@2.28.4.patch:985
- The reverse conversion has the same Hardhat 2 compatibility problem:
l1HardforkToString(L1Hardfork.Merge)producesmerge, but callers ofedrSpecIdToEthereumHardforkreceive aHardforkNameand Hardhat 2 expectsmergeForkBlockfor that member. Returning the EDR string directly violates the declared return contract for Merge; map this fork back to Hardhat's value or align the Hardhat enum first.
+ // EDR's hardfork names match Hardhat's, so no conversion is needed.
+ return l1HardforkToString(specId) as HardforkName;
| -exports.httpHeadersToEdr = exports.edrTracingMessageToMinimalMessage = exports.edrTracingMessageResultToMinimalEVMResult = exports.edrTracingStepToMinimalInterpreterStep = exports.edrRpcDebugTraceToHardhat = exports.ethereumjsMempoolOrderToEdrMineOrdering = exports.ethereumjsIntervalMiningConfigToEdr = exports.edrSpecIdToEthereumHardfork = exports.ethereumsjsHardforkToEdrSpecId = void 0; | ||
| -const edr_1 = require("@nomicfoundation/edr"); | ||
| +exports.httpHeadersToEdr = exports.edrTracingMessageToMinimalMessage = exports.edrTracingMessageResultToMinimalEVMResult = exports.edrTracingStepToMinimalInterpreterStep = exports.ethereumjsMempoolOrderToEdrMineOrdering = exports.ethereumjsIntervalMiningConfigToEdr = exports.edrSpecIdToEthereumHardfork = exports.ethereumsjsHardforkToEdrSpecId = void 0; |
There was a problem hiding this comment.
This is not part of this PR changes
aa0c689 to
b624ed7
Compare
10a7cd3 to
15fcecc
Compare
|
Draft HH PR associated to these changes: NomicFoundation/hardhat#8574 |
15fcecc to
24f5b67
Compare
Wodann
left a comment
There was a problem hiding this comment.
Overall this looks good. However, I do have some suggestions w.r.t the wording of the changesets that I think should be applied to match our changelog, before merging.
Feel free to apply them locally and merge with re-approval; or ask me to have a quick look if you deem it necessary.
| /// The fork block predates the chain's oldest supported hardfork | ||
| #[error( | ||
| "Cannot fork {chain_name} from block {fork_block_number}. The hardfork must be at least Spurious Dragon, but {hardfork:?} was detected." | ||
| "Cannot fork {chain_name} from block {fork_block_number}. The block precedes {oldest_hardfork:?}, which is the chain's oldest supported hardfork." |
There was a problem hiding this comment.
Now that we support Display for HardforkT, can this use the Display instead of the Debug trait implementation?
| "Cannot fork {chain_name} from block {fork_block_number}. The block precedes {oldest_hardfork:?}, which is the chain's oldest supported hardfork." | |
| "Cannot fork {chain_name} from block {fork_block_number}. The block precedes {oldest_hardfork}, which is the chain's oldest supported hardfork." |
There was a problem hiding this comment.
Not so easily. To use Display here we'd have to add a Display bound for HardforkT generic type of this enum, and since this error nests into ProviderError, the bound ripples through every generic consumer that renders provider errors.
Containing that ripple means adding Display to the ProtocolHardfork umbrella trait itself, which I think goes against the style guide from #1618: error-formatting bounds belong at usage sites.
So I'd keep Debug formatting here, unless we want to deliberately declare Display part of ProtocolHardfork's contract.
…om L1Hardfork The mainnet activation schedule, the NAPI `SpecId` enum and its exported name constants follow. Discriminants of the remaining variants are unchanged, so `Byzantium` stays 6. Forking previously rejected pre-Spurious-Dragon blocks by inspecting the resolved hardfork. With those activations gone, `hardfork_at_block` returns `None` below block 4,370,000, which the old `if let Some` skipped silently. Resolving a hardfork is now mandatory for any chain that has an activation schedule, and failing to do so is `UnsupportedForkBlock`, which names the oldest hardfork the chain supports via the new `HardforkActivations::oldest_hardfork`.
Co-authored-by: Wodann <Wodann@users.noreply.github.com>
07a5b1e to
ef78b21
Compare
Closes #1634.
Now that the hardfork type is owned by EDR (#1601) instead of mirroring revm's
SpecId, we can shape the exposed API freely: this PR aligns it with Hardhat's own hardfork definitions, which will enable removing the conversion layers in Hardhat that translate between Hardhat and EDR names on every boundary crossing.Breaking changes (TS API)
SpecIdenum toL1Hardfork, mirroringOpHardfork. Discriminants are unchanged.Frontier,FrontierThawing,Homestead,DaoFork,Tangerine,SpuriousDragon). Forking a chain from a block that precedes its oldest supported hardfork now fails with an error naming that hardfork, instead of silently skipping hardfork validation."Byzantium"→"byzantium","MuirGlacier"→"muirGlacier","Bedrock"→"bedrock").l1HardforkToString/opHardforkToStringreturn the new names;l1HardforkFromString/opHardforkFromStringand provider configs accept only them.BYZANTIUM, …,AMSTERDAM,BEDROCK, …,ISTHMUS). Hardhat 3 imports them only in two test files; Hardhat 2 uses them inconvertToEdr.ts, which already requires a rewrite for the changes above. Suggested instead to replace then with the corresponding enumToString function. E.g.: replaceOSAKAwithl1HardforkToString(L1Hardfork.Osaka)Internal changes
#[strum(serialize_all = "camelCase")]directive, replacing the per-variantserializeoverrides. The expected strings are pinned as literals in each crate'sname_round_triptest, which is the guard against unintended renames.hardfork::namestring-constant modules fromedr_chain_l1andedr_op.L1Hardfork,OpHardfork) delegate parsing and string conversion to the domain types. The napiOpHardforkexcludesJovian/Interop, which are not exposed over N-API yet; its parse rejects them explicitly. Unit tests pin that the hand-written conversion tables between the napi and domain enums are inverses.Notes for reviewers
L1Hardfork, colliding grep-wise withedr_chain_l1::L1Hardfork(the OP side already had this collision). An alternative is a role-based Rust name (e.g.ExposedL1Hardfork) with#[napi(js_name = "L1Hardfork")], keeping the TS API unchanged - open for discussion.Hardhat migration
Hardhat's
L1HardforkName/OpHardforkNamevalues are now exactly EDR's hardfork strings, so the string-conversion mappings in HH3'sconvert-to-edr.tsand HH2'sconvertToEdr.tscan collapse to pass-throughs for supported forks.