[trace] introduce trace failed to Ext (#11019)

* [trace] add trace_failed to Ext, manage stack of trace data

* [evm] call trace_failed only if self.do_trace

* [evm] add a comment about do_trace set to true

* [vm] improve the doc in trace_prepare_execute

* [trace] add the bounds check back

ethcore/evm/src/interpreter/mod.rs

@@ -279,6 +279,8 @@ impl<Cost: CostType> Interpreter<Cost> {
 			cache, params, reader, informant,
 			valid_jump_destinations, gasometer, stack,
 			done: false,
+			// Overridden in `step_inner` based on
+			// the result of `ext.trace_next_instruction`.
 			do_trace: true,
 			mem: Vec::new(),
 			return_data: ReturnData::empty(),
@@ -353,6 +355,9 @@ impl<Cost: CostType> Interpreter<Cost> {
 					ext.trace_prepare_execute(self.reader.position - 1, opcode, requirements.gas_cost.as_u256(), Self::mem_written(instruction, &self.stack), Self::store_written(instruction, &self.stack));
 				}
 				if let Err(e) = self.gasometer.as_mut().expect(GASOMETER_PROOF).verify_gas(&requirements.gas_cost) {
+					if self.do_trace {
+						ext.trace_failed();
+					}
 					return InterpreterResult::Done(Err(e));
 				}
 				self.mem.expand(requirements.memory_required_size);
@@ -366,7 +371,12 @@ impl<Cost: CostType> Interpreter<Cost> {
 				let result = match self.exec_instruction(
 					current_gas, ext, instruction, requirements.provide_gas
 				) {
-					Err(x) => return InterpreterResult::Done(Err(x)),
+					Err(x) => {
+						if self.do_trace {
+							ext.trace_failed();
+						}
+						return InterpreterResult::Done(Err(x));
+					},
 					Ok(x) => x,
 				};
 				evm_debug!({ self.informant.after_instruction(instruction) });

ethcore/machine/src/externalities.rs

@@ -440,6 +440,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
 		self.vm_tracer.trace_prepare_execute(pc, instruction, gas_cost, mem_written, store_written)
 	}
 
+	fn trace_failed(&mut self) {
+		self.vm_tracer.trace_failed();
+	}
+
 	fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem: &[u8]) {
 		self.vm_tracer.trace_executed(gas_used, stack_push, mem)
 	}

ethcore/trace/src/executive_tracer.rs

@@ -16,6 +16,7 @@
 
 //! Simple executive tracer.
 
+use std::cmp::min;
 use ethereum_types::{U256, Address};
 use vm::{Error as VmError, ActionParams};
 use log::{debug, warn};
@@ -196,12 +197,16 @@ impl Tracer for ExecutiveTracer {
 	}
 }
 
+struct TraceData {
+	mem_written: Option<(usize, usize)>,
+	store_written: Option<(U256, U256)>,
+}
+
 /// Simple VM tracer. Traces all operations.
 pub struct ExecutiveVMTracer {
 	data: VMTrace,
 	depth: usize,
-	last_mem_written: Option<(usize, usize)>,
-	last_store_written: Option<(U256, U256)>,
+	trace_stack: Vec<TraceData>,
 }
 
 impl ExecutiveVMTracer {
@@ -215,8 +220,7 @@ impl ExecutiveVMTracer {
 				subs: vec![],
 			},
 			depth: 0,
-			last_mem_written: None,
-			last_store_written: None,
+			trace_stack: vec![],
 		}
 	}
 
@@ -243,30 +247,27 @@ impl VMTracer for ExecutiveVMTracer {
 				executed: None,
 			});
 		});
-		self.last_mem_written = mem_written;
-		self.last_store_written = store_written;
+		self.trace_stack.push(TraceData { mem_written, store_written });
+	}
+
+	fn trace_failed(&mut self) {
+		let _ = self.trace_stack.pop().expect("pushed in trace_prepare_execute; qed");
 	}
 
 	fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem: &[u8]) {
-		let mem_diff = self.last_mem_written.take().map(|(o, s)| {
+		let TraceData { mem_written, store_written } = self.trace_stack.pop().expect("pushed in trace_prepare_execute; qed");
+		let mem_diff = mem_written.map(|(o, s)| {
 			if o + s > mem.len() {
-				warn!(
-					target: "trace",
-					"Last mem written is out of bounds {} (mem is {})",
-					o + s,
-					mem.len(),
-				);
-				(o, &[][..])
-			} else {
-				(o, &(mem[o..o+s]))
+				warn!(target: "trace", "mem_written is out of bounds");
 			}
+			(o, &mem[min(mem.len(), o)..min(o + s, mem.len())])
 		});
-		let store_diff = self.last_store_written.take();
+		let store_diff = store_written;
 		Self::with_trace_in_depth(&mut self.data, self.depth, move |trace| {
 			let ex = VMExecutedOperation {
 				gas_used: gas_used,
-				stack_push: stack_push.iter().cloned().collect(),
-				mem_diff: mem_diff.map(|(s, r)| MemoryDiff { offset: s, data: r.iter().cloned().collect() }),
+				stack_push: stack_push.to_vec(),
+				mem_diff: mem_diff.map(|(s, r)| MemoryDiff { offset: s, data: r.to_vec() }),
 				store_diff: store_diff.map(|(l, v)| StorageDiff { location: l, value: v }),
 			};
 			trace.operations.last_mut().expect("trace_executed is always called after a trace_prepare_execute; trace.operations cannot be empty; qed").executed = Some(ex);

ethcore/trace/src/lib.rs

@@ -93,6 +93,9 @@ pub trait VMTracer: Send {
 	/// Trace the preparation to execute a single valid instruction.
 	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256, _mem_written: Option<(usize, usize)>, _store_written: Option<(U256, U256)>) {}
 
+	/// Trace the execution failure of a single instruction.
+	fn trace_failed(&mut self) {}
+
 	/// Trace the finalised execution of a single valid instruction.
 	fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem: &[u8]) {}
 

ethcore/vm/src/ext.rs

@@ -164,8 +164,12 @@ pub trait Ext {
 	fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _current_gas: U256) -> bool { false }
 
 	/// Prepare to trace an operation. Passthrough for the VM trace.
+	/// For each call of `trace_prepare_execute` either `trace_failed` or `trace_executed` MUST be called.
 	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256, _mem_written: Option<(usize, usize)>, _store_written: Option<(U256, U256)>) {}
 
+	/// Trace the execution failure of a single instruction.
+	fn trace_failed(&mut self) {}
+
 	/// Trace the finalised execution of a single instruction.
 	fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem: &[u8]) {}