Fix output of parity-evm in case of bad instruction (#5955)

* Fix output of evmbin.

* Use foundation chain settings by default.

ethcore/src/evm/ext.rs

@@ -129,8 +129,11 @@ pub trait Ext {
 	/// Increments sstore refunds count by 1.
 	fn inc_sstore_clears(&mut self);
 
+	/// Decide if any more operations should be traced. Passthrough for the VM trace.
+	fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8) -> bool { false }
+
 	/// Prepare to trace an operation. Passthrough for the VM trace.
-	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: &U256) -> bool { false }
+	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256) {}
 
 	/// Trace the finalised execution of a single instruction.
 	fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem_diff: Option<(usize, &[u8])>, _store_diff: Option<(U256, U256)>) {}

ethcore/src/evm/interpreter/mod.rs

@@ -111,6 +111,7 @@ impl<Cost: CostType> evm::Evm for Interpreter<Cost> {
 		self.mem.clear();
 
 		let mut informant = informant::EvmInformant::new(ext.depth());
+		let mut do_trace = true;
 
 		let code = &params.code.as_ref().expect("exec always called with code; qed");
 		let mut valid_jump_destinations = None;
@@ -124,13 +125,17 @@ impl<Cost: CostType> evm::Evm for Interpreter<Cost> {
 			let instruction = code[reader.position];
 			reader.position += 1;
 
+			// TODO: make compile-time removable if too much of a performance hit.
+			do_trace = do_trace && ext.trace_next_instruction(reader.position - 1, instruction);
+
 			let info = &infos[instruction as usize];
 			self.verify_instruction(ext, instruction, info, &stack)?;
 
 			// Calculate gas cost
 			let requirements = gasometer.requirements(ext, instruction, info, &stack, self.mem.size())?;
-			// TODO: make compile-time removable if too much of a performance hit.
+			if do_trace {
-			let trace_executed = ext.trace_prepare_execute(reader.position - 1, instruction, &requirements.gas_cost.as_u256());
+				ext.trace_prepare_execute(reader.position - 1, instruction, requirements.gas_cost.as_u256());
+			}
 
 			gasometer.verify_gas(&requirements.gas_cost)?;
 			self.mem.expand(requirements.memory_required_size);
@@ -139,7 +144,7 @@ impl<Cost: CostType> evm::Evm for Interpreter<Cost> {
 
 			evm_debug!({ informant.before_instruction(reader.position, instruction, info, &gasometer.current_gas, &stack) });
 
-			let (mem_written, store_written) = match trace_executed {
+			let (mem_written, store_written) = match do_trace {
 				true => (Self::mem_written(instruction, &stack), Self::store_written(instruction, &stack)),
 				false => (None, None),
 			};
@@ -155,7 +160,7 @@ impl<Cost: CostType> evm::Evm for Interpreter<Cost> {
 				gasometer.current_gas = gasometer.current_gas + *gas;
 			}
 
-			if trace_executed {
+			if do_trace {
 				ext.trace_executed(gasometer.current_gas.as_u256(), stack.peek_top(info.ret), mem_written.map(|(o, s)| (o, &(self.mem[o..(o + s)]))), store_written);
 			}
 

ethcore/src/executive.rs

@@ -376,7 +376,7 @@ impl<'a, B: 'a + StateBackend, E: Engine + ?Sized> Executive<'a, B, E> {
 					self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut()), &mut subtracer, &mut subvmtracer)
 				};
 
-				vm_tracer.done_subtrace(subvmtracer, res.is_ok());
+				vm_tracer.done_subtrace(subvmtracer);
 
 				trace!(target: "executive", "res={:?}", res);
 
@@ -457,7 +457,7 @@ impl<'a, B: 'a + StateBackend, E: Engine + ?Sized> Executive<'a, B, E> {
 			self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::InitContract(trace_output.as_mut()), &mut subtracer, &mut subvmtracer)
 		};
 
-		vm_tracer.done_subtrace(subvmtracer, res.is_ok());
+		vm_tracer.done_subtrace(subvmtracer);
 
 		match res {
 			Ok(ref res) => tracer.trace_create(

ethcore/src/externalities.rs

@@ -379,7 +379,11 @@ impl<'a, T: 'a, V: 'a, B: 'a, E: 'a> Ext for Externalities<'a, T, V, B, E>
 		self.substate.sstore_clears_count = self.substate.sstore_clears_count + U256::one();
 	}
 
-	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: &U256) -> bool {
+	fn trace_next_instruction(&mut self, pc: usize, instruction: u8) -> bool {
+		self.vm_tracer.trace_next_instruction(pc, instruction)
+	}
+
+	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: U256) {
 		self.vm_tracer.trace_prepare_execute(pc, instruction, gas_cost)
 	}
 

ethcore/src/trace/executive_tracer.rs

@@ -192,14 +192,15 @@ impl ExecutiveVMTracer {
 }
 
 impl VMTracer for ExecutiveVMTracer {
-	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: &U256) -> bool {
+	fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8) -> bool { true }
+
+	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: U256) {
 		self.data.operations.push(VMOperation {
 			pc: pc,
 			instruction: instruction,
-			gas_cost: gas_cost.clone(),
+			gas_cost: gas_cost,
 			executed: None,
 		});
-		true
 	}
 
 	fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem_diff: Option<(usize, &[u8])>, store_diff: Option<(U256, U256)>) {
@@ -221,7 +222,7 @@ impl VMTracer for ExecutiveVMTracer {
 		}}
 	}
 
