diff --git a/crates/database/src/in_memory_db.rs b/crates/database/src/in_memory_db.rs index ecfadf3009..5705e5f745 100644 --- a/crates/database/src/in_memory_db.rs +++ b/crates/database/src/in_memory_db.rs @@ -430,26 +430,28 @@ impl BenchmarkDB { /// BYTECODE address pub const FFADDRESS: Address = address!("ffffffffffffffffffffffffffffffffffffffff"); pub const BENCH_TARGET: Address = FFADDRESS; +pub const BENCH_TARGET_BALANCE: U256 = U256::from_limbs([10000000, 0, 0, 0]); /// CALLER address pub const EEADDRESS: Address = address!("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"); pub const BENCH_CALLER: Address = EEADDRESS; +pub const BENCH_CALLER_BALANCE: U256 = U256::from_limbs([10000000, 0, 0, 0]); impl Database for BenchmarkDB { type Error = Infallible; /// Get basic account information. fn basic(&mut self, address: Address) -> Result, Self::Error> { - if address == FFADDRESS { + if address == BENCH_TARGET { return Ok(Some(AccountInfo { nonce: 1, - balance: U256::from(10000000), + balance: BENCH_TARGET_BALANCE, code: Some(self.0.clone()), code_hash: self.1, })); } - if address == EEADDRESS { + if address == BENCH_CALLER { return Ok(Some(AccountInfo { nonce: 0, - balance: U256::from(10000000), + balance: BENCH_CALLER_BALANCE, code: None, code_hash: KECCAK_EMPTY, })); diff --git a/crates/handler/src/handler.rs b/crates/handler/src/handler.rs index 5e371f1b03..f8fcfec605 100644 --- a/crates/handler/src/handler.rs +++ b/crates/handler/src/handler.rs @@ -115,12 +115,25 @@ pub trait Handler { fn run( &mut self, evm: &mut Self::Evm, + ) -> Result, Self::Error> { + // run inner handler and catch all errors to handle cleanup. + match self.run_without_catch_error(evm) { + Ok(output) => Ok(output), + Err(e) => self.catch_error(evm, e), + } + } + + #[inline] + fn run_without_catch_error( + &mut self, + evm: &mut Self::Evm, ) -> Result, Self::Error> { let init_and_floor_gas = self.validate(evm)?; let eip7702_refund = self.pre_execution(evm)? as i64; let exec_result = self.execution(evm, &init_and_floor_gas)?; self.post_execution(evm, exec_result, init_and_floor_gas, eip7702_refund) } + /// Call all validation functions #[inline] fn validate(&self, evm: &mut Self::Evm) -> Result { @@ -174,11 +187,7 @@ pub trait Handler { // Reward beneficiary self.reward_beneficiary(evm, &mut exec_result)?; // Prepare output of transaction. - let output = self.output(evm, exec_result)?; - // Clear any internal state. - self.clear(evm); - // Return output - Ok(output) + self.output(evm, exec_result) } /* VALIDATION */ @@ -371,28 +380,24 @@ pub trait Handler { ) -> Result, Self::Error> { let ctx = evm.ctx(); mem::replace(ctx.error(), Ok(()))?; - Ok(post_execution::output(ctx, result)) + let output = post_execution::output(ctx, result); + + // clear journal + evm.ctx().journal().clear(); + Ok(output) } - /// Called when execution ends. + /// Called every time at the end of execution. Used for clearing the journal. /// /// End handle in comparison to output handle will be called every time after execution. - /// - /// While output will be omitted in case of the error. #[inline] - fn end( + fn catch_error( &self, - _evm: &mut Self::Evm, - end_output: Result, Self::Error>, + evm: &mut Self::Evm, + error: Self::Error, ) -> Result, Self::Error> { - end_output - } - - /// Clean handler. It resets internal Journal state to default one. - /// - /// This handle is called every time regardless of the result of the transaction. - #[inline] - fn clear(&self, evm: &mut Self::Evm) { + // do the cleanup of journal if error is caught evm.ctx().journal().clear(); + Err(error) } } diff --git a/crates/inspector/src/inspector.rs b/crates/inspector/src/inspector.rs index 5e5ae2c7f4..1043ccf10c 100644 --- a/crates/inspector/src/inspector.rs +++ b/crates/inspector/src/inspector.rs @@ -190,6 +190,16 @@ where fn inspect_run( &mut self, evm: &mut Self::Evm, + ) -> Result, Self::Error> { + match self.inspect_run_without_catch_error(evm) { + Ok(output) => Ok(output), + Err(e) => self.catch_error(evm, e), + } + } + + fn inspect_run_without_catch_error( + &mut self, + evm: &mut Self::Evm, ) -> Result, Self::Error> { let init_and_floor_gas = self.validate(evm)?; let eip7702_refund = self.pre_execution(evm)? as i64; diff --git a/crates/optimism/src/evm.rs b/crates/optimism/src/evm.rs index defdf4b30f..4bd96520f5 100644 --- a/crates/optimism/src/evm.rs +++ b/crates/optimism/src/evm.rs @@ -75,3 +75,78 @@ where (&mut self.0.data.ctx, &mut self.0.precompiles) } } + +#[cfg(test)] +mod tests { + use crate::{ + transaction::deposit::DEPOSIT_TRANSACTION_TYPE, DefaultOp, OpBuilder, OpHaltReason, + OpSpecId, + }; + use database::{BenchmarkDB, BENCH_CALLER, BENCH_CALLER_BALANCE, BENCH_TARGET}; + use precompile::Address; + use revm::{ + bytecode::opcode, + context::result::ExecutionResult, + primitives::{TxKind, U256}, + state::Bytecode, + Context, ExecuteEvm, + }; + + #[test] + fn test_deposit_tx() { + let ctx = Context::op() + .modify_tx_chained(|tx| { + tx.enveloped_tx = None; + tx.deposit.mint = Some(100); + tx.base.tx_type = DEPOSIT_TRANSACTION_TYPE; + }) + .modify_cfg_chained(|cfg| cfg.spec = OpSpecId::HOLOCENE); + + let mut evm = ctx.build_op(); + + let output = evm.transact_previous().unwrap(); + + // balance should be 100 + assert_eq!( + output + .state + .get(&Address::default()) + .map(|a| a.info.balance), + Some(U256::from(100)) + ); + } + + #[test] + fn test_halted_deposit_tx() { + let ctx = Context::op() + .modify_tx_chained(|tx| { + tx.enveloped_tx = None; + tx.deposit.mint = Some(100); + tx.base.tx_type = DEPOSIT_TRANSACTION_TYPE; + tx.base.caller = BENCH_CALLER; + tx.base.kind = TxKind::Call(BENCH_TARGET); + }) + .modify_cfg_chained(|cfg| cfg.spec = OpSpecId::HOLOCENE) + .with_db(BenchmarkDB::new_bytecode(Bytecode::new_legacy( + [opcode::POP].into(), + ))); + + // POP would return a halt. + let mut evm = ctx.build_op(); + + let output = evm.transact_previous().unwrap(); + + // balance should be 100 + previous balance + assert_eq!( + output.result, + ExecutionResult::Halt { + reason: OpHaltReason::FailedDeposit, + gas_used: 30_000_000 + } + ); + assert_eq!( + output.state.get(&BENCH_CALLER).map(|a| a.info.balance), + Some(U256::from(100) + BENCH_CALLER_BALANCE) + ); + } +} diff --git a/crates/optimism/src/handler.rs b/crates/optimism/src/handler.rs index c63e0ad61c..cbbee17776 100644 --- a/crates/optimism/src/handler.rs +++ b/crates/optimism/src/handler.rs @@ -387,79 +387,76 @@ where return Err(ERROR::from(OpTransactionError::HaltedDepositPostRegolith)); } } + evm.ctx().chain().clear_tx_l1_cost(); Ok(result) } - fn end( + fn catch_error( &self, evm: &mut Self::Evm, - end_output: Result, Self::Error>, + error: Self::Error, ) -> Result, Self::Error> { - //end_output - let is_deposit = evm.ctx().tx().tx_type() == DEPOSIT_TRANSACTION_TYPE; - end_output.or_else(|err| { - if err.is_tx_error() && is_deposit { - let ctx = evm.ctx(); - let spec = ctx.cfg().spec(); - let tx = ctx.tx(); - let caller = tx.caller(); - let mint = tx.mint(); - let is_system_tx = tx.is_system_transaction(); - let gas_limit = tx.gas_limit(); - // If the transaction is a deposit transaction and it failed - // for any reason, the caller nonce must be bumped, and the - // gas reported must be altered depending on the Hardfork. This is - // also returned as a special Halt variant so that consumers can more - // easily distinguish between a failed deposit and a failed - // normal transaction. - - // Increment sender nonce and account balance for the mint amount. Deposits - // always persist the mint amount, even if the transaction fails. - let account = { - let mut acc = Account::from( - evm.ctx() - .db() - .basic(caller) - .unwrap_or_default() - .unwrap_or_default(), - ); - acc.info.nonce = acc.info.nonce.saturating_add(1); - acc.info.balance = acc - .info - .balance - .saturating_add(U256::from(mint.unwrap_or_default())); - acc.mark_touch(); - acc - }; - let state = HashMap::from_iter([(caller, account)]); - - // The gas used of a failed deposit post-regolith is the gas - // limit of the transaction. pre-regolith, it is the gas limit - // of the transaction for non system transactions and 0 for system - // transactions. - let gas_used = if spec.is_enabled_in(OpSpecId::REGOLITH) || !is_system_tx { - gas_limit - } else { - 0 - }; - - Ok(ResultAndState { - result: ExecutionResult::Halt { - reason: OpHaltReason::FailedDeposit, - gas_used, - }, - state, - }) + let output = if error.is_tx_error() && is_deposit { + let ctx = evm.ctx(); + let spec = ctx.cfg().spec(); + let tx = ctx.tx(); + let caller = tx.caller(); + let mint = tx.mint(); + let is_system_tx = tx.is_system_transaction(); + let gas_limit = tx.gas_limit(); + // If the transaction is a deposit transaction and it failed + // for any reason, the caller nonce must be bumped, and the + // gas reported must be altered depending on the Hardfork. This is + // also returned as a special Halt variant so that consumers can more + // easily distinguish between a failed deposit and a failed + // normal transaction. + + // Increment sender nonce and account balance for the mint amount. Deposits + // always persist the mint amount, even if the transaction fails. + let account = { + let mut acc = Account::from( + evm.ctx() + .db() + .basic(caller) + .unwrap_or_default() + .unwrap_or_default(), + ); + acc.info.nonce = acc.info.nonce.saturating_add(1); + acc.info.balance = acc + .info + .balance + .saturating_add(U256::from(mint.unwrap_or_default())); + acc.mark_touch(); + acc + }; + let state = HashMap::from_iter([(caller, account)]); + + // The gas used of a failed deposit post-regolith is the gas + // limit of the transaction. pre-regolith, it is the gas limit + // of the transaction for non system transactions and 0 for system + // transactions. + let gas_used = if spec.is_enabled_in(OpSpecId::REGOLITH) || !is_system_tx { + gas_limit } else { - Err(err) - } - }) - } - - fn clear(&self, evm: &mut Self::Evm) { + 0 + }; + // clear the journal + Ok(ResultAndState { + result: ExecutionResult::Halt { + reason: OpHaltReason::FailedDeposit, + gas_used, + }, + state, + }) + } else { + Err(error) + }; + // do cleanup evm.ctx().chain().clear_tx_l1_cost(); - self.mainnet.clear(evm); + evm.ctx().journal().clear(); + + output } }