From 178d8e1a41ea2d8b987881ff1cfeae6f7b3c2022 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Mon, 13 Jun 2022 10:10:47 +0200 Subject: [PATCH 1/8] `ExitError` specify invalid opcode --- core/src/error.rs | 6 +++--- gasometer/src/lib.rs | 2 +- runtime/src/handler.rs | 4 ++-- src/executor/stack/executor.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/error.rs b/core/src/error.rs index f6c70fc5a..2b5390d51 100644 --- 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(u8), /// An opcode accesses external information, but the request is off offset /// limit (runtime). diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 32163ca4a..1abc11f42 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -610,7 +610,7 @@ pub fn dynamic_opcode_cost( } } - _ => GasCost::Invalid, + _ => return Err(ExitError::InvalidCode(opcode.0)), }; let memory_cost = match opcode { diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index ccb52d502..90a1d3b85 100644 --- 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.0)) } } diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 3d1750031..187c79337 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -648,7 +648,7 @@ 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); + return Err(ExitError::InvalidCode(0xef)); } } Ok(()) From d2c0df50d04ed15a528c0a473e78ea97fd5eda79 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Tue, 14 Jun 2022 16:36:15 +0200 Subject: [PATCH 2/8] Do not early return, propagate instead --- gasometer/src/lib.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 1abc11f42..6a1150f45 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -450,19 +450,19 @@ pub fn dynamic_opcode_cost( 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.0), Opcode::CHAINID if config.has_chain_id => GasCost::Base, - Opcode::CHAINID => GasCost::Invalid, + Opcode::CHAINID => GasCost::Invalid(opcode.0), 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.0), Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, - Opcode::SELFBALANCE => GasCost::Invalid, + Opcode::SELFBALANCE => GasCost::Invalid(opcode.0), Opcode::BASEFEE if config.has_base_fee => GasCost::Base, - Opcode::BASEFEE => GasCost::Invalid, + Opcode::BASEFEE => GasCost::Invalid(opcode.0), Opcode::EXTCODESIZE => { let target = stack.peek(0)?.into(); @@ -487,7 +487,7 @@ pub fn dynamic_opcode_cost( target_is_cold: handler.is_cold(target, None), } } - Opcode::EXTCODEHASH => GasCost::Invalid, + Opcode::EXTCODEHASH => GasCost::Invalid(opcode.0), Opcode::CALLCODE => { let target = stack.peek(1)?.into(); @@ -542,13 +542,13 @@ pub fn dynamic_opcode_cost( target_exists: handler.exists(target), } } - Opcode::DELEGATECALL => GasCost::Invalid, + Opcode::DELEGATECALL => GasCost::Invalid(opcode.0), 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.0), Opcode::SSTORE if !is_static => { let index = stack.peek(0)?; @@ -610,7 +610,7 @@ pub fn dynamic_opcode_cost( } } - _ => return Err(ExitError::InvalidCode(opcode.0)), + _ => GasCost::Invalid(opcode.0), }; 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(u8), /// Gas cost for `EXTCODESIZE`. ExtCodeSize { From f53ff0f55ddc2bdf80cec41698e055b41d4c7142 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 09:28:15 +0200 Subject: [PATCH 3/8] `InvalidCode(Opcode)` --- core/src/error.rs | 2 +- core/src/opcode.rs | 5 +++++ gasometer/src/lib.rs | 20 ++++++++++---------- runtime/src/handler.rs | 2 +- src/executor/stack/executor.rs | 2 +- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/core/src/error.rs b/core/src/error.rs index 2b5390d51..3c3eeddf1 100644 --- a/core/src/error.rs +++ b/core/src/error.rs @@ -129,7 +129,7 @@ pub enum ExitError { CreateContractLimit, /// 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(u8), + 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 index e02e3c694..814223c68 100644 --- 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. diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 6a1150f45..564a2d3e0 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -450,19 +450,19 @@ pub fn dynamic_opcode_cost( Opcode::MLOAD | Opcode::MSTORE | Opcode::MSTORE8 => GasCost::VeryLow, Opcode::REVERT if config.has_revert => GasCost::Zero, - Opcode::REVERT => GasCost::Invalid(opcode.0), + Opcode::REVERT => GasCost::Invalid(opcode), Opcode::CHAINID if config.has_chain_id => GasCost::Base, - Opcode::CHAINID => GasCost::Invalid(opcode.0), + 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.0), + Opcode::SHL | Opcode::SHR | Opcode::SAR => GasCost::Invalid(opcode), Opcode::SELFBALANCE if config.has_self_balance => GasCost::Low, - Opcode::SELFBALANCE => GasCost::Invalid(opcode.0), + Opcode::SELFBALANCE => GasCost::Invalid(opcode), Opcode::BASEFEE if config.has_base_fee => GasCost::Base, - Opcode::BASEFEE => GasCost::Invalid(opcode.0), + Opcode::BASEFEE => GasCost::Invalid(opcode), Opcode::EXTCODESIZE => { let target = stack.peek(0)?.into(); @@ -487,7 +487,7 @@ pub fn dynamic_opcode_cost( target_is_cold: handler.is_cold(target, None), } } - Opcode::EXTCODEHASH => GasCost::Invalid(opcode.0), + Opcode::EXTCODEHASH => GasCost::Invalid(opcode), Opcode::CALLCODE => { let target = stack.peek(1)?.into(); @@ -542,13 +542,13 @@ pub fn dynamic_opcode_cost( target_exists: handler.exists(target), } } - Opcode::DELEGATECALL => GasCost::Invalid(opcode.0), + 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.0), + 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( } } - _ => GasCost::Invalid(opcode.0), + _ => GasCost::Invalid(opcode), }; let memory_cost = match opcode { @@ -852,7 +852,7 @@ pub enum GasCost { /// Low gas cost. Low, /// Fail the gasometer. - Invalid(u8), + Invalid(Opcode), /// Gas cost for `EXTCODESIZE`. ExtCodeSize { diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 90a1d3b85..4c3aea80a 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -116,6 +116,6 @@ pub trait Handler { ) -> Result<(), ExitError>; /// Handle other unknown external opcodes. fn other(&mut self, opcode: Opcode, _stack: &mut Machine) -> Result<(), ExitError> { - Err(ExitError::InvalidCode(opcode.0)) + Err(ExitError::InvalidCode(opcode)) } } diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 187c79337..4d2938319 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -648,7 +648,7 @@ 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(0xef)); + return Err(ExitError::InvalidCode(Opcode(0xef))); } } Ok(()) From 1c669ab664f416839a4e5a9770140d0b98d79af0 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 09:30:44 +0200 Subject: [PATCH 4/8] Use const --- src/executor/stack/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 4d2938319..71f5e09bd 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -648,7 +648,7 @@ 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(Opcode(0xef))); + return Err(ExitError::InvalidCode(Opcode::INVALID)); } } Ok(()) From 16f44e91f1deb9003e8ad46071dcb7a20e871b38 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 11:52:25 +0200 Subject: [PATCH 5/8] Use constant --- src/executor/stack/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 71f5e09bd..0517243a2 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -647,7 +647,7 @@ 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) { + if Some(&Opcode::INVALID.as_u8()) == code.get(0) { return Err(ExitError::InvalidCode(Opcode::INVALID)); } } From 79e7157a8308c674c227337e21530e3eb52b6537 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 11:57:46 +0200 Subject: [PATCH 6/8] clippy --- src/executor/stack/executor.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 0517243a2..2fab1f389 100644 --- 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 Some(&Opcode::INVALID.as_u8()) == code.get(0) { - return Err(ExitError::InvalidCode(Opcode::INVALID)); - } + if config.disallow_executable_format && Some(&Opcode::INVALID.as_u8()) == code.get(0) { + return Err(ExitError::InvalidCode(Opcode::INVALID)); } Ok(()) } From 19b9221e9ba986365dde3fd159498ebae70932f6 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 14:37:30 +0200 Subject: [PATCH 7/8] Revert "Use const" This reverts commit 1c669ab664f416839a4e5a9770140d0b98d79af0. --- src/executor/stack/executor.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 2fab1f389..4d2938319 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -646,8 +646,10 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } fn check_first_byte(config: &Config, code: &[u8]) -> Result<(), ExitError> { - if config.disallow_executable_format && Some(&Opcode::INVALID.as_u8()) == code.get(0) { - return Err(ExitError::InvalidCode(Opcode::INVALID)); + if config.disallow_executable_format { + if let Some(0xef) = code.get(0) { + return Err(ExitError::InvalidCode(Opcode(0xef))); + } } Ok(()) } From 517af5a209182dfcf638c60a9dc17dcb3fe8fb4d Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 15 Jun 2022 14:50:07 +0200 Subject: [PATCH 8/8] Add 0xef const --- core/src/opcode.rs | 3 +++ src/executor/stack/executor.rs | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/opcode.rs b/core/src/opcode.rs index 814223c68..6820c2f46 100644 --- a/core/src/opcode.rs +++ b/core/src/opcode.rs @@ -171,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/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 4d2938319..a1fc1a619 100644 --- 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(Opcode(0xef))); - } + if config.disallow_executable_format && Some(&Opcode::EOFMAGIC.as_u8()) == code.get(0) { + return Err(ExitError::InvalidCode(Opcode::EOFMAGIC)); } Ok(()) }