facelift for traces, added errors (#2042)

* evm errors facelift

* facelift for traces, added errors with description

* additional tests for traces json serialization

ethcore/src/evm/evm.rs

@@ -16,11 +16,13 @@
 
 //! Evm interface.
 
-use common::*;
+use std::{ops, cmp, fmt};
+use util::{U128, U256, U512, Uint};
+use action_params::ActionParams;
 use evm::Ext;
 
 /// Evm errors.
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 pub enum Error {
 	/// `OutOfGas` is returned when transaction execution runs out of gas.
 	/// The state should be reverted to the state from before the
@@ -63,6 +65,21 @@ pub enum Error {
 	Internal,
 }
 
+impl fmt::Display for Error {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		use self::Error::*;
+		let message = match *self {
+			OutOfGas => "Out of gas",
+			BadJumpDestination { .. } => "Bad jump destination",
+			BadInstruction { .. } => "Bad instruction",
+			StackUnderflow { .. } => "Stack underflow",
+			OutOfStack { .. } => "Out of stack",
+			Internal => "Internal error",
+		};
+		message.fmt(f)
+	}
+}
+
 /// A specialized version of Result over EVM errors.
 pub type Result<T> = ::std::result::Result<T, Error>;
 
@@ -193,53 +210,55 @@ pub trait Evm {
 	fn exec(&mut self, params: ActionParams, ext: &mut Ext) -> Result<GasLeft>;
 }
 
-
-#[test]
 #[cfg(test)]
-fn should_calculate_overflow_mul_shr_without_overflow() {
-	// given
-	let num = 1048576;
+mod tests {
+	use util::{U256, Uint};
+	use super::CostType;
 
-	// when
-	let (res1, o1) = U256::from(num).overflow_mul_shr(U256::from(num), 20);
-	let (res2, o2) = num.overflow_mul_shr(num, 20);
+	#[test]
+	fn should_calculate_overflow_mul_shr_without_overflow() {
+		// given
+		let num = 1048576;
 
-	// then
-	assert_eq!(res1, U256::from(num));
-	assert!(!o1);
-	assert_eq!(res2, num);
-	assert!(!o2);
-}
-
-#[test]
-#[cfg(test)]
-fn should_calculate_overflow_mul_shr_with_overflow() {
-	// given
-	let max = ::std::u64::MAX;
-	let num1 = U256([max, max, max, max]);
-	let num2 = ::std::usize::MAX;
-
-	// when
-	let (res1, o1) = num1.overflow_mul_shr(num1, 256);
-	let (res2, o2) = num2.overflow_mul_shr(num2, 64);
-
-	// then
-	assert_eq!(res2, num2 - 1);
-	assert!(o2);
-
-	assert_eq!(res1, !U256::zero() - U256::one());
-	assert!(o1);
-}
-
-#[test]
-#[cfg(test)]
-fn should_validate_u256_to_usize_conversion() {
-	// given
-	let v = U256::from(::std::usize::MAX) + U256::from(1);
-
-	// when
-	let res = usize::from_u256(v);
-
-	// then
-	assert!(res.is_err());
+		// when
+		let (res1, o1) = U256::from(num).overflow_mul_shr(U256::from(num), 20);
+		let (res2, o2) = num.overflow_mul_shr(num, 20);
+
+		// then
+		assert_eq!(res1, U256::from(num));
+		assert!(!o1);
+		assert_eq!(res2, num);
+		assert!(!o2);
+	}
+
+	#[test]
+	fn should_calculate_overflow_mul_shr_with_overflow() {
+		// given
+		let max = u64::max_value();
+		let num1 = U256([max, max, max, max]);
+		let num2 = usize::max_value();
+
+		// when
+		let (res1, o1) = num1.overflow_mul_shr(num1, 256);
+		let (res2, o2) = num2.overflow_mul_shr(num2, 64);
+
+		// then
+		assert_eq!(res2, num2 - 1);
+		assert!(o2);
+
+		assert_eq!(res1, !U256::zero() - U256::one());
+		assert!(o1);
+	}
+
+	#[test]
+	fn should_validate_u256_to_usize_conversion() {
+		// given
+		let v = U256::from(usize::max_value()) + U256::from(1);
+
+		// when
+		let res = usize::from_u256(v);
+
+		// then
+		assert!(res.is_err());
+	}
 }

ethcore/src/executive.rs

@@ -286,7 +286,7 @@ impl<'a> Executive<'a> {
 				// just drain the whole gas
 				self.state.revert_snapshot();
 
-				tracer.trace_failed_call(trace_info, vec![]);
+				tracer.trace_failed_call(trace_info, vec![], evm::Error::OutOfGas.into());
 
 				Err(evm::Error::OutOfGas)
 			}
@@ -320,7 +320,7 @@ impl<'a> Executive<'a> {
 						trace_output,
 						traces
 					),
-					_ => tracer.trace_failed_call(trace_info, traces),
+					Err(e) => tracer.trace_failed_call(trace_info, traces, e.into()),
 				};
 
 				trace!(target: "executive", "substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate);
@@ -385,7 +385,7 @@ impl<'a> Executive<'a> {
 				created,
 				subtracer.traces()
 			),
-			_ => tracer.trace_failed_create(trace_info, subtracer.traces())
+			Err(e) => tracer.trace_failed_create(trace_info, subtracer.traces(), e.into())
 		};
 
 		self.enact_result(&res, substate, unconfirmed_substate);

ethcore/src/state/mod.rs

@@ -444,8 +444,7 @@ use env_info::*;
 use spec::*;
 use transaction::*;
 use util::log::init_log;
-use trace::trace;
-use trace::FlatTrace;
+use trace::{FlatTrace, TraceError, trace};
 use types::executed::CallType;
 
 #[test]
@@ -538,7 +537,7 @@ fn should_trace_failed_create_transaction() {
 			gas: 78792.into(),
 			init: vec![91, 96, 0, 86],
 		}),
