ExitError specify invalid opcode (#126)

* `ExitError` specify invalid opcode * Do not early return, propagate instead * `InvalidCode(Opcode)` * Use const * Use constant * clippy * Revert "Use const" This reverts commit 1c669ab. * Add 0xef const
rust-ethereum · Jun 15, 2022 · 51b8c2c · 51b8c2c
1 parent 01bcbd2
commit 51b8c2c
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 20 deletions.
diff --git a/core/src/error.rs b/core/src/error.rs
@@ -127,9 +127,9 @@ pub enum ExitError {
 	/// Create init code exceeds limit (runtime).
 	#[cfg_attr(feature = "with-codec", codec(index = 7))]
 	CreateContractLimit,
-	/// Starting byte must not begin with 0xef. See [EIP-3541](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-3541.md).
-	#[cfg_attr(feature = "with-codec", codec(index = 14))]
-	InvalidCode,
+	/// Invalid opcode during execution or starting byte is 0xef. See [EIP-3541](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-3541.md).
+	#[cfg_attr(feature = "with-codec", codec(index = 15))]
+	InvalidCode(Opcode),
 
 	/// An opcode accesses external information, but the request is off offset
 	/// limit (runtime).

diff --git a/core/src/opcode.rs b/core/src/opcode.rs
@@ -1,5 +1,10 @@
 /// Opcode enum. One-to-one corresponding to an `u8` value.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[cfg_attr(
+	feature = "with-codec",
+	derive(codec::Encode, codec::Decode, scale_info::TypeInfo)
+)]
+#[cfg_attr(feature = "with-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Opcode(pub u8);
 
 // Core opcodes.
@@ -166,6 +171,9 @@ impl Opcode {
 
 	/// `INVALID`
 	pub const INVALID: Opcode = Opcode(0xfe);
+
+	/// See [EIP-3541](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-3541.md)
+	pub const EOFMAGIC: Opcode = Opcode(0xef);
 }
 
 // External opcodes

diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs
@@ -450,19 +450,19 @@ pub fn dynamic_opcode_cost<H: Handler>(
 		Opcode::MLOAD | Opcode::MSTORE | Opcode::MSTORE8 => GasCost::VeryLow,
 
 		Opcode::REVERT if config.has_revert => GasCost::Zero,
-		Opcode::REVERT => GasCost::Invalid,
+		Opcode::REVERT => GasCost::Invalid(opcode),
 
 		Opcode::CHAINID if config.has_chain_id => GasCost::Base,
-		Opcode::CHAINID => GasCost::Invalid,
+		Opcode::CHAINID => GasCost::Invalid(opcode),
 
 		Opcode::SHL | Opcode::SHR | Opcode::SAR if config.has_bitwise_shifting => GasCost::VeryLow,
-		Opcode::SHL | Opcode::SHR | Opcode::SAR => GasCost::Invalid,
+		Opcode::SHL | Opcode::SHR | Opcode::SAR => GasCost::Invalid(opcode),
 
 		Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low,
-		Opcode::SELFBALANCE => GasCost::Invalid,
+		Opcode::SELFBALANCE => GasCost::Invalid(opcode),
 
 		Opcode::BASEFEE if config.has_base_fee => GasCost::Base,
-		Opcode::BASEFEE => GasCost::Invalid,
+		Opcode::BASEFEE => GasCost::Invalid(opcode),
 
 		Opcode::EXTCODESIZE => {
 			let target = stack.peek(0)?.into();
@@ -487,7 +487,7 @@ pub fn dynamic_opcode_cost<H: Handler>(
 				target_is_cold: handler.is_cold(target, None),
 			}
 		}
-		Opcode::EXTCODEHASH => GasCost::Invalid,
+		Opcode::EXTCODEHASH => GasCost::Invalid(opcode),
 
 		Opcode::CALLCODE => {
 			let target = stack.peek(1)?.into();
@@ -542,13 +542,13 @@ pub fn dynamic_opcode_cost<H: Handler>(
 				target_exists: handler.exists(target),
 			}
 		}
-		Opcode::DELEGATECALL => GasCost::Invalid,
+		Opcode::DELEGATECALL => GasCost::Invalid(opcode),
 
 		Opcode::RETURNDATASIZE if config.has_return_data => GasCost::Base,
 		Opcode::RETURNDATACOPY if config.has_return_data => GasCost::VeryLowCopy {
 			len: U256::from_big_endian(&stack.peek(2)?[..]),
 		},
-		Opcode::RETURNDATASIZE | Opcode::RETURNDATACOPY => GasCost::Invalid,
+		Opcode::RETURNDATASIZE | Opcode::RETURNDATACOPY => GasCost::Invalid(opcode),
 
 		Opcode::SSTORE if !is_static => {
 			let index = stack.peek(0)?;
@@ -610,7 +610,7 @@ pub fn dynamic_opcode_cost<H: Handler>(
 			}
 		}
 
-		_ => GasCost::Invalid,
+		_ => GasCost::Invalid(opcode),
 	};
 
 	let memory_cost = match opcode {
@@ -801,7 +801,7 @@ impl<'config> Inner<'config> {
 			GasCost::Base => consts::G_BASE,
 			GasCost::VeryLow => consts::G_VERYLOW,
 			GasCost::Low => consts::G_LOW,
-			GasCost::Invalid => return Err(ExitError::OutOfGas),
+			GasCost::Invalid(opcode) => return Err(ExitError::InvalidCode(opcode)),
 
 			GasCost::ExtCodeSize { target_is_cold } => {
 				costs::address_access_cost(target_is_cold, self.config.gas_ext_code, self.config)
@@ -852,7 +852,7 @@ pub enum GasCost {
 	/// Low gas cost.
 	Low,
 	/// Fail the gasometer.
-	Invalid,
+	Invalid(Opcode),
 
 	/// Gas cost for `EXTCODESIZE`.
 	ExtCodeSize {

diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs
@@ -115,7 +115,7 @@ pub trait Handler {
 		stack: &Stack,
 	) -> Result<(), ExitError>;
 	/// Handle other unknown external opcodes.
-	fn other(&mut self, _opcode: Opcode, _stack: &mut Machine) -> Result<(), ExitError> {
-		Err(ExitError::OutOfGas)
+	fn other(&mut self, opcode: Opcode, _stack: &mut Machine) -> Result<(), ExitError> {
+		Err(ExitError::InvalidCode(opcode))
 	}
 }
diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs
@@ -646,10 +646,8 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet>
 		}
 
 		fn check_first_byte(config: &Config, code: &[u8]) -> Result<(), ExitError> {
-			if config.disallow_executable_format {
-				if let Some(0xef) = code.get(0) {
-					return Err(ExitError::InvalidCode);
-				}
+			if config.disallow_executable_format && Some(&Opcode::EOFMAGIC.as_u8()) == code.get(0) {
+				return Err(ExitError::InvalidCode(Opcode::EOFMAGIC));
 			}
 			Ok(())
 		}