From ff716e77990fecd5a64ba80ddda98e8b412c899b Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 14 Aug 2018 05:27:13 +0800 Subject: [PATCH] Remove pass-by-reference return data value from executive (#9211) * Remove pass-by-reference return data value from executive * Fix tests * Fix a missing test output * typo: wasm_activation_test * Tracing change in output * json_tests: fix compile * typo: 0..32 -> ..32 to keep it consistent with other occurance * Fix tests --- ethcore/evm/src/evm.rs | 2 +- ethcore/evm/src/interpreter/mod.rs | 15 ++-- ethcore/src/client/evm_test_client.rs | 4 +- ethcore/src/executive.rs | 102 ++++++++++++++------------ ethcore/src/externalities.rs | 79 +++++++++----------- ethcore/src/json_tests/executive.rs | 13 ++-- ethcore/src/machine.rs | 13 ++-- ethcore/src/spec/spec.rs | 2 +- ethcore/src/tests/evm.rs | 15 ++-- ethcore/vm/src/ext.rs | 1 - ethcore/vm/src/tests.rs | 1 - ethcore/wasm/src/runtime.rs | 12 ++- 12 files changed, 137 insertions(+), 122 deletions(-) diff --git a/ethcore/evm/src/evm.rs b/ethcore/evm/src/evm.rs index 9d2ff0cb1..08b4b0981 100644 --- a/ethcore/evm/src/evm.rs +++ b/ethcore/evm/src/evm.rs @@ -45,7 +45,7 @@ impl Finalize for Result { fn finalize(self, ext: E) -> Result { match self { Ok(GasLeft::Known(gas_left)) => Ok(FinalizationResult { gas_left: gas_left, apply_state: true, return_data: ReturnData::empty() }), - Ok(GasLeft::NeedsReturn {gas_left, data, apply_state}) => ext.ret(&gas_left, &data, apply_state).map(|gas_left| FinalizationResult { + Ok(GasLeft::NeedsReturn { gas_left, data, apply_state }) => ext.ret(&gas_left, &data, apply_state).map(|gas_left| FinalizationResult { gas_left: gas_left, apply_state: apply_state, return_data: data, diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index fefa3a60b..b535b0833 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -523,20 +523,25 @@ impl Interpreter { } let call_result = { - // we need to write and read from memory in the same time - // and we don't want to copy - let input = unsafe { ::std::mem::transmute(self.mem.read_slice(in_off, in_size)) }; - let output = self.mem.writeable_slice(out_off, out_size); - ext.call(&call_gas.as_u256(), sender_address, receive_address, value, input, &code_address, output, call_type) + let input = self.mem.read_slice(in_off, in_size); + ext.call(&call_gas.as_u256(), sender_address, receive_address, value, input, &code_address, call_type) }; + let output = self.mem.writeable_slice(out_off, out_size); + return match call_result { MessageCallResult::Success(gas_left, data) => { + let len = cmp::min(output.len(), data.len()); + (&mut output[..len]).copy_from_slice(&data[..len]); + self.stack.push(U256::one()); self.return_data = data; Ok(InstructionResult::UnusedGas(Cost::from_u256(gas_left).expect("Gas left cannot be greater than current one"))) }, MessageCallResult::Reverted(gas_left, data) => { + let len = cmp::min(output.len(), data.len()); + (&mut output[..len]).copy_from_slice(&data[..len]); + self.stack.push(U256::zero()); self.return_data = data; Ok(InstructionResult::UnusedGas(Cost::from_u256(gas_left).expect("Gas left cannot be greater than current one"))) diff --git a/ethcore/src/client/evm_test_client.rs b/ethcore/src/client/evm_test_client.rs index ace761723..c54dc2a98 100644 --- a/ethcore/src/client/evm_test_client.rs +++ b/ethcore/src/client/evm_test_client.rs @@ -19,7 +19,7 @@ use std::fmt; use std::sync::Arc; use ethereum_types::{H256, U256, H160}; -use {factory, journaldb, trie, kvdb_memorydb, bytes}; +use {factory, journaldb, trie, kvdb_memorydb}; use kvdb::{self, KeyValueDB}; use {state, state_db, client, executive, trace, transaction, db, spec, pod_state, log_entry, receipt}; use factory::Factories; @@ -183,14 +183,12 @@ impl<'a> EvmTestClient<'a> { gas_limit: *genesis.gas_limit(), }; let mut substate = state::Substate::new(); - let mut output = vec![]; let machine = self.spec.engine.machine(); let schedule = machine.schedule(info.number); let mut executive = executive::Executive::new(&mut self.state, &info, &machine, &schedule); executive.call( params, &mut substate, - bytes::BytesRef::Flexible(&mut output), tracer, vm_tracer, ).map_err(EvmTestError::Evm) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 1d1a733ad..17d870fcb 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -204,7 +204,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { &'any mut self, origin_info: OriginInfo, substate: &'any mut Substate, - output: OutputPolicy<'any, 'any>, + output: OutputPolicy, tracer: &'any mut T, vm_tracer: &'any mut V, static_call: bool, @@ -312,8 +312,12 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { call_type: CallType::None, params_type: vm::ParamsType::Embedded, }; - let mut out = if output_from_create { Some(vec![]) } else { None }; - (self.create(params, &mut substate, &mut out, &mut tracer, &mut vm_tracer), out.unwrap_or_else(Vec::new)) + let res = self.create(params, &mut substate, &mut tracer, &mut vm_tracer); + let out = match &res { + Ok(res) if output_from_create => res.return_data.to_vec(), + _ => Vec::new(), + }; + (res, out) }, Action::Call(ref address) => { let params = ActionParams { @@ -330,8 +334,12 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { call_type: CallType::Call, params_type: vm::ParamsType::Separate, }; - let mut out = vec![]; - (self.call(params, &mut substate, BytesRef::Flexible(&mut out), &mut tracer, &mut vm_tracer), out) + let res = self.call(params, &mut substate, &mut tracer, &mut vm_tracer); + let out = match &res { + Ok(res) => res.return_data.to_vec(), + _ => Vec::new(), + }; + (res, out) } }; @@ -382,7 +390,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { &mut self, params: ActionParams, substate: &mut Substate, - mut output: BytesRef, tracer: &mut T, vm_tracer: &mut V ) -> vm::Result where T: Tracer, V: VMTracer { @@ -432,7 +439,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { Err(evm_err) } else { self.state.discard_checkpoint(); - output.write(0, &builtin_out_buffer); // Trace only top level calls and calls with balance transfer to builtins. The reason why we don't // trace all internal calls to builtin contracts is that memcpy (IDENTITY) is a heavily used @@ -444,7 +450,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { if self.depth == 0 || is_transferred { let mut trace_output = tracer.prepare_trace_output(); if let Some(out) = trace_output.as_mut() { - *out = output.to_owned(); + *out = builtin_out_buffer.clone(); } tracer.trace_call( @@ -485,7 +491,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let mut subvmtracer = vm_tracer.prepare_subtrace(params.code.as_ref().expect("scope is conditional on params.code.is_some(); qed")); let res = { - self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut()), &mut subtracer, &mut subvmtracer) + self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return, &mut subtracer, &mut subvmtracer) }; vm_tracer.done_subtrace(subvmtracer); @@ -494,12 +500,15 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let traces = subtracer.drain(); match res { - Ok(ref res) if res.apply_state => tracer.trace_call( - trace_info, - gas - res.gas_left, - trace_output, - traces - ), + Ok(ref res) if res.apply_state => { + trace_output.as_mut().map(|d| *d = res.return_data.to_vec()); + tracer.trace_call( + trace_info, + gas - res.gas_left, + trace_output, + traces + ); + }, Ok(_) => tracer.trace_failed_call(trace_info, traces, vm::Error::Reverted.into()), Err(ref e) => tracer.trace_failed_call(trace_info, traces, e.into()), }; @@ -530,7 +539,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { &mut self, params: ActionParams, substate: &mut Substate, - output: &mut Option, tracer: &mut T, vm_tracer: &mut V, ) -> vm::Result where T: Tracer, V: VMTracer { @@ -579,7 +587,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let res = self.exec_vm( params, &mut unconfirmed_substate, - OutputPolicy::InitContract(output.as_mut().or(trace_output.as_mut())), + OutputPolicy::InitContract, &mut subtracer, &mut subvmtracer ); @@ -587,13 +595,16 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { vm_tracer.done_subtrace(subvmtracer); match res { - Ok(ref res) if res.apply_state => tracer.trace_create( - trace_info, - gas - res.gas_left, - trace_output.map(|data| output.as_ref().map(|out| out.to_vec()).unwrap_or(data)), - created, - subtracer.drain() - ), + Ok(ref res) if res.apply_state => { + trace_output.as_mut().map(|trace| *trace = res.return_data.to_vec()); + tracer.trace_create( + trace_info, + gas - res.gas_left, + trace_output, + created, + subtracer.drain() + ); + } Ok(_) => tracer.trace_failed_create(trace_info, subtracer.drain(), vm::Error::Reverted.into()), Err(ref e) => tracer.trace_failed_create(trace_info, subtracer.drain(), e.into()) }; @@ -715,7 +726,6 @@ mod tests { use ethkey::{Generator, Random}; use super::*; use ethereum_types::{H256, U256, U512, Address}; - use bytes::BytesRef; use vm::{ActionParams, ActionValue, CallType, EnvInfo, CreateContractAddress}; use evm::{Factory, VMType}; use error::ExecutionError; @@ -766,7 +776,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; assert_eq!(gas_left, U256::from(79_975)); @@ -824,7 +834,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; assert_eq!(gas_left, U256::from(62_976)); @@ -868,8 +878,7 @@ mod tests { let mut vm_tracer = ExecutiveVMTracer::toplevel(); let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - let output = BytesRef::Fixed(&mut[0u8;0]); - ex.call(params, &mut substate, output, &mut tracer, &mut vm_tracer).unwrap(); + ex.call(params, &mut substate, &mut tracer, &mut vm_tracer).unwrap(); assert_eq!(tracer.drain(), vec![FlatTrace { action: trace::Action::Call(trace::Call { @@ -896,7 +905,7 @@ mod tests { call_type: CallType::Call }), result: trace::Res::Call(trace::CallResult { gas_used: 600.into(), - output: vec![] + output: vec![0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 156, 17, 133, 165, 197, 233, 252, 84, 97, 40, 8, 151, 126, 232, 245, 72, 178, 37, 141, 49] }), subtraces: 0, trace_address: vec![0].into_iter().collect(), @@ -954,8 +963,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - let output = BytesRef::Fixed(&mut[0u8;0]); - ex.call(params, &mut substate, output, &mut tracer, &mut vm_tracer).unwrap() + ex.call(params, &mut substate, &mut tracer, &mut vm_tracer).unwrap() }; assert_eq!(gas_left, U256::from(44_752)); @@ -1071,8 +1079,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - let output = BytesRef::Fixed(&mut[0u8;0]); - ex.call(params, &mut substate, output, &mut tracer, &mut vm_tracer).unwrap() + ex.call(params, &mut substate, &mut tracer, &mut vm_tracer).unwrap() }; assert_eq!(gas_left, U256::from(62967)); @@ -1144,7 +1151,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params.clone(), &mut substate, &mut None, &mut tracer, &mut vm_tracer).unwrap() + ex.create(params.clone(), &mut substate, &mut tracer, &mut vm_tracer).unwrap() }; assert_eq!(gas_left, U256::from(96_776)); @@ -1230,7 +1237,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; assert_eq!(gas_left, U256::from(62_976)); @@ -1282,7 +1289,7 @@ mod tests { { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer).unwrap(); + ex.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap(); } assert_eq!(substate.contracts_created.len(), 1); @@ -1343,7 +1350,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.call(params, &mut substate, BytesRef::Fixed(&mut []), &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; assert_eq!(gas_left, U256::from(73_237)); @@ -1388,7 +1395,7 @@ mod tests { let FinalizationResult { gas_left, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.call(params, &mut substate, BytesRef::Fixed(&mut []), &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; assert_eq!(gas_left, U256::from(59_870)); @@ -1562,7 +1569,7 @@ mod tests { let result = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer) + ex.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer) }; match result { @@ -1595,10 +1602,11 @@ mod tests { let mut substate = Substate::new(); let mut output = [0u8; 14]; - let FinalizationResult { gas_left: result, .. } = { + let FinalizationResult { gas_left: result, return_data, .. } = { let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.call(params, &mut substate, BytesRef::Fixed(&mut output), &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer).unwrap() }; + (&mut output).copy_from_slice(&return_data[..(cmp::min(14, return_data.len()))]); assert_eq!(result, U256::from(1)); assert_eq!(output[..], returns[..]); @@ -1638,11 +1646,12 @@ mod tests { let machine = ::ethereum::new_kovan_wasm_test_machine(); let mut output = [0u8; 20]; - let FinalizationResult { gas_left: result, .. } = { + let FinalizationResult { gas_left: result, return_data, .. } = { let schedule = machine.schedule(info.number); let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.call(params.clone(), &mut Substate::new(), BytesRef::Fixed(&mut output), &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.call(params.clone(), &mut Substate::new(), &mut NoopTracer, &mut NoopVMTracer).unwrap() }; + (&mut output).copy_from_slice(&return_data[..(cmp::min(20, return_data.len()))]); assert_eq!(result, U256::from(18433)); // Transaction successfully returned sender @@ -1652,11 +1661,12 @@ mod tests { info.number = 1; let mut output = [0u8; 20]; - let FinalizationResult { gas_left: result, .. } = { + let FinalizationResult { gas_left: result, return_data, .. } = { let schedule = machine.schedule(info.number); let mut ex = Executive::new(&mut state, &info, &machine, &schedule); - ex.call(params, &mut Substate::new(), BytesRef::Fixed(&mut output), &mut NoopTracer, &mut NoopVMTracer).unwrap() + ex.call(params, &mut Substate::new(), &mut NoopTracer, &mut NoopVMTracer).unwrap() }; + (&mut output[..((cmp::min(20, return_data.len())))]).copy_from_slice(&return_data[..(cmp::min(20, return_data.len()))]); assert_eq!(result, U256::from(20025)); // Since transaction errored due to wasm was not activated, result is just empty diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 68d2c0817..74fedd0fd 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -18,7 +18,7 @@ use std::cmp; use std::sync::Arc; use ethereum_types::{H256, U256, Address}; -use bytes::{Bytes, BytesRef}; +use bytes::Bytes; use state::{Backend as StateBackend, State, Substate, CleanupMode}; use machine::EthereumMachine as Machine; use executive::*; @@ -32,12 +32,12 @@ use transaction::UNSIGNED_SENDER; use trace::{Tracer, VMTracer}; /// Policy for handling output data on `RETURN` opcode. -pub enum OutputPolicy<'a, 'b> { +pub enum OutputPolicy { /// Return reference to fixed sized output. /// Used for message calls. - Return(BytesRef<'a>, Option<&'b mut Bytes>), + Return, /// Init new contract as soon as `RETURN` is called. - InitContract(Option<&'b mut Bytes>), + InitContract, } /// Transaction properties that externalities need to know about. @@ -71,7 +71,7 @@ pub struct Externalities<'a, T: 'a, V: 'a, B: 'a> { substate: &'a mut Substate, machine: &'a Machine, schedule: &'a Schedule, - output: OutputPolicy<'a, 'a>, + output: OutputPolicy, tracer: &'a mut T, vm_tracer: &'a mut V, static_flag: bool, @@ -89,7 +89,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Externalities<'a, T, V, B> depth: usize, origin_info: OriginInfo, substate: &'a mut Substate, - output: OutputPolicy<'a, 'a>, + output: OutputPolicy, tracer: &'a mut T, vm_tracer: &'a mut V, static_flag: bool, @@ -171,9 +171,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> params_type: vm::ParamsType::Separate, }; - let mut output = H256::new(); let mut ex = Executive::new(self.state, self.env_info, self.machine, self.schedule); - let r = ex.call(params, self.substate, BytesRef::Fixed(&mut output), self.tracer, self.vm_tracer); + let r = ex.call(params, self.substate, self.tracer, self.vm_tracer); + let output = match &r { + Ok(ref r) => H256::from(&r.return_data[..32]), + _ => H256::new(), + }; trace!("ext: blockhash contract({}) -> {:?}({}) self.env_info.number={}\n", number, r, output, self.env_info.number); output } else { @@ -194,7 +197,13 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> } } - fn create(&mut self, gas: &U256, value: &U256, code: &[u8], address_scheme: CreateContractAddress) -> ContractCreateResult { + fn create( + &mut self, + gas: &U256, + value: &U256, + code: &[u8], + address_scheme: CreateContractAddress + ) -> ContractCreateResult { // create new contract address let (address, code_hash) = match self.state.nonce(&self.origin_info.address) { Ok(nonce) => contract_address(address_scheme, &self.origin_info.address, &nonce, &code), @@ -231,7 +240,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> let mut ex = Executive::from_parent(self.state, self.env_info, self.machine, self.schedule, self.depth, self.static_flag); // TODO: handle internal error separately - match ex.create(params, self.substate, &mut None, self.tracer, self.vm_tracer) { + match ex.create(params, self.substate, self.tracer, self.vm_tracer) { Ok(FinalizationResult{ gas_left, apply_state: true, .. }) => { self.substate.contracts_created.push(address.clone()); ContractCreateResult::Created(address, gas_left) @@ -243,14 +252,14 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> } } - fn call(&mut self, + fn call( + &mut self, gas: &U256, sender_address: &Address, receive_address: &Address, value: Option, data: &[u8], code_address: &Address, - output: &mut [u8], call_type: CallType ) -> MessageCallResult { trace!(target: "externalities", "call"); @@ -284,7 +293,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> let mut ex = Executive::from_parent(self.state, self.env_info, self.machine, self.schedule, self.depth, self.static_flag); - match ex.call(params, self.substate, BytesRef::Fixed(output), self.tracer, self.vm_tracer) { + match ex.call(params, self.substate, self.tracer, self.vm_tracer) { Ok(FinalizationResult{ gas_left, return_data, apply_state: true }) => MessageCallResult::Success(gas_left, return_data), Ok(FinalizationResult{ gas_left, return_data, apply_state: false }) => MessageCallResult::Reverted(gas_left, return_data), _ => MessageCallResult::Failed @@ -303,27 +312,13 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> Ok(self.state.code_size(address)?) } - fn ret(mut self, gas: &U256, data: &ReturnData, apply_state: bool) -> vm::Result + fn ret(self, gas: &U256, data: &ReturnData, apply_state: bool) -> vm::Result where Self: Sized { - let handle_copy = |to: &mut Option<&mut Bytes>| { - to.as_mut().map(|b| **b = data.to_vec()); - }; match self.output { - OutputPolicy::Return(BytesRef::Fixed(ref mut slice), ref mut copy) => { - handle_copy(copy); - - let len = cmp::min(slice.len(), data.len()); - (&mut slice[..len]).copy_from_slice(&data[..len]); + OutputPolicy::Return => { Ok(*gas) }, - OutputPolicy::Return(BytesRef::Flexible(ref mut vec), ref mut copy) => { - handle_copy(copy); - - vec.clear(); - vec.extend_from_slice(&*data); - Ok(*gas) - }, - OutputPolicy::InitContract(ref mut copy) if apply_state => { + OutputPolicy::InitContract if apply_state => { let return_cost = U256::from(data.len()) * U256::from(self.schedule.create_data_gas); if return_cost > *gas || data.len() > self.schedule.create_data_limit { return match self.schedule.exceptional_failed_code_deposit { @@ -331,11 +326,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> false => Ok(*gas) } } - handle_copy(copy); self.state.init_code(&self.origin_info.address, data.to_vec())?; Ok(*gas - return_cost) }, - OutputPolicy::InitContract(_) => { + OutputPolicy::InitContract => { Ok(*gas) }, } @@ -479,7 +473,7 @@ mod tests { let mut tracer = NoopTracer; let mut vm_tracer = NoopVMTracer; - let ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); assert_eq!(ext.env_info().number, 100); } @@ -491,7 +485,7 @@ mod tests { let mut tracer = NoopTracer; let mut vm_tracer = NoopVMTracer; - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); let hash = ext.blockhash(&"0000000000000000000000000000000000000000000000000000000000120000".parse::().unwrap()); @@ -515,7 +509,7 @@ mod tests { let mut tracer = NoopTracer; let mut vm_tracer = NoopVMTracer; - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); let hash = ext.blockhash(&"0000000000000000000000000000000000000000000000000000000000120000".parse::().unwrap()); @@ -530,9 +524,7 @@ mod tests { let mut tracer = NoopTracer; let mut vm_tracer = NoopVMTracer; - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); - - let mut output = vec![]; + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); // this should panic because we have no balance on any account ext.call( @@ -542,7 +534,6 @@ mod tests { Some("0000000000000000000000000000000000000000000000000000000000150000".parse::().unwrap()), &[], &Address::new(), - &mut output, CallType::Call ); } @@ -558,7 +549,7 @@ mod tests { let mut vm_tracer = NoopVMTracer; { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); ext.log(log_topics, &log_data).unwrap(); } @@ -575,7 +566,7 @@ mod tests { let mut vm_tracer = NoopVMTracer; { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); ext.suicide(refund_account).unwrap(); } @@ -592,7 +583,7 @@ mod tests { let mut vm_tracer = NoopVMTracer; let address = { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderAndNonce) { ContractCreateResult::Created(address, _) => address, _ => panic!("Test create failed; expected Created, got Failed/Reverted."), @@ -612,8 +603,8 @@ mod tests { let mut vm_tracer = NoopVMTracer; let address = { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); - + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract, &mut tracer, &mut vm_tracer, false); + match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderSaltAndCodeHash(H256::default())) { ContractCreateResult::Created(address, _) => address, _ => panic!("Test create failed; expected Created, got Failed/Reverted."), diff --git a/ethcore/src/json_tests/executive.rs b/ethcore/src/json_tests/executive.rs index 3a04bbe5e..20e7ebb20 100644 --- a/ethcore/src/json_tests/executive.rs +++ b/ethcore/src/json_tests/executive.rs @@ -30,7 +30,7 @@ use test_helpers::get_temp_state; use ethjson; use trace::{Tracer, NoopTracer}; use trace::{VMTracer, NoopVMTracer}; -use bytes::{Bytes, BytesRef}; +use bytes::Bytes; use ethtrie; use rlp::RlpStream; use hash::keccak; @@ -90,7 +90,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> TestExt<'a, T, V, B> depth: usize, origin_info: OriginInfo, substate: &'a mut Substate, - output: OutputPolicy<'a, 'a>, + output: OutputPolicy, address: Address, tracer: &'a mut T, vm_tracer: &'a mut V, @@ -154,7 +154,6 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> value: Option, data: &[u8], _code_address: &Address, - _output: &mut [u8], _call_type: CallType ) -> MessageCallResult { self.callcreates.push(CallCreate { @@ -262,7 +261,6 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8] let mut substate = Substate::new(); let mut tracer = NoopTracer; let mut vm_tracer = NoopVMTracer; - let mut output = vec![]; let vm_factory = state.vm_factory(); // execute @@ -276,7 +274,7 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8] 0, OriginInfo::from(¶ms), &mut substate, - OutputPolicy::Return(BytesRef::Flexible(&mut output), None), + OutputPolicy::Return, params.address.clone(), &mut tracer, &mut vm_tracer, @@ -288,6 +286,11 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8] (res.finalize(ex), callcreates) }; + let output = match &res { + Ok(res) => res.return_data.to_vec(), + Err(_) => Vec::new(), + }; + let log_hash = { let mut rlp = RlpStream::new_list(substate.logs.len()); for l in &substate.logs { diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index d487fff9b..fdeed4c8e 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -33,7 +33,6 @@ use transaction::{self, SYSTEM_ADDRESS, UnverifiedTransaction, SignedTransaction use tx_filter::TransactionFilter; use ethereum_types::{U256, Address}; -use bytes::BytesRef; use rlp::Rlp; use vm::{CallType, ActionParams, ActionValue, ParamsType}; use vm::{EnvInfo, Schedule, CreateContractAddress}; @@ -148,10 +147,14 @@ impl EthereumMachine { let schedule = self.schedule(env_info.number); let mut ex = Executive::new(&mut state, &env_info, self, &schedule); let mut substate = Substate::new(); - let mut output = Vec::new(); - if let Err(e) = ex.call(params, &mut substate, BytesRef::Flexible(&mut output), &mut NoopTracer, &mut NoopVMTracer) { - warn!("Encountered error on making system call: {}", e); - } + let res = ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer); + let output = match res { + Ok(res) => res.return_data.to_vec(), + Err(e) => { + warn!("Encountered error on making system call: {}", e); + Vec::new() + } + }; Ok(output) } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index e842835f7..bcfa64ed1 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -655,7 +655,7 @@ impl Spec { let machine = self.engine.machine(); let schedule = machine.schedule(env_info.number); let mut exec = Executive::new(&mut state, &env_info, &machine, &schedule); - if let Err(e) = exec.create(params, &mut substate, &mut None, &mut NoopTracer, &mut NoopVMTracer) { + if let Err(e) = exec.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer) { warn!(target: "spec", "Genesis constructor execution at {} failed: {}.", address, e); } } diff --git a/ethcore/src/tests/evm.rs b/ethcore/src/tests/evm.rs index 239905fac..7befa47f6 100644 --- a/ethcore/src/tests/evm.rs +++ b/ethcore/src/tests/evm.rs @@ -29,7 +29,6 @@ use transaction::SYSTEM_ADDRESS; use rustc_hex::FromHex; use ethereum_types::{H256, Address}; -use bytes::BytesRef; evm_test!{test_blockhash_eip210: test_blockhash_eip210_int} fn test_blockhash_eip210(factory: Factory) { @@ -65,8 +64,7 @@ fn test_blockhash_eip210(factory: Factory) { let schedule = machine.schedule(env_info.number); let mut ex = Executive::new(&mut state, &env_info, &machine, &schedule); let mut substate = Substate::new(); - let mut output = []; - if let Err(e) = ex.call(params, &mut substate, BytesRef::Fixed(&mut output), &mut NoopTracer, &mut NoopVMTracer) { + if let Err(e) = ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer) { panic!("Encountered error on updating last hashes: {}", e); } } @@ -89,9 +87,12 @@ fn test_blockhash_eip210(factory: Factory) { let schedule = machine.schedule(env_info.number); let mut ex = Executive::new(&mut state, &env_info, &machine, &schedule); let mut substate = Substate::new(); - let mut output = H256::new(); - if let Err(e) = ex.call(params, &mut substate, BytesRef::Fixed(&mut output), &mut NoopTracer, &mut NoopVMTracer) { - panic!("Encountered error on getting last hash: {}", e); - } + let res = ex.call(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer); + let output = match res { + Ok(res) => H256::from(&res.return_data[..32]), + Err(e) => { + panic!("Encountered error on getting last hash: {}", e); + }, + }; assert_eq!(output, 255.into()); } diff --git a/ethcore/vm/src/ext.rs b/ethcore/vm/src/ext.rs index c1ce1b79f..1eb696f97 100644 --- a/ethcore/vm/src/ext.rs +++ b/ethcore/vm/src/ext.rs @@ -101,7 +101,6 @@ pub trait Ext { value: Option, data: &[u8], code_address: &Address, - output: &mut [u8], call_type: CallType ) -> MessageCallResult; diff --git a/ethcore/vm/src/tests.rs b/ethcore/vm/src/tests.rs index 4930e4219..3dac74484 100644 --- a/ethcore/vm/src/tests.rs +++ b/ethcore/vm/src/tests.rs @@ -155,7 +155,6 @@ impl Ext for FakeExt { value: Option, data: &[u8], code_address: &Address, - _output: &mut [u8], _call_type: CallType ) -> MessageCallResult { diff --git a/ethcore/wasm/src/runtime.rs b/ethcore/wasm/src/runtime.rs index 347023dd9..1c814ab7c 100644 --- a/ethcore/wasm/src/runtime.rs +++ b/ethcore/wasm/src/runtime.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::cmp; use ethereum_types::{U256, H256, Address}; use vm::{self, CallType}; use wasmi::{self, MemoryRef, RuntimeArgs, RuntimeValue, Error as InterpreterError, Trap, TrapKind}; @@ -447,12 +448,14 @@ impl<'a> Runtime<'a> { val, &payload, &address, - &mut result[..], call_type, ); match call_result { - vm::MessageCallResult::Success(gas_left, _) => { + vm::MessageCallResult::Success(gas_left, data) => { + let len = cmp::min(result.len(), data.len()); + (&mut result[..len]).copy_from_slice(&data[..len]); + // cannot overflow, before making call gas_counter was incremented with gas, and gas_left < gas self.gas_counter = self.gas_counter - gas_left.low_u64() * self.ext.schedule().wasm().opcodes_div as u64 @@ -461,7 +464,10 @@ impl<'a> Runtime<'a> { self.memory.set(result_ptr, &result)?; Ok(0i32.into()) }, - vm::MessageCallResult::Reverted(gas_left, _) => { + vm::MessageCallResult::Reverted(gas_left, data) => { + let len = cmp::min(result.len(), data.len()); + (&mut result[..len]).copy_from_slice(&data[..len]); + // cannot overflow, before making call gas_counter was incremented with gas, and gas_left < gas self.gas_counter = self.gas_counter - gas_left.low_u64() * self.ext.schedule().wasm().opcodes_div as u64