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.

Sign up for GitHub

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

refactor: use EvmEnv when setting up Evm #13800

Merged
merged 4 commits into from
Jan 15, 2025
+553 −587
Merged

refactor: use EvmEnv when setting up Evm #13800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e44dfb8
feat: use EvmEnv when setting up Evm
klkvr Jan 15, 2025
199435a
fix
klkvr Jan 15, 2025
e456c03
fix
klkvr Jan 15, 2025
6ccfbf2
fix
klkvr Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

108 changes: 67 additions & 41 deletions crates/ethereum/evm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ mod tests {
primitives::{BlockEnv, CfgEnv, SpecId},
JournaledState,
};
use revm_primitives::{EnvWithHandlerCfg, HandlerCfg};
use revm_primitives::HandlerCfg;
use std::collections::HashSet;

#[test]
Expand Down Expand Up @@ -272,12 +272,13 @@ mod tests {

let db = CacheDB::<EmptyDBTyped<ProviderError>>::default();

let env_with_handler = EnvWithHandlerCfg::default();
let evm_env = EvmEnv::default();

let evm = evm_config.evm_with_env(db, env_with_handler.clone());
let evm = evm_config.evm_with_env(db, evm_env.clone(), Default::default());

// Check that the EVM environment
assert_eq!(evm.context.evm.env, env_with_handler.env);
assert_eq!(evm.context.evm.env.block, evm_env.block_env);
assert_eq!(evm.context.evm.env.cfg, evm_env.cfg_env_with_handler_cfg.cfg_env);

// Default spec ID
assert_eq!(evm.handler.spec_id(), SpecId::LATEST);
Expand All @@ -296,16 +297,15 @@ mod tests {
// Create a custom configuration environment with a chain ID of 111
let cfg = CfgEnv::default().with_chain_id(111);

let env_with_handler = EnvWithHandlerCfg {
env: Box::new(Env {
cfg: cfg.clone(),
block: BlockEnv::default(),
tx: TxEnv::default(),
}),
handler_cfg: Default::default(),
let evm_env = EvmEnv {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
cfg_env: cfg.clone(),
handler_cfg: Default::default(),
},
..Default::default()
};

let evm = evm_config.evm_with_env(db, env_with_handler);
let evm = evm_config.evm_with_env(db, evm_env, Default::default());

// Check that the EVM environment is initialized with the custom environment
assert_eq!(evm.context.evm.inner.env.cfg, cfg);
Expand Down Expand Up @@ -333,16 +333,19 @@ mod tests {
};
let tx = TxEnv { gas_limit: 5_000_000, gas_price: U256::from(50), ..Default::default() };

let env_with_handler = EnvWithHandlerCfg {
env: Box::new(Env { cfg: CfgEnv::default(), block, tx }),
handler_cfg: Default::default(),
let evm_env = EvmEnv {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
cfg_env: CfgEnv::default(),
handler_cfg: Default::default(),
},
block_env: block,
};

let evm = evm_config.evm_with_env(db, env_with_handler.clone());
let evm = evm_config.evm_with_env(db, evm_env.clone(), tx.clone());

// Verify that the block and transaction environments are set correctly
assert_eq!(evm.context.evm.env.block, env_with_handler.env.block);
assert_eq!(evm.context.evm.env.tx, env_with_handler.env.tx);
assert_eq!(evm.context.evm.env.block, evm_env.block_env);
assert_eq!(evm.context.evm.env.tx, tx);

// Default spec ID
assert_eq!(evm.handler.spec_id(), SpecId::LATEST);
Expand All @@ -360,9 +363,15 @@ mod tests {

let handler_cfg = HandlerCfg { spec_id: SpecId::CONSTANTINOPLE, ..Default::default() };

let env_with_handler = EnvWithHandlerCfg { env: Box::new(Env::default()), handler_cfg };
let evm_env = EvmEnv {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
cfg_env: Default::default(),
handler_cfg,
},
..Default::default()
};

let evm = evm_config.evm_with_env(db, env_with_handler);
let evm = evm_config.evm_with_env(db, evm_env, Default::default());

// Check that the spec ID is setup properly
assert_eq!(evm.handler.spec_id(), SpecId::CONSTANTINOPLE);
Expand Down Expand Up @@ -422,13 +431,18 @@ mod tests {
let evm_config = EthEvmConfig::new(MAINNET.clone());
let db = CacheDB::<EmptyDBTyped<ProviderError>>::default();

let env_with_handler = EnvWithHandlerCfg::default();
let evm_env = EvmEnv::default();

let evm =
evm_config.evm_with_env_and_inspector(db, env_with_handler.clone(), NoOpInspector);
let evm = evm_config.evm_with_env_and_inspector(
db,
evm_env.clone(),
Default::default(),
NoOpInspector,
);

// Check that the EVM environment is set to default values
assert_eq!(evm.context.evm.env, env_with_handler.env);
assert_eq!(evm.context.evm.env.block, evm_env.block_env);
assert_eq!(evm.context.evm.env.cfg, evm_env.cfg_env_with_handler_cfg.cfg_env);
assert_eq!(evm.context.external, NoOpInspector);
assert_eq!(evm.handler.spec_id(), SpecId::LATEST);

Expand All @@ -442,18 +456,21 @@ mod tests {
let evm_config = EthEvmConfig::new(MAINNET.clone());
let db = CacheDB::<EmptyDBTyped<ProviderError>>::default();

let cfg = CfgEnv::default().with_chain_id(111);
let cfg_env = CfgEnv::default().with_chain_id(111);
let block = BlockEnv::default();
let tx = TxEnv::default();
let env_with_handler = EnvWithHandlerCfg {
env: Box::new(Env { cfg: cfg.clone(), block, tx }),
handler_cfg: Default::default(),
let evm_env = EvmEnv {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
cfg_env: cfg_env.clone(),
handler_cfg: Default::default(),
},
block_env: block,
};

let evm = evm_config.evm_with_env_and_inspector(db, env_with_handler, NoOpInspector);
let evm = evm_config.evm_with_env_and_inspector(db, evm_env, tx, NoOpInspector);

// Check that the EVM environment is set with custom configuration
assert_eq!(evm.context.evm.env.cfg, cfg);
assert_eq!(evm.context.evm.env.cfg, cfg_env);
assert_eq!(evm.context.external, NoOpInspector);
assert_eq!(evm.handler.spec_id(), SpecId::LATEST);

Expand All @@ -475,17 +492,14 @@ mod tests {
..Default::default()
};
let tx = TxEnv { gas_limit: 5_000_000, gas_price: U256::from(50), ..Default::default() };
let env_with_handler = EnvWithHandlerCfg {
env: Box::new(Env { cfg: CfgEnv::default(), block, tx }),
handler_cfg: Default::default(),
};
let evm_env = EvmEnv { block_env: block, ..Default::default() };

let evm =
evm_config.evm_with_env_and_inspector(db, env_with_handler.clone(), NoOpInspector);
evm_config.evm_with_env_and_inspector(db, evm_env.clone(), tx.clone(), NoOpInspector);

// Verify that the block and transaction environments are set correctly
assert_eq!(evm.context.evm.env.block, env_with_handler.env.block);
assert_eq!(evm.context.evm.env.tx, env_with_handler.env.tx);
assert_eq!(evm.context.evm.env.block, evm_env.block_env);
assert_eq!(evm.context.evm.env.tx, tx);
assert_eq!(evm.context.external, NoOpInspector);
assert_eq!(evm.handler.spec_id(), SpecId::LATEST);

Expand All @@ -500,14 +514,26 @@ mod tests {
let db = CacheDB::<EmptyDBTyped<ProviderError>>::default();

let handler_cfg = HandlerCfg { spec_id: SpecId::CONSTANTINOPLE, ..Default::default() };
let env_with_handler = EnvWithHandlerCfg { env: Box::new(Env::default()), handler_cfg };
let evm_env = EvmEnv {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
handler_cfg,
cfg_env: Default::default(),
},
..Default::default()
};

let evm =
evm_config.evm_with_env_and_inspector(db, env_with_handler.clone(), NoOpInspector);
let evm = evm_config.evm_with_env_and_inspector(
db,
evm_env.clone(),
Default::default(),
NoOpInspector,
);

// Check that the spec ID is set properly
assert_eq!(evm.handler.spec_id(), SpecId::CONSTANTINOPLE);
assert_eq!(evm.context.evm.env, env_with_handler.env);
assert_eq!(evm.context.evm.env.block, evm_env.block_env);
assert_eq!(evm.context.evm.env.cfg, evm_env.cfg_env_with_handler_cfg.cfg_env);
assert_eq!(evm.context.evm.env.tx, Default::default());
assert_eq!(evm.context.external, NoOpInspector);

// No Optimism
Expand Down
46 changes: 18 additions & 28 deletions crates/ethereum/payload/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ use reth_transaction_pool::{
};
use revm::{
db::{states::bundle_state::BundleRetention, State},
primitives::{
BlockEnv, CfgEnvWithHandlerCfg, EVMError, EnvWithHandlerCfg, InvalidTransaction,
ResultAndState, TxEnv,
},
primitives::{EVMError, InvalidTransaction, ResultAndState},
DatabaseCommit,
};
use std::sync::Arc;
Expand Down Expand Up @@ -77,7 +74,7 @@ impl<EvmConfig> EthereumPayloadBuilder<EvmConfig>
where
EvmConfig: ConfigureEvm<Header = Header>,
{
/// Returns the configured [`CfgEnvWithHandlerCfg`] and [`BlockEnv`] for the targeted payload
/// Returns the configured [`EvmEnv`] for the targeted payload
/// (that has the `parent` as its parent).
fn cfg_and_block_env(
&self,
Expand Down Expand Up @@ -108,7 +105,7 @@ where
&self,
args: BuildArguments<Pool, Client, EthPayloadBuilderAttributes, EthBuiltPayload>,
) -> Result<BuildOutcome<EthBuiltPayload>, PayloadBuilderError> {
let EvmEnv { cfg_env_with_handler_cfg, block_env } = self
let evm_env = self
.cfg_and_block_env(&args.config, &args.config.parent_header)
.map_err(PayloadBuilderError::other)?;

Expand All @@ -117,8 +114,7 @@ where
self.evm_config.clone(),
self.builder_config.clone(),
args,
cfg_env_with_handler_cfg,
block_env,
evm_env,
|attributes| pool.best_transactions_with_attributes(attributes),
)
}
Expand All @@ -138,7 +134,7 @@ where
None,
);

let EvmEnv { cfg_env_with_handler_cfg, block_env } = self
let evm_env = self
.cfg_and_block_env(&args.config, &args.config.parent_header)
.map_err(PayloadBuilderError::other)?;

Expand All @@ -148,8 +144,7 @@ where
self.evm_config.clone(),
self.builder_config.clone(),
args,
cfg_env_with_handler_cfg,
block_env,
Comment on lines -151 to -152
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is a lot better

evm_env,
|attributes| pool.best_transactions_with_attributes(attributes),
)?
.into_payload()
Expand All @@ -167,8 +162,7 @@ pub fn default_ethereum_payload<EvmConfig, Pool, Client, F>(
evm_config: EvmConfig,
builder_config: EthereumBuilderConfig,
args: BuildArguments<Pool, Client, EthPayloadBuilderAttributes, EthBuiltPayload>,
initialized_cfg: CfgEnvWithHandlerCfg,
initialized_block_env: BlockEnv,
evm_env: EvmEnv,
best_txs: F,
) -> Result<BuildOutcome<EthBuiltPayload>, PayloadBuilderError>
where
Expand All @@ -189,28 +183,29 @@ where
debug!(target: "payload_builder", id=%attributes.id, parent_header = ?parent_header.hash(), parent_number = parent_header.number, "building new payload");
let mut cumulative_gas_used = 0;
let mut sum_blob_gas_used = 0;
let block_gas_limit: u64 = initialized_block_env.gas_limit.to::<u64>();
let base_fee = initialized_block_env.basefee.to::<u64>();
let block_gas_limit: u64 = evm_env.block_env.gas_limit.to::<u64>();
let base_fee = evm_env.block_env.basefee.to::<u64>();

let mut executed_txs = Vec::new();
let mut executed_senders = Vec::new();

let mut best_txs = best_txs(BestTransactionsAttributes::new(
base_fee,
initialized_block_env.get_blob_gasprice().map(|gasprice| gasprice as u64),
evm_env.block_env.get_blob_gasprice().map(|gasprice| gasprice as u64),
));
let mut total_fees = U256::ZERO;

let block_number = initialized_block_env.number.to::<u64>();
let block_number = evm_env.block_env.number.to::<u64>();
let beneficiary = evm_env.block_env.coinbase;

let mut system_caller = SystemCaller::new(evm_config.clone(), chain_spec.clone());

// apply eip-4788 pre block contract call
system_caller
.pre_block_beacon_root_contract_call(
&mut db,
&initialized_cfg,
&initialized_block_env,
evm_env.cfg_env_with_handler_cfg(),
evm_env.block_env(),
attributes.parent_beacon_block_root,
)
.map_err(|err| {
Expand All @@ -225,21 +220,16 @@ where
// apply eip-2935 blockhashes update
system_caller.pre_block_blockhashes_contract_call(
&mut db,
&initialized_cfg,
&initialized_block_env,
evm_env.cfg_env_with_handler_cfg(),
evm_env.block_env(),
parent_header.hash(),
)
.map_err(|err| {
warn!(target: "payload_builder", parent_hash=%parent_header.hash(), %err, "failed to update parent header blockhashes for payload");
PayloadBuilderError::Internal(err.into())
})?;

let env = EnvWithHandlerCfg::new_with_cfg_env(
initialized_cfg.clone(),
initialized_block_env.clone(),
TxEnv::default(),
);
let mut evm = evm_config.evm_with_env(&mut db, env);
let mut evm = evm_config.evm_with_env(&mut db, evm_env, Default::default());

let mut receipts = Vec::new();
while let Some(pool_tx) = best_txs.next() {
Expand Down Expand Up @@ -458,7 +448,7 @@ where
let header = Header {
parent_hash: parent_header.hash(),
ommers_hash: EMPTY_OMMER_ROOT_HASH,
beneficiary: initialized_block_env.coinbase,
beneficiary,
state_root,
transactions_root,
receipts_root,
Expand Down
13 changes: 13 additions & 0 deletions crates/evm/src/env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ pub struct EvmEnv {
pub block_env: BlockEnv,
}

impl Default for EvmEnv {
fn default() -> Self {
Self {
cfg_env_with_handler_cfg: CfgEnvWithHandlerCfg {
cfg_env: Default::default(),
// Will set `is_optimism` if `revm/optimism-default-handler` is enabled.
handler_cfg: Default::default(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a sanity doc here that handler_cfg::default will configure optimism if revm's optimism feature is enabled?

},
block_env: BlockEnv::default(),
}
}
}

impl EvmEnv {
/// Create a new `EvmEnv` from its components.
///
Expand Down
Loading
Loading