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

Possible redundant reset HandlerCfg #43

Open
0xurb opened this issue Oct 15, 2024 · 4 comments
Open

Possible redundant reset HandlerCfg #43

0xurb opened this issue Oct 15, 2024 · 4 comments

Comments

@0xurb
Copy link
Contributor

0xurb commented Oct 15, 2024

Not sure also about that

cfg_env.handler_cfg.spec_id = spec_id;
cfg_env.handler_cfg.is_optimism = true;

Because, Odyssey's evm used reth's trait ConfigureEvm with .optimism(). So, we are set spec id here and is_optimism = true on ConfigureEvm::evm (via optimism register handler) than do it again via ConfigureEvmEnv::fill_cfg_and_block_env?

@onbjerg
Copy link
Member

onbjerg commented Oct 15, 2024

I don't follow, can you rephrase?

@0xurb
Copy link
Contributor Author

0xurb commented Oct 15, 2024

For sure. On Evm configuration we are setup evm builder with .optimism() here:

impl ConfigureEvm for OdysseyEvmConfig {
type DefaultExternalContext<'a> = ();
fn evm<DB: Database>(&self, db: DB) -> Evm<'_, Self::DefaultExternalContext<'_>, DB> {
EvmBuilder::default()
.with_db(db)
.optimism()
// add additional precompiles
.append_handler_register(Self::set_precompiles)
.build()
}

That means we are modifying handler config with:

    /// Handler for optimism
    #[cfg(feature = "optimism")]
    pub fn optimism<SPEC: Spec>() -> Self {
        let mut handler = Self::mainnet::<SPEC>();
        handler.cfg.is_optimism = true;
        handler.append_handler_register(HandleRegisters::Plain(
            crate::optimism::optimism_handle_register::<DB, EXT>,
        ));
        handler
    }

Usage in reth for example here
https://github.com/paradigmxyz/reth/blob/c4d7b591834055bdf3afed96b696e0a2cef0f383/crates/optimism/evm/src/execute.rs#L276-L288

Question: do we need to do below, as we are already set that by calling .optimism():

cfg_env.handler_cfg.spec_id = spec_id;
cfg_env.handler_cfg.is_optimism = true;

@onbjerg
Copy link
Member

onbjerg commented Oct 17, 2024

got it, you're correct

@0xurb
Copy link
Contributor Author

0xurb commented Oct 17, 2024

Then same on reth revm optimism

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants