feat: use OpChainSpec in OptimismNode and its components
#11304
Conversation
klkvr
force-pushed
the
klkvr/optimism-chain-spec
branch
from
68e19a6
to
48a4702
Compare
| @@ -50,14 +50,14 @@ pub struct EnvironmentArgs<C: ChainSpecParser> { | |||
| pub db: DatabaseArgs, | |||
| } | |||
|
|
|||
| impl<C: ChainSpecParser<ChainSpec = ChainSpec>> EnvironmentArgs<C> { | |||
| impl<C: ChainSpecParser<ChainSpec: EthChainSpec + EthereumHardforks>> EnvironmentArgs<C> { | |||
There was a problem hiding this comment.
we're using this combo a few times,
what we could do is introduce a helper trait that combines both, but perhaps not useful. maybe we introduce EthereumChainSpec and rename EthChainSpec
| fn deposit_contract(&self) -> Option<&DepositContract> { | ||
| self.inner.deposit_contract() | ||
| } |
There was a problem hiding this comment.
followup here would be to check if we actually need this in the trait because this is just an ethereum setting.
it might be useful to split EthChainSpec into two traits
There was a problem hiding this comment.
this is only used for pruning configuration
reth/crates/node/core/src/args/pruning.rs
Lines 106 to 111 in 48a4702
I think we can either move prune_config fn to chainspec trait or introduce trait(s) which would be able to produce a concrete *Config type based on concrete cli *Args type and a chainspec
we also have similar pattern in NetworkArgs
reth/crates/node/core/src/args/network.rs
Line 199 in 48a4702
There was a problem hiding this comment.
I see, I guess this doesn't matter really if this is part of the trait since option
Closes #11238
Closes #10909
Closes #10770
This PR relaxes chainspec bounds on CLI, changes
OpChainSpecParserAT toOpChainSpecand makes optimism-specific CLI types to useOpChainSpec