Remove pass-by-reference return data value from executive

ethcore/evm/src/evm.rs

@@ -45,7 +45,7 @@ impl Finalize for Result<GasLeft> {
 	fn finalize<E: Ext>(self, ext: E) -> Result<FinalizationResult> {
 		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,

ethcore/evm/src/interpreter/mod.rs

@@ -409,20 +409,25 @@ impl<Cost: CostType> Interpreter<Cost> {
 				}
 
 				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]);
+
 						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]);
+
 						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")))

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)

ethcore/src/executive.rs

@@ -202,7 +202,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,
@@ -310,8 +310,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 {
@@ -328,8 +332,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)
 			}
 		};
 
@@ -381,7 +389,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<FinalizationResult> where T: Tracer, V: VMTracer {
@@ -431,7 +438,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
@@ -443,7 +449,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(
@@ -484,7 +490,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);
@@ -493,12 +499,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()),
 				};
@@ -529,7 +538,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 		&mut self,
 		params: ActionParams,
 		substate: &mut Substate,
-		output: &mut Option<Bytes>,
 		tracer: &mut T,
 		vm_tracer: &mut V,
 	) -> vm::Result<FinalizationResult> where T: Tracer, V: VMTracer {
@@ -578,21 +586,24 @@ 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
 		);
 
 		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())
 		};
@@ -714,7 +725,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;
@@ -765,7 +775,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));
@@ -823,7 +833,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));
@@ -867,8 +877,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 {
@@ -895,7 +904,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(),
@@ -953,8 +962,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));
@@ -1070,8 +1078,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));
@@ -1143,7 +1150,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));
@@ -1229,7 +1236,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));
@@ -1281,7 +1288,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);
@@ -1342,7 +1349,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));
@@ -1387,7 +1394,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));
@@ -1561,7 +1568,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 {
@@ -1594,10 +1601,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[..]);
@@ -1637,11 +1645,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
@@ -1651,11 +1660,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