Remove prepare_trace_output and make sure prepare_trace_call and trace*call are balanced (#9353)

This refactors `prepare_trace_output` to instead directly take the reference of return values, so that it's simpler and we save a stack item. This should also fixes [the issue](https://github.com/paritytech/parity-ethereum/pull/9236#issuecomment-408444995) @udoprog is facing. Replaces #9236

ethcore/src/executive.rs

@@ -423,8 +423,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 			let default = [];
 			let data = if let Some(ref d) = params.data { d as &[u8] } else { &default as &[u8] };
 
-			let trace_info = tracer.prepare_trace_call(&params);
-
 			let cost = builtin.cost(data);
 			if cost <= params.gas {
 				let mut builtin_out_buffer = Vec::new();
@@ -435,7 +433,12 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 				if let Err(e) = result {
 					self.state.revert_to_checkpoint();
 					let evm_err: vm::Error = e.into();
-					tracer.trace_failed_call(trace_info, vec![], evm_err.clone().into());
+					let trace_info = tracer.prepare_trace_call(&params);
+					tracer.trace_failed_call(
+						trace_info,
+						vec![],
+						evm_err.clone().into()
+					);
 					Err(evm_err)
 				} else {
 					self.state.discard_checkpoint();
@@ -448,15 +451,11 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 						ActionValue::Apparent(_) => false,
 					};
 					if self.depth == 0 || is_transferred {
-						let mut trace_output = tracer.prepare_trace_output();
+						let trace_info = tracer.prepare_trace_call(&params);
-						if let Some(out) = trace_output.as_mut() {
-							*out = builtin_out_buffer.clone();
-						}
-
 						tracer.trace_call(
 							trace_info,
 							cost,
-							trace_output,
+							&builtin_out_buffer,
 							vec![]
 						);
 					}
@@ -472,13 +471,17 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 				// just drain the whole gas
 				self.state.revert_to_checkpoint();
 
-				tracer.trace_failed_call(trace_info, vec![], vm::Error::OutOfGas.into());
+				let trace_info = tracer.prepare_trace_call(&params);
+				tracer.trace_failed_call(
+					trace_info,
+					vec![],
+					vm::Error::OutOfGas.into()
+				);
 
 				Err(vm::Error::OutOfGas)
 			}
 		} else {
 			let trace_info = tracer.prepare_trace_call(&params);
-			let mut trace_output = tracer.prepare_trace_output();
 			let mut subtracer = tracer.subtracer();
 
 			let gas = params.gas;
@@ -501,11 +504,10 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 				let traces = subtracer.drain();
 				match res {
 					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,
+							&res.return_data,
 							traces
 						);
 					},
@@ -522,7 +524,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 				// otherwise it's just a basic transaction, only do tracing, if necessary.
 				self.state.discard_checkpoint();
 
