-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add interop hardfork in all relevant places. #14582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add interop hardfork in all relevant places. #14582
Conversation
2 places commented out, because need changes in alloys and revm
mattsse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm,
the comments and todo! are helpful
@klkvr for vis
| (OpHardfork::Granite.boxed(), genesis_info.granite_time), | ||
| (OpHardfork::Holocene.boxed(), genesis_info.holocene_time), | ||
| (OpHardfork::Isthmus.boxed(), genesis_info.isthmus_time), | ||
| // (OpHardfork::Interop.boxed(), genesis_info.interop_time), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this we need to add to the genesis info type
| // if chain_spec.is_interop_active_at_timestamp(timestamp) { | ||
| // OpSpecId::INTEROP | ||
| // } else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik there are no evm changes for interop?
possible that we don't need to roll a new spec but probably should anyway for consistency
FYI @klkvr
|
@mattsse thanks for the lightning speed! |
<!-- Thank you for your Pull Request. Please provide a description above and review the requirements below. Bug fixes and new features should include tests. Contributors guide: https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md The contributors guide includes instructions for running rustfmt and building the documentation. --> <!-- ** Please select "Allow edits from maintainers" in the PR Options ** --> ## Motivation Closes #446 For paradigmxyz/reth#14582 ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
emhane
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! next we need to open an issue in https://github.com/bluealloy/revm to add the OpSpecId::Interop as well
| Holocene, | ||
| /// Isthmus: <https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/isthmus/overview.md> | ||
| Isthmus, | ||
| /// Interop: <https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/interop/overview.md> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
link is 404. better add a code comment // todo: add link to specs when available
|
@emhane Thanks! |
<!-- Thank you for your Pull Request. Please provide a description above and review the requirements below. Bug fixes and new features should include tests. Contributors guide: https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md The contributors guide includes instructions for running rustfmt and building the documentation. --> <!-- ** Please select "Allow edits from maintainers" in the PR Options ** --> ## Motivation Closes alloy-rs/op-alloy#446 For paradigmxyz/reth#14582 ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
Closes #14553
Adds interop hardfork to all relevant places.
2 places commented out, because it needs changes in alloys and revm