From e5e945c9f8134155cbb24b3004373fdfcfda9f2b Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Sun, 27 Aug 2023 21:33:51 +0200 Subject: [PATCH 01/13] returndatacopy opcode --- crates/evm/src/errors.cairo | 10 ++ .../environmental_information.cairo | 20 +++- .../test_environment_information.cairo | 92 ++++++++++++++++++- crates/evm/src/tests/test_utils.cairo | 14 +++ crates/utils/Scarb.toml | 1 + crates/utils/src/helpers.cairo | 11 ++- 6 files changed, 142 insertions(+), 6 deletions(-) diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index 717e6c840..176230948 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -8,10 +8,18 @@ const STACK_UNDERFLOW: felt252 = 'Kakarot: StackUnderflow'; // INSTRUCTIONS const PC_OUT_OF_BOUNDS: felt252 = 'Kakarot: pc >= bytecode length'; +// TYPE CONVERSION +const TYPE_CONVERSION_ERROR: felt252 = 'Kakarot: type conversion error'; + +// RETURNDATA +const RETURNDATA_OUT_OF_BOUNDS_ERROR: felt252 = 'Kakarot: out of bounds'; + #[derive(Drop, Copy, PartialEq)] enum EVMError { StackError: felt252, InvalidProgramCounter: felt252, + TypeConversionError: felt252, + ReturnDataError: felt252 } @@ -20,6 +28,8 @@ impl EVMErrorIntoU256 of Into { match self { EVMError::StackError(error_message) => error_message.into(), EVMError::InvalidProgramCounter(error_message) => error_message.into(), + EVMError::TypeConversionError(error_message) => error_message.into(), + EVMError::ReturnDataError(error_message) => error_message.into(), } } } diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 1ffdbd6db..413c40757 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -6,8 +6,9 @@ use evm::context::ExecutionContext; use evm::context::ExecutionContextTrait; use evm::context::CallContextTrait; use evm::context::BoxDynamicExecutionContextDestruct; -use evm::errors::EVMError; -use utils::helpers::EthAddressIntoU256; +use evm::errors::{EVMError, RETURNDATA_OUT_OF_BOUNDS_ERROR}; +use utils::helpers::{EthAddressIntoU256, u256_to_u32}; +use evm::memory::MemoryTrait; #[generate_trait] impl EnvironmentInformationImpl of EnvironmentInformationTrait { @@ -113,6 +114,21 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait { /// Save word to memory. /// # Specification: https://www.evm.codes/#3e?fork=shanghai fn exec_returndatacopy(ref self: ExecutionContext) -> Result<(), EVMError> { + let popped = self.stack.pop_n(3)?; + let destOffset: u32 = u256_to_u32(*popped[0])?; + let offset: u32 = u256_to_u32(*popped[1])?; + let size: u32 = u256_to_u32(*popped[2])?; + + let return_data: Span = self.return_data(); + + let mut slice_size = size; + if (offset + size > return_data.len()) { + return Result::Err(EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR)); + } + + let data_to_copy: Span = return_data.slice(offset, slice_size); + self.memory.store_n(data_to_copy, destOffset); + Result::Ok(()) } diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 279aad7f3..d1e172ab5 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -1,10 +1,14 @@ +use evm::context::{BoxDynamicExecutionContextDestruct, ExecutionContextTrait}; use evm::instructions::EnvironmentInformationTrait; -use evm::tests::test_utils::{setup_execution_context, evm_address, callvalue}; +use evm::memory::{InternalMemoryTrait, MemoryTrait}; +use evm::tests::test_utils::{ + setup_execution_context, setup_execution_context_with_returndata, evm_address, callvalue +}; use evm::stack::StackTrait; use option::OptionTrait; use starknet::EthAddressIntoFelt252; -use evm::context::BoxDynamicExecutionContextDestruct; -use utils::helpers::EthAddressIntoU256; +use utils::helpers::{EthAddressIntoU256, u256_to_bytes_array}; +use evm::errors::{EVMError, TYPE_CONVERSION_ERROR, RETURNDATA_OUT_OF_BOUNDS_ERROR}; #[test] #[available_gas(20000000)] @@ -41,3 +45,85 @@ fn test__exec_callvalue() { assert(ctx.stack.len() == 1, 'stack should have one element'); assert(ctx.stack.pop().unwrap() == callvalue(), 'should be `123456789'); } + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_type_conversion_error() { + // Given + let mut ctx = setup_execution_context_with_returndata(array![1, 2, 3, 4, 5]); + + ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); + ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); + ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); + + // When + let res = ctx.exec_returndatacopy(); + + // Then + assert(res.is_err(), 'should return error'); + assert( + res.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR), + 'should return ConversionError' + ); +} + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_basic() { + test_returndata_copy(32, 0, 0); +} + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_with_offset() { + test_returndata_copy(32, 2, 0); +} + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_with_out_of_bound_bytes() { + test_returndata_copy(32, 0, 8); +} + +fn test_returndata_copy(destOffset: u32, offset: u32, mut size: u32) { + // Given + + let mut ctx = setup_execution_context_with_returndata(array![1, 2, 3, 4, 5]); + let return_data: Span = ctx.return_data(); + + if (size == 0) { + size = return_data.len() - offset; + } + + ctx.stack.push(size.into()); + ctx.stack.push(offset.into()); + ctx.stack.push(destOffset.into()); + + // When + let res = ctx.exec_returndatacopy(); + + // Then + assert(ctx.stack.is_empty(), 'stack should be empty'); + + if (offset + size > return_data.len()) { + assert(res.is_err(), 'should return error'); + assert( + res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), + 'should return out of bounds' + ); + } else { + let result: u256 = ctx.memory.load_internal(destOffset).into(); + let mut results: Array = u256_to_bytes_array(result); + + let mut i = 0; + loop { + if (i == size) { + break; + } + + assert(*results[i] == *return_data[i + offset], 'wrong data value'); + + i += 1; + }; + } +} diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index 094457fd0..81bb42b54 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -42,6 +42,20 @@ fn setup_execution_context() -> ExecutionContext { ) } +fn setup_execution_context_with_returndata(return_data: Array) -> ExecutionContext { + let call_context = setup_call_context(); + let starknet_address: ContractAddress = starknet_address(); + let evm_address: EthAddress = evm_address(); + let gas_limit: u64 = 1000; + let gas_price: u64 = 10; + let read_only: bool = false; + let returned_data = return_data; + + ExecutionContextTrait::new( + call_context, starknet_address, evm_address, gas_limit, gas_price, returned_data, read_only + ) +} + impl CallContextPartialEq of PartialEq { fn eq(lhs: @CallContext, rhs: @CallContext) -> bool { lhs.bytecode() == rhs.bytecode() && lhs.call_data == rhs.call_data && lhs.value == rhs.value diff --git a/crates/utils/Scarb.toml b/crates/utils/Scarb.toml index d784eea29..5d05d7d9a 100644 --- a/crates/utils/Scarb.toml +++ b/crates/utils/Scarb.toml @@ -5,3 +5,4 @@ version = "0.1.0" # See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html [dependencies] +evm = { path = "../evm" } \ No newline at end of file diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index 62a32357b..5d5fa6483 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -4,7 +4,8 @@ use traits::{Into, TryInto}; use option::OptionTrait; use debug::PrintTrait; use starknet::{EthAddress, EthAddressIntoFelt252}; - +use result::ResultTrait; +use evm::errors::{EVMError, TYPE_CONVERSION_ERROR}; use utils::{constants}; @@ -219,3 +220,11 @@ impl EthAddressIntoU256 of Into { intermediate.into() } } + +// Try converting u256 to u32 +fn u256_to_u32(value: u256) -> Result { + match value.try_into() { + Option::Some(value) => Result::Ok(value), + Option::None(_) => Result::Err(EVMError::TypeConversionError(TYPE_CONVERSION_ERROR)) + } +} From bf87cfc39756ec3459b0d18e6f3cb6e1fa049250 Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Tue, 29 Aug 2023 00:14:59 +0200 Subject: [PATCH 02/13] RETURNDATACOPY Opcode --- crates/evm/src/helpers.cairo | 14 ++++++++++++++ .../instructions/environmental_information.cairo | 14 +++++++------- crates/evm/src/lib.cairo | 3 +++ crates/utils/Scarb.toml | 1 - crates/utils/src/helpers.cairo | 12 +----------- 5 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 crates/evm/src/helpers.cairo diff --git a/crates/evm/src/helpers.cairo b/crates/evm/src/helpers.cairo new file mode 100644 index 000000000..fdc53c860 --- /dev/null +++ b/crates/evm/src/helpers.cairo @@ -0,0 +1,14 @@ +use traits::{Into, TryInto}; +use option::OptionTrait; +use result::ResultTrait; +use evm::errors::{EVMError, TYPE_CONVERSION_ERROR}; + +// Try converting u256 to u32 +impl U256IntoResultU32 of Into> { + fn into(self: u256) -> Result { + match self.try_into() { + Option::Some(value) => Result::Ok(value), + Option::None(_) => Result::Err(EVMError::TypeConversionError(TYPE_CONVERSION_ERROR)) + } + } +} \ No newline at end of file diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 413c40757..262344712 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -7,7 +7,8 @@ use evm::context::ExecutionContextTrait; use evm::context::CallContextTrait; use evm::context::BoxDynamicExecutionContextDestruct; use evm::errors::{EVMError, RETURNDATA_OUT_OF_BOUNDS_ERROR}; -use utils::helpers::{EthAddressIntoU256, u256_to_u32}; +use evm::helpers::U256IntoResultU32; +use utils::helpers::EthAddressIntoU256; use evm::memory::MemoryTrait; #[generate_trait] @@ -115,19 +116,18 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait { /// # Specification: https://www.evm.codes/#3e?fork=shanghai fn exec_returndatacopy(ref self: ExecutionContext) -> Result<(), EVMError> { let popped = self.stack.pop_n(3)?; - let destOffset: u32 = u256_to_u32(*popped[0])?; - let offset: u32 = u256_to_u32(*popped[1])?; - let size: u32 = u256_to_u32(*popped[2])?; + let dest_offset: u32 = Into::>::into((*popped[0]))?; + let offset: u32 = Into::>::into((*popped[1]))?; + let size: u32 = Into::>::into((*popped[2]))?; let return_data: Span = self.return_data(); - let mut slice_size = size; if (offset + size > return_data.len()) { return Result::Err(EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR)); } - let data_to_copy: Span = return_data.slice(offset, slice_size); - self.memory.store_n(data_to_copy, destOffset); + let data_to_copy: Span = return_data.slice(offset, size); + self.memory.store_n(data_to_copy, dest_offset); Result::Ok(()) } diff --git a/crates/evm/src/lib.cairo b/crates/evm/src/lib.cairo index 926df2763..663d8c86e 100644 --- a/crates/evm/src/lib.cairo +++ b/crates/evm/src/lib.cairo @@ -22,6 +22,9 @@ mod model; // Errors module mod errors; +// Helpers module +mod helpers; + // tests #[cfg(test)] mod tests; diff --git a/crates/utils/Scarb.toml b/crates/utils/Scarb.toml index 5d05d7d9a..d784eea29 100644 --- a/crates/utils/Scarb.toml +++ b/crates/utils/Scarb.toml @@ -5,4 +5,3 @@ version = "0.1.0" # See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html [dependencies] -evm = { path = "../evm" } \ No newline at end of file diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index da5d6beaa..e3bad1c49 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -4,8 +4,6 @@ use traits::{Into, TryInto}; use option::OptionTrait; use debug::PrintTrait; use starknet::{EthAddress, EthAddressIntoFelt252}; -use result::ResultTrait; -use evm::errors::{EVMError, TYPE_CONVERSION_ERROR}; use cmp::min; use utils::constants; @@ -222,12 +220,4 @@ impl EthAddressIntoU256 of Into { let intermediate: felt252 = self.into(); intermediate.into() } -} - -// Try converting u256 to u32 -fn u256_to_u32(value: u256) -> Result { - match value.try_into() { - Option::Some(value) => Result::Ok(value), - Option::None(_) => Result::Err(EVMError::TypeConversionError(TYPE_CONVERSION_ERROR)) - } -} +} \ No newline at end of file From 2354bb04a0835237dfad3111227635659e5ef2ee Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Tue, 29 Aug 2023 20:20:06 +0200 Subject: [PATCH 03/13] Refactoring --- .../environmental_information.cairo | 1 - .../test_environment_information.cairo | 104 +++++++++++------- 2 files changed, 66 insertions(+), 39 deletions(-) diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 2f5707132..f4cddb1dd 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -8,7 +8,6 @@ use evm::context::{ ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait }; use utils::helpers::EthAddressIntoU256; -use evm::helpers::U256IntoResultU32; use evm::memory::MemoryTrait; #[generate_trait] diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index b6dff453a..12844e0d2 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -1,11 +1,9 @@ -use evm::context::{BoxDynamicExecutionContextDestruct, ExecutionContextTrait}; use evm::instructions::EnvironmentInformationTrait; use evm::memory::{InternalMemoryTrait, MemoryTrait}; use evm::tests::test_utils::{ setup_execution_context, setup_execution_context_with_bytecode, evm_address, callvalue }; use evm::stack::StackTrait; -use evm::memory::{InternalMemoryTrait, MemoryTrait}; use option::OptionTrait; use starknet::EthAddressIntoFelt252; use utils::helpers::{EthAddressIntoU256, u256_to_bytes_array}; @@ -14,6 +12,10 @@ use evm::context::{ ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait }; +// ************************************************************************* +// 0x30: ADDRESS +// ************************************************************************* + #[test] #[available_gas(20000000)] fn test_address_basic() { @@ -36,6 +38,10 @@ fn test_address_nested_call() { // A (EOA) -(calls)-> B (smart contract) -(calls // ref: https://github.com/kkrt-labs/kakarot-ssj/issues/183 } +// ************************************************************************* +// 0x34: CALLVALUE +// ************************************************************************* + #[test] #[available_gas(120000)] fn test__exec_callvalue() { @@ -50,36 +56,9 @@ fn test__exec_callvalue() { assert(ctx.stack.pop().unwrap() == callvalue(), 'should be `123456789'); } -#[test] -#[available_gas(20000000)] -fn test_gasprice() { - // Given - let mut ctx = setup_execution_context(); - - // When - ctx.exec_gasprice(); - - // Then - assert(ctx.stack.len() == 1, 'stack should have one element'); - assert(ctx.stack.peek().unwrap() == 10, 'stack top should be 10'); -} - -#[test] -#[available_gas(20000000)] -fn test_returndatasize() { - // Given - let return_data: Array = array![1, 2, 3, 4, 5]; - let size = return_data.len(); - let mut ctx = setup_execution_context(); - ctx.set_return_data(return_data); - - // When - ctx.exec_returndatasize(); - - // Then - assert(ctx.stack.len() == 1, 'stack should have one element'); - assert(ctx.stack.pop().unwrap() == size.into(), 'wrong returndatasize'); -} +// ************************************************************************* +// 0x36: CALLDATASIZE +// ************************************************************************* #[test] #[available_gas(20000000)] @@ -96,6 +75,10 @@ fn test_calldata_size() { assert(ctx.stack.peek().unwrap() == call_data.len().into(), 'stack top is not calldatasize'); } +// ************************************************************************* +// 0x38: CODESIZE +// ************************************************************************* + #[test] #[available_gas(20000000)] fn test_codesize() { @@ -111,6 +94,10 @@ fn test_codesize() { assert(ctx.stack.pop().unwrap() == bytecode.len().into(), 'wrong codesize'); } +// ************************************************************************* +// 0x39: CODECOPY +// ************************************************************************* + #[test] #[available_gas(20000000)] fn test_codecopy_type_conversion_error() { @@ -199,15 +186,55 @@ fn test_codecopy(dest_offset: u32, offset: u32, mut size: u32) { }; } +// ************************************************************************* +// 0x3A: GASPRICE +// ************************************************************************* + #[test] #[available_gas(20000000)] -fn test_returndata_copy_type_conversion_error() { +fn test_gasprice() { // Given - let mut ctx = setup_execution_context_with_returndata(array![1, 2, 3, 4, 5]); - + let mut ctx = setup_execution_context(); + + // When + ctx.exec_gasprice(); + + // Then + assert(ctx.stack.len() == 1, 'stack should have one element'); + assert(ctx.stack.peek().unwrap() == 10, 'stack top should be 10'); +} + +// ************************************************************************* +// 0x3D: RETURNDATASIZE +// ************************************************************************* + +#[test] +#[available_gas(20000000)] +fn test_returndatasize() { // Given - let bytecode: Span = array![1, 2, 3, 4, 5].span(); - let mut ctx = setup_execution_context_with_bytecode(bytecode); + let return_data: Array = array![1, 2, 3, 4, 5]; + let size = return_data.len(); + let mut ctx = setup_execution_context(); + ctx.set_return_data(return_data); + + // When + ctx.exec_returndatasize(); + + // Then + assert(ctx.stack.len() == 1, 'stack should have one element'); + assert(ctx.stack.pop().unwrap() == size.into(), 'wrong returndatasize'); +} + +// ************************************************************************* +// 0x3E: RETURNDATACOPY +// ************************************************************************* + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_type_conversion_error() { + // Given + let mut ctx = setup_execution(); + ctx.set_return_data(array![1, 2, 3, 4, 5]); ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); @@ -243,8 +270,9 @@ fn test_returndata_copy_with_out_of_bound_bytes() { fn test_returndata_copy(destOffset: u32, offset: u32, mut size: u32) { // Given + let mut ctx = setup_execution_context(); + ctx.set_return_data(array![1, 2, 3, 4, 5]); - let mut ctx = setup_execution_context_with_returndata(array![1, 2, 3, 4, 5]); let return_data: Span = ctx.return_data(); if (size == 0) { From fa51662595faf0293dd3fa43e2b04ed03535cf7e Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Tue, 29 Aug 2023 20:25:52 +0200 Subject: [PATCH 04/13] RETURNDATACOPY --- crates/evm/src/helpers.cairo | 2 +- .../test_instructions/test_environment_information.cairo | 6 +++--- crates/evm/src/tests/test_utils.cairo | 2 +- crates/utils/src/helpers.cairo | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/evm/src/helpers.cairo b/crates/evm/src/helpers.cairo index 34e64bced..1e5642410 100644 --- a/crates/evm/src/helpers.cairo +++ b/crates/evm/src/helpers.cairo @@ -11,4 +11,4 @@ impl U256IntoResultU32 of Into> { Option::None(_) => Result::Err(EVMError::TypeConversionError(TYPE_CONVERSION_ERROR)) } } -} \ No newline at end of file +} diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 12844e0d2..46c3819a1 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -111,7 +111,7 @@ fn test_codecopy_type_conversion_error() { // When let res = ctx.exec_codecopy(); - + // Then assert(res.is_err(), 'should return error'); assert( @@ -233,7 +233,7 @@ fn test_returndatasize() { #[available_gas(20000000)] fn test_returndata_copy_type_conversion_error() { // Given - let mut ctx = setup_execution(); + let mut ctx = setup_execution_context(); ctx.set_return_data(array![1, 2, 3, 4, 5]); ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); @@ -241,7 +241,7 @@ fn test_returndata_copy_type_conversion_error() { ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); // When - let res = ctx.exec_returndatacopy(); + let res = ctx.exec_returndatacopy(); // Then assert(res.is_err(), 'should return error'); assert( diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index 6eff817b8..7f959b931 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -57,7 +57,7 @@ fn setup_execution_context_with_bytecode(bytecode: Span) -> ExecutionContext let gas_price: u64 = 10; let read_only: bool = false; let returned_data = Default::default(); - + ExecutionContextTrait::new( call_context, starknet_address, evm_address, gas_limit, gas_price, returned_data, read_only ) diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index e3bad1c49..6adb6a6a0 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -220,4 +220,4 @@ impl EthAddressIntoU256 of Into { let intermediate: felt252 = self.into(); intermediate.into() } -} \ No newline at end of file +} From 13a439f040e70b8d7cc26988dd3c7e33fcadad64 Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Tue, 29 Aug 2023 20:55:27 +0200 Subject: [PATCH 05/13] RETURNDATACOPY --- crates/evm/src/errors.cairo | 2 +- .../src/instructions/environmental_information.cairo | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index 176230948..63943d745 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -12,7 +12,7 @@ const PC_OUT_OF_BOUNDS: felt252 = 'Kakarot: pc >= bytecode length'; const TYPE_CONVERSION_ERROR: felt252 = 'Kakarot: type conversion error'; // RETURNDATA -const RETURNDATA_OUT_OF_BOUNDS_ERROR: felt252 = 'Kakarot: out of bounds'; +const RETURNDATA_OUT_OF_BOUNDS_ERROR: felt252 = 'Kakarot: ReturnDataOutOfBounds'; #[derive(Drop, Copy, PartialEq)] enum EVMError { diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index f4cddb1dd..fd000a892 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -9,6 +9,7 @@ use evm::context::{ }; use utils::helpers::EthAddressIntoU256; use evm::memory::MemoryTrait; +use integer::u32_overflowing_add; #[generate_trait] impl EnvironmentInformationImpl of EnvironmentInformationTrait { @@ -154,8 +155,15 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait { let return_data: Span = self.return_data(); - if (offset + size > return_data.len()) { - return Result::Err(EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR)); + match u32_overflowing_add(offset, size) { + Result::Ok(x) => { + if (x > return_data.len()) { + return Result::Err(EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR)); + } + }, + Result::Err(x) => { + return Result::Err(EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR)); + } } let data_to_copy: Span = return_data.slice(offset, size); From 9fc2ee9fd99a3d78fbe27372a626aea44dcfe6d4 Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Wed, 30 Aug 2023 18:46:44 +0200 Subject: [PATCH 06/13] Add out of bounds and overflowing add tests --- .../test_environment_information.cairo | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 46c3819a1..be786620b 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -250,6 +250,48 @@ fn test_returndata_copy_type_conversion_error() { ); } +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_overflowing_add_error() { + // Given + let mut ctx = setup_execution_context(); + ctx.set_return_data(array![1, 2, 3, 4, 5]); + + ctx.stack.push(0xFFFFFFFF); + ctx.stack.push(0xFFFFFFFF); + ctx.stack.push(0xFFFFFFFF); + + // When + let res = ctx.exec_returndatacopy(); + // Then + assert(res.is_err(), 'should return error'); + assert( + res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), + 'should return OutOfBounds' + ); +} + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_out_of_bounds_error() { + // Given + let mut ctx = setup_execution_context(); + ctx.set_return_data(array![1, 2, 3, 4, 5]); + + ctx.stack.push(10); + ctx.stack.push(0); + ctx.stack.push(0); + + // When + let res = ctx.exec_returndatacopy(); + // Then + assert(res.is_err(), 'should return error'); + assert( + res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), + 'should return OutOfBounds' + ); +} + #[test] #[available_gas(20000000)] fn test_returndata_copy_basic() { From b1abae74ff7d0a75b2de371af48f9e41dc536d68 Mon Sep 17 00:00:00 2001 From: khaeljy Date: Mon, 4 Sep 2023 19:53:49 +0200 Subject: [PATCH 07/13] Update crates/evm/src/tests/test_instructions/test_environment_information.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- .../tests/test_instructions/test_environment_information.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 4f64862f9..95d4cbb37 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -316,7 +316,7 @@ fn test_returndata_copy_with_out_of_bound_bytes() { test_returndata_copy(32, 0, 8); } -fn test_returndata_copy(destOffset: u32, offset: u32, mut size: u32) { +fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { // Given let mut ctx = setup_execution_context(); ctx.set_return_data(array![1, 2, 3, 4, 5]); From 26a69cb5f33b25b99238c50f064b870ac40a4ac1 Mon Sep 17 00:00:00 2001 From: khaeljy Date: Mon, 4 Sep 2023 19:54:00 +0200 Subject: [PATCH 08/13] Update crates/evm/src/tests/test_instructions/test_environment_information.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- .../tests/test_instructions/test_environment_information.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 95d4cbb37..84a3ce1d0 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -344,7 +344,7 @@ fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { 'should return out of bounds' ); } else { - let result: u256 = ctx.memory.load_internal(destOffset).into(); + let result: u256 = ctx.memory.load_internal(dest_offset).into(); let mut results: Array = u256_to_bytes_array(result); let mut i = 0; From dffade33dec9fe5c11c52de61665b1938c2e2a5d Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Mon, 4 Sep 2023 19:55:12 +0200 Subject: [PATCH 09/13] Add line breaks --- .../tests/test_instructions/test_environment_information.cairo | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 9e35dd28a..322d4c4f7 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -248,6 +248,7 @@ fn test_returndata_copy_type_conversion_error() { // When let res = ctx.exec_returndatacopy(); + // Then assert(res.is_err(), 'should return error'); assert( @@ -269,6 +270,7 @@ fn test_returndata_copy_overflowing_add_error() { // When let res = ctx.exec_returndatacopy(); + // Then assert(res.is_err(), 'should return error'); assert( @@ -290,6 +292,7 @@ fn test_returndata_copy_out_of_bounds_error() { // When let res = ctx.exec_returndatacopy(); + // Then assert(res.is_err(), 'should return error'); assert( From e5c7c2478ec676cc3586026ea892246d28a7837a Mon Sep 17 00:00:00 2001 From: khaeljy Date: Mon, 4 Sep 2023 19:56:10 +0200 Subject: [PATCH 10/13] Update crates/evm/src/tests/test_instructions/test_environment_information.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- .../tests/test_instructions/test_environment_information.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 322d4c4f7..76f46cb1c 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -332,7 +332,7 @@ fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { ctx.stack.push(size.into()); ctx.stack.push(offset.into()); - ctx.stack.push(destOffset.into()); + ctx.stack.push(dest_offset.into()); // When let res = ctx.exec_returndatacopy(); From ab7b4786198f33d6e62b577e332180af24d5b900 Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Mon, 4 Sep 2023 19:56:41 +0200 Subject: [PATCH 11/13] Remove unused function --- crates/evm/src/tests/test_utils.cairo | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index f9fce6964..ec6783927 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -63,20 +63,6 @@ fn setup_execution_context_with_bytecode(bytecode: Span) -> ExecutionContext ) } -fn setup_execution_context_with_returndata(return_data: Array) -> ExecutionContext { - let call_context = setup_call_context(); - let starknet_address: ContractAddress = starknet_address(); - let evm_address: EthAddress = evm_address(); - let gas_limit: u64 = 1000; - let gas_price: u64 = 10; - let read_only: bool = false; - let returned_data = return_data; - - ExecutionContextTrait::new( - call_context, starknet_address, evm_address, gas_limit, gas_price, returned_data, read_only - ) -} - impl CallContextPartialEq of PartialEq { fn eq(lhs: @CallContext, rhs: @CallContext) -> bool { lhs.bytecode() == rhs.bytecode() && lhs.calldata == rhs.calldata && lhs.value == rhs.value From 8b1ef54acff300426298df3873bac5afa79187ed Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Mon, 4 Sep 2023 19:59:52 +0200 Subject: [PATCH 12/13] Remove useless asserts --- .../test_instructions/test_environment_information.cairo | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 76f46cb1c..6abab738a 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -250,7 +250,6 @@ fn test_returndata_copy_type_conversion_error() { let res = ctx.exec_returndatacopy(); // Then - assert(res.is_err(), 'should return error'); assert( res.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR), 'should return ConversionError' @@ -272,7 +271,6 @@ fn test_returndata_copy_overflowing_add_error() { let res = ctx.exec_returndatacopy(); // Then - assert(res.is_err(), 'should return error'); assert( res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), 'should return OutOfBounds' @@ -292,9 +290,8 @@ fn test_returndata_copy_out_of_bounds_error() { // When let res = ctx.exec_returndatacopy(); - + // Then - assert(res.is_err(), 'should return error'); assert( res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), 'should return OutOfBounds' @@ -341,7 +338,6 @@ fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { assert(ctx.stack.is_empty(), 'stack should be empty'); if (offset + size > return_data.len()) { - assert(res.is_err(), 'should return error'); assert( res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), 'should return out of bounds' From e7c55b1127081bf750eefc9f0b6f278c69d947ed Mon Sep 17 00:00:00 2001 From: Khaeljy Date: Mon, 4 Sep 2023 23:07:31 +0200 Subject: [PATCH 13/13] Test refactoring --- crates/evm/src/memory.cairo | 2 +- .../test_environment_information.cairo | 145 +++++++++++------- 2 files changed, 89 insertions(+), 58 deletions(-) diff --git a/crates/evm/src/memory.cairo b/crates/evm/src/memory.cairo index 10c9cac2b..35d75644c 100644 --- a/crates/evm/src/memory.cairo +++ b/crates/evm/src/memory.cairo @@ -262,7 +262,7 @@ impl InternalMemoryMethods of InternalMemoryTrait { self.items.insert(chunk_index.into(), current.try_into().unwrap()); chunk_index += 1; - elements = elements.slice(0, 16); + elements = elements.slice(16, elements.len() - 16); } } diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 6abab738a..ed9f869af 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -11,6 +11,8 @@ use evm::errors::{EVMError, TYPE_CONVERSION_ERROR, RETURNDATA_OUT_OF_BOUNDS_ERRO use evm::context::{ ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait }; +use utils::helpers::{ArrayExtension, ArrayExtensionTrait}; +use integer::u32_overflowing_add; // ************************************************************************* // 0x30: ADDRESS @@ -240,7 +242,6 @@ fn test_returndatasize() { fn test_returndata_copy_type_conversion_error() { // Given let mut ctx = setup_execution_context(); - ctx.set_return_data(array![1, 2, 3, 4, 5]); ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); ctx.stack.push(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); @@ -259,43 +260,7 @@ fn test_returndata_copy_type_conversion_error() { #[test] #[available_gas(20000000)] fn test_returndata_copy_overflowing_add_error() { - // Given - let mut ctx = setup_execution_context(); - ctx.set_return_data(array![1, 2, 3, 4, 5]); - - ctx.stack.push(0xFFFFFFFF); - ctx.stack.push(0xFFFFFFFF); - ctx.stack.push(0xFFFFFFFF); - - // When - let res = ctx.exec_returndatacopy(); - - // Then - assert( - res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), - 'should return OutOfBounds' - ); -} - -#[test] -#[available_gas(20000000)] -fn test_returndata_copy_out_of_bounds_error() { - // Given - let mut ctx = setup_execution_context(); - ctx.set_return_data(array![1, 2, 3, 4, 5]); - - ctx.stack.push(10); - ctx.stack.push(0); - ctx.stack.push(0); - - // When - let res = ctx.exec_returndatacopy(); - - // Then - assert( - res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), - 'should return OutOfBounds' - ); + test_returndata_copy(0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF); } #[test] @@ -313,13 +278,59 @@ fn test_returndata_copy_with_offset() { #[test] #[available_gas(20000000)] fn test_returndata_copy_with_out_of_bound_bytes() { - test_returndata_copy(32, 0, 8); + test_returndata_copy(32, 30, 10); +} + +#[test] +#[available_gas(20000000)] +fn test_returndata_copy_with_multiple_words() { + test_returndata_copy(32, 0, 33); } fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { // Given let mut ctx = setup_execution_context(); - ctx.set_return_data(array![1, 2, 3, 4, 5]); + ctx + .set_return_data( + array![ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36 + ] + ); let return_data: Span = ctx.return_data(); @@ -337,24 +348,44 @@ fn test_returndata_copy(dest_offset: u32, offset: u32, mut size: u32) { // Then assert(ctx.stack.is_empty(), 'stack should be empty'); - if (offset + size > return_data.len()) { - assert( - res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), - 'should return out of bounds' - ); - } else { - let result: u256 = ctx.memory.load_internal(dest_offset).into(); - let mut results: Array = u256_to_bytes_array(result); - - let mut i = 0; - loop { - if (i == size) { - break; + match u32_overflowing_add(offset, size) { + Result::Ok(x) => { + if (x > return_data.len()) { + assert( + res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), + 'should return out of bounds' + ); + return; } + }, + Result::Err(x) => { + assert( + res.unwrap_err() == EVMError::ReturnDataError(RETURNDATA_OUT_OF_BOUNDS_ERROR), + 'should return out of bounds' + ); + return; + } + } - assert(*results[i] == *return_data[i + offset], 'wrong data value'); + let result: u256 = ctx.memory.load_internal(dest_offset).into(); + let mut results: Array = ArrayTrait::new(); - i += 1; - }; - } + let mut i = 0; + loop { + if i == (size / 32) + 1 { + break; + } + + let result: u256 = ctx.memory.load_internal(dest_offset + (i * 32)).into(); + let result_span = u256_to_bytes_array(result).span(); + + if ((i + 1) * 32 > size) { + ArrayExtensionTrait::concat(ref results, result_span.slice(0, size - (i * 32))); + } else { + ArrayExtensionTrait::concat(ref results, result_span); + } + + i += 1; + }; + assert(results.span() == return_data.slice(offset, size), 'wrong data value'); }