From 0938504f4a265eec74e0b833ab5c41d66649bdee Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 1 May 2024 19:32:25 +0200 Subject: [PATCH] chore: reduce number of Evm monomorphizations (#8030) --- crates/payload/builder/src/database.rs | 12 ++-- crates/payload/ethereum/src/lib.rs | 79 ++++++++++++++------- crates/payload/optimism/src/builder.rs | 67 +++++++++++------ crates/rpc/rpc/src/debug.rs | 36 ++++------ crates/rpc/rpc/src/eth/api/call.rs | 4 +- crates/rpc/rpc/src/eth/api/pending_block.rs | 5 +- crates/rpc/rpc/src/eth/api/transactions.rs | 31 +++----- crates/rpc/rpc/src/eth/revm_utils.rs | 7 +- crates/rpc/rpc/src/trace.rs | 2 +- 9 files changed, 138 insertions(+), 105 deletions(-) diff --git a/crates/payload/builder/src/database.rs b/crates/payload/builder/src/database.rs index 5b5239fddc..ac36de98c3 100644 --- a/crates/payload/builder/src/database.rs +++ b/crates/payload/builder/src/database.rs @@ -61,10 +61,13 @@ impl CachedReads { } } +/// A [Database] that caches reads inside [CachedReads]. #[derive(Debug)] -struct CachedReadsDbMut<'a, DB> { - cached: &'a mut CachedReads, - db: DB, +pub struct CachedReadsDbMut<'a, DB> { + /// The cache of reads. + pub cached: &'a mut CachedReads, + /// The underlying database. + pub db: DB, } impl<'a, DB: DatabaseRef> Database for CachedReadsDbMut<'a, DB> { @@ -126,7 +129,8 @@ impl<'a, DB: DatabaseRef> Database for CachedReadsDbMut<'a, DB> { /// `revm::db::State` for repeated payload build jobs. #[derive(Debug)] pub struct CachedReadsDBRef<'a, DB> { - inner: RefCell>, + /// The inner cache reads db mut. + pub inner: RefCell>, } impl<'a, DB: DatabaseRef> DatabaseRef for CachedReadsDBRef<'a, DB> { diff --git a/crates/payload/ethereum/src/lib.rs b/crates/payload/ethereum/src/lib.rs index f1c0a215b8..e34287f765 100644 --- a/crates/payload/ethereum/src/lib.rs +++ b/crates/payload/ethereum/src/lib.rs @@ -73,36 +73,54 @@ where debug!(target: "payload_builder", parent_hash = ?parent_block.hash(), parent_number = parent_block.number, "building empty payload"); let state = client.state_by_block_hash(parent_block.hash()).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to get state for empty payload"); - err - })?; + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to get state for empty payload" + ); + err + })?; let mut db = State::builder() - .with_database_boxed(Box::new(StateProviderDatabase::new(&state))) + .with_database(StateProviderDatabase::new(state)) .with_bundle_update() .build(); let base_fee = initialized_block_env.basefee.to::(); let block_number = initialized_block_env.number.to::(); - let block_gas_limit: u64 = initialized_block_env.gas_limit.try_into().unwrap_or(u64::MAX); + let block_gas_limit = initialized_block_env.gas_limit.try_into().unwrap_or(u64::MAX); // apply eip-4788 pre block contract call pre_block_beacon_root_contract_call( - &mut db, - &chain_spec, - block_number, - &initialized_cfg, - &initialized_block_env, - &attributes, - ).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to apply beacon root contract call for empty payload"); - err - })?; + &mut db, + &chain_spec, + block_number, + &initialized_cfg, + &initialized_block_env, + &attributes, + ) + .map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to apply beacon root contract call for empty payload" + ); + err + })?; - let WithdrawalsOutcome { withdrawals_root, withdrawals } = - commit_withdrawals(&mut db, &chain_spec, attributes.timestamp, attributes.withdrawals.clone()).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to commit withdrawals for empty payload"); - err - })?; + let WithdrawalsOutcome { withdrawals_root, withdrawals } = commit_withdrawals( + &mut db, + &chain_spec, + attributes.timestamp, + attributes.withdrawals.clone(), + ) + .map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to commit withdrawals for empty payload" + ); + err + })?; // merge all transitions into bundle state, this would apply the withdrawal balance // changes and 4788 contract call @@ -110,10 +128,14 @@ where // calculate the state root let bundle_state = db.take_bundle(); - let state_root = state.state_root(&bundle_state).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to calculate state root for empty payload"); - err - })?; + let state_root = db.database.state_root(&bundle_state).map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to calculate state root for empty payload" + ); + err + })?; let mut excess_blob_gas = None; let mut blob_gas_used = None; @@ -178,9 +200,9 @@ where let BuildArguments { client, pool, mut cached_reads, config, cancel, best_payload } = args; let state_provider = client.state_by_block_hash(config.parent_block.hash())?; - let state = StateProviderDatabase::new(&state_provider); + let state = StateProviderDatabase::new(state_provider); let mut db = - State::builder().with_database_ref(cached_reads.as_db(&state)).with_bundle_update().build(); + State::builder().with_database_ref(cached_reads.as_db(state)).with_bundle_update().build(); let extra_data = config.extra_data(); let PayloadConfig { initialized_block_env, @@ -349,7 +371,10 @@ where let logs_bloom = bundle.block_logs_bloom(block_number).expect("Number is in range"); // calculate the state root - let state_root = state_provider.state_root(bundle.state())?; + let state_root = { + let state_provider = db.database.0.inner.borrow_mut(); + state_provider.db.state_root(bundle.state())? + }; // create the block header let transactions_root = proofs::calculate_transaction_root(&executed_txs); diff --git a/crates/payload/optimism/src/builder.rs b/crates/payload/optimism/src/builder.rs index 7d8efa6899..8e8bfb8f0f 100644 --- a/crates/payload/optimism/src/builder.rs +++ b/crates/payload/optimism/src/builder.rs @@ -123,7 +123,7 @@ where err })?; let mut db = State::builder() - .with_database_boxed(Box::new(StateProviderDatabase::new(&state))) + .with_database(StateProviderDatabase::new(state)) .with_bundle_update() .build(); @@ -133,22 +133,36 @@ where // apply eip-4788 pre block contract call pre_block_beacon_root_contract_call( - &mut db, - &chain_spec, - block_number, - &initialized_cfg, - &initialized_block_env, - &attributes, - ).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to apply beacon root contract call for empty payload"); - err - })?; + &mut db, + &chain_spec, + block_number, + &initialized_cfg, + &initialized_block_env, + &attributes, + ) + .map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to apply beacon root contract call for empty payload" + ); + err + })?; - let WithdrawalsOutcome { withdrawals_root, withdrawals } = - commit_withdrawals(&mut db, &chain_spec, attributes.payload_attributes.timestamp, attributes.payload_attributes.withdrawals.clone()).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to commit withdrawals for empty payload"); - err - })?; + let WithdrawalsOutcome { withdrawals_root, withdrawals } = commit_withdrawals( + &mut db, + &chain_spec, + attributes.payload_attributes.timestamp, + attributes.payload_attributes.withdrawals.clone(), + ) + .map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to commit withdrawals for empty payload" + ); + err + })?; // merge all transitions into bundle state, this would apply the withdrawal balance // changes and 4788 contract call @@ -156,10 +170,14 @@ where // calculate the state root let bundle_state = db.take_bundle(); - let state_root = state.state_root(&bundle_state).map_err(|err| { - warn!(target: "payload_builder", parent_hash=%parent_block.hash(), %err, "failed to calculate state root for empty payload"); - err - })?; + let state_root = db.database.state_root(&bundle_state).map_err(|err| { + warn!(target: "payload_builder", + parent_hash=%parent_block.hash(), + %err, + "failed to calculate state root for empty payload" + ); + err + })?; let mut excess_blob_gas = None; let mut blob_gas_used = None; @@ -236,9 +254,9 @@ where let BuildArguments { client, pool, mut cached_reads, config, cancel, best_payload } = args; let state_provider = client.state_by_block_hash(config.parent_block.hash())?; - let state = StateProviderDatabase::new(&state_provider); + let state = StateProviderDatabase::new(state_provider); let mut db = - State::builder().with_database_ref(cached_reads.as_db(&state)).with_bundle_update().build(); + State::builder().with_database_ref(cached_reads.as_db(state)).with_bundle_update().build(); let extra_data = config.extra_data(); let PayloadConfig { initialized_block_env, @@ -510,7 +528,10 @@ where let logs_bloom = bundle.block_logs_bloom(block_number).expect("Number is in range"); // calculate the state root - let state_root = state_provider.state_root(bundle.state())?; + let state_root = { + let state_provider = db.database.0.inner.borrow_mut(); + state_provider.db.state_root(bundle.state())? + }; // create the block header let transactions_root = proofs::calculate_transaction_root(&executed_txs); diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index e47ccc4661..b21adf5205 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -323,14 +323,11 @@ where self.inner .eth_api .spawn_with_call_at(call, at, overrides, move |db, env| { - let (res, _, db) = this.eth_api().inspect_and_return_db( - db, - env, - &mut inspector, - )?; + let (res, _) = + this.eth_api().inspect(&mut *db, env, &mut inspector)?; let frame = inspector .into_geth_builder() - .geth_prestate_traces(&res, prestate_config, &db)?; + .geth_prestate_traces(&res, prestate_config, db)?; Ok(frame) }) .await?; @@ -348,12 +345,9 @@ where .inner .eth_api .spawn_with_call_at(call, at, overrides, move |db, env| { - let (res, _, db) = this.eth_api().inspect_and_return_db( - db, - env, - &mut inspector, - )?; - let frame = inspector.try_into_mux_frame(&res, &db)?; + let (res, _) = + this.eth_api().inspect(&mut *db, env, &mut inspector)?; + let frame = inspector.try_into_mux_frame(&res, db)?; Ok(frame.into()) }) .await?; @@ -370,12 +364,9 @@ where .eth_api .spawn_with_call_at(call, at, overrides, move |db, env| { let mut inspector = JsInspector::new(code, config)?; - let (res, _, db) = this.eth_api().inspect_and_return_db( - db, - env.clone(), - &mut inspector, - )?; - Ok(inspector.json_result(res, &env, &db)?) + let (res, _) = + this.eth_api().inspect(&mut *db, env.clone(), &mut inspector)?; + Ok(inspector.json_result(res, &env, db)?) }) .await?; @@ -564,8 +555,7 @@ where let mut inspector = TracingInspector::new( TracingInspectorConfig::from_geth_prestate_config(&prestate_config), ); - let (res, _, db) = - self.eth_api().inspect_and_return_db(db, env, &mut inspector)?; + let (res, _) = self.eth_api().inspect(&mut *db, env, &mut inspector)?; let frame = inspector.into_geth_builder().geth_prestate_traces( &res, @@ -585,8 +575,7 @@ where let mut inspector = MuxInspector::try_from_config(mux_config)?; - let (res, _, db) = - self.eth_api().inspect_and_return_db(db, env, &mut inspector)?; + let (res, _) = self.eth_api().inspect(&mut *db, env, &mut inspector)?; let frame = inspector.try_into_mux_frame(&res, db)?; return Ok((frame.into(), res.state)) } @@ -598,8 +587,7 @@ where config, transaction_context.unwrap_or_default(), )?; - let (res, env, db) = - self.eth_api().inspect_and_return_db(db, env, &mut inspector)?; + let (res, env) = self.eth_api().inspect(&mut *db, env, &mut inspector)?; let state = res.state.clone(); let result = inspector.json_result(res, &env, db)?; diff --git a/crates/rpc/rpc/src/eth/api/call.rs b/crates/rpc/rpc/src/eth/api/call.rs index 191406f96a..8ef2af2f52 100644 --- a/crates/rpc/rpc/src/eth/api/call.rs +++ b/crates/rpc/rpc/src/eth/api/call.rs @@ -451,14 +451,14 @@ where &self, env_gas_limit: U256, mut env: EnvWithHandlerCfg, - mut db: &mut CacheDB>, + db: &mut CacheDB>, ) -> EthApiError where S: StateProvider, { let req_gas_limit = env.tx.gas_limit; env.tx.gas_limit = env_gas_limit.try_into().unwrap_or(u64::MAX); - let (res, _) = match self.transact(&mut db, env) { + let (res, _) = match self.transact(db, env) { Ok(res) => res, Err(err) => return err, }; diff --git a/crates/rpc/rpc/src/eth/api/pending_block.rs b/crates/rpc/rpc/src/eth/api/pending_block.rs index aa18bf7ecc..dbb148981b 100644 --- a/crates/rpc/rpc/src/eth/api/pending_block.rs +++ b/crates/rpc/rpc/src/eth/api/pending_block.rs @@ -52,8 +52,8 @@ impl PendingBlockEnv { let parent_hash = origin.build_target_hash(); let state_provider = client.history_by_block_hash(parent_hash)?; - let state = StateProviderDatabase::new(&state_provider); - let mut db = State::builder().with_database(Box::new(state)).with_bundle_update().build(); + let state = StateProviderDatabase::new(state_provider); + let mut db = State::builder().with_database(state).with_bundle_update().build(); let mut cumulative_gas_used = 0; let mut sum_blob_gas_used = 0; @@ -230,6 +230,7 @@ impl PendingBlockEnv { let logs_bloom = bundle.block_logs_bloom(block_number).expect("Block is present"); // calculate the state root + let state_provider = &db.database; let state_root = state_provider.state_root(bundle.state())?; // create the block header diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index 3e582821b5..15e2b6f565 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -287,7 +287,7 @@ pub trait EthTransactions: Send + Sync { f: F, ) -> EthResult where - F: FnOnce(StateCacheDB, EnvWithHandlerCfg) -> EthResult + Send + 'static, + F: FnOnce(&mut StateCacheDB, EnvWithHandlerCfg) -> EthResult + Send + 'static, R: Send + 'static; /// Executes the call request at the given [BlockId]. @@ -308,7 +308,7 @@ pub trait EthTransactions: Send + Sync { inspector: I, ) -> EthResult<(ResultAndState, EnvWithHandlerCfg)> where - I: Inspector + Send + 'static; + I: for<'a> Inspector<&'a mut StateCacheDB> + Send + 'static; /// Executes the transaction on top of the given [BlockId] with a tracer configured by the /// config. @@ -571,10 +571,7 @@ where ::Error: Into, I: GetInspector, { - let mut evm = self.inner.evm_config.evm_with_env_and_inspector(db, env, inspector); - let res = evm.transact()?; - let (_, env) = evm.into_db_and_env_with_handler_cfg(); - Ok((res, env)) + self.inspect_and_return_db(db, env, inspector).map(|(res, env, _)| (res, env)) } fn inspect_and_return_db( @@ -1066,7 +1063,7 @@ where f: F, ) -> EthResult where - F: FnOnce(StateCacheDB, EnvWithHandlerCfg) -> EthResult + Send + 'static, + F: FnOnce(&mut StateCacheDB, EnvWithHandlerCfg) -> EthResult + Send + 'static, R: Send + 'static, { let (cfg, block_env, at) = self.evm_env_at(at).await?; @@ -1085,7 +1082,7 @@ where &mut db, overrides, )?; - f(db, env) + f(&mut db, env) }) .await .map_err(|_| EthApiError::InternalBlockingTaskError)? @@ -1098,10 +1095,7 @@ where overrides: EvmOverrides, ) -> EthResult<(ResultAndState, EnvWithHandlerCfg)> { let this = self.clone(); - self.spawn_with_call_at(request, at, overrides, move |mut db, env| { - this.transact(&mut db, env) - }) - .await + self.spawn_with_call_at(request, at, overrides, move |db, env| this.transact(db, env)).await } async fn spawn_inspect_call_at( @@ -1112,7 +1106,7 @@ where inspector: I, ) -> EthResult<(ResultAndState, EnvWithHandlerCfg)> where - I: Inspector + Send + 'static, + I: for<'a> Inspector<&'a mut StateCacheDB> + Send + 'static, { let this = self.clone(); self.spawn_with_call_at(request, at, overrides, move |db, env| { @@ -1133,11 +1127,9 @@ where { let this = self.clone(); self.with_state_at_block(at, |state| { - let db = CacheDB::new(StateProviderDatabase::new(state)); - + let mut db = CacheDB::new(StateProviderDatabase::new(state)); let mut inspector = TracingInspector::new(config); - let (res, _) = this.inspect(db, env, &mut inspector)?; - + let (res, _) = this.inspect(&mut db, env, &mut inspector)?; f(inspector, res) }) } @@ -1155,10 +1147,9 @@ where { let this = self.clone(); self.spawn_with_state_at_block(at, move |state| { - let db = CacheDB::new(StateProviderDatabase::new(state)); + let mut db = CacheDB::new(StateProviderDatabase::new(state)); let mut inspector = TracingInspector::new(config); - let (res, _, db) = this.inspect_and_return_db(db, env, &mut inspector)?; - + let (res, _) = this.inspect(&mut db, env, &mut inspector)?; f(inspector, res, db) }) .await diff --git a/crates/rpc/rpc/src/eth/revm_utils.rs b/crates/rpc/rpc/src/eth/revm_utils.rs index c80aee99d5..c2855163ba 100644 --- a/crates/rpc/rpc/src/eth/revm_utils.rs +++ b/crates/rpc/rpc/src/eth/revm_utils.rs @@ -278,7 +278,10 @@ pub(crate) fn create_txn_env( } /// Caps the configured [TxEnv] `gas_limit` with the allowance of the caller. -pub(crate) fn cap_tx_gas_limit_with_caller_allowance(db: DB, env: &mut TxEnv) -> EthResult<()> +pub(crate) fn cap_tx_gas_limit_with_caller_allowance( + db: &mut DB, + env: &mut TxEnv, +) -> EthResult<()> where DB: Database, EthApiError: From<::Error>, @@ -296,7 +299,7 @@ where /// /// Returns an error if the caller has insufficient funds. /// Caution: This assumes non-zero `env.gas_price`. Otherwise, zero allowance will be returned. -pub(crate) fn caller_gas_allowance(mut db: DB, env: &TxEnv) -> EthResult +pub(crate) fn caller_gas_allowance(db: &mut DB, env: &TxEnv) -> EthResult where DB: Database, EthApiError: From<::Error>, diff --git a/crates/rpc/rpc/src/trace.rs b/crates/rpc/rpc/src/trace.rs index 0479190367..5ee089a91f 100644 --- a/crates/rpc/rpc/src/trace.rs +++ b/crates/rpc/rpc/src/trace.rs @@ -86,7 +86,7 @@ where let this = self.clone(); self.eth_api() .spawn_with_call_at(trace_request.call, at, overrides, move |db, env| { - let (res, _, db) = this.eth_api().inspect_and_return_db(db, env, &mut inspector)?; + let (res, _) = this.eth_api().inspect(&mut *db, env, &mut inspector)?; let trace_res = inspector.into_parity_builder().into_trace_results_with_state( &res, &trace_request.trace_types,