From 2d94acc64939d30acdc2c3da6bc032dc093f687a Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Sat, 12 Oct 2024 02:10:59 +0200 Subject: [PATCH 1/3] Refactoring Executor: create_inner and gas_limit calc --- runtime/src/eval/system.rs | 4 +- runtime/src/handler.rs | 2 +- src/executor/stack/executor.rs | 159 +++++++++++++++------------------ 3 files changed, 73 insertions(+), 92 deletions(-) diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 29973f9e..a4de7cf3 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -448,8 +448,8 @@ pub fn create(runtime: &mut Runtime, is_create2: bool, handler: &mut }; match handler.create(runtime.context.address, scheme, value, code, None) { - Capture::Exit((reason, address, return_data)) => { - match super::finish_create(runtime, reason, address, return_data) { + Capture::Exit((reason, return_data)) => { + match super::finish_create(runtime, reason, None, return_data) { Ok(()) => Control::Continue, Err(e) => Control::Exit(e), } diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index d73f44a3..309b80db 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -103,7 +103,7 @@ pub trait Handler { value: U256, init_code: Vec, target_gas: Option, - ) -> Capture<(ExitReason, Option, Vec), Self::CreateInterrupt>; + ) -> Capture<(ExitReason, Vec), Self::CreateInterrupt>; /// Feed in create feedback. /// /// # Errors diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 597e09ae..ad745e9b 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -36,9 +36,21 @@ macro_rules! emit_exit { (reason, return_value) }}; } +macro_rules! try_or_fail { + ( $e:expr ) => { + match $e { + Ok(v) => v, + Err(e) => return Capture::Exit((e.into(), Vec::new())), + } + }; +} const DEFAULT_CALL_STACK_CAPACITY: usize = 4; +const fn l64(gas: u64) -> u64 { + gas - gas / 64 +} + pub enum StackExitKind { Succeeded, Reverted, @@ -542,7 +554,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Some(gas_limit), false, ) { - Capture::Exit((s, _, v)) => emit_exit!(s, v), + Capture::Exit((s, v)) => emit_exit!(s, v), Capture::Trap(rt) => { let mut cs = Vec::with_capacity(DEFAULT_CALL_STACK_CAPACITY); cs.push(rt.0); @@ -649,7 +661,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Some(gas_limit), false, ) { - Capture::Exit((s, _, v)) => emit_exit!(s, v), + Capture::Exit((s, v)) => emit_exit!(s, v), Capture::Trap(rt) => { let mut cs = Vec::with_capacity(DEFAULT_CALL_STACK_CAPACITY); cs.push(rt.0); @@ -821,6 +833,30 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } } + /// Calculate gas limit and record it in the gasometer. + fn calc_gas_limit_and_record( + &mut self, + target_gas: Option, + take_l64: bool, + ) -> Result { + let after_gas = if take_l64 && self.config.call_l64_after_gas { + if self.config.estimate { + let initial_after_gas = self.state.metadata().gasometer.gas(); + let diff = initial_after_gas - l64(initial_after_gas); + self.state.metadata_mut().gasometer.record_cost(diff)?; + self.state.metadata().gasometer.gas() + } else { + l64(self.state.metadata().gasometer.gas()) + } + } else { + self.state.metadata().gasometer.gas() + }; + let target_gas = target_gas.unwrap_or(after_gas); + let gas_limit = min(target_gas, after_gas); + self.state.metadata_mut().gasometer.record_cost(gas_limit)?; + Ok(gas_limit) + } + fn create_inner( &mut self, caller: H160, @@ -829,26 +865,13 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> init_code: Vec, target_gas: Option, take_l64: bool, - ) -> Capture<(ExitReason, Option, Vec), StackExecutorCreateInterrupt<'static>> { - const fn l64(gas: u64) -> u64 { - gas - gas / 64 - } - + ) -> Capture<(ExitReason, Vec), StackExecutorCreateInterrupt<'static>> { if self.nonce(caller) >= U64_MAX { - return Capture::Exit((ExitError::MaxNonce.into(), None, Vec::new())); - } - - macro_rules! try_or_fail { - ( $e:expr ) => { - match $e { - Ok(v) => v, - Err(e) => return Capture::Exit((e.into(), None, Vec::new())), - } - }; + return Capture::Exit((ExitError::MaxNonce.into(), Vec::new())); } + // Warm address for EIP-2929 let address = self.create_address(scheme); - self.state .metadata_mut() .access_addresses([caller, address].iter().copied()); @@ -867,49 +890,34 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> // early to verify exceeding Stack limit. It allows avoid // issue with wrong detection `CallTooDeep` for Create. if depth + 1 > self.config.call_stack_limit { - return Capture::Exit((ExitError::CallTooDeep.into(), None, Vec::new())); + return Capture::Exit((ExitError::CallTooDeep.into(), Vec::new())); } } + // Check is transfer value is enough if self.balance(caller) < value { - return Capture::Exit((ExitError::OutOfFund.into(), None, Vec::new())); + return Capture::Exit((ExitError::OutOfFund.into(), Vec::new())); } - let after_gas = if take_l64 && self.config.call_l64_after_gas { - if self.config.estimate { - let initial_after_gas = self.state.metadata().gasometer.gas(); - let diff = initial_after_gas - l64(initial_after_gas); - try_or_fail!(self.state.metadata_mut().gasometer.record_cost(diff)); - self.state.metadata().gasometer.gas() - } else { - l64(self.state.metadata().gasometer.gas()) - } - } else { - self.state.metadata().gasometer.gas() - }; + let gas_limit = try_or_fail!(self.calc_gas_limit_and_record(target_gas, take_l64)); - let target_gas = target_gas.unwrap_or(after_gas); - - let gas_limit = min(after_gas, target_gas); - try_or_fail!(self.state.metadata_mut().gasometer.record_cost(gas_limit)); + // Check nonce and increment it for caller + try_or_fail!(self.state.inc_nonce(caller)); - if let Err(e) = self.state.inc_nonce(caller) { - return Capture::Exit((e.into(), None, Vec::new())); + // Check create collision: EIP-7610 + if self.is_create_collision(address) { + return Capture::Exit((ExitError::CreateCollision.into(), Vec::new())); } + // Enter to execution substate self.enter_substate(gas_limit, false); - // Check create collision: EIP-7610 - if self.is_create_collision(address) { - let _ = self.exit_substate(&StackExitKind::Failed); - return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); + // Check nonce and increment it for created address after entering substate + if self.config.create_increase_nonce { + try_or_fail!(self.state.inc_nonce(address)); } - let context = Context { - address, - caller, - apparent_value: value, - }; + // Transfer funds if needed let transfer = Transfer { source: caller, target: address, @@ -919,19 +927,19 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Ok(()) => (), Err(e) => { let _ = self.exit_substate(&StackExitKind::Reverted); - return Capture::Exit((ExitReason::Error(e), None, Vec::new())); + return Capture::Exit((ExitReason::Error(e), Vec::new())); } } // It needed for CANCUN hard fork EIP-6780 we should mark account as created // to handle SELFDESTRUCT in the same transaction self.state.set_created(address); - if self.config.create_increase_nonce { - if let Err(e) = self.state.inc_nonce(address) { - return Capture::Exit((e.into(), None, Vec::new())); - } - } - + // Init EVM runtime in Context + let context = Context { + address, + caller, + apparent_value: value, + }; let runtime = Runtime::new( Rc::new(init_code), Rc::new(Vec::new()), @@ -940,6 +948,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.config.memory_limit, ); + // Set Runtime kind with pre-init Runtime and return Trap, that mean continue execution Capture::Trap(StackExecutorCreateInterrupt(TaggedRuntime { kind: RuntimeKind::Create(address), inner: MaybeBorrowed::Owned(runtime), @@ -958,19 +967,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> take_stipend: bool, context: Context, ) -> Capture<(ExitReason, Vec), StackExecutorCallInterrupt<'static>> { - macro_rules! try_or_fail { - ( $e:expr ) => { - match $e { - Ok(v) => v, - Err(e) => return Capture::Exit((e.into(), Vec::new())), - } - }; - } - - const fn l64(gas: u64) -> u64 { - gas - gas / 64 - } - event!(Call { code_address, transfer: &transfer, @@ -980,23 +976,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> context: &context, }); - let after_gas = if take_l64 && self.config.call_l64_after_gas { - if self.config.estimate { - let initial_after_gas = self.state.metadata().gasometer.gas(); - let diff = initial_after_gas - l64(initial_after_gas); - try_or_fail!(self.state.metadata_mut().gasometer.record_cost(diff)); - self.state.metadata().gasometer.gas() - } else { - l64(self.state.metadata().gasometer.gas()) - } - } else { - self.state.metadata().gasometer.gas() - }; - - let target_gas = target_gas.unwrap_or(after_gas); - let mut gas_limit = min(target_gas, after_gas); - - try_or_fail!(self.state.metadata_mut().gasometer.record_cost(gas_limit)); + let mut gas_limit = try_or_fail!(self.calc_gas_limit_and_record(target_gas, take_l64)); if let Some(transfer) = transfer.as_ref() { if take_stipend && transfer.value != U256::zero() { @@ -1016,6 +996,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } } + // Transfer funds if needed if let Some(transfer) = transfer { match self.state.transfer(transfer) { Ok(()) => (), @@ -1439,11 +1420,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler value: U256, init_code: Vec, target_gas: Option, - ) -> Capture<(ExitReason, Option, Vec), Self::CreateInterrupt> { + ) -> Capture<(ExitReason, Vec), Self::CreateInterrupt> { if let Err(e) = self.maybe_record_init_code_cost(&init_code) { let reason: ExitReason = e.into(); emit_exit!(reason.clone()); - return Capture::Exit((reason, None, Vec::new())); + return Capture::Exit((reason, Vec::new())); } self.create_inner(caller, scheme, value, init_code, target_gas, true) } @@ -1456,16 +1437,16 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler value: U256, init_code: Vec, target_gas: Option, - ) -> Capture<(ExitReason, Option, Vec), Self::CreateInterrupt> { + ) -> Capture<(ExitReason, Vec), Self::CreateInterrupt> { if let Err(e) = self.maybe_record_init_code_cost(&init_code) { let reason: ExitReason = e.into(); emit_exit!(reason.clone()); - return Capture::Exit((reason, None, Vec::new())); + return Capture::Exit((reason, Vec::new())); } let capture = self.create_inner(caller, scheme, value, init_code, target_gas, true); - if let Capture::Exit((ref reason, _, ref return_value)) = capture { + if let Capture::Exit((ref reason, ref return_value)) = capture { emit_exit!(reason, return_value); } From 7424b3ef62477510b95c5423b39deddcb6eceedc Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Mon, 14 Oct 2024 23:04:09 +0200 Subject: [PATCH 2/3] Added debug_assert, comments. Changed version to v0.46.0 --- Cargo.toml | 10 +++++----- runtime/src/eval/system.rs | 9 +++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a06c1717..9c698ed8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,10 +9,10 @@ keywords.workspace = true edition.workspace = true [workspace.dependencies] -evm = { version = "0.45.2", path = "." } -evm-core = { version = "0.45.2", path = "core", default-features = false } -evm-gasometer = { version = "0.45.2", path = "gasometer", default-features = false } -evm-runtime = { version = "0.45.2", path = "runtime", default-features = false } +evm = { version = "0.46.0", path = "." } +evm-core = { version = "0.46.0", path = "core", default-features = false } +evm-gasometer = { version = "0.46.0", path = "gasometer", default-features = false } +evm-runtime = { version = "0.46.0", path = "runtime", default-features = false } primitive-types = { version = "0.12", default-features = false } auto_impl = "1.0" sha3 = { version = "0.10", default-features = false } @@ -87,7 +87,7 @@ create-fixed = [] print-debug = ["evm-gasometer/print-debug"] [workspace.package] -version = "0.45.2" +version = "0.46.0" license = "Apache-2.0" authors = ["Aurora Labs ", "Wei Tang ", "Parity Technologies "] description = "Portable Ethereum Virtual Machine implementation written in pure Rust." diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index a4de7cf3..7d942391 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -449,6 +449,15 @@ pub fn create(runtime: &mut Runtime, is_create2: bool, handler: &mut match handler.create(runtime.context.address, scheme, value, code, None) { Capture::Exit((reason, return_data)) => { + // The `Capture::Exit` case used to always have `address: None` because it was only returned when + // the call to `create_inner` encountered an error and therefore no contract was created. The happy + // path is returning `Capture::Trap` and `finish_create` ends up being called later + // with the created address as part of the EVM call stack handling logic. + // And it means that the `Capture::Exit` case only happens if there was an error - `ExitError`. + debug_assert!( + reason.is_error(), + "ExitReason for finish_create should be only ExitError" + ); match super::finish_create(runtime, reason, None, return_data) { Ok(()) => Control::Continue, Err(e) => Control::Exit(e), From 456d5218fbdd653a9bc3d1a9a4cb83a039d7f814 Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Tue, 15 Oct 2024 12:15:42 +0200 Subject: [PATCH 3/3] Improve gas variable --- src/executor/stack/executor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index ad745e9b..91b99482 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -839,17 +839,17 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> target_gas: Option, take_l64: bool, ) -> Result { + let initial_after_gas = self.state.metadata().gasometer.gas(); let after_gas = if take_l64 && self.config.call_l64_after_gas { if self.config.estimate { - let initial_after_gas = self.state.metadata().gasometer.gas(); let diff = initial_after_gas - l64(initial_after_gas); self.state.metadata_mut().gasometer.record_cost(diff)?; - self.state.metadata().gasometer.gas() + initial_after_gas } else { - l64(self.state.metadata().gasometer.gas()) + l64(initial_after_gas) } } else { - self.state.metadata().gasometer.gas() + initial_after_gas }; let target_gas = target_gas.unwrap_or(after_gas); let gas_limit = min(target_gas, after_gas);