-				tracer.trace_call(trace_info, U256::zero(), trace_output, vec![]);
+				tracer.trace_call(trace_info, U256::zero(), &[], vec![]);
 				Ok(FinalizationResult {
 					gas_left: params.gas,
 					return_data: ReturnData::empty(),
@@ -577,7 +579,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 		}
 
 		let trace_info = tracer.prepare_trace_create(&params);
-		let mut trace_output = tracer.prepare_trace_output();
 		let mut subtracer = tracer.subtracer();
 		let gas = params.gas;
 		let created = params.address.clone();
@@ -596,11 +597,10 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 
 		match res {
 			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,
+					&res.return_data,
 					created,
 					subtracer.drain()
 				);

ethcore/src/trace/executive_tracer.rs

@@ -17,7 +17,6 @@
 //! Simple executive tracer.
 
 use ethereum_types::{U256, Address};
-use bytes::Bytes;
 use vm::ActionParams;
 use trace::trace::{Call, Create, Action, Res, CreateResult, CallResult, VMTrace, VMOperation, VMExecutedOperation, MemoryDiff, StorageDiff, Suicide, Reward, RewardType};
 use trace::{Tracer, VMTracer, FlatTrace, TraceError};
@@ -92,18 +91,14 @@ impl Tracer for ExecutiveTracer {
 		Some(Create::from(params.clone()))
 	}
 
-	fn prepare_trace_output(&self) -> Option<Bytes> {
+	fn trace_call(&mut self, call: Option<Call>, gas_used: U256, output: &[u8], subs: Vec<FlatTrace>) {
-		Some(vec![])
-	}
-
-	fn trace_call(&mut self, call: Option<Call>, gas_used: U256, output: Option<Bytes>, subs: Vec<FlatTrace>) {
 		let trace = FlatTrace {
 			trace_address: Default::default(),
 			subtraces: top_level_subtraces(&subs),
 			action: Action::Call(call.expect("self.prepare_trace_call().is_some(): so we must be tracing: qed")),
 			result: Res::Call(CallResult {
 				gas_used: gas_used,
-				output: output.expect("self.prepare_trace_output().is_some(): so we must be tracing: qed")
+				output: output.into()
 			}),
 		};
 		debug!(target: "trace", "Traced call {:?}", trace);
@@ -111,13 +106,13 @@ impl Tracer for ExecutiveTracer {
 		self.traces.extend(prefix_subtrace_addresses(subs));
 	}
 
-	fn trace_create(&mut self, create: Option<Create>, gas_used: U256, code: Option<Bytes>, address: Address, subs: Vec<FlatTrace>) {
+	fn trace_create(&mut self, create: Option<Create>, gas_used: U256, code: &[u8], address: Address, subs: Vec<FlatTrace>) {
 		let trace = FlatTrace {
 			subtraces: top_level_subtraces(&subs),
 			action: Action::Create(create.expect("self.prepare_trace_create().is_some(): so we must be tracing: qed")),
 			result: Res::Create(CreateResult {
 				gas_used: gas_used,
-				code: code.expect("self.prepare_trace_output.is_some(): so we must be tracing: qed"),
+				code: code.into(),
 				address: address
 			}),
 			trace_address: Default::default(),

ethcore/src/trace/mod.rs

@@ -38,7 +38,6 @@ pub use self::types::filter::{Filter, AddressesFilter};
 
 use ethereum_types::{H256, U256, Address};
 use kvdb::DBTransaction;
-use bytes::Bytes;
 use self::trace::{Call, Create};
 use vm::ActionParams;
 use header::BlockNumber;
@@ -58,9 +57,6 @@ pub trait Tracer: Send {
 	/// This is called before a create has been executed.
 	fn prepare_trace_create(&self, params: &ActionParams) -> Option<Create>;
 
-	/// Prepare trace output. Noop tracer should return None.
-	fn prepare_trace_output(&self) -> Option<Bytes>;
-
 	/// Stores trace call info.
 	///
 	/// This is called after a call has completed successfully.
@@ -68,7 +64,7 @@ pub trait Tracer: Send {
 		&mut self,
 		call: Option<Call>,
 		gas_used: U256,
-		output: Option<Bytes>,
+		output: &[u8],
 		subs: Vec<Self::Output>,
 	);
 
@@ -79,7 +75,7 @@ pub trait Tracer: Send {
 		&mut self,
 		create: Option<Create>,
 		gas_used: U256,
-		code: Option<Bytes>,
+		code: &[u8],
 		address: Address,
 		subs: Vec<Self::Output>
 	);

ethcore/src/trace/noop_tracer.rs

@@ -17,7 +17,6 @@
 //! Nonoperative tracer.
 
 use ethereum_types::{U256, Address};
-use bytes::Bytes;
 use vm::ActionParams;
 use trace::{Tracer, VMTracer, FlatTrace, TraceError};
 use trace::trace::{Call, Create, VMTrace, RewardType};
@@ -36,18 +35,12 @@ impl Tracer for NoopTracer {
 		None
 	}
 
-	fn prepare_trace_output(&self) -> Option<Bytes> {
+	fn trace_call(&mut self, call: Option<Call>, _: U256, _: &[u8], _: Vec<FlatTrace>) {
-		None
-	}
-
-	fn trace_call(&mut self, call: Option<Call>, _: U256, output: Option<Bytes>, _: Vec<FlatTrace>) {
 		assert!(call.is_none(), "self.prepare_trace_call().is_none(): so we can't be tracing: qed");
-		assert!(output.is_none(), "self.prepare_trace_output().is_none(): so we can't be tracing: qed");
 	}
 
-	fn trace_create(&mut self, create: Option<Create>, _: U256, code: Option<Bytes>, _: Address, _: Vec<FlatTrace>) {
+	fn trace_create(&mut self, create: Option<Create>, _: U256, _: &[u8], _: Address, _: Vec<FlatTrace>) {
 		assert!(create.is_none(), "self.prepare_trace_create().is_none(): so we can't be tracing: qed");
-		assert!(code.is_none(), "self.prepare_trace_output().is_none(): so we can't be tracing: qed");
 	}
 
 	fn trace_failed_call(&mut self, call: Option<Call>, _: Vec<FlatTrace>, _: TraceError) {