Skip to content
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

Parse Optimism hardforks from Genesis in op-reth --chain genesis.json #7043

Closed
hai-rise opened this issue Mar 8, 2024 · 2 comments · Fixed by #7935
Closed

Parse Optimism hardforks from Genesis in op-reth --chain genesis.json #7043

hai-rise opened this issue Mar 8, 2024 · 2 comments · Fixed by #7935
Labels
A-op-reth Related to Optimism and op-reth C-enhancement New feature or request M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity

Comments

@hai-rise
Copy link
Contributor

hai-rise commented Mar 8, 2024

Describe the feature

Currently, op-reth doesn't parse Optimism hardforks from Genesis files, with common chains like Base's being hardcoded.
This leads to several execution errors for custom OP chains (like executing a Canyon tx with a Shanghai executor).
Adding this feature will make it easier to run op-reth for most OP chains, including devnets!

Additional context

The relevant code is here:

pub fn genesis_value_parser(s: &str) -> eyre::Result<Arc<ChainSpec>, eyre::Error> {
Ok(match s {
#[cfg(not(feature = "optimism"))]
"mainnet" => MAINNET.clone(),
#[cfg(not(feature = "optimism"))]
"goerli" => GOERLI.clone(),
#[cfg(not(feature = "optimism"))]
"sepolia" => SEPOLIA.clone(),
#[cfg(not(feature = "optimism"))]
"holesky" => HOLESKY.clone(),
#[cfg(not(feature = "optimism"))]
"dev" => DEV.clone(),
#[cfg(feature = "optimism")]
"base_sepolia" | "base-sepolia" => BASE_SEPOLIA.clone(),
#[cfg(feature = "optimism")]
"base" => BASE_MAINNET.clone(),
_ => {
// try to read json from path first
let raw = match fs::read_to_string(PathBuf::from(shellexpand::full(s)?.into_owned())) {
Ok(raw) => raw,
Err(io_err) => {
// valid json may start with "\n", but must contain "{"
if s.contains('{') {
s.to_string()
} else {
return Err(io_err.into()) // assume invalid path
}
}
};
// both serialized Genesis and ChainSpec structs supported
let genesis: AllGenesisFormats = serde_json::from_str(&raw)?;
Arc::new(genesis.into())
}
})
}

Ideally, alloy's Genesis would parse bedrockBlock, regolithTime, etc., for us to update impl From<Genesis> for ChainSpec accordingly.
Nevertheless, they have strong reasons not to: alloy-rs/alloy#243.
So, we had to use this current patch to support our custom OP devnet:

--- a/crates/node-core/src/args/utils.rs
+++ b/crates/node-core/src/args/utils.rs
@@ -1,6 +1,6 @@
 //! Clap parser utilities
 
-use reth_primitives::{fs, AllGenesisFormats, BlockHashOrNumber, ChainSpec, B256};
+use reth_primitives::{fs, AllGenesisFormats, BlockHashOrNumber, ChainSpec, Hardfork, ForkCondition, B256};
 use std::{
     net::{IpAddr, Ipv4Addr, SocketAddr, ToSocketAddrs},
     path::PathBuf,
@@ -95,8 +95,47 @@ pub fn genesis_value_parser(s: &str) -> eyre::Result<Arc<ChainSpec>, eyre::Error
 
             // both serialized Genesis and ChainSpec structs supported
             let genesis: AllGenesisFormats = serde_json::from_str(&raw)?;
+            let mut chain_spec: ChainSpec = genesis.into();
 
-            Arc::new(genesis.into())
+            // RISE TODO: Ideally, alloy's Genesis would parse `bedrockBlock`, `regolithTime`, and `canyonTime`.
+            // They have good reasons not to: https://github.com/alloy-rs/alloy/issues/243.
+            // This hack is therefore needed until we have better OP types in reth, and
+            // `genesis.into()` would set up the custom OP Stack chain's hardforks correctly.
+            #[cfg(feature = "optimism")]
+            {
+                let raw_genesis: serde_json::Value = serde_json::from_str(&raw)?;
+                match raw_genesis["config"]["bedrockBlock"].as_u64() {
+                    Some(block) => {
+                        chain_spec.hardforks.insert(
+                            Hardfork::Bedrock,
+                            ForkCondition::Block(block),
+                        );
+                    }
+                    None => ()
+                }
+                match raw_genesis["config"]["regolithTime"].as_u64() {
+                    Some(time) => {
+                        chain_spec.hardforks.insert(
+                            Hardfork::Regolith,
+                            ForkCondition::Timestamp(time),
+                        );
+                        chain_spec.fork_timestamps = chain_spec.fork_timestamps.regolith(time);
+                    }
+                    None => ()
+                }
+                match raw_genesis["config"]["canyonTime"].as_u64() {
+                    Some(time) => {
+                        chain_spec.hardforks.insert(
+                            Hardfork::Canyon,
+                            ForkCondition::Timestamp(time),
+                        );
+                        chain_spec.fork_timestamps = chain_spec.fork_timestamps.canyon(time);
+                    }
+                    None => ()
+                }
+            }
+
+            Arc::new(chain_spec)
         }
     })
 }

Perhaps the actual solution is to build alloy-op or extend the Genesis type here in reth?
We'd love to contribute PRs once a firm direction is decided :).

@hai-rise hai-rise added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Mar 8, 2024
@gakonst
Copy link
Member

gakonst commented Mar 10, 2024

Good flag. @mattsse how should we approach?

@emhane emhane added A-op-reth Related to Optimism and op-reth and removed S-needs-triage This issue needs to be labelled labels Mar 12, 2024
Copy link
Contributor

github-actions bot commented Apr 3, 2024

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Apr 3, 2024
@onbjerg onbjerg added M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity and removed S-stale This issue/PR is stale and will close with no further activity labels Apr 3, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth C-enhancement New feature or request M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants