diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 49c8dde73..615634d32 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -36,10 +36,12 @@ jobs: - name: Run tests run: | cargo run --release --verbose -p jsontests -- \ + jsontests/res/ethtests/GeneralStateTests/stCodeCopyTest/ \ jsontests/res/ethtests/GeneralStateTests/stExample/ \ jsontests/res/ethtests/GeneralStateTests/stSLoadTest/ \ jsontests/res/ethtests/GeneralStateTests/VMTests/vmArithmeticTest/ \ jsontests/res/ethtests/GeneralStateTests/VMTests/vmBitwiseLogicOperation/ \ jsontests/res/ethtests/GeneralStateTests/VMTests/vmIOandFlowOperations/ \ - jsontests/res/ethtests/GeneralStateTests/VMTests/vmLogTest/ + jsontests/res/ethtests/GeneralStateTests/VMTests/vmLogTest/ \ + jsontests/res/ethtests/GeneralStateTests/VMTests/vmTests/ diff --git a/interpreter/src/eval/misc.rs b/interpreter/src/eval/misc.rs index df8365e35..32a642def 100644 --- a/interpreter/src/eval/misc.rs +++ b/interpreter/src/eval/misc.rs @@ -207,6 +207,7 @@ pub fn ret(state: &mut Machine) -> Control { pop_u256!(state, start, len); try_or_fail!(state.memory.resize_offset(start, len)); state.memory.resize_to_range(start..(start + len)); + state.memory.swap_and_clear(&mut state.retval); Control::Exit(ExitSucceed::Returned.into()) } @@ -215,5 +216,6 @@ pub fn revert(state: &mut Machine) -> Control { pop_u256!(state, start, len); try_or_fail!(state.memory.resize_offset(start, len)); state.memory.resize_to_range(start..(start + len)); + state.memory.swap_and_clear(&mut state.retval); Control::Exit(ExitError::Reverted.into()) } diff --git a/interpreter/src/eval/system.rs b/interpreter/src/eval/system.rs index 90fad99c7..c74dd0e87 100644 --- a/interpreter/src/eval/system.rs +++ b/interpreter/src/eval/system.rs @@ -1,7 +1,7 @@ use super::Control; use crate::{ ExitException, ExitFatal, ExitSucceed, Log, Machine, RuntimeBackend, RuntimeEnvironment, - RuntimeState, + RuntimeState, Transfer, }; use alloc::vec::Vec; use primitive_types::{H256, U256}; @@ -47,7 +47,7 @@ pub fn balance, H: RuntimeEnvironment + RuntimeBackend, T handler: &mut H, ) -> Control { pop!(machine, address); - try_or_fail!(handler.mark_hot(address.into(), None)); + handler.mark_hot(address.into(), None); push_u256!(machine, handler.balance(address.into())); Control::Continue @@ -127,7 +127,7 @@ pub fn extcodesize, H: RuntimeEnvironment + RuntimeBacken handler: &mut H, ) -> Control { pop!(machine, address); - try_or_fail!(handler.mark_hot(address.into(), None)); + handler.mark_hot(address.into(), None); let code_size = handler.code_size(address.into()); push_u256!(machine, code_size); @@ -139,7 +139,7 @@ pub fn extcodehash, H: RuntimeEnvironment + RuntimeBacken handler: &mut H, ) -> Control { pop!(machine, address); - try_or_fail!(handler.mark_hot(address.into(), None)); + handler.mark_hot(address.into(), None); let code_hash = handler.code_hash(address.into()); push!(machine, code_hash); @@ -153,7 +153,7 @@ pub fn extcodecopy, H: RuntimeEnvironment + RuntimeBacken pop!(machine, address); pop_u256!(machine, memory_offset, code_offset, len); - try_or_fail!(handler.mark_hot(address.into(), None)); + handler.mark_hot(address.into(), None); try_or_fail!(machine.memory.resize_offset(memory_offset, len)); let code = handler.code(address.into()); @@ -265,7 +265,7 @@ pub fn sload, H: RuntimeEnvironment + RuntimeBackend, Tr> handler: &mut H, ) -> Control { pop!(machine, index); - try_or_fail!(handler.mark_hot(machine.state.as_ref().context.address, Some(index))); + handler.mark_hot(machine.state.as_ref().context.address, Some(index)); let value = handler.storage(machine.state.as_ref().context.address, index); push!(machine, value); @@ -277,7 +277,7 @@ pub fn sstore, H: RuntimeEnvironment + RuntimeBackend, Tr handler: &mut H, ) -> Control { pop!(machine, index, value); - try_or_fail!(handler.mark_hot(machine.state.as_ref().context.address, Some(index))); + handler.mark_hot(machine.state.as_ref().context.address, Some(index)); match handler.set_storage(machine.state.as_ref().context.address, index, value) { Ok(()) => Control::Continue, @@ -335,12 +335,23 @@ pub fn suicide, H: RuntimeEnvironment + RuntimeBackend, T machine: &mut Machine, handler: &mut H, ) -> Control { - pop!(machine, target); + let address = machine.state.as_ref().context.address; - match handler.mark_delete(machine.state.as_ref().context.address, target.into()) { - Ok(()) => (), - Err(e) => return Control::Exit(e.into()), - } + match machine.stack.perform_pop1_push0(|target| { + let balance = handler.balance(address); + + handler.transfer(Transfer { + source: address, + target: (*target).into(), + value: balance, + })?; - Control::Exit(ExitSucceed::Suicided.into()) + handler.mark_delete(address); + handler.reset_balance(address); + + Ok(((), ())) + }) { + Ok(()) => Control::Exit(ExitSucceed::Suicided.into()), + Err(e) => Control::Exit(Err(e)), + } } diff --git a/interpreter/src/lib.rs b/interpreter/src/lib.rs index d0e39455a..927d3933d 100644 --- a/interpreter/src/lib.rs +++ b/interpreter/src/lib.rs @@ -40,6 +40,10 @@ pub struct Machine { position: usize, /// Code validity maps. valids: Valids, + /// Return value. Note the difference between `retbuf`. + /// A `retval` holds what's returned by the current machine, with `RETURN` or `REVERT` opcode. + /// A `retbuf` holds the buffer of returned value by sub-calls. + pub retval: Vec, /// Memory. pub memory: Memory, /// Stack. @@ -69,6 +73,7 @@ impl Machine { code, position: 0, valids, + retval: Vec::new(), memory: Memory::new(memory_limit), stack: Stack::new(stack_limit), state, @@ -93,11 +98,6 @@ impl Machine { self.position = self.code.len(); } - /// Return value of the machine. - pub fn into_retbuf(self) -> Vec { - self.memory.into_data() - } - /// Inspect the machine's next opcode and current stack. pub fn inspect(&self) -> Option<(Opcode, &Stack)> { self.code diff --git a/interpreter/src/memory.rs b/interpreter/src/memory.rs index c2cdd8400..c363a37ed 100644 --- a/interpreter/src/memory.rs +++ b/interpreter/src/memory.rs @@ -1,7 +1,7 @@ use crate::{ExitException, ExitFatal}; use alloc::vec::Vec; -use core::cmp::min; use core::ops::{BitAnd, Not, Range}; +use core::{cmp::min, mem}; use primitive_types::U256; /// A sequencial memory. It uses Rust's `Vec` for internal @@ -48,9 +48,10 @@ impl Memory { &self.data } - /// Into the full data. - pub fn into_data(self) -> Vec { - self.data + pub(crate) fn swap_and_clear(&mut self, other: &mut Vec) { + mem::swap(&mut self.data, other); + self.data = Vec::new(); + self.effective_len = U256::zero(); } /// Resize the memory, making it cover the memory region of `offset..(offset diff --git a/interpreter/src/runtime.rs b/interpreter/src/runtime.rs index 3da06c1d7..aa5b30eb0 100644 --- a/interpreter/src/runtime.rs +++ b/interpreter/src/runtime.rs @@ -133,13 +133,13 @@ pub trait RuntimeBackend: RuntimeBaseBackend { } /// Mark an address or (address, index) pair as hot. - fn mark_hot(&mut self, address: H160, index: Option) -> Result<(), ExitError>; + fn mark_hot(&mut self, address: H160, index: Option); /// Set storage value of address at index. fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError>; /// Create a log owned by address with given topics and data. fn log(&mut self, log: Log) -> Result<(), ExitError>; - /// Mark an address to be deleted, with funds transferred to target. - fn mark_delete(&mut self, address: H160, target: H160) -> Result<(), ExitError>; + /// Mark an address to be deleted. + fn mark_delete(&mut self, address: H160); /// Fully delete storages of an account. fn reset_storage(&mut self, address: H160); /// Set code of an account. diff --git a/interpreter/tests/performance.rs b/interpreter/tests/performance.rs index 4a97f2147..acbb7cebf 100644 --- a/interpreter/tests/performance.rs +++ b/interpreter/tests/performance.rs @@ -15,7 +15,7 @@ macro_rules! ret_test { vm.run(&mut (), &ETABLE), Capture::Exit(Ok(ExitSucceed::Returned.into())) ); - assert_eq!(vm.into_retbuf(), hex::decode($ret).unwrap()); + assert_eq!(vm.retval, hex::decode($ret).unwrap()); } }; } diff --git a/interpreter/tests/usability.rs b/interpreter/tests/usability.rs index 4a28da8da..c6aa6da5a 100644 --- a/interpreter/tests/usability.rs +++ b/interpreter/tests/usability.rs @@ -25,7 +25,7 @@ fn etable_wrap() { let mut vm = Machine::new(Rc::new(code), Rc::new(data), 1024, 10000, ()); let result = vm.run(&mut (), &wrapped_etable); assert_eq!(result, Capture::Exit(Ok(ExitSucceed::Returned.into()))); - assert_eq!(vm.into_retbuf(), hex::decode(&RET1).unwrap()); + assert_eq!(vm.retval, hex::decode(&RET1).unwrap()); } #[test] @@ -125,7 +125,7 @@ impl<'a> RuntimeBackend for UnimplementedHandler { unimplemented!() } - fn mark_hot(&mut self, _address: H160, _index: Option) -> Result<(), ExitError> { + fn mark_hot(&mut self, _address: H160, _index: Option) { unimplemented!() } @@ -135,7 +135,7 @@ impl<'a> RuntimeBackend for UnimplementedHandler { fn log(&mut self, _log: Log) -> Result<(), ExitError> { unimplemented!() } - fn mark_delete(&mut self, _address: H160, _target: H160) -> Result<(), ExitError> { + fn mark_delete(&mut self, _address: H160) { unimplemented!() } @@ -194,5 +194,5 @@ fn etable_runtime() { let res = vm.run(&mut handler, &RUNTIME_ETABLE).exit().unwrap(); assert_eq!(res, Ok(ExitSucceed::Returned.into())); - assert_eq!(vm.into_retbuf(), hex::decode(&RET1).unwrap()); + assert_eq!(vm.retval, hex::decode(&RET1).unwrap()); } diff --git a/jsontests/src/run.rs b/jsontests/src/run.rs index 6ab3df142..75a01afaa 100644 --- a/jsontests/src/run.rs +++ b/jsontests/src/run.rs @@ -94,6 +94,7 @@ pub fn run_test(_filename: &str, _test_name: &str, test: Test, debug: bool) -> R &invoker, &etable, ); + run_backend.layers[0].clear_pending(); // Step if debug { @@ -116,6 +117,7 @@ pub fn run_test(_filename: &str, _test_name: &str, test: Test, debug: bool) -> R break result; } }); + step_backend.layers[0].clear_pending(); } let state_root = crate::hash::state_root(&run_backend); diff --git a/src/backend/in_memory.rs b/src/backend/in_memory.rs index c63441118..c1b8554b8 100644 --- a/src/backend/in_memory.rs +++ b/src/backend/in_memory.rs @@ -4,6 +4,7 @@ use crate::{ }; use alloc::collections::{BTreeMap, BTreeSet}; use primitive_types::{H160, H256, U256}; +use std::mem; #[derive(Clone, Debug)] pub struct InMemoryEnvironment { @@ -30,7 +31,6 @@ pub struct InMemoryAccount { #[derive(Clone, Debug)] pub struct InMemorySuicideInfo { pub address: H160, - pub target: H160, } #[derive(Clone, Debug)] @@ -41,6 +41,18 @@ pub struct InMemoryLayer { pub hots: BTreeSet<(H160, Option)>, } +impl InMemoryLayer { + pub fn clear_pending(&mut self) { + self.hots.clear(); + + let mut suicides = Vec::new(); + mem::swap(&mut suicides, &mut self.suicides); + for suicide in suicides { + self.state.remove(&suicide.address); + } + } +} + #[derive(Clone, Debug)] pub struct InMemoryBackend { pub environment: InMemoryEnvironment, @@ -168,9 +180,8 @@ impl RuntimeBackend for InMemoryBackend { !self.current_layer().hots.contains(&(address, index)) } - fn mark_hot(&mut self, address: H160, index: Option) -> Result<(), ExitError> { + fn mark_hot(&mut self, address: H160, index: Option) { self.current_layer_mut().hots.insert((address, index)); - Ok(()) } fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError> { @@ -189,11 +200,10 @@ impl RuntimeBackend for InMemoryBackend { Ok(()) } - fn mark_delete(&mut self, address: H160, target: H160) -> Result<(), ExitError> { + fn mark_delete(&mut self, address: H160) { self.current_layer_mut() .suicides - .push(InMemorySuicideInfo { address, target }); - Ok(()) + .push(InMemorySuicideInfo { address }); } fn reset_storage(&mut self, address: H160) { diff --git a/src/gasometer.rs b/src/gasometer.rs index d64203f2b..3b9e1329d 100644 --- a/src/gasometer.rs +++ b/src/gasometer.rs @@ -37,7 +37,12 @@ pub trait Gasometer: Sized { } fn record_codedeposit(&mut self, len: usize) -> Result<(), ExitError>; fn gas(&self) -> U256; - fn submeter(&mut self, gas_limit: U256, code: &[u8]) -> Result; + fn submeter( + &mut self, + gas_limit: U256, + call_has_value: bool, + code: &[u8], + ) -> Result; fn merge(&mut self, other: Self, strategy: MergeStrategy); } diff --git a/src/standard/gasometer/mod.rs b/src/standard/gasometer/mod.rs index 9b6333974..fe3e69a17 100644 --- a/src/standard/gasometer/mod.rs +++ b/src/standard/gasometer/mod.rs @@ -203,14 +203,24 @@ impl<'config, S: AsRef, H: RuntimeBackend> GasometerT for Ga self.gas().into() } - fn submeter(&mut self, gas_limit: U256, code: &[u8]) -> Result { - let gas_limit = if gas_limit > U256::from(u64::MAX) { + fn submeter( + &mut self, + gas_limit: U256, + call_has_value: bool, + code: &[u8], + ) -> Result { + let mut gas_limit = if gas_limit > U256::from(u64::MAX) { return Err(ExitException::OutOfGas.into()); } else { gas_limit.as_u64() }; self.record_cost(gas_limit)?; + + if call_has_value { + gas_limit = gas_limit.saturating_add(self.config.call_stipend); + } + Ok(Self::new(gas_limit, code, self.config)) } diff --git a/src/standard/invoker/mod.rs b/src/standard/invoker/mod.rs index 8d7985a72..565208b3c 100644 --- a/src/standard/invoker/mod.rs +++ b/src/standard/invoker/mod.rs @@ -194,9 +194,9 @@ where .. } => { for (address, keys) in &access_list { - handler.mark_hot(*address, None)?; + handler.mark_hot(*address, None); for key in keys { - handler.mark_hot(*address, Some(*key))?; + handler.mark_hot(*address, Some(*key)); } } @@ -224,10 +224,10 @@ where if self.config.increase_state_access_gas { if self.config.warm_coinbase_address { let coinbase = handler.block_coinbase(); - handler.mark_hot(coinbase, None)?; + handler.mark_hot(coinbase, None); } - handler.mark_hot(caller, None)?; - handler.mark_hot(address, None)?; + handler.mark_hot(caller, None); + handler.mark_hot(address, None); } handler.inc_nonce(caller)?; @@ -286,7 +286,7 @@ where let work = || -> Result { if result.is_ok() { if let Some(address) = invoke.create_address { - let retbuf = machine.machine.into_retbuf(); + let retbuf = machine.machine.retval; routines::deploy_create_code( self, @@ -356,14 +356,12 @@ where machine.gasometer.gas() }; let target_gas = trap_data.target_gas().unwrap_or(after_gas); - let mut gas_limit = min(after_gas, target_gas); + let gas_limit = min(after_gas, target_gas); - match &trap_data { - CallCreateTrapData::Call(call) if call.has_value() => { - gas_limit = gas_limit.saturating_add(U256::from(self.config.call_stipend)); - } - _ => (), - } + let call_has_value = match &trap_data { + CallCreateTrapData::Call(call) if call.has_value() => true, + _ => false, + }; let is_static = if machine.is_static { true @@ -377,7 +375,7 @@ where let transaction_context = machine.machine.state.as_ref().transaction_context.clone(); let code = trap_data.code(handler); - let submeter = match machine.gasometer.submeter(gas_limit, &code) { + let submeter = match machine.gasometer.submeter(gas_limit, call_has_value, &code) { Ok(submeter) => submeter, Err(err) => return Capture::Exit(Err(err)), }; @@ -444,7 +442,7 @@ where match trap_data { SubstackInvoke::Create { address, trap } => { - let retbuf = child.machine.memory.into_data(); + let retbuf = child.machine.retval; parent.machine.state.merge(child.machine.state, strategy); let mut child_gasometer = child.gasometer; @@ -470,7 +468,7 @@ where SubstackInvoke::Call { trap } => { parent.machine.state.merge(child.machine.state, strategy); - let retbuf = child.machine.memory.into_data(); + let retbuf = child.machine.retval; handler.pop_substate(strategy); GasometerT::::merge(&mut parent.gasometer, child.gasometer, strategy); diff --git a/src/standard/invoker/routines.rs b/src/standard/invoker/routines.rs index edea34817..362dac86e 100644 --- a/src/standard/invoker/routines.rs +++ b/src/standard/invoker/routines.rs @@ -22,7 +22,7 @@ where G: GasometerT, H: RuntimeEnvironment + RuntimeBackend + TransactionalBackend, { - handler.mark_hot(state.as_ref().context.address, None)?; + handler.mark_hot(state.as_ref().context.address, None); if let Some(transfer) = transfer { handler.transfer(transfer)?; @@ -66,8 +66,8 @@ where } } - handler.mark_hot(caller, None)?; - handler.mark_hot(state.as_ref().context.address, None)?; + handler.mark_hot(caller, None); + handler.mark_hot(state.as_ref().context.address, None); handler.transfer(transfer)?;