feat: Support unnamed OP chains #8488
Conversation
BrianBland
changed the title
crates/primitives/src/chain/spec.rs
Outdated
| #[cfg(feature = "optimism")] | ||
| if self.hardforks.contains_key(&Hardfork::Bedrock) { | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
this seems reasonable, doesn't impact regular eth nodes
There was a problem hiding this comment.
I considered simply returning false if the optimism feature is disabled. Is it worth also making this change or simply keeping the previous name check?
There was a problem hiding this comment.
we are trying to get rid of the feature all together, so this is good the way it is #7649
crates/primitives/src/chain/spec.rs
Outdated
| let eip1559_elasticity = genesis.config.extra_fields.get("optimism").and_then(|value| { | ||
| value | ||
| .as_object() | ||
| .and_then(|obj| obj.get("eip1559Elasticity").and_then(|value| value.as_u64())) | ||
| }); | ||
| let eip1559_denominator = genesis.config.extra_fields.get("optimism").and_then(|value| { | ||
| value | ||
| .as_object() | ||
| .and_then(|obj| obj.get("eip1559Denominator").and_then(|value| value.as_u64())) | ||
| }); | ||
| let base_fee_params = if let (Some(elasticity), Some(denominator)) = | ||
| (eip1559_elasticity, eip1559_denominator) | ||
| { | ||
| BaseFeeParams::new(denominator as u128, elasticity as u128) | ||
| } else { | ||
| BaseFeeParams::ethereum() | ||
| }; |
There was a problem hiding this comment.
this does the "optimism" lookup twice, I think we can nest this with an if let Some
BrianBland
force-pushed
the
support-unnamed-op-chains
branch
from
cb634ca
to
ff53ce3
Compare
| @@ -573,6 +573,14 @@ impl ChainSpec { | |||
|
|
|||
| /// Returns `true` if this chain contains Optimism configuration. | |||
| #[inline] | |||
| #[cfg(feature = "optimism")] | |||
There was a problem hiding this comment.
Note: This change attempts to preserve the const behavior when possible, and also prioritizes the const self.chain.is_optimism() case over the slower runtime-evaluated hardfork check. The performance penalty should only impact unnamed chains or non-OP chains when the optimism feature is enabled.
| @@ -573,6 +573,14 @@ impl ChainSpec { | |||
|
|
|||
| /// Returns `true` if this chain contains Optimism configuration. | |||
| #[inline] | |||
| #[cfg(feature = "optimism")] | |||
BrianBland
force-pushed
the
support-unnamed-op-chains
branch
from
eb76e88
to
ada6f4a
Compare
BrianBland
force-pushed
the
support-unnamed-op-chains
branch
from
ada6f4a
to
8f0ee33
Compare
ChainSpec.is_optimismto return true if the chain has the Bedrock hardfork configured (past or present), which allows arbitrary genesis files to be used for unnamed OP chains.