-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: mev_simBundle #12218
feat: mev_simBundle #12218
Conversation
# Conflicts: # Cargo.lock # Cargo.toml
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.
some style nits first,
please also take a look @klkvr
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.
looking good, left some suggestions for simplifications and a question regarding block env
crates/rpc/rpc/src/eth/sim_bundle.rs
Outdated
if let Some(base_fee) = base_fee { | ||
block_env.basefee = U256::from(base_fee); | ||
} else if cfg.handler_cfg.spec_id.is_enabled_in(SpecId::LONDON) { | ||
if let Some(base_fee) = parent_header.next_block_base_fee( | ||
RpcNodeCore::provider(&self.inner.eth_api) | ||
.chain_spec() | ||
.base_fee_params_at_block(block_env.number.saturating_to::<u64>()), | ||
) { | ||
block_env.basefee = U256::from(base_fee); | ||
} | ||
} | ||
|
||
if cfg.handler_cfg.spec_id.is_enabled_in(SpecId::CANCUN) { | ||
let excess_blob_gas = calc_excess_blob_gas( | ||
parent_header.excess_blob_gas.unwrap_or_default(), | ||
parent_header.blob_gas_used.unwrap_or_default(), | ||
); | ||
|
||
block_env.set_blob_excess_gas_and_price(excess_blob_gas); | ||
} |
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.
I think those should already be set when evm_env_at
is called
also wondering if we should fill env with pending block vs latest?
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.
you're right, excess_blob_gas
doesn't need to be set here, but base_fee
is an override so we should keep it
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.
golang impl uses latest not pending, but I dont see a reason why not use pending to simulate bundles: https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/internal/ethapi/sbundle_api.go#L227
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.
basically we need to ensure that block env fields correspond to block latest + 1
I think this can be done by just using pending block for env, and latest block for state
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
7693af8
to
172522a
Compare
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!
Added the function implementation of mev_simBundle according to the builder implementation reference:
https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/internal/ethapi/sbundle_api.go#L205
https://github.com/flashbots/builder/blob/a742641e24df68bc2fc476199b012b0abce40ffe/core/sbundle_sim.go#L101
We are flattening the
SendBundleRequest
so that we can iterate over all the tx in an easier way.Fixes #9472