feat(l1): add amsterdam fork to chain config #5715
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Amsterdam fork to the chain configuration, which is required for implementing engine_newPayloadV5. The changes follow the existing pattern established for BPO5 and other recent forks.
Key Changes:
- Added
amsterdamfield toBlobScheduleandamsterdam_timetoChainConfigstructs - Added
Amsterdamvariant to theForkenum with value 25 - Implemented
is_amsterdam_activatedhelper method for activation checks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn is_amsterdam_activated(&self, block_timestamp: u64) -> bool { | ||
| self.amsterdam_time | ||
| .is_some_and(|time| time <= block_timestamp) | ||
| } |
There was a problem hiding this comment.
The is_amsterdam_activated method is placed before is_bpo1_activated, breaking the chronological ordering convention used for other fork activation methods. The methods should follow the chronological fork order (oldest to newest) for consistency. Consider moving this method to after is_bpo5_activated to maintain the established pattern: Cancun → Shanghai → Prague → Osaka → BPO1-5 → Amsterdam.
There was a problem hiding this comment.
The existing code already had them out of order by placing BPO5 right above Osaka. The correct fix is reordering the BPOs, not placing Amsterdam after BPO5.
|
We would have to update revm to v30.0.0 when Amsterdam support was added. |
crates/networking/rpc/rpc.rs
Outdated
| "bpo5Time": null, | ||
| "amsterdamTime": null, |
There was a problem hiding this comment.
The indentation is off by one level
| pub fn is_amsterdam_activated(&self, block_timestamp: u64) -> bool { | ||
| self.amsterdam_time | ||
| .is_some_and(|time| time <= block_timestamp) | ||
| } |
There was a problem hiding this comment.
The existing code already had them out of order by placing BPO5 right above Osaka. The correct fix is reordering the BPOs, not placing Amsterdam after BPO5.
|
@iovoid fixed the order and indentaion. |
2 participants
Motivation
Adds Amsterdam fork which is required for implementing
engine_newPayloadV5and such.Description
BlobScheduleandChainConfigfor Amsterdam fork details.NOTE: We would have to update revm to v30.0.0 when Amsterdam support was added.