-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Move OptimismHardfork
to new crate reth_optimism_forks
#10963
Conversation
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.
that makes sense, but I wonder if this breaks --dev mode for op
reth/crates/chainspec/src/spec.rs
Lines 108 to 115 in 0746479
pub static DEV: Lazy<Arc<ChainSpec>> = Lazy::new(|| { | |
ChainSpec { | |
chain: Chain::dev(), | |
genesis: serde_json::from_str(include_str!("../res/genesis/dev.json")) | |
.expect("Can't deserialize Dev testnet genesis json"), | |
genesis_hash: once_cell_set(DEV_GENESIS_HASH), | |
paris_block_and_final_difficulty: Some((0, U256::from(0))), | |
hardforks: DEV_HARDFORKS.clone(), |
let me try :) |
3de9db1
to
c818da1
Compare
ff6f9dc
to
c00d88b
Compare
c00d88b
to
a9b2e02
Compare
re-request review @mattsse |
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.
cool, incremental progress.
one q re CI change, otherwise lgtm
.github/workflows/unit.yml
Outdated
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.
why do we need to change this?
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.
because of the parse_dev
test
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.
could we remove that, not worth changing the ci for and adding a single expectation in ci
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.
then I have to remove the whole test
Closes #10518
reth-optimism-forks
reth_ethereum_forks::OptimismHardfork
andreth_ethereum_forks::OptimismHardforks
toreth-optimism-forks