-	fn done_subtrace(&mut self, sub: Self, _is_successful: bool) {
+	fn done_subtrace(&mut self, sub: Self) {
 		self.data.subs.push(sub.data);
 	}
 

ethcore/src/trace/mod.rs

@@ -87,18 +87,23 @@ pub trait Tracer: Send {
 
 /// Used by executive to build VM traces.
 pub trait VMTracer: Send {
-	/// Trace the preparation to execute a single instruction.
-	/// @returns true if `trace_executed` should be called.
-	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: &U256) -> bool { false }
 
-	/// Trace the finalised execution of a single instruction.
+	/// Trace the progression of interpreter to next instruction.
+	/// If tracer returns `false` it won't be called again.
+	/// @returns true if `trace_prepare_execute` and `trace_executed` should be called.
+	fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8) -> bool { false }
+
+	/// Trace the preparation to execute a single valid instruction.
+	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256) {}
+
+	/// Trace the finalised execution of a single valid instruction.
 	fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem_diff: Option<(usize, &[u8])>, _store_diff: Option<(U256, U256)>) {}
 
 	/// Spawn subtracer which will be used to trace deeper levels of execution.
 	fn prepare_subtrace(&self, code: &[u8]) -> Self where Self: Sized;
 
 	/// Finalize subtracer.
-	fn done_subtrace(&mut self, sub: Self, is_successful: bool) where Self: Sized;
+	fn done_subtrace(&mut self, sub: Self) where Self: Sized;
 
 	/// Consumes self and returns the VM trace.
 	fn drain(self) -> Option<VMTrace>;

ethcore/src/trace/noop_tracer.rs

@@ -71,18 +71,15 @@ impl Tracer for NoopTracer {
 pub struct NoopVMTracer;
 
 impl VMTracer for NoopVMTracer {
-	/// Trace the preparation to execute a single instruction.
+	fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8) -> bool { false }
-	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: &U256) -> bool { false }
+
+	fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256) {}
 
-	/// Trace the finalised execution of a single instruction.
 	fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem_diff: Option<(usize, &[u8])>, _store_diff: Option<(U256, U256)>) {}
 
-	/// Spawn subtracer which will be used to trace deeper levels of execution.
 	fn prepare_subtrace(&self, _code: &[u8]) -> Self { NoopVMTracer }
 
-	/// Spawn subtracer which will be used to trace deeper levels of execution.
+	fn done_subtrace(&mut self, _sub: Self) {}
-	fn done_subtrace(&mut self, _sub: Self, _is_successful: bool) {}
 
-	/// Consumes self and returns all VM traces.
 	fn drain(self) -> Option<VMTrace> { None }
 }

evmbin/src/display/json.rs

@@ -81,13 +81,18 @@ impl vm::Informant for Informant {
 }
 
 impl trace::VMTracer for Informant {
-	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: &U256) -> bool {
+	fn trace_next_instruction(&mut self, pc: usize, instruction: u8) -> bool {
 		self.pc = pc;
 		self.instruction = instruction;
-		self.gas_cost = *gas_cost;
 		true
 	}
 
+	fn trace_prepare_execute(&mut self, pc: usize, instruction: u8, gas_cost: U256) {
+		self.pc = pc;
+		self.instruction = instruction;
+		self.gas_cost = gas_cost;
+	}
+
 	fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem_diff: Option<(usize, &[u8])>, store_diff: Option<(U256, U256)>) {
 		let info = evm::INSTRUCTIONS[self.instruction as usize];
 
@@ -127,17 +132,9 @@ impl trace::VMTracer for Informant {
 		vm
 	}
 
-	fn done_subtrace(&mut self, mut sub: Self, is_successful: bool) where Self: Sized {
+	fn done_subtrace(&mut self, mut sub: Self) {
 		if sub.depth == 1 {
 			// print last line with final state:
-			if is_successful {
-				sub.pc += 1;
-				sub.instruction = 0;
-			} else {
-				let push_bytes = evm::push_bytes(sub.instruction);
-				sub.pc += if push_bytes > 0 { push_bytes + 1 } else { 0 };
-				sub.instruction = if sub.pc < sub.code.len() { sub.code[sub.pc] } else { 0 };
-			}
 			sub.gas_cost = 0.into();
 			let gas_used = sub.gas_used;
 			trace::VMTracer::trace_executed(&mut sub, gas_used, &[], None, None);

evmbin/src/display/simple.rs

@@ -44,6 +44,6 @@ impl vm::Informant for Informant {
 
 impl trace::VMTracer for Informant {
 	fn prepare_subtrace(&self, _code: &[u8]) -> Self where Self: Sized { Default::default() }
-	fn done_subtrace(&mut self, _sub: Self, _is_successful: bool) where Self: Sized {}
+	fn done_subtrace(&mut self, _sub: Self) {}
 	fn drain(self) -> Option<trace::VMTrace> { None }
 }

evmbin/src/main.rs

@@ -137,7 +137,7 @@ impl Args {
 				spec::Spec::load(file)?
 			},
 			None => {
-				spec::Spec::new_instant()
+				ethcore::ethereum::new_foundation()
 			},
 		})
 	}