-		result: trace::Res::FailedCreate,
+		result: trace::Res::FailedCreate(TraceError::OutOfGas),
 		subtraces: 0
 	}];
 
@@ -869,7 +868,7 @@ fn should_trace_failed_call_transaction() {
 			input: vec![],
 			call_type: CallType::Call,
 		}),
-		result: trace::Res::FailedCall,
+		result: trace::Res::FailedCall(TraceError::OutOfGas),
 		subtraces: 0,
 	}];
 
@@ -1084,7 +1083,7 @@ fn should_trace_failed_subcall_transaction() {
 			input: vec![],
 			call_type: CallType::Call,
 		}),
-		result: trace::Res::FailedCall,
+		result: trace::Res::FailedCall(TraceError::OutOfGas),
 	}];
 
 	assert_eq!(result.trace, expected_trace);
@@ -1217,7 +1216,7 @@ fn should_trace_failed_subcall_with_subcall_transaction() {
 			input: vec![],
 			call_type: CallType::Call,
 		}),
-		result: trace::Res::FailedCall,
+		result: trace::Res::FailedCall(TraceError::OutOfGas),
 	}, FlatTrace {
 		trace_address: vec![0, 0].into_iter().collect(),
 		subtraces: 0,

ethcore/src/trace/db.rs

@@ -420,7 +420,7 @@ mod tests {
 	use devtools::RandomTempPath;
 	use header::BlockNumber;
 	use trace::{Config, Switch, TraceDB, Database as TraceDatabase, DatabaseExtras, ImportRequest};
-	use trace::{Filter, LocalizedTrace, AddressesFilter};
+	use trace::{Filter, LocalizedTrace, AddressesFilter, TraceError};
 	use trace::trace::{Call, Action, Res};
 	use trace::flat::{FlatTrace, FlatBlockTraces, FlatTransactionTraces};
 	use types::executed::CallType;
@@ -560,7 +560,7 @@ mod tests {
 					input: vec![],
 					call_type: CallType::Call,
 				}),
-				result: Res::FailedCall,
+				result: Res::FailedCall(TraceError::OutOfGas),
 			}])]),
 			block_hash: block_hash.clone(),
 			block_number: block_number,
@@ -579,7 +579,7 @@ mod tests {
 				input: vec![],
 				call_type: CallType::Call,
 			}),
-			result: Res::FailedCall,
+			result: Res::FailedCall(TraceError::OutOfGas),
 			trace_address: vec![],
 			subtraces: 0,
 			transaction_number: 0,

ethcore/src/trace/executive_tracer.rs

@@ -19,7 +19,7 @@
 use util::{Bytes, Address, U256};
 use action_params::ActionParams;
 use trace::trace::{Call, Create, Action, Res, CreateResult, CallResult, VMTrace, VMOperation, VMExecutedOperation, MemoryDiff, StorageDiff, Suicide};
-use trace::{Tracer, VMTracer, FlatTrace};
+use trace::{Tracer, VMTracer, FlatTrace, TraceError};
 
 /// Simple executive tracer. Traces all calls and creates. Ignores delegatecalls.
 #[derive(Default)]
@@ -112,23 +112,23 @@ impl Tracer for ExecutiveTracer {
 		self.traces.extend(update_trace_address(subs));
 	}
 
-	fn trace_failed_call(&mut self, call: Option<Call>, subs: Vec<FlatTrace>) {
+	fn trace_failed_call(&mut self, call: Option<Call>, subs: Vec<FlatTrace>, error: TraceError) {
 		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::FailedCall,
+			result: Res::FailedCall(error),
 		};
 		debug!(target: "trace", "Traced failed call {:?}", trace);
 		self.traces.push(trace);
 		self.traces.extend(update_trace_address(subs));
 	}
 
-	fn trace_failed_create(&mut self, create: Option<Create>, subs: Vec<FlatTrace>) {
+	fn trace_failed_create(&mut self, create: Option<Create>, subs: Vec<FlatTrace>, error: TraceError) {
 		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::FailedCreate,
+			result: Res::FailedCreate(error),
 			trace_address: Default::default(),
 		};
 		debug!(target: "trace", "Traced failed create {:?}", trace);

ethcore/src/trace/mod.rs

@@ -24,7 +24,8 @@ mod executive_tracer;
 mod import;
 mod noop_tracer;
 
-pub use types::trace_types::*;
+pub use types::trace_types::{filter, flat, localized, trace};
+pub use types::trace_types::error::Error as TraceError;
 pub use self::config::{Config, Switch};
 pub use self::db::TraceDB;
 pub use self::error::Error;
@@ -71,10 +72,10 @@ pub trait Tracer: Send {
 	);
 
 	/// Stores failed call trace.
-	fn trace_failed_call(&mut self, call: Option<Call>, subs: Vec<FlatTrace>);
+	fn trace_failed_call(&mut self, call: Option<Call>, subs: Vec<FlatTrace>, error: TraceError);
 
 	/// Stores failed create trace.
-	fn trace_failed_create(&mut self, create: Option<Create>, subs: Vec<FlatTrace>);
+	fn trace_failed_create(&mut self, create: Option<Create>, subs: Vec<FlatTrace>, error: TraceError);
 
 	/// Stores suicide info.
 	fn trace_suicide(&mut self, address: Address, balance: U256, refund_address: Address);

ethcore/src/trace/noop_tracer.rs

@@ -18,7 +18,7 @@
 
 use util::{Bytes, Address, U256};
 use action_params::ActionParams;
-use trace::{Tracer, VMTracer, FlatTrace};
+use trace::{Tracer, VMTracer, FlatTrace, TraceError};
 use trace::trace::{Call, Create, VMTrace};
 
 /// Nonoperative tracer. Does not trace anything.
@@ -47,11 +47,11 @@ impl Tracer for NoopTracer {
 		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>) {
+	fn trace_failed_call(&mut self, call: Option<Call>, _: Vec<FlatTrace>, _: TraceError) {
 		assert!(call.is_none(), "self.prepare_trace_call().is_none(): so we can't be tracing: qed");
 	}
 
-	fn trace_failed_create(&mut self, create: Option<Create>, _: Vec<FlatTrace>) {
+	fn trace_failed_create(&mut self, create: Option<Create>, _: Vec<FlatTrace>, _: TraceError) {
 		assert!(create.is_none(), "self.prepare_trace_create().is_none(): so we can't be tracing: qed");
 	}
 

ethcore/src/types/trace_types/error.rs

@@ -0,0 +1,99 @@
+// Copyright 2015, 2016 Ethcore (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Trace errors.
+
+use std::fmt;
+use rlp::{Encodable, RlpStream, Decodable, Decoder, DecoderError, Stream, View};
+use evm::Error as EvmError;
+
+/// Trace evm errors.
+#[derive(Debug, PartialEq, Clone, Binary)]
+pub enum Error {
+	/// `OutOfGas` is returned when transaction execution runs out of gas.
+	OutOfGas,
+	/// `BadJumpDestination` is returned when execution tried to move
+	/// to position that wasn't marked with JUMPDEST instruction
+	BadJumpDestination,
+	/// `BadInstructions` is returned when given instruction is not supported
+	BadInstruction,
+	/// `StackUnderflow` when there is not enough stack elements to execute instruction
+	StackUnderflow,
+	/// When execution would exceed defined Stack Limit
+	OutOfStack,
+	/// Returned on evm internal error. Should never be ignored during development.
+	/// Likely to cause consensus issues.
+	Internal,
+}
+
+impl From<EvmError> for Error {
+	fn from(e: EvmError) -> Self {
+		match e {
+			EvmError::OutOfGas => Error::OutOfGas,
+			EvmError::BadJumpDestination { .. } => Error::BadJumpDestination,
+			EvmError::BadInstruction { .. } => Error::BadInstruction,
+			EvmError::StackUnderflow { .. } => Error::StackUnderflow,
+			EvmError::OutOfStack { .. } => Error::OutOfStack,
+			EvmError::Internal => Error::Internal,
+		}
+	}
+}
+
+impl fmt::Display for Error {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		use self::Error::*;
+		let message = match *self {
+			OutOfGas => "Out of gas",
+			BadJumpDestination => "Bad jump destination",
+			BadInstruction => "Bad instruction",
+			StackUnderflow => "Stack underflow",
+			OutOfStack => "Out of stack",
+			Internal => "Internal error",
+		};
+		message.fmt(f)
+	}
+}
+
+impl Encodable for Error {
+	fn rlp_append(&self, s: &mut RlpStream) {
+		use self::Error::*;
+		let value = match *self {
+			OutOfGas => 0u8,
+			BadJumpDestination => 1,
+			BadInstruction => 2,
+			StackUnderflow => 3,
+			OutOfStack => 4,
+			Internal => 5,
+		};
+		s.append(&value);
+	}
+}
+
+impl Decodable for Error {
+	fn decode<D>(decoder: &D) -> Result<Self, DecoderError> where D: Decoder {
+		use self::Error::*;
+		let value: u8 = try!(decoder.as_rlp().as_val());
+		match value {
+			0 => Ok(OutOfGas),
+			1 => Ok(BadJumpDestination),
+			2 => Ok(BadInstruction),
+			3 => Ok(StackUnderflow),
+			4 => Ok(OutOfStack),
+			5 => Ok(Internal),
+			_ => Err(DecoderError::Custom("Invalid error type")),
+		}
+	}
+}

ethcore/src/types/trace_types/filter.rs

@@ -140,7 +140,7 @@ mod tests {
 	use util::bloom::Bloomable;
 	use trace::trace::{Action, Call, Res, Create, CreateResult, Suicide};
 	use trace::flat::FlatTrace;
-	use trace::{Filter, AddressesFilter};
+	use trace::{Filter, AddressesFilter, TraceError};
 	use types::executed::CallType;
 
 	#[test]
@@ -286,7 +286,7 @@ mod tests {
 				input: vec![0x5],
 				call_type: CallType::Call,
 			}),
-			result: Res::FailedCall,
+			result: Res::FailedCall(TraceError::OutOfGas),
 			trace_address: vec![0].into_iter().collect(),
 			subtraces: 0,
 		};

ethcore/src/types/trace_types/mod.rs

@@ -16,6 +16,7 @@
 
 //! Types used in the public api
 
+pub mod error;
 pub mod filter;
 pub mod flat;
 pub mod trace;

ethcore/src/types/trace_types/trace.rs

@@ -24,6 +24,7 @@ use rlp::*;
 use action_params::ActionParams;
 use basic_types::LogBloom;
 use types::executed::CallType;
+use super::error::Error;
 
 /// `Call` result.
 #[derive(Debug, Clone, PartialEq, Default, Binary)]
@@ -322,9 +323,9 @@ pub enum Res {
 	/// Successful create action result.
 	Create(CreateResult),
 	/// Failed call.
-	FailedCall,
+	FailedCall(Error),
 	/// Failed create.
-	FailedCreate,
+	FailedCreate(Error),
 	/// None
 	None,
 }
@@ -342,13 +343,15 @@ impl Encodable for Res {
 				s.append(&1u8);
 				s.append(create);
 			},
-			Res::FailedCall => {
-				s.begin_list(1);
+			Res::FailedCall(ref err) => {
+				s.begin_list(2);
 				s.append(&2u8);
+				s.append(err);
 			},
-			Res::FailedCreate => {
-				s.begin_list(1);
+			Res::FailedCreate(ref err) => {
+				s.begin_list(2);
 				s.append(&3u8);
+				s.append(err);
 			},
 			Res::None => {
 				s.begin_list(1);
@@ -365,8 +368,8 @@ impl Decodable for Res {
 		match action_type {
 			0 => d.val_at(1).map(Res::Call),
 			1 => d.val_at(1).map(Res::Create),
-			2 => Ok(Res::FailedCall),
-			3 => Ok(Res::FailedCreate),
+			2 => d.val_at(1).map(Res::FailedCall),
+			3 => d.val_at(1).map(Res::FailedCreate),
 			4 => Ok(Res::None),
 			_ => Err(DecoderError::Custom("Invalid result type.")),
 		}
@@ -378,7 +381,7 @@ impl Res {
 	pub fn bloom(&self) -> LogBloom {
 		match *self {
 			Res::Create(ref create) => create.bloom(),
-			Res::Call(_) | Res::FailedCall | Res::FailedCreate | Res::None => Default::default(),
+			Res::Call(_) | Res::FailedCall(_) | Res::FailedCreate(_) | Res::None => Default::default(),
 		}
 	}
 }