Refactor evm Instruction to be a c-like enum (#8914)

* Add a basic instruction c-like enum

* Fix all compiling errors

* Fix tests

* Access instruction info as a Instruction impl

* Use macro to avoid duplication in from_u8

* Use single space instead of multiple tabs to avoid formatting issue

* Fix evmbin compile

* typo: indentation

* Use if let to remove an expect

* Address grumbles

ethcore/evm/src/interpreter/gasometer.rs

@@ -113,7 +113,7 @@ impl<Gas: evm::CostType> Gasometer<Gas> {
 		current_mem_size: usize,
 	) -> vm::Result<InstructionRequirements<Gas>> {
 		let schedule = ext.schedule();
-		let tier = instructions::get_tier_idx(info.tier);
+		let tier = info.tier.idx();
 		let default_gas = Gas::from(schedule.tier_step_gas[tier]);
 
 		let cost = match instruction {
@@ -179,8 +179,8 @@ impl<Gas: evm::CostType> Gasometer<Gas> {
 			instructions::EXTCODECOPY => {
 				Request::GasMemCopy(schedule.extcodecopy_base_gas.into(), mem_needed(stack.peek(1), stack.peek(3))?, Gas::from_u256(*stack.peek(3))?)
 			},
-			instructions::LOG0...instructions::LOG4 => {
+			instructions::LOG0 | instructions::LOG1 | instructions::LOG2 | instructions::LOG3 | instructions::LOG4 => {
-				let no_of_topics = instructions::get_log_topics(instruction);
+				let no_of_topics = instruction.log_topics().expect("log_topics always return some for LOG* instructions; qed");
 				let log_gas = schedule.log_gas + schedule.log_topic_gas * no_of_topics;
 
 				let data_gas = overflowing!(Gas::from_u256(*stack.peek(1))?.overflow_mul(Gas::from(schedule.log_data_gas)));

ethcore/evm/src/interpreter/mod.rs

@@ -64,7 +64,6 @@ struct CodeReader<'a> {
 }
 
 impl<'a> CodeReader<'a> {
-
 	/// Create new code reader - starting at position 0.
 	fn new(code: &'a [u8]) -> Self {
 		CodeReader {
@@ -81,7 +80,7 @@ impl<'a> CodeReader<'a> {
 		U256::from(&self.code[pos..max])
 	}
 
-	fn len (&self) -> usize {
+	fn len(&self) -> usize {
 		self.code.len()
 	}
 }
@@ -124,24 +123,31 @@ impl<Cost: CostType> vm::Vm for Interpreter<Cost> {
 		let mut gasometer = Gasometer::<Cost>::new(Cost::from_u256(params.gas)?);
 		let mut stack = VecStack::with_capacity(ext.schedule().stack_limit, U256::zero());
 		let mut reader = CodeReader::new(code);
-		let infos = &*instructions::INSTRUCTIONS;
 
 		while reader.position < code.len() {
-			let instruction = code[reader.position];
+			let opcode = code[reader.position];
+			let instruction = Instruction::from_u8(opcode);
 			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, gasometer.current_gas.as_u256(),
+				reader.position - 1, opcode, gasometer.current_gas.as_u256(),
 			);
 
-			let info = &infos[instruction as usize];
+			if instruction.is_none() {
+				return Err(vm::Error::BadInstruction {
+					instruction: opcode
+				});
+			}
+			let instruction = instruction.expect("None case is checked above; qed");
+
+			let info = instruction.info();
 			self.verify_instruction(ext, instruction, info, &stack)?;
 
 			// Calculate gas cost
 			let requirements = gasometer.requirements(ext, instruction, info, &stack, self.mem.size())?;
 			if do_trace {
-				ext.trace_prepare_execute(reader.position - 1, instruction, requirements.gas_cost.as_u256());
+				ext.trace_prepare_execute(reader.position - 1, opcode, requirements.gas_cost.as_u256());
 			}
 
 			gasometer.verify_gas(&requirements.gas_cost)?;
@@ -227,13 +233,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 			((instruction == instructions::SHL || instruction == instructions::SHR || instruction == instructions::SAR) && !schedule.have_bitwise_shifting) {
 
 			return Err(vm::Error::BadInstruction {
-				instruction: instruction
+				instruction: instruction as u8
-			});
-		}
-
-		if info.tier == instructions::GasPriceTier::Invalid {
-			return Err(vm::Error::BadInstruction {
-				instruction: instruction
 			});
 		}
 
@@ -396,7 +396,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 					},
 					instructions::DELEGATECALL => (&params.sender, &params.address, true, CallType::DelegateCall),
 					instructions::STATICCALL => (&params.address, &code_address, true, CallType::StaticCall),
-					_ => panic!(format!("Unexpected instruction {} in CALL branch.", instruction))
+					_ => panic!(format!("Unexpected instruction {:?} in CALL branch.", instruction))
 				};
 
 				// clear return data buffer before creating new call frame.
@@ -453,8 +453,8 @@ impl<Cost: CostType> Interpreter<Cost> {
 				ext.suicide(&u256_to_address(&address))?;
 				return Ok(InstructionResult::StopExecution);
 			},
-			instructions::LOG0...instructions::LOG4 => {
+			instructions::LOG0 | instructions::LOG1 | instructions::LOG2 | instructions::LOG3 | instructions::LOG4 => {
-				let no_of_topics = instructions::get_log_topics(instruction);
+				let no_of_topics = instruction.log_topics().expect("log_topics always return some for LOG* instructions; qed");
 
 				let offset = stack.pop_back();
 				let size = stack.pop_back();
@@ -464,8 +464,15 @@ impl<Cost: CostType> Interpreter<Cost> {
 					.collect();
 				ext.log(topics, self.mem.read_slice(offset, size))?;
 			},
-			instructions::PUSH1...instructions::PUSH32 => {
+			instructions::PUSH1 | instructions::PUSH2 | instructions::PUSH3 | instructions::PUSH4 |
-				let bytes = instructions::get_push_bytes(instruction);
+			instructions::PUSH5 | instructions::PUSH6 | instructions::PUSH7 | instructions::PUSH8 |
+			instructions::PUSH9 | instructions::PUSH10 | instructions::PUSH11 | instructions::PUSH12 |
+			instructions::PUSH13 | instructions::PUSH14 | instructions::PUSH15 | instructions::PUSH16 |
+			instructions::PUSH17 | instructions::PUSH18 | instructions::PUSH19 | instructions::PUSH20 |
+			instructions::PUSH21 | instructions::PUSH22 | instructions::PUSH23 | instructions::PUSH24 |
+			instructions::PUSH25 | instructions::PUSH26 | instructions::PUSH27 | instructions::PUSH28 |
+			instructions::PUSH29 | instructions::PUSH30 | instructions::PUSH31 | instructions::PUSH32 => {
+				let bytes = instruction.push_bytes().expect("push_bytes always return some for PUSH* instructions");
 				let val = code.read(bytes);
 				stack.push(val);
 			},
@@ -609,73 +616,22 @@ impl<Cost: CostType> Interpreter<Cost> {
 			instructions::GASLIMIT => {
 				stack.push(ext.env_info().gas_limit.clone());
 			},
-			_ => {
-				self.exec_stack_instruction(instruction, stack)?;
-			}
-		};
-		Ok(InstructionResult::Ok)
-	}
 
-	fn copy_data_to_memory(mem: &mut Vec<u8>, stack: &mut Stack<U256>, source: &[u8]) {
+			// Stack instructions
-		let dest_offset = stack.pop_back();
-		let source_offset = stack.pop_back();
-		let size = stack.pop_back();
-		let source_size = U256::from(source.len());
 
-		let output_end = match source_offset > source_size || size > source_size || source_offset + size > source_size {
+			instructions::DUP1 | instructions::DUP2 | instructions::DUP3 | instructions::DUP4 |
-			true => {
+			instructions::DUP5 | instructions::DUP6 | instructions::DUP7 | instructions::DUP8 |
-				let zero_slice = if source_offset > source_size {
+			instructions::DUP9 | instructions::DUP10 | instructions::DUP11 | instructions::DUP12 |
-					mem.writeable_slice(dest_offset, size)
+			instructions::DUP13 | instructions::DUP14 | instructions::DUP15 | instructions::DUP16 => {
-				} else {
+				let position = instruction.dup_position().expect("dup_position always return some for DUP* instructions");
-					mem.writeable_slice(dest_offset + source_size - source_offset, source_offset + size - source_size)
-				};
-				for i in zero_slice.iter_mut() {
-					*i = 0;
-				}
-				source.len()
-			},
-			false => (size.low_u64() + source_offset.low_u64()) as usize
-		};
-
-		if source_offset < source_size {
-			let output_begin = source_offset.low_u64() as usize;
-			mem.write_slice(dest_offset, &source[output_begin..output_end]);
-		}
-	}
-
-	fn verify_jump(&self, jump_u: U256, valid_jump_destinations: &BitSet) -> vm::Result<usize> {
-		let jump = jump_u.low_u64() as usize;
-
-		if valid_jump_destinations.contains(jump) && U256::from(jump) == jump_u {
-			Ok(jump)
-		} else {
-			Err(vm::Error::BadJumpDestination {
-				destination: jump
-			})
-		}
-	}
-
-	fn is_zero(&self, val: &U256) -> bool {
-		val.is_zero()
-	}
-
-	fn bool_to_u256(&self, val: bool) -> U256 {
-		if val {
-			U256::one()
-		} else {
-			U256::zero()
-		}
-	}
-
-	fn exec_stack_instruction(&self, instruction: Instruction, stack: &mut Stack<U256>) -> vm::Result<()> {
-		match instruction {
-			instructions::DUP1...instructions::DUP16 => {
-				let position = instructions::get_dup_position(instruction);
 				let val = stack.peek(position).clone();
 				stack.push(val);
 			},
-			instructions::SWAP1...instructions::SWAP16 => {
+			instructions::SWAP1 | instructions::SWAP2 | instructions::SWAP3 | instructions::SWAP4 |
-				let position = instructions::get_swap_position(instruction);
+			instructions::SWAP5 | instructions::SWAP6 | instructions::SWAP7 | instructions::SWAP8 |
+			instructions::SWAP9 | instructions::SWAP10 | instructions::SWAP11 | instructions::SWAP12 |
+			instructions::SWAP13 | instructions::SWAP14 | instructions::SWAP15 | instructions::SWAP16 => {
+				let position = instruction.swap_position().expect("swap_position always return some for SWAP* instructions");
 				stack.swap_with_top(position)
 			},
 			instructions::POP => {
@@ -923,15 +879,60 @@ impl<Cost: CostType> Interpreter<Cost> {
 				};
 				stack.push(result);
 			},
-			_ => {
+		};
-				return Err(vm::Error::BadInstruction {
+		Ok(InstructionResult::Ok)
-					instruction: instruction
-				});
-			}
-		}
-		Ok(())
 	}
 
+	fn copy_data_to_memory(mem: &mut Vec<u8>, stack: &mut Stack<U256>, source: &[u8]) {
+		let dest_offset = stack.pop_back();
+		let source_offset = stack.pop_back();
+		let size = stack.pop_back();
+		let source_size = U256::from(source.len());
+
+		let output_end = match source_offset > source_size || size > source_size || source_offset + size > source_size {
+			true => {
+				let zero_slice = if source_offset > source_size {
+					mem.writeable_slice(dest_offset, size)
+				} else {
+					mem.writeable_slice(dest_offset + source_size - source_offset, source_offset + size - source_size)
+				};
+				for i in zero_slice.iter_mut() {
+					*i = 0;
+				}
+				source.len()
+			},
+			false => (size.low_u64() + source_offset.low_u64()) as usize
+		};
+
+		if source_offset < source_size {
+			let output_begin = source_offset.low_u64() as usize;
+			mem.write_slice(dest_offset, &source[output_begin..output_end]);
+		}
+	}
+
+	fn verify_jump(&self, jump_u: U256, valid_jump_destinations: &BitSet) -> vm::Result<usize> {
+		let jump = jump_u.low_u64() as usize;
+
+		if valid_jump_destinations.contains(jump) && U256::from(jump) == jump_u {
+			Ok(jump)
+		} else {
+			Err(vm::Error::BadJumpDestination {
+				destination: jump
+			})
+		}
+	}
+
+	fn is_zero(&self, val: &U256) -> bool {
+		val.is_zero()
+	}
+
+	fn bool_to_u256(&self, val: bool) -> U256 {
+		if val {
+			U256::one()
+		} else {
+			U256::zero()
+		}
+	}
 }
 
 fn get_and_reset_sign(value: U256) -> (U256, bool) {

ethcore/evm/src/interpreter/shared_cache.rs

@@ -21,7 +21,7 @@ use ethereum_types::H256;
 use parking_lot::Mutex;
 use memory_cache::MemoryLruCache;
 use bit_set::BitSet;
-use super::super::instructions;
+use super::super::instructions::{self, Instruction};
 
 const DEFAULT_CACHE_SIZE: usize = 4 * 1024 * 1024;
 
@@ -70,12 +70,14 @@ impl SharedCache {
 		let mut position = 0;
 
 		while position < code.len() {
-			let instruction = code[position];
+			let instruction = Instruction::from_u8(code[position]);
 
+			if let Some(instruction) = instruction {
 				if instruction == instructions::JUMPDEST {
 					jump_dests.insert(position);
-			} else if instructions::is_push(instruction) {
+				} else if let Some(push_bytes) = instruction.push_bytes() {
-				position += instructions::get_push_bytes(instruction);
+					position += push_bytes;
+				}
 			}
 			position += 1;
 		}

ethcore/evm/src/lib.rs

@@ -52,6 +52,6 @@ pub use vm::{
     GasLeft, ReturnData
 };
 pub use self::evm::{Finalize, FinalizationResult, CostType};
-pub use self::instructions::{InstructionInfo, INSTRUCTIONS, push_bytes};
+pub use self::instructions::{InstructionInfo, Instruction};
 pub use self::vmtype::VMType;
 pub use self::factory::Factory;

evmbin/src/display/json.rs

@@ -121,13 +121,13 @@ impl trace::VMTracer for Informant {
 	}
 
 	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];
+		let info = ::evm::Instruction::from_u8(self.instruction).map(|i| i.info());
 
 		let trace = format!(
 			"{{\"pc\":{pc},\"op\":{op},\"opName\":\"{name}\",\"gas\":\"0x{gas:x}\",\"gasCost\":\"0x{gas_cost:x}\",\"memory\":{memory},\"stack\":{stack},\"storage\":{storage},\"depth\":{depth}}}",
 			pc = self.pc,
 			op = self.instruction,
-			name = info.name,
+			name = info.map(|i| i.name).unwrap_or(""),
 			gas = gas_used.saturating_add(self.gas_cost),
 			gas_cost = self.gas_cost,
 			memory = self.memory(),
@@ -141,7 +141,8 @@ impl trace::VMTracer for Informant {
 		self.gas_used = gas_used;
 
 		let len = self.stack.len();
-		self.stack.truncate(if len > info.args { len - info.args } else { 0 });
+		let info_args = info.map(|i| i.args).unwrap_or(0);
+		self.stack.truncate(if len > info_args { len - info_args } else { 0 });
 		self.stack.extend_from_slice(stack_push);
 
 		// TODO [ToDr] Align memory?

evmbin/src/display/std_json.rs

@@ -118,7 +118,7 @@ impl<T: Writer> trace::VMTracer for Informant<T> {
 	type Output = ();
 
 	fn trace_next_instruction(&mut self, pc: usize, instruction: u8, current_gas: U256) -> bool {
-		let info = ::evm::INSTRUCTIONS[instruction as usize];
+		let info = ::evm::Instruction::from_u8(instruction).map(|i| i.info());
 		self.instruction = instruction;
 		let storage = self.storage();
 		let stack = self.stack();
@@ -128,7 +128,7 @@ impl<T: Writer> trace::VMTracer for Informant<T> {
 			"{{\"pc\":{pc},\"op\":{op},\"opName\":\"{name}\",\"gas\":\"0x{gas:x}\",\"stack\":{stack},\"storage\":{storage},\"depth\":{depth}}}",
 			pc = pc,
 			op = instruction,
-			name = info.name,
+			name = info.map(|i| i.name).unwrap_or(""),
 			gas = current_gas,
 			stack = stack,
 			storage = storage,
@@ -142,10 +142,11 @@ impl<T: Writer> trace::VMTracer for Informant<T> {
 	}
 
 	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];
+		let info = ::evm::Instruction::from_u8(self.instruction).map(|i| i.info());
 
 		let len = self.stack.len();
-		self.stack.truncate(if len > info.args { len - info.args } else { 0 });
+		let info_args = info.map(|i| i.args).unwrap_or(0);
+		self.stack.truncate(if len > info_args { len - info_args } else { 0 });
 		self.stack.extend_from_slice(stack_push);
 
 		if let Some((pos, val)) = store_diff {