-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(anvil): add support for injecting precompiles #7589
feat(anvil): add support for injecting precompiles #7589
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.
this is great!
Before making any changes to the evm crate, I think we can make this work for anvil but injecting precompiles to the evm directly which does not require changes in the utils crate
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.
this is great, this should work.
for some reason fmt keeps appending ;
on some machines...
I'd appreciate if you could remove those from unchanged files so this doesn't cause conflicts with other prs that perhaps touch that code.
lgtm otherwise
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.
okay, I realized this needs a few more changes,
when we create the backend, we need to clone the precompilesfactory and keep this separately, so it's no longer behind the lock.
we also need to pass this to the executor, because currently the executor doesn't inject the precompiles, so the executor needs a Arc<dyn>
clone as well
let mut evm = new_evm_with_inspector_ref(&*db, env, &mut inspector); | ||
|
||
let ResultAndState { result, state } = | ||
new_evm_with_inspector_ref(&*db, env, &mut inspector).transact()?; | ||
let cfg = self.node_config.read().await; | ||
if let Some(ref factory) = cfg.precompile_factory { | ||
inject_precompiles(&mut evm, factory.precompiles()); | ||
} |
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.
we can put this in a function of Backend::new_evm_with_inspector_ref
I see, that makes sense 👍 |
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.
lgtm,
we have some open prs that likely introduce conflicts, will resolve them and get this merged asap
Lovely! Thank you for the patience and the pointers. |
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.
this is great,
once you have arb precompiles lmk, we'd love to inject them by default if the forked chain is arb for example
Motivation
Resolves #7498
Solution
We add a new object-safe trait to
anvil
's public API called:PrecompileFactory
.Alongside this trait we add a new
precompile_factory
field toNodeConfig
which we populate in thewith_precompile_factory
builder function:This is used in the newly added
inject_precompiles()
:Usage
From a consumer's standpoint, this looks like this (simplified for showcasing):
This looks quite clean IMO. This should mark the first step in adding support for Arbitrum precompiles and being able to decouple Optimism from the codebase.
Notes
Evm::builder
because of this comment.