From 92561a1242eb53c4c84d92574b40afd794ae664b Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 15:00:56 +0200 Subject: [PATCH 01/29] mstore tests impl --- .../src/instructions/memory_operations.cairo | 1 + .../test_memory_operations.cairo | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 64d61b077..2203b7c77 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -19,6 +19,7 @@ impl MemoryOperation of MemoryOperationTrait { /// Save word to memory. /// # Specification: https://www.evm.codes/#52?fork=shanghai fn exec_mstore(ref self: ExecutionContext) -> Result<(), EVMError> { + panic_with_felt252('MSTORE not implement yet'); Result::Ok(()) } diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index a291197be..d2dbfb7db 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -77,3 +77,82 @@ fn test_exec_pop_should_stack_underflow() { result.unwrap_err() == EVMError::StackError(STACK_UNDERFLOW), 'should return StackUnderflow' ); } + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_only_F_offset_1() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff); + ctx.stack.push(0x01); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeed'); + let (stored, _) = ctx.memory.load(1); + assert( + stored == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, + 'should have store only Fs' + ); +} + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_1_offset_1() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(0x01); + ctx.stack.push(0x01); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeed'); + let (stored, _) = ctx.memory.load(1); + assert(stored == 0x01, 'should have store 0x01'); +} + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_0xFF_offset_1() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(0xFF); + ctx.stack.push(0x01); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeed'); + let (stored, _) = ctx.memory.load(0); + assert(stored == 0x00, 'should be 0s'); +} + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_0xFF00_offset_1() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(0xFF); + ctx.stack.push(0x01); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeed'); + let (stored, _) = ctx.memory.load(0); + assert(stored == 0xFF, 'should be 0xFF'); +} From b6804f587fc360d0f43688462e7b0cd012dff272 Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 15:09:47 +0200 Subject: [PATCH 02/29] made exec_mstore panic --- crates/evm/src/instructions/memory_operations.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 2203b7c77..97cd1d3f4 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -6,7 +6,6 @@ use evm::context::{ use evm::errors::EVMError; use evm::stack::StackTrait; - #[generate_trait] impl MemoryOperation of MemoryOperationTrait { /// MLOAD operation. From 80edce95dbbc4a08dbf512ae8b05f0e992c6d1bb Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 15:20:34 +0200 Subject: [PATCH 03/29] corrected test case error --- .../src/tests/test_instructions/test_memory_operations.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index d2dbfb7db..f22d0ef90 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -145,7 +145,7 @@ fn test_exec_mstore_should_store_0xFF00_offset_1() { // Given let mut ctx = setup_execution_context(); - ctx.stack.push(0xFF); + ctx.stack.push(0xFF00); ctx.stack.push(0x01); // When From a4f459753a1388e7ff291b396058ce8c4a4eace1 Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 15:22:14 +0200 Subject: [PATCH 04/29] mstore opcode impl --- crates/evm/src/instructions/memory_operations.cairo | 7 ++++++- .../tests/test_instructions/test_memory_operations.cairo | 4 ---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 97cd1d3f4..51d6becf2 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -5,6 +5,8 @@ use evm::context::{ }; use evm::errors::EVMError; use evm::stack::StackTrait; +use evm::memory::MemoryTrait; +use evm::helpers::U256IntoResultU32; #[generate_trait] impl MemoryOperation of MemoryOperationTrait { @@ -18,7 +20,10 @@ impl MemoryOperation of MemoryOperationTrait { /// Save word to memory. /// # Specification: https://www.evm.codes/#52?fork=shanghai fn exec_mstore(ref self: ExecutionContext) -> Result<(), EVMError> { - panic_with_felt252('MSTORE not implement yet'); + let offset: u32 = Into::>::into((self.stack.pop()?))?; + let value: u256 = self.stack.pop()?; + + self.memory.store(value, offset); Result::Ok(()) } diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index f22d0ef90..0990ef14e 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -80,7 +80,6 @@ fn test_exec_pop_should_stack_underflow() { #[test] #[available_gas(20000000)] -#[should_panic(expected: ('MSTORE not implement yet',))] fn test_exec_mstore_should_store_only_F_offset_1() { // Given let mut ctx = setup_execution_context(); @@ -102,7 +101,6 @@ fn test_exec_mstore_should_store_only_F_offset_1() { #[test] #[available_gas(20000000)] -#[should_panic(expected: ('MSTORE not implement yet',))] fn test_exec_mstore_should_store_1_offset_1() { // Given let mut ctx = setup_execution_context(); @@ -121,7 +119,6 @@ fn test_exec_mstore_should_store_1_offset_1() { #[test] #[available_gas(20000000)] -#[should_panic(expected: ('MSTORE not implement yet',))] fn test_exec_mstore_should_store_0xFF_offset_1() { // Given let mut ctx = setup_execution_context(); @@ -140,7 +137,6 @@ fn test_exec_mstore_should_store_0xFF_offset_1() { #[test] #[available_gas(20000000)] -#[should_panic(expected: ('MSTORE not implement yet',))] fn test_exec_mstore_should_store_0xFF00_offset_1() { // Given let mut ctx = setup_execution_context(); From 80264aca52d57c33448b2a38574d213b70b7be0f Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 21:43:06 +0200 Subject: [PATCH 05/29] review correction + add 2 tests --- .../test_memory_operations.cairo | 66 ++++++++++++++++--- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index f22d0ef90..2c53fff3f 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -11,6 +11,7 @@ use evm::errors::{EVMError, STACK_UNDERFLOW}; use evm::context::{ ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait, }; +use integer::BoundedInt; #[test] @@ -78,6 +79,26 @@ fn test_exec_pop_should_stack_underflow() { ); } +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_only_F_offset_0() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 32, 'memory should be 32 bytes long'); + let (stored, _) = ctx.memory.load(0); + assert(stored == BoundedInt::::max(), 'should have store only Fs'); +} + #[test] #[available_gas(20000000)] #[should_panic(expected: ('MSTORE not implement yet',))] @@ -85,19 +106,17 @@ fn test_exec_mstore_should_store_only_F_offset_1() { // Given let mut ctx = setup_execution_context(); - ctx.stack.push(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff); + ctx.stack.push(BoundedInt::::max()); ctx.stack.push(0x01); // When let result = ctx.exec_mstore(); // Then - assert(result.is_ok(), 'should have succeed'); + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); let (stored, _) = ctx.memory.load(1); - assert( - stored == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, - 'should have store only Fs' - ); + assert(stored == BoundedInt::::max(), 'should have store only Fs'); } #[test] @@ -114,7 +133,8 @@ fn test_exec_mstore_should_store_1_offset_1() { let result = ctx.exec_mstore(); // Then - assert(result.is_ok(), 'should have succeed'); + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); let (stored, _) = ctx.memory.load(1); assert(stored == 0x01, 'should have store 0x01'); } @@ -133,9 +153,12 @@ fn test_exec_mstore_should_store_0xFF_offset_1() { let result = ctx.exec_mstore(); // Then - assert(result.is_ok(), 'should have succeed'); + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); let (stored, _) = ctx.memory.load(0); assert(stored == 0x00, 'should be 0s'); + let (stored, _) = ctx.memory.load(2); + assert(stored == 0xFF00, 'should be 0xFF00'); } #[test] @@ -152,7 +175,32 @@ fn test_exec_mstore_should_store_0xFF00_offset_1() { let result = ctx.exec_mstore(); // Then - assert(result.is_ok(), 'should have succeed'); + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); + let (stored, _) = ctx.memory.load(0); + assert(stored == 0xFF, 'should be 0xFF'); +} + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('MSTORE not implement yet',))] +fn test_exec_mstore_should_store_0xFF00_offset_0x20() { + // Given + let mut ctx = setup_execution_context(); + + ctx.stack.push(0xFF00); + ctx.stack.push(0x20); + + // When + let result = ctx.exec_mstore(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.memory.bytes_len == 96, 'memory should be 96 bytes long'); let (stored, _) = ctx.memory.load(0); + assert(stored == 0x00, 'should be 0x00'); + let (stored, _) = ctx.memory.load(0x20); + assert(stored == 0xFF00, 'should be 0xFF00'); + let (stored, _) = ctx.memory.load(0xF9); assert(stored == 0xFF, 'should be 0xFF'); } From 3b06efa165c5ddafec6e809054bf0fb9afafff6c Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 22:03:12 +0200 Subject: [PATCH 06/29] correction test case --- .../src/tests/test_instructions/test_memory_operations.cairo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index 2c53fff3f..21793bdb7 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -196,11 +196,11 @@ fn test_exec_mstore_should_store_0xFF00_offset_0x20() { // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.memory.bytes_len == 96, 'memory should be 96 bytes long'); + assert(ctx.memory.bytes_len == 64, 'memory should be 96 bytes long'); let (stored, _) = ctx.memory.load(0); assert(stored == 0x00, 'should be 0x00'); let (stored, _) = ctx.memory.load(0x20); assert(stored == 0xFF00, 'should be 0xFF00'); - let (stored, _) = ctx.memory.load(0xF9); + let (stored, _) = ctx.memory.load(0x1F); assert(stored == 0xFF, 'should be 0xFF'); } From 0224cfe6ab89e7aa26a3cc11b5887d28c9b92d25 Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 22:26:02 +0200 Subject: [PATCH 07/29] used pop_n instead --- crates/evm/src/instructions/memory_operations.cairo | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 51d6becf2..7a0fec7fe 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -20,10 +20,9 @@ impl MemoryOperation of MemoryOperationTrait { /// Save word to memory. /// # Specification: https://www.evm.codes/#52?fork=shanghai fn exec_mstore(ref self: ExecutionContext) -> Result<(), EVMError> { - let offset: u32 = Into::>::into((self.stack.pop()?))?; - let value: u256 = self.stack.pop()?; + let popped = self.stack.pop_n(2)?; - self.memory.store(value, offset); + self.memory.store(*popped[1], Into::>::into(*popped[0])?); Result::Ok(()) } From 6042b2ea250094a4a5d28b2f73819e16532011ea Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 6 Sep 2023 23:16:09 +0200 Subject: [PATCH 08/29] corrected #263 succeed into succeeded --- .../src/tests/test_instructions/test_memory_operations.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index e39d7b7a6..83cfeaa50 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -58,7 +58,7 @@ fn test_exec_pop_should_pop_an_item_from_stack() { let result = ctx.exec_pop(); // Then - assert(result.is_ok(), 'should have succeed'); + assert(result.is_ok(), 'should have succeeded'); assert(ctx.stack.len() == 1, 'stack should have one element'); assert(ctx.stack.peek().unwrap() == 0x01, 'stack peek should return 0x01'); } From 86cc9b1aed40bd9838ad18948c41c9db80f67a7e Mon Sep 17 00:00:00 2001 From: Quentash Date: Thu, 7 Sep 2023 14:44:48 +0200 Subject: [PATCH 09/29] reverted to single pop + removed tests --- .../src/instructions/memory_operations.cairo | 5 +- .../test_memory_operations.cairo | 90 +------------------ 2 files changed, 7 insertions(+), 88 deletions(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 7a0fec7fe..51d6becf2 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -20,9 +20,10 @@ impl MemoryOperation of MemoryOperationTrait { /// Save word to memory. /// # Specification: https://www.evm.codes/#52?fork=shanghai fn exec_mstore(ref self: ExecutionContext) -> Result<(), EVMError> { - let popped = self.stack.pop_n(2)?; + let offset: u32 = Into::>::into((self.stack.pop()?))?; + let value: u256 = self.stack.pop()?; - self.memory.store(*popped[1], Into::>::into(*popped[0])?); + self.memory.store(value, offset); Result::Ok(()) } diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index 83cfeaa50..a6090f5fc 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -81,7 +81,7 @@ fn test_exec_pop_should_stack_underflow() { #[test] #[available_gas(20000000)] -fn test_exec_mstore_should_store_only_F_offset_0() { +fn test_exec_mstore_should_store_max_uint256_offset_0() { // Given let mut ctx = setup_execution_context(); @@ -95,12 +95,12 @@ fn test_exec_mstore_should_store_only_F_offset_0() { assert(result.is_ok(), 'should have succeeded'); assert(ctx.memory.bytes_len == 32, 'memory should be 32 bytes long'); let (stored, _) = ctx.memory.load(0); - assert(stored == BoundedInt::::max(), 'should have store only Fs'); + assert(stored == BoundedInt::::max(), 'should have stored max_uint256'); } #[test] #[available_gas(20000000)] -fn test_exec_mstore_should_store_only_F_offset_1() { +fn test_exec_mstore_should_store_max_uint256_offset_1() { // Given let mut ctx = setup_execution_context(); @@ -114,87 +114,5 @@ fn test_exec_mstore_should_store_only_F_offset_1() { assert(result.is_ok(), 'should have succeeded'); assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); let (stored, _) = ctx.memory.load(1); - assert(stored == BoundedInt::::max(), 'should have store only Fs'); -} - -#[test] -#[available_gas(20000000)] -fn test_exec_mstore_should_store_1_offset_1() { - // Given - let mut ctx = setup_execution_context(); - - ctx.stack.push(0x01); - ctx.stack.push(0x01); - - // When - let result = ctx.exec_mstore(); - - // Then - assert(result.is_ok(), 'should have succeeded'); - assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); - let (stored, _) = ctx.memory.load(1); - assert(stored == 0x01, 'should have store 0x01'); -} - -#[test] -#[available_gas(20000000)] -fn test_exec_mstore_should_store_0xFF_offset_1() { - // Given - let mut ctx = setup_execution_context(); - - ctx.stack.push(0xFF); - ctx.stack.push(0x01); - - // When - let result = ctx.exec_mstore(); - - // Then - assert(result.is_ok(), 'should have succeeded'); - assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); - let (stored, _) = ctx.memory.load(0); - assert(stored == 0x00, 'should be 0s'); - let (stored, _) = ctx.memory.load(2); - assert(stored == 0xFF00, 'should be 0xFF00'); -} - -#[test] -#[available_gas(20000000)] -fn test_exec_mstore_should_store_0xFF00_offset_1() { - // Given - let mut ctx = setup_execution_context(); - - ctx.stack.push(0xFF00); - ctx.stack.push(0x01); - - // When - let result = ctx.exec_mstore(); - - // Then - assert(result.is_ok(), 'should have succeeded'); - assert(ctx.memory.bytes_len == 64, 'memory should be 64 bytes long'); - let (stored, _) = ctx.memory.load(0); - assert(stored == 0xFF, 'should be 0xFF'); -} - -#[test] -#[available_gas(20000000)] -fn test_exec_mstore_should_store_0xFF00_offset_0x20() { - // Given - let mut ctx = setup_execution_context(); - - ctx.stack.push(0xFF00); - ctx.stack.push(0x20); - - // When - let result = ctx.exec_mstore(); - - // Then - assert(result.is_ok(), 'should have succeeded'); - assert(ctx.memory.bytes_len == 64, 'memory should be 96 bytes long'); - let (stored, _) = ctx.memory.load(0); - assert(stored == 0x00, 'should be 0x00'); - let (stored, _) = ctx.memory.load(0x20); - assert(stored == 0xFF00, 'should be 0xFF00'); - let (stored, _) = ctx.memory.load(0x1F); - assert(stored == 0xFF, 'should be 0xFF'); + assert(stored == BoundedInt::::max(), 'should have stored max_uint256'); } From 34c0d3b389acfac00284b8af6dc717d78e1d47da Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 8 Sep 2023 00:13:35 +0200 Subject: [PATCH 10/29] first commit --- crates/evm/src/errors.cairo | 7 +- crates/evm/src/instructions.cairo | 1 + .../src/instructions/logging_operations.cairo | 133 ++++++++++---- crates/evm/src/interpreter.cairo | 17 +- crates/evm/src/tests/test_instructions.cairo | 1 + .../test_logging_operations.cairo | 171 ++++++++++++++++++ 6 files changed, 288 insertions(+), 42 deletions(-) create mode 100644 crates/evm/src/tests/test_instructions/test_logging_operations.cairo diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index 63943d745..876dd2697 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -14,12 +14,16 @@ const TYPE_CONVERSION_ERROR: felt252 = 'Kakarot: type conversion error'; // RETURNDATA const RETURNDATA_OUT_OF_BOUNDS_ERROR: felt252 = 'Kakarot: ReturnDataOutOfBounds'; +// EVM STATE +const STATE_MODIFICATION_ERROR: felt252 = 'Kakarot: StateModificationError'; + #[derive(Drop, Copy, PartialEq)] enum EVMError { StackError: felt252, InvalidProgramCounter: felt252, TypeConversionError: felt252, - ReturnDataError: felt252 + ReturnDataError: felt252, + StateModificationError: felt252 } @@ -30,6 +34,7 @@ impl EVMErrorIntoU256 of Into { EVMError::InvalidProgramCounter(error_message) => error_message.into(), EVMError::TypeConversionError(error_message) => error_message.into(), EVMError::ReturnDataError(error_message) => error_message.into(), + EVMError::StateModificationError(error_message) => error_message.into(), } } } diff --git a/crates/evm/src/instructions.cairo b/crates/evm/src/instructions.cairo index 7a5c7c9c6..8aa84e549 100644 --- a/crates/evm/src/instructions.cairo +++ b/crates/evm/src/instructions.cairo @@ -14,6 +14,7 @@ use environmental_information::EnvironmentInformationTrait; mod exchange_operations; mod logging_operations; +use logging_operations::LoggingOperationsTrait; mod memory_operations; use memory_operations::MemoryOperationTrait; diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 54b92444a..c3a298552 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -2,49 +2,116 @@ // Internal imports use evm::context::ExecutionContext; -use evm::context::ExecutionContextTrait; +use evm::errors::EVMError; mod internal { - use evm::context::ExecutionContext; - use evm::context::ExecutionContextTrait; + use evm::context::{ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct}; + use evm::stack::StackTrait; + use evm::memory::MemoryTrait; + use evm::model::Event; + use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; + use core::array::ArrayTrait; + use box::BoxTrait; + use evm::helpers::U256IntoResultU32; + use utils::helpers::u256_to_bytes_array; + + use debug::PrintTrait; /// Generic logging operation. /// Append log record with n topics. - fn exec_log_i(ref context: ExecutionContext, topics_len: u8) {} -} + fn exec_log_i(ref self: ExecutionContext, topics_len: u8) -> Result<(), EVMError> { + if self.read_only() { + return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); + } -/// 0xA0 - LOG0 operation -/// Append log record with no topic. -/// # Specification: https://www.evm.codes/#a0?fork=shanghai -fn exec_log0(ref context: ExecutionContext) { - internal::exec_log_i(ref context, 0); -} + let popped = self.stack.pop_n(2 + topics_len.into())?; + let offset: u32 = Into::>::into(*popped[0])?; + let size: u32 = Into::>::into(*popped[1])?; + let mut topics: Array = ArrayTrait::new(); + let mut i = 0; + loop { + if i == topics_len { + break; + } + topics.append(*popped[2 + i.into()]); -/// 0xA1 - LOG1 -/// Append log record with one topic. -/// # Specification: https://www.evm.codes/#a1?fork=shanghai -fn exec_log1(ref context: ExecutionContext) { - internal::exec_log_i(ref context, 1); -} + i += 1; + }; -/// 0xA2 - LOG2 -/// Append log record with two topics. -/// # Specification: https://www.evm.codes/#a2?fork=shanghai -fn exec_log2(ref context: ExecutionContext) { - internal::exec_log_i(ref context, 2); -} + let mut datas: Array = ArrayTrait::new(); + + let mut i = 0; + loop { + if 31 + i > size { + if i != size { + let (mut loaded, _) = self.memory.load(offset + i); + let mut chunk: Array = u256_to_bytes_array(loaded); + let mut last_elem = 0; + let mut j = 0; + loop { + if j + i == size { + break; + } + last_elem *= 256; + last_elem += (*chunk[j]).into(); + j += 1; + }; + datas.append(last_elem); + } + break; + }; + let (mut loaded, _) = self.memory.load(offset + i); + loaded /= 256; + datas.append(loaded.try_into().unwrap()); + i += 31; + }; + + let event: Event = Event { keys: topics, data: datas }; -/// 0xA3 - LOG3 -/// Append log record with three topics. -/// # Specification: https://www.evm.codes/#a3?fork=shanghai -fn exec_log3(ref context: ExecutionContext) { - internal::exec_log_i(ref context, 3); + let mut dyn_ctx = self.dynamic_context.unbox(); + dyn_ctx.events.append(event); + self.dynamic_context = BoxTrait::new(dyn_ctx); + + Result::Ok(()) + } } -/// 0xA4 - LOG4 -/// Append log record with 4 topics. -/// # Specification: https://www.evm.codes/#a4?fork=shanghai -fn exec_log4(ref context: ExecutionContext) { - internal::exec_log_i(ref context, 4); +#[generate_trait] +impl LoggingOperations of LoggingOperationsTrait { + /// 0xA0 - LOG0 operation + /// Append log record with no topic. + /// # Specification: https://www.evm.codes/#a0?fork=shanghai + fn exec_log0(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 0) + } + + + /// 0xA1 - LOG1 + /// Append log record with one topic. + /// # Specification: https://www.evm.codes/#a1?fork=shanghai + fn exec_log1(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 1) + } + + /// 0xA2 - LOG2 + /// Append log record with two topics. + /// # Specification: https://www.evm.codes/#a2?fork=shanghai + fn exec_log2(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 2) + } + + /// 0xA3 - LOG3 + /// Append log record with three topics. + /// # Specification: https://www.evm.codes/#a3?fork=shanghai + fn exec_log3(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 3) + } + + /// 0xA4 - LOG4 + /// Append log record with 4 topics. + /// # Specification: https://www.evm.codes/#a4?fork=shanghai + fn exec_log4(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 4) + } } diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index 48afc882f..8b4d1ba33 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -13,9 +13,10 @@ use utils::{helpers::u256_to_bytes_array}; use evm::errors::{EVMError, PC_OUT_OF_BOUNDS}; use evm::instructions::{ duplication_operations, environmental_information, exchange_operations, logging_operations, - memory_operations, sha3, StopAndArithmeticOperationsTrait, ComparisonAndBitwiseOperationsTrait, - system_operations, BlockInformationTrait, DuplicationOperationsTrait, - EnvironmentInformationTrait, PushOperationsTrait, MemoryOperationTrait + LoggingOperationsTrait, memory_operations, sha3, StopAndArithmeticOperationsTrait, + ComparisonAndBitwiseOperationsTrait, system_operations, BlockInformationTrait, + DuplicationOperationsTrait, EnvironmentInformationTrait, PushOperationsTrait, + MemoryOperationTrait }; use result::ResultTrait; @@ -603,23 +604,23 @@ impl EVMInterpreterImpl of EVMInterpreterTrait { } if opcode == 160 { // LOG0 - logging_operations::exec_log0(ref context); + return context.exec_log0(); } if opcode == 161 { // LOG1 - logging_operations::exec_log1(ref context); + return context.exec_log1(); } if opcode == 162 { // LOG2 - logging_operations::exec_log2(ref context); + return context.exec_log2(); } if opcode == 163 { // LOG3 - logging_operations::exec_log3(ref context); + return context.exec_log3(); } if opcode == 164 { // LOG4 - logging_operations::exec_log4(ref context); + return context.exec_log4(); } if opcode == 240 { // CREATE diff --git a/crates/evm/src/tests/test_instructions.cairo b/crates/evm/src/tests/test_instructions.cairo index cefcd4b22..26c346ae8 100644 --- a/crates/evm/src/tests/test_instructions.cairo +++ b/crates/evm/src/tests/test_instructions.cairo @@ -5,3 +5,4 @@ mod test_block_information; mod test_environment_information; mod test_push_operations; mod test_memory_operations; +mod test_logging_operations; diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo new file mode 100644 index 000000000..08f219d58 --- /dev/null +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -0,0 +1,171 @@ +use evm::instructions::LoggingOperationsTrait; +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}; +use evm::errors::{EVMError, STACK_UNDERFLOW}; +use evm::context::{ + ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait, +}; +use integer::BoundedInt; + +use debug::PrintTrait; + +#[test] +#[available_gas(20000000)] +fn test_exec_log0() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x1F); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log0(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 4, 'stack size should be 4'); + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 0, 'event should not have keys'); + assert(event.data.len() == 1, 'event should have one data'); + assert( + *event.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, + 'event data should be max_u248' + ); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log1() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x20); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 3, 'stack size should be 3'); + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 1, 'event should have one key'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + assert(event.data.len() == 2, 'event should have one data'); + assert( + *event.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, + 'event data0 should be max_u248' + ); + assert(*event.data[1] == 0xff, 'event data1 should be 0xFF'); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log2() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x05); + ctx.stack.push(0x05); + + // When + let result = ctx.exec_log2(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 2, 'stack size should be 2'); + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 2, 'event should have two keys'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); + assert(event.data.len() == 1, 'event should have one data'); + assert(*event.data[0] == 0xFFFFFFFFFF, 'event data is not correct'); +} + + +#[test] +#[available_gas(20000000)] +fn test_exec_log3_and_log4() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x0A); + ctx.stack.push(0x20); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x28); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log3(); + let result = ctx.exec_log4(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 0, 'stack size should be 0'); + + let mut events = ctx.events(); + assert(events.len() == 2, 'context should have 2 events'); + + let event1 = events.pop_front().unwrap(); + assert(event1.keys.len() == 3, 'event1 should have 3 keys'); + assert(*event1.keys[0] == 0x0123456789ABCDEF, 'event1 key is not correct'); + assert(*event1.keys[1] == BoundedInt::::max(), 'event1 key is not correct'); + assert(*event1.keys[2] == 0x00, 'event1 key is not correct'); + + assert(event1.data.len() == 2, 'event1 should have 2 datas'); + assert( + *event1.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, + 'event1 data is not correct' + ); + assert(*event1.data[1] == 0xff0123456789ABCDEF, 'event1 data is not correct'); + + let event2 = events.pop_front().unwrap(); + assert(event2.keys.len() == 4, 'event2 should have 4 keys'); + assert(*event2.keys[0] == 0x0123456789ABCDEF, 'event2 key is not correct'); + assert(*event2.keys[1] == BoundedInt::::max(), 'event2 key is not correct'); + assert(*event2.keys[2] == 0x00, 'event2 key is not correct'); + assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); + + assert(event2.data.len() == 1, 'event2 should have one data'); + (*event2.data[0]).print(); + assert(*event2.data[0] == 0x0123456789ABCDEF0000, 'event2 data is not correct'); +} From 0a6cea7472a9ce36e0bac8dca1dd7af49cc177bc Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 8 Sep 2023 15:48:22 +0200 Subject: [PATCH 11/29] removed print and used default for array --- crates/evm/src/instructions/logging_operations.cairo | 6 ++---- .../tests/test_instructions/test_logging_operations.cairo | 3 --- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index c3a298552..cf6ea2140 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -15,8 +15,6 @@ mod internal { use evm::helpers::U256IntoResultU32; use utils::helpers::u256_to_bytes_array; - use debug::PrintTrait; - /// Generic logging operation. /// Append log record with n topics. fn exec_log_i(ref self: ExecutionContext, topics_len: u8) -> Result<(), EVMError> { @@ -28,7 +26,7 @@ mod internal { let offset: u32 = Into::>::into(*popped[0])?; let size: u32 = Into::>::into(*popped[1])?; - let mut topics: Array = ArrayTrait::new(); + let mut topics: Array = Default::default(); let mut i = 0; loop { if i == topics_len { @@ -39,7 +37,7 @@ mod internal { i += 1; }; - let mut datas: Array = ArrayTrait::new(); + let mut datas: Array = Default::default(); let mut i = 0; loop { diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 08f219d58..2c6770ce7 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -13,8 +13,6 @@ use evm::context::{ }; use integer::BoundedInt; -use debug::PrintTrait; - #[test] #[available_gas(20000000)] fn test_exec_log0() { @@ -166,6 +164,5 @@ fn test_exec_log3_and_log4() { assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); assert(event2.data.len() == 1, 'event2 should have one data'); - (*event2.data[0]).print(); assert(*event2.data[0] == 0x0123456789ABCDEF0000, 'event2 data is not correct'); } From 45f9e27642fa0e5b3b3c39f8dc5416d51d83321d Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 8 Sep 2023 16:06:35 +0200 Subject: [PATCH 12/29] added readonly test & cleaned imports --- .../src/instructions/logging_operations.cairo | 7 ++-- .../test_logging_operations.cairo | 42 ++++++++++++++----- crates/evm/src/tests/test_utils.cairo | 14 +++++++ 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index cf6ea2140..3fcb66e4b 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -10,14 +10,13 @@ mod internal { use evm::memory::MemoryTrait; use evm::model::Event; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; - use core::array::ArrayTrait; - use box::BoxTrait; use evm::helpers::U256IntoResultU32; use utils::helpers::u256_to_bytes_array; /// Generic logging operation. /// Append log record with n topics. fn exec_log_i(ref self: ExecutionContext, topics_len: u8) -> Result<(), EVMError> { + // Revert if the transaction is in a read only context if self.read_only() { return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); } @@ -26,6 +25,7 @@ mod internal { let offset: u32 = Into::>::into(*popped[0])?; let size: u32 = Into::>::into(*popped[1])?; + // Feed topics array for the new event let mut topics: Array = Default::default(); let mut i = 0; loop { @@ -37,8 +37,8 @@ mod internal { i += 1; }; + // Feed data array for the new event let mut datas: Array = Default::default(); - let mut i = 0; loop { if 31 + i > size { @@ -65,6 +65,7 @@ mod internal { i += 31; }; + // Create and store the new event in the dynamic context let event: Event = Event { keys: topics, data: datas }; let mut dyn_ctx = self.dynamic_context.unbox(); diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 2c6770ce7..7b13166a6 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -1,16 +1,11 @@ -use evm::instructions::LoggingOperationsTrait; -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}; -use evm::errors::{EVMError, STACK_UNDERFLOW}; use evm::context::{ ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait, }; +use evm::stack::StackTrait; +use evm::memory::MemoryTrait; +use evm::tests::test_utils::{setup_execution_context, setup_read_only_execution_context}; +use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; +use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; #[test] @@ -79,6 +74,32 @@ fn test_exec_log1() { assert(*event.data[1] == 0xff, 'event data1 should be 0xFF'); } +#[test] +#[available_gas(20000000)] +fn test_exec_log1_read_only_context() { + // Given + let mut ctx = setup_read_only_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x20); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_err(), 'should have returned an error'); + assert( + result.unwrap_err() == EVMError::StateModificationError(STATE_MODIFICATION_ERROR), + 'err != StateModificationError' + ); +} + #[test] #[available_gas(20000000)] fn test_exec_log2() { @@ -110,7 +131,6 @@ fn test_exec_log2() { assert(*event.data[0] == 0xFFFFFFFFFF, 'event data is not correct'); } - #[test] #[available_gas(20000000)] fn test_exec_log3_and_log4() { diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index 6b8ac76c6..e9c6bc2cb 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -84,6 +84,20 @@ fn setup_execution_context_with_calldata(calldata: Span) -> ExecutionContext ) } +fn setup_read_only_execution_context() -> 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 = true; + let return_data = Default::default(); + + ExecutionContextTrait::new( + call_context, starknet_address, evm_address, gas_limit, gas_price, return_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 c31e984ace026c25afd8879682fd083219e4ea6b Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 13 Sep 2023 14:35:48 +0200 Subject: [PATCH 13/29] added internal get_datas --- .../src/instructions/logging_operations.cairo | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 3fcb66e4b..0c60004de 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -39,6 +39,23 @@ mod internal { // Feed data array for the new event let mut datas: Array = Default::default(); + get_datas(ref self, ref datas, size, offset); + + // Create and store the new event in the dynamic context + let event: Event = Event { keys: topics, data: datas }; + + let mut dyn_ctx = self.dynamic_context.unbox(); + dyn_ctx.events.append(event); + self.dynamic_context = BoxTrait::new(dyn_ctx); + + Result::Ok(()) + } + + /// Generic logging operation. + /// Append log record with n topics. + fn get_datas( + ref self: ExecutionContext, ref datas_array: Array, size: u32, offset: u32 + ) -> Result<(), EVMError> { let mut i = 0; loop { if 31 + i > size { @@ -55,23 +72,15 @@ mod internal { last_elem += (*chunk[j]).into(); j += 1; }; - datas.append(last_elem); + datas_array.append(last_elem); } break; }; let (mut loaded, _) = self.memory.load(offset + i); loaded /= 256; - datas.append(loaded.try_into().unwrap()); + datas_array.append(loaded.try_into().unwrap()); i += 31; }; - - // Create and store the new event in the dynamic context - let event: Event = Event { keys: topics, data: datas }; - - let mut dyn_ctx = self.dynamic_context.unbox(); - dyn_ctx.events.append(event); - self.dynamic_context = BoxTrait::new(dyn_ctx); - Result::Ok(()) } } From 211719b0e4f656da93d9b524b59a4cc8ada1da0b Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 15 Sep 2023 13:05:23 +0200 Subject: [PATCH 14/29] need to merge main --- .../src/instructions/logging_operations.cairo | 133 ++++++++++-------- 1 file changed, 73 insertions(+), 60 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 0c60004de..f5795e042 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -4,6 +4,45 @@ use evm::context::ExecutionContext; use evm::errors::EVMError; +#[generate_trait] +impl LoggingOperations of LoggingOperationsTrait { + /// 0xA0 - LOG0 operation + /// Append log record with no topic. + /// # Specification: https://www.evm.codes/#a0?fork=shanghai + fn exec_log0(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 0) + } + + + /// 0xA1 - LOG1 + /// Append log record with one topic. + /// # Specification: https://www.evm.codes/#a1?fork=shanghai + fn exec_log1(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 1) + } + + /// 0xA2 - LOG2 + /// Append log record with two topics. + /// # Specification: https://www.evm.codes/#a2?fork=shanghai + fn exec_log2(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 2) + } + + /// 0xA3 - LOG3 + /// Append log record with three topics. + /// # Specification: https://www.evm.codes/#a3?fork=shanghai + fn exec_log3(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 3) + } + + /// 0xA4 - LOG4 + /// Append log record with four topics. + /// # Specification: https://www.evm.codes/#a4?fork=shanghai + fn exec_log4(ref self: ExecutionContext) -> Result<(), EVMError> { + internal::exec_log_i(ref self, 4) + } +} + mod internal { use evm::context::{ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct}; use evm::stack::StackTrait; @@ -12,9 +51,16 @@ mod internal { use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; use evm::helpers::U256IntoResultU32; use utils::helpers::u256_to_bytes_array; - - /// Generic logging operation. - /// Append log record with n topics. + use utils::helpers::ArrayExtensionTrait; + use array::ArrayTrait; + + /// This generic function will store a new event in the dynamic context + /// using topics popped from the stack and data from the memory. + /// + /// # Arguments + /// + /// * `self` - The context to which the event will be added + /// * `topics_len` - The amount of topics to pop from the stack fn exec_log_i(ref self: ExecutionContext, topics_len: u8) -> Result<(), EVMError> { // Revert if the transaction is in a read only context if self.read_only() { @@ -27,34 +73,40 @@ mod internal { // Feed topics array for the new event let mut topics: Array = Default::default(); - let mut i = 0; - loop { - if i == topics_len { - break; - } - topics.append(*popped[2 + i.into()]); - - i += 1; - }; + ArrayExtensionTrait::concat(ref topics, popped.span().slice(2, popped.len() - 2)); // Feed data array for the new event - let mut datas: Array = Default::default(); - get_datas(ref self, ref datas, size, offset); + let mut data: Array = Default::default(); + load_data_from_memory(ref self, ref data, size, offset); // Create and store the new event in the dynamic context - let event: Event = Event { keys: topics, data: datas }; + store_event_in_dyn_context(ref self, topics, data); + + Result::Ok(()) + } + /// This function will store a new event in the dynamic context + /// using given topics and data. + /// The dynamic context has to be recreated to be modified. + /// + /// # Arguments + /// + /// * `self` - The context to which the event will be added + /// * `topics` - Topics of the event + /// * `data` - Data of the event + fn store_event_in_dyn_context( + ref self: ExecutionContext, topics: Array, data: Array + ) { + let event: Event = Event { keys: topics, data }; let mut dyn_ctx = self.dynamic_context.unbox(); dyn_ctx.events.append(event); self.dynamic_context = BoxTrait::new(dyn_ctx); - - Result::Ok(()) } /// Generic logging operation. /// Append log record with n topics. - fn get_datas( - ref self: ExecutionContext, ref datas_array: Array, size: u32, offset: u32 + fn load_data_from_memory( + ref self: ExecutionContext, ref data_array: Array, size: u32, offset: u32 ) -> Result<(), EVMError> { let mut i = 0; loop { @@ -72,54 +124,15 @@ mod internal { last_elem += (*chunk[j]).into(); j += 1; }; - datas_array.append(last_elem); + data_array.append(last_elem); } break; }; let (mut loaded, _) = self.memory.load(offset + i); loaded /= 256; - datas_array.append(loaded.try_into().unwrap()); + data_array.append(loaded.try_into().unwrap()); i += 31; }; Result::Ok(()) } } - -#[generate_trait] -impl LoggingOperations of LoggingOperationsTrait { - /// 0xA0 - LOG0 operation - /// Append log record with no topic. - /// # Specification: https://www.evm.codes/#a0?fork=shanghai - fn exec_log0(ref self: ExecutionContext) -> Result<(), EVMError> { - internal::exec_log_i(ref self, 0) - } - - - /// 0xA1 - LOG1 - /// Append log record with one topic. - /// # Specification: https://www.evm.codes/#a1?fork=shanghai - fn exec_log1(ref self: ExecutionContext) -> Result<(), EVMError> { - internal::exec_log_i(ref self, 1) - } - - /// 0xA2 - LOG2 - /// Append log record with two topics. - /// # Specification: https://www.evm.codes/#a2?fork=shanghai - fn exec_log2(ref self: ExecutionContext) -> Result<(), EVMError> { - internal::exec_log_i(ref self, 2) - } - - /// 0xA3 - LOG3 - /// Append log record with three topics. - /// # Specification: https://www.evm.codes/#a3?fork=shanghai - fn exec_log3(ref self: ExecutionContext) -> Result<(), EVMError> { - internal::exec_log_i(ref self, 3) - } - - /// 0xA4 - LOG4 - /// Append log record with 4 topics. - /// # Specification: https://www.evm.codes/#a4?fork=shanghai - fn exec_log4(ref self: ExecutionContext) -> Result<(), EVMError> { - internal::exec_log_i(ref self, 4) - } -} From c221d5cf76448eb6fdd865d3b916142cf7396b5c Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 15 Sep 2023 13:11:15 +0200 Subject: [PATCH 15/29] removed ',' from merge --- crates/evm/src/errors.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index e02a21fa5..773b1e5c5 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -23,7 +23,7 @@ enum EVMError { InvalidProgramCounter: felt252, TypeConversionError: felt252, ReturnDataError: felt252, - JumpError: felt252,, + JumpError: felt252, StateModificationError: felt252 } From 1f13db4a8c1563f6c2d2932a6baeb609724a09c4 Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 15 Sep 2023 15:33:56 +0200 Subject: [PATCH 16/29] check load vs loadn --- crates/evm/src/context.cairo | 16 +++++ .../src/instructions/logging_operations.cairo | 66 +++++++++---------- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index 42632ec0a..12e309262 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -343,6 +343,22 @@ impl ExecutionContextImpl of ExecutionContextTrait { fn set_pc(ref self: ExecutionContext, value: u32) { self.program_counter = value; } + + /// This function will store a new event in the dynamic context + /// using given topics and data. + /// The dynamic context has to be recreated to be modified. + /// + /// # Arguments + /// + /// * `self` - The context to which the event will be added + /// * `topics` - Topics of the event + /// * `data` - Data of the event + fn set_events(ref self: ExecutionContext, topics: Array, data: Array) { + let event: Event = Event { keys: topics, data }; + let mut dyn_ctx = self.dynamic_context.unbox(); + dyn_ctx.events.append(event); + self.dynamic_context = BoxTrait::new(dyn_ctx); + } } impl DefaultExecutionContext of Default { diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index c2d4e2491..fa638ed03 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -52,7 +52,6 @@ mod internal { use evm::helpers::U256IntoResultU32; use utils::helpers::u256_to_bytes_array; use utils::helpers::ArrayExtensionTrait; - use array::ArrayTrait; /// This generic function will store a new event in the dynamic context /// using topics popped from the stack and data from the memory. @@ -67,45 +66,20 @@ mod internal { return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); } - let popped = self.stack.pop_n(2 + topics_len.into())?; - let offset: u32 = Into::>::into(*popped[0])?; - let size: u32 = Into::>::into(*popped[1])?; + let offset = (self.stack.pop_usize())?; + let size = (self.stack.pop_usize())?; + let topics: Array = self.stack.pop_n(topics_len.into())?; - // Feed topics array for the new event - let mut topics: Array = Default::default(); - ArrayExtensionTrait::concat(ref topics, popped.span().slice(2, popped.len() - 2)); - - // Feed data array for the new event let mut data: Array = Default::default(); load_data_from_memory(ref self, ref data, size, offset); - // Create and store the new event in the dynamic context - store_event_in_dyn_context(ref self, topics, data); + self.set_events(topics, data); Result::Ok(()) } - /// This function will store a new event in the dynamic context - /// using given topics and data. - /// The dynamic context has to be recreated to be modified. - /// - /// # Arguments - /// - /// * `self` - The context to which the event will be added - /// * `topics` - Topics of the event - /// * `data` - Data of the event - fn store_event_in_dyn_context( - ref self: ExecutionContext, topics: Array, data: Array - ) { - let event: Event = Event { keys: topics, data }; - let mut dyn_ctx = self.dynamic_context.unbox(); - dyn_ctx.events.append(event); - self.dynamic_context = BoxTrait::new(dyn_ctx); - } - - - /// This function will load data from the memory by 32Bytes - /// using given topics and data. + /// This function will load data from the memory by 32 Bytes + /// chunk and fill . /// The dynamic context has to be recreated to be modified. /// /// # Arguments @@ -115,7 +89,7 @@ mod internal { /// * `data` - Data of the event fn load_data_from_memory( ref self: ExecutionContext, ref data_array: Array, size: u32, offset: u32 - ) -> Result<(), EVMError> { + ) { let mut i = 0; loop { if 31 + i > size { @@ -141,6 +115,30 @@ mod internal { data_array.append(loaded.try_into().unwrap()); i += 31; }; - Result::Ok(()) + } + + fn loadn_data_from_memory( + ref self: ExecutionContext, ref data_array: Array, size: u32, offset: u32 + ) { + let mut loaded: Array = Default::default(); + self.memory.load_n(size, ref loaded, offset); + let loaded_len = loaded.len(); + let mut i = 0; + let mut element: felt252 = 0; + loop { + if i == loaded_len { + break; + } + element *= 256; + element += (*loaded[i]).into(); + i += 1; + if i % 31 == 0 { + data_array.append(element); + element = 0; + }; + }; + if element != 0 { + data_array.append(element); + } } } From 89b487245b922fe387d08e2b020e3334e2431b96 Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 22 Sep 2023 09:54:16 +0200 Subject: [PATCH 17/29] switch data array from felt to u8 --- crates/evm/src/context.cairo | 13 +- .../src/instructions/logging_operations.cairo | 113 +++++++----------- crates/evm/src/model.cairo | 2 +- .../test_logging_operations.cairo | 47 ++++---- crates/evm/src/tests/test_utils.cairo | 14 --- 5 files changed, 81 insertions(+), 108 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index 12e309262..0915c94f7 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -344,8 +344,15 @@ impl ExecutionContextImpl of ExecutionContextTrait { self.program_counter = value; } - /// This function will store a new event in the dynamic context - /// using given topics and data. + #[inline(always)] + fn set_read_only(ref self: ExecutionContext, value: bool) { + let mut static_ctx = self.static_context.unbox(); + static_ctx.read_only = value; + self.static_context = BoxTrait::new(static_ctx); + } + + /// Store a new event in the dynamic context using given + /// topics and data. /// The dynamic context has to be recreated to be modified. /// /// # Arguments @@ -353,7 +360,7 @@ impl ExecutionContextImpl of ExecutionContextTrait { /// * `self` - The context to which the event will be added /// * `topics` - Topics of the event /// * `data` - Data of the event - fn set_events(ref self: ExecutionContext, topics: Array, data: Array) { + fn set_events(ref self: ExecutionContext, topics: Array, data: Array) { let event: Event = Event { keys: topics, data }; let mut dyn_ctx = self.dynamic_context.unbox(); dyn_ctx.events.append(event); diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index fa638ed03..b6632009c 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -53,8 +53,8 @@ mod internal { use utils::helpers::u256_to_bytes_array; use utils::helpers::ArrayExtensionTrait; - /// This generic function will store a new event in the dynamic context - /// using topics popped from the stack and data from the memory. + /// Store a new event in the dynamic context using topics + /// popped from the stack and data from the memory. /// /// # Arguments /// @@ -70,75 +70,54 @@ mod internal { let size = (self.stack.pop_usize())?; let topics: Array = self.stack.pop_n(topics_len.into())?; - let mut data: Array = Default::default(); - load_data_from_memory(ref self, ref data, size, offset); + let mut data: Array = Default::default(); + self.memory.load_n(size, ref data, offset); + + //let mut data: Array = Default::default(); + //load_data_from_memory(ref self, ref data, size, offset); self.set_events(topics, data); Result::Ok(()) } +/// Load data from the memory by extracting 32 bytes words +/// and put the 31 most significant bytes in a felt. +/// The dynamic context has to be recreated to be modified. +/// +/// # Arguments +/// +/// * `self` - The context to which the event will be added +/// * `data_array` - Array containing the data +/// * `size` - Amount of bytes to load +/// * `offset` - Offset in the memory +// fn load_data_from_memory( +// ref self: ExecutionContext, ref data_array: Array, mut size: u32, mut offset: u32 +// ) { +// let mut i = 0; +// loop { - /// This function will load data from the memory by 32 Bytes - /// chunk and fill . - /// The dynamic context has to be recreated to be modified. - /// - /// # Arguments - /// - /// * `self` - The context to which the event will be added - /// * `topics` - Topics of the event - /// * `data` - Data of the event - fn load_data_from_memory( - ref self: ExecutionContext, ref data_array: Array, size: u32, offset: u32 - ) { - let mut i = 0; - loop { - if 31 + i > size { - if i != size { - let loaded = self.memory.load(offset + i); - let mut chunk: Array = u256_to_bytes_array(loaded); - let mut last_elem = 0; - let mut j = 0; - loop { - if j + i == size { - break; - } - last_elem *= 256; - last_elem += (*chunk[j]).into(); - j += 1; - }; - data_array.append(last_elem); - } - break; - }; - let mut loaded = self.memory.load(offset + i); - loaded /= 256; - data_array.append(loaded.try_into().unwrap()); - i += 31; - }; - } - - fn loadn_data_from_memory( - ref self: ExecutionContext, ref data_array: Array, size: u32, offset: u32 - ) { - let mut loaded: Array = Default::default(); - self.memory.load_n(size, ref loaded, offset); - let loaded_len = loaded.len(); - let mut i = 0; - let mut element: felt252 = 0; - loop { - if i == loaded_len { - break; - } - element *= 256; - element += (*loaded[i]).into(); - i += 1; - if i % 31 == 0 { - data_array.append(element); - element = 0; - }; - }; - if element != 0 { - data_array.append(element); - } - } +// if size < 31 + i { +// if i != size { +// let loaded = self.memory.load(offset + i); +// let mut chunk: Array = u256_to_bytes_array(loaded); +// let mut last_elem = 0; +// let mut j = 0; +// loop { +// if j + i == size { +// break; +// } +// last_elem *= 256; +// last_elem += (*chunk[j]).into(); +// j += 1; +// }; +// data_array.append(last_elem); +// } +// break; +// }; +// let mut loaded = self.memory.load(offset + i); +// loaded /= 256; +// data_array.append(loaded.try_into().unwrap()); +// i += 31; +// }; +// } } diff --git a/crates/evm/src/model.cairo b/crates/evm/src/model.cairo index 8ac93588b..26441b251 100644 --- a/crates/evm/src/model.cairo +++ b/crates/evm/src/model.cairo @@ -1,5 +1,5 @@ #[derive(Drop)] struct Event { keys: Array, - data: Array, + data: Array, } diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 7b13166a6..b091f1773 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -3,10 +3,12 @@ use evm::context::{ }; use evm::stack::StackTrait; use evm::memory::MemoryTrait; -use evm::tests::test_utils::{setup_execution_context, setup_read_only_execution_context}; +use evm::tests::test_utils::setup_execution_context; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; +use utils::helpers::u256_to_bytes_array; +use debug::PrintTrait; #[test] #[available_gas(20000000)] @@ -33,11 +35,10 @@ fn test_exec_log0() { assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); assert(event.keys.len() == 0, 'event should not have keys'); - assert(event.data.len() == 1, 'event should have one data'); - assert( - *event.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, - 'event data should be max_u248' - ); + assert(event.data.len() == 31, 'event should have 31 data'); + assert(*event.data[0] == 0xff, 'event data should be max_u8'); + assert(*event.data[15] == 0xff, 'event data should be max_u8'); + assert(*event.data[30] == 0xff, 'event data should be max_u8'); } #[test] @@ -66,19 +67,18 @@ fn test_exec_log1() { let event = events.pop_front().unwrap(); assert(event.keys.len() == 1, 'event should have one key'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); - assert(event.data.len() == 2, 'event should have one data'); - assert( - *event.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, - 'event data0 should be max_u248' - ); - assert(*event.data[1] == 0xff, 'event data1 should be 0xFF'); + assert(event.data.len() == 32, 'event should have one data'); + assert(*event.data[0] == 0xff, 'event data should be max_u8'); + assert(*event.data[15] == 0xff, 'event data should be max_u8'); + assert(*event.data[31] == 0xff, 'event data should be max_u8'); } #[test] #[available_gas(20000000)] fn test_exec_log1_read_only_context() { // Given - let mut ctx = setup_read_only_execution_context(); + let mut ctx = setup_execution_context(); + ctx.set_read_only(true); ctx.memory.store(BoundedInt::::max(), 0); @@ -127,8 +127,9 @@ fn test_exec_log2() { assert(event.keys.len() == 2, 'event should have two keys'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); - assert(event.data.len() == 1, 'event should have one data'); - assert(*event.data[0] == 0xFFFFFFFFFF, 'event data is not correct'); + assert(event.data.len() == 5, 'event should have one data'); + assert(*event.data[0] == 0xff, 'event data should be max_u8'); + assert(*event.data[4] == 0xff, 'event data should be max_u8'); } #[test] @@ -169,12 +170,10 @@ fn test_exec_log3_and_log4() { assert(*event1.keys[1] == BoundedInt::::max(), 'event1 key is not correct'); assert(*event1.keys[2] == 0x00, 'event1 key is not correct'); - assert(event1.data.len() == 2, 'event1 should have 2 datas'); - assert( - *event1.data[0] == 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, - 'event1 data is not correct' - ); - assert(*event1.data[1] == 0xff0123456789ABCDEF, 'event1 data is not correct'); + assert(event1.data.len() == 40, 'event1 should have 38 data'); + assert(*event1.data[0] == 0xff, 'event data should be max_u8'); + assert(*event1.data[4] == 0xff, 'event data should be max_u8'); + assert(*event1.data[39] == 0xef, 'event data should be 0xEF'); let event2 = events.pop_front().unwrap(); assert(event2.keys.len() == 4, 'event2 should have 4 keys'); @@ -183,6 +182,8 @@ fn test_exec_log3_and_log4() { assert(*event2.keys[2] == 0x00, 'event2 key is not correct'); assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); - assert(event2.data.len() == 1, 'event2 should have one data'); - assert(*event2.data[0] == 0x0123456789ABCDEF0000, 'event2 data is not correct'); + assert(event2.data.len() == 10, 'event2 should have 10 bytes'); + assert(*event2.data[0] == 0x01, 'event data should be 0x01'); + assert(*event2.data[5] == 0xAB, 'event data should be 0xAB'); + assert(*event2.data[9] == 0x00, 'event data should be 0x00'); } diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index 000529ec9..b00bd3ff0 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -83,20 +83,6 @@ fn setup_execution_context_with_calldata(calldata: Span) -> ExecutionContext ) } -fn setup_read_only_execution_context() -> 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 = true; - let return_data = Default::default(); - - ExecutionContextTrait::new( - call_context, starknet_address, evm_address, gas_limit, gas_price, return_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 a800070e518ef4e714640c1d243c1a8dd4ea935e Mon Sep 17 00:00:00 2001 From: Quentash Date: Fri, 22 Sep 2023 10:23:47 +0200 Subject: [PATCH 18/29] cleaned code/tests --- .../src/instructions/logging_operations.cairo | 43 ------------------- .../test_logging_operations.cairo | 38 ++++++++-------- 2 files changed, 18 insertions(+), 63 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index b6632009c..b1e438e9a 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -73,51 +73,8 @@ mod internal { let mut data: Array = Default::default(); self.memory.load_n(size, ref data, offset); - //let mut data: Array = Default::default(); - //load_data_from_memory(ref self, ref data, size, offset); - self.set_events(topics, data); Result::Ok(()) } -/// Load data from the memory by extracting 32 bytes words -/// and put the 31 most significant bytes in a felt. -/// The dynamic context has to be recreated to be modified. -/// -/// # Arguments -/// -/// * `self` - The context to which the event will be added -/// * `data_array` - Array containing the data -/// * `size` - Amount of bytes to load -/// * `offset` - Offset in the memory -// fn load_data_from_memory( -// ref self: ExecutionContext, ref data_array: Array, mut size: u32, mut offset: u32 -// ) { -// let mut i = 0; -// loop { - -// if size < 31 + i { -// if i != size { -// let loaded = self.memory.load(offset + i); -// let mut chunk: Array = u256_to_bytes_array(loaded); -// let mut last_elem = 0; -// let mut j = 0; -// loop { -// if j + i == size { -// break; -// } -// last_elem *= 256; -// last_elem += (*chunk[j]).into(); -// j += 1; -// }; -// data_array.append(last_elem); -// } -// break; -// }; -// let mut loaded = self.memory.load(offset + i); -// loaded /= 256; -// data_array.append(loaded.try_into().unwrap()); -// i += 31; -// }; -// } } diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index b091f1773..4969eb73b 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -7,8 +7,6 @@ use evm::tests::test_utils::setup_execution_context; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; -use utils::helpers::u256_to_bytes_array; -use debug::PrintTrait; #[test] #[available_gas(20000000)] @@ -35,10 +33,10 @@ fn test_exec_log0() { assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); assert(event.keys.len() == 0, 'event should not have keys'); - assert(event.data.len() == 31, 'event should have 31 data'); - assert(*event.data[0] == 0xff, 'event data should be max_u8'); - assert(*event.data[15] == 0xff, 'event data should be max_u8'); - assert(*event.data[30] == 0xff, 'event data should be max_u8'); + assert(event.data.len() == 31, 'event should have 31 bytes'); + assert(*event.data[0] == 0xff, 'event byte should be max_u8'); + assert(*event.data[15] == 0xff, 'event byte should be max_u8'); + assert(*event.data[30] == 0xff, 'event byte should be max_u8'); } #[test] @@ -67,10 +65,10 @@ fn test_exec_log1() { let event = events.pop_front().unwrap(); assert(event.keys.len() == 1, 'event should have one key'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); - assert(event.data.len() == 32, 'event should have one data'); - assert(*event.data[0] == 0xff, 'event data should be max_u8'); - assert(*event.data[15] == 0xff, 'event data should be max_u8'); - assert(*event.data[31] == 0xff, 'event data should be max_u8'); + assert(event.data.len() == 32, 'event should have 32 bytes'); + assert(*event.data[0] == 0xff, 'event byte should be max_u8'); + assert(*event.data[15] == 0xff, 'event byte should be max_u8'); + assert(*event.data[31] == 0xff, 'event byte should be max_u8'); } #[test] @@ -127,9 +125,9 @@ fn test_exec_log2() { assert(event.keys.len() == 2, 'event should have two keys'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); - assert(event.data.len() == 5, 'event should have one data'); - assert(*event.data[0] == 0xff, 'event data should be max_u8'); - assert(*event.data[4] == 0xff, 'event data should be max_u8'); + assert(event.data.len() == 5, 'event should have 5 bytes'); + assert(*event.data[0] == 0xff, 'event byte should be max_u8'); + assert(*event.data[4] == 0xff, 'event byte should be max_u8'); } #[test] @@ -170,10 +168,10 @@ fn test_exec_log3_and_log4() { assert(*event1.keys[1] == BoundedInt::::max(), 'event1 key is not correct'); assert(*event1.keys[2] == 0x00, 'event1 key is not correct'); - assert(event1.data.len() == 40, 'event1 should have 38 data'); - assert(*event1.data[0] == 0xff, 'event data should be max_u8'); - assert(*event1.data[4] == 0xff, 'event data should be max_u8'); - assert(*event1.data[39] == 0xef, 'event data should be 0xEF'); + assert(event1.data.len() == 40, 'event1 should have 40 bytes'); + assert(*event1.data[0] == 0xff, 'event1 byte should be max_u8'); + assert(*event1.data[4] == 0xff, 'event1 byte should be max_u8'); + assert(*event1.data[39] == 0xef, 'event1 byte should be 0xEF'); let event2 = events.pop_front().unwrap(); assert(event2.keys.len() == 4, 'event2 should have 4 keys'); @@ -183,7 +181,7 @@ fn test_exec_log3_and_log4() { assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); assert(event2.data.len() == 10, 'event2 should have 10 bytes'); - assert(*event2.data[0] == 0x01, 'event data should be 0x01'); - assert(*event2.data[5] == 0xAB, 'event data should be 0xAB'); - assert(*event2.data[9] == 0x00, 'event data should be 0x00'); + assert(*event2.data[0] == 0x01, 'event2 byte should be 0x01'); + assert(*event2.data[5] == 0xAB, 'event2 byte should be 0xAB'); + assert(*event2.data[9] == 0x00, 'event2 byte should be 0x00'); } From f8bce7ae661e23ead82be118cb1dc2e1f0fa37d1 Mon Sep 17 00:00:00 2001 From: Quentash Date: Mon, 25 Sep 2023 10:35:04 +0200 Subject: [PATCH 19/29] review correction --- crates/evm/src/context.cairo | 9 +- .../src/instructions/logging_operations.cairo | 7 +- .../test_logging_operations.cairo | 130 ++++++++++++++---- 3 files changed, 107 insertions(+), 39 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index 0915c94f7..c1d287536 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -351,17 +351,14 @@ impl ExecutionContextImpl of ExecutionContextTrait { self.static_context = BoxTrait::new(static_ctx); } - /// Store a new event in the dynamic context using given - /// topics and data. + /// Store a new event in the dynamic context. /// The dynamic context has to be recreated to be modified. /// /// # Arguments /// /// * `self` - The context to which the event will be added - /// * `topics` - Topics of the event - /// * `data` - Data of the event - fn set_events(ref self: ExecutionContext, topics: Array, data: Array) { - let event: Event = Event { keys: topics, data }; + /// * `event` - Event to append + fn append_event(ref self: ExecutionContext, event: Event) { let mut dyn_ctx = self.dynamic_context.unbox(); dyn_ctx.events.append(event); self.dynamic_context = BoxTrait::new(dyn_ctx); diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index b1e438e9a..4fcad55e8 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -66,14 +66,15 @@ mod internal { return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); } - let offset = (self.stack.pop_usize())?; - let size = (self.stack.pop_usize())?; + let offset = self.stack.pop_usize()?; + let size = self.stack.pop_usize()?; let topics: Array = self.stack.pop_n(topics_len.into())?; let mut data: Array = Default::default(); self.memory.load_n(size, ref data, offset); - self.set_events(topics, data); + let event: Event = Event { keys: topics, data }; + self.append_event(event); Result::Ok(()) } diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 4969eb73b..edc891959 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -16,10 +16,6 @@ fn test_exec_log0() { ctx.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); ctx.stack.push(0x1F); ctx.stack.push(0x00); @@ -28,9 +24,11 @@ fn test_exec_log0() { // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 4, 'stack size should be 4'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + let mut events = ctx.events(); assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); assert(event.keys.len() == 0, 'event should not have keys'); assert(event.data.len() == 31, 'event should have 31 bytes'); @@ -47,9 +45,6 @@ fn test_exec_log1() { ctx.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); ctx.stack.push(0x0123456789ABCDEF); ctx.stack.push(0x20); ctx.stack.push(0x00); @@ -59,9 +54,11 @@ fn test_exec_log1() { // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 3, 'stack size should be 3'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + let mut events = ctx.events(); assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); assert(event.keys.len() == 1, 'event should have one key'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); @@ -73,66 +70,139 @@ fn test_exec_log1() { #[test] #[available_gas(20000000)] -fn test_exec_log1_read_only_context() { +fn test_exec_log2() { // Given let mut ctx = setup_execution_context(); - ctx.set_read_only(true); ctx.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); ctx.stack.push(BoundedInt::::max()); ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x20); - ctx.stack.push(0x00); + ctx.stack.push(0x05); + ctx.stack.push(0x05); // When - let result = ctx.exec_log1(); + let result = ctx.exec_log2(); // Then - assert(result.is_err(), 'should have returned an error'); - assert( - result.unwrap_err() == EVMError::StateModificationError(STATE_MODIFICATION_ERROR), - 'err != StateModificationError' - ); + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 2, 'event should have two keys'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); + assert(event.data.len() == 5, 'event should have 5 bytes'); + assert(*event.data[0] == 0xff, 'event byte should be max_u8'); + assert(*event.data[4] == 0xff, 'event byte should be max_u8'); } #[test] #[available_gas(20000000)] -fn test_exec_log2() { +fn test_exec_log3() { // Given let mut ctx = setup_execution_context(); ctx.memory.store(BoundedInt::::max(), 0); + ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); - ctx.stack.push(BoundedInt::::max()); ctx.stack.push(0x00); ctx.stack.push(BoundedInt::::max()); ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x05); - ctx.stack.push(0x05); + ctx.stack.push(0x28); + ctx.stack.push(0x00); // When - let result = ctx.exec_log2(); + let result = ctx.exec_log3(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 2, 'stack size should be 2'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + let mut events = ctx.events(); assert(events.len() == 1, 'context should have one event'); + let event = events.pop_front().unwrap(); - assert(event.keys.len() == 2, 'event should have two keys'); + assert(event.keys.len() == 3, 'event should have 3 keys'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); - assert(event.data.len() == 5, 'event should have 5 bytes'); + assert(*event.keys[2] == 0x00, 'event key is not correct'); + + assert(event.data.len() == 40, 'event should have 40 bytes'); assert(*event.data[0] == 0xff, 'event byte should be max_u8'); assert(*event.data[4] == 0xff, 'event byte should be max_u8'); + assert(*event.data[39] == 0xef, 'event byte should be 0xEF'); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log4() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); + + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x0A); + ctx.stack.push(0x20); + + // When + let result = ctx.exec_log4(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 4, 'event2 should have 4 keys'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event2 key is not correct'); + assert(*event.keys[1] == BoundedInt::::max(), 'event2 key is not correct'); + assert(*event.keys[2] == 0x00, 'event2 key is not correct'); + assert(*event.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); + + assert(event.data.len() == 10, 'event2 should have 10 bytes'); + assert(*event.data[0] == 0x01, 'event2 byte should be 0x01'); + assert(*event.data[5] == 0xAB, 'event2 byte should be 0xAB'); + assert(*event.data[9] == 0x00, 'event2 byte should be 0x00'); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log1_read_only_context() { + // Given + let mut ctx = setup_execution_context(); + ctx.set_read_only(true); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x20); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_err(), 'should have returned an error'); + assert( + result.unwrap_err() == EVMError::StateModificationError(STATE_MODIFICATION_ERROR), + 'err != StateModificationError' + ); } #[test] #[available_gas(20000000)] -fn test_exec_log3_and_log4() { +fn test_exec_log_multiple_events() { // Given let mut ctx = setup_execution_context(); From b9512c42dc78c069c27b4a664d081a846918be6b Mon Sep 17 00:00:00 2001 From: Quentash Date: Mon, 25 Sep 2023 12:34:09 +0200 Subject: [PATCH 20/29] compare all bytes in tests + inlined app_event --- crates/evm/src/context.cairo | 1 + .../test_logging_operations.cairo | 72 ++++++++++++------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index c1d287536..694d79eaf 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -358,6 +358,7 @@ impl ExecutionContextImpl of ExecutionContextTrait { /// /// * `self` - The context to which the event will be added /// * `event` - Event to append + #[inline(always)] fn append_event(ref self: ExecutionContext, event: Event) { let mut dyn_ctx = self.dynamic_context.unbox(); dyn_ctx.events.append(event); diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index edc891959..45bb0d624 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -7,6 +7,7 @@ use evm::tests::test_utils::setup_execution_context; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; +use utils::helpers::u256_to_bytes_array; #[test] #[available_gas(20000000)] @@ -31,10 +32,10 @@ fn test_exec_log0() { let event = events.pop_front().unwrap(); assert(event.keys.len() == 0, 'event should not have keys'); + assert(event.data.len() == 31, 'event should have 31 bytes'); - assert(*event.data[0] == 0xff, 'event byte should be max_u8'); - assert(*event.data[15] == 0xff, 'event byte should be max_u8'); - assert(*event.data[30] == 0xff, 'event byte should be max_u8'); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 31); + assert(event.data.span() == data_expected, 'event data are incorrect'); } #[test] @@ -62,10 +63,10 @@ fn test_exec_log1() { let event = events.pop_front().unwrap(); assert(event.keys.len() == 1, 'event should have one key'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + assert(event.data.len() == 32, 'event should have 32 bytes'); - assert(*event.data[0] == 0xff, 'event byte should be max_u8'); - assert(*event.data[15] == 0xff, 'event byte should be max_u8'); - assert(*event.data[31] == 0xff, 'event byte should be max_u8'); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 32); + assert(event.data.span() == data_expected, 'event data are incorrect'); } #[test] @@ -95,9 +96,10 @@ fn test_exec_log2() { assert(event.keys.len() == 2, 'event should have two keys'); assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); + assert(event.data.len() == 5, 'event should have 5 bytes'); - assert(*event.data[0] == 0xff, 'event byte should be max_u8'); - assert(*event.data[4] == 0xff, 'event byte should be max_u8'); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 5); + assert(event.data.span() == data_expected, 'event data are incorrect'); } #[test] @@ -132,9 +134,14 @@ fn test_exec_log3() { assert(*event.keys[2] == 0x00, 'event key is not correct'); assert(event.data.len() == 40, 'event should have 40 bytes'); - assert(*event.data[0] == 0xff, 'event byte should be max_u8'); - assert(*event.data[4] == 0xff, 'event byte should be max_u8'); - assert(*event.data[39] == 0xef, 'event byte should be 0xEF'); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 32); + assert(event.data.span().slice(0, 32) == data_expected, 'event data are incorrect'); + let data_expected = u256_to_bytes_array( + 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 + ) + .span() + .slice(0, 8); + assert(event.data.span().slice(32, 8) == data_expected, 'event data are incorrect'); } #[test] @@ -164,16 +171,19 @@ fn test_exec_log4() { assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); - assert(event.keys.len() == 4, 'event2 should have 4 keys'); - assert(*event.keys[0] == 0x0123456789ABCDEF, 'event2 key is not correct'); - assert(*event.keys[1] == BoundedInt::::max(), 'event2 key is not correct'); - assert(*event.keys[2] == 0x00, 'event2 key is not correct'); - assert(*event.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); - - assert(event.data.len() == 10, 'event2 should have 10 bytes'); - assert(*event.data[0] == 0x01, 'event2 byte should be 0x01'); - assert(*event.data[5] == 0xAB, 'event2 byte should be 0xAB'); - assert(*event.data[9] == 0x00, 'event2 byte should be 0x00'); + assert(event.keys.len() == 4, 'event should have 4 keys'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + assert(*event.keys[1] == BoundedInt::::max(), 'event key is not correct'); + assert(*event.keys[2] == 0x00, 'event key is not correct'); + assert(*event.keys[3] == BoundedInt::::max(), 'event key is not correct'); + + assert(event.data.len() == 10, 'event should have 10 bytes'); + let data_expected = u256_to_bytes_array( + 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 + ) + .span() + .slice(0, 10); + assert(event.data.span() == data_expected, 'event data are incorrect'); } #[test] @@ -239,9 +249,14 @@ fn test_exec_log_multiple_events() { assert(*event1.keys[2] == 0x00, 'event1 key is not correct'); assert(event1.data.len() == 40, 'event1 should have 40 bytes'); - assert(*event1.data[0] == 0xff, 'event1 byte should be max_u8'); - assert(*event1.data[4] == 0xff, 'event1 byte should be max_u8'); - assert(*event1.data[39] == 0xef, 'event1 byte should be 0xEF'); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 32); + assert(event1.data.span().slice(0, 32) == data_expected, 'event1 data are incorrect'); + let data_expected = u256_to_bytes_array( + 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 + ) + .span() + .slice(0, 8); + assert(event1.data.span().slice(32, 8) == data_expected, 'event1 data are incorrect'); let event2 = events.pop_front().unwrap(); assert(event2.keys.len() == 4, 'event2 should have 4 keys'); @@ -251,7 +266,10 @@ fn test_exec_log_multiple_events() { assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); assert(event2.data.len() == 10, 'event2 should have 10 bytes'); - assert(*event2.data[0] == 0x01, 'event2 byte should be 0x01'); - assert(*event2.data[5] == 0xAB, 'event2 byte should be 0xAB'); - assert(*event2.data[9] == 0x00, 'event2 byte should be 0x00'); + let data_expected = u256_to_bytes_array( + 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 + ) + .span() + .slice(0, 10); + assert(event2.data.span() == data_expected, 'event2 data are incorrect'); } From 2e8de3c78ead27141ff277627acbcb96f2e0b7d4 Mon Sep 17 00:00:00 2001 From: Quentash Date: Mon, 25 Sep 2023 13:06:30 +0200 Subject: [PATCH 21/29] stop useless slicing --- .../src/tests/test_instructions/test_logging_operations.cairo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 45bb0d624..99b93ac2f 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -134,7 +134,7 @@ fn test_exec_log3() { assert(*event.keys[2] == 0x00, 'event key is not correct'); assert(event.data.len() == 40, 'event should have 40 bytes'); - let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 32); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span(); assert(event.data.span().slice(0, 32) == data_expected, 'event data are incorrect'); let data_expected = u256_to_bytes_array( 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 @@ -249,7 +249,7 @@ fn test_exec_log_multiple_events() { assert(*event1.keys[2] == 0x00, 'event1 key is not correct'); assert(event1.data.len() == 40, 'event1 should have 40 bytes'); - let data_expected = u256_to_bytes_array(BoundedInt::::max()).span().slice(0, 32); + let data_expected = u256_to_bytes_array(BoundedInt::::max()).span(); assert(event1.data.span().slice(0, 32) == data_expected, 'event1 data are incorrect'); let data_expected = u256_to_bytes_array( 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 From 717a6e44d3b554cf00b520dd731854c09be046a7 Mon Sep 17 00:00:00 2001 From: Quentash Date: Mon, 25 Sep 2023 15:33:16 +0200 Subject: [PATCH 22/29] added tests cases --- .../test_logging_operations.cairo | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 99b93ac2f..efd60fdfe 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -4,7 +4,7 @@ use evm::context::{ use evm::stack::StackTrait; use evm::memory::MemoryTrait; use evm::tests::test_utils::setup_execution_context; -use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; +use evm::errors::{EVMError, STATE_MODIFICATION_ERROR, TYPE_CONVERSION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; use utils::helpers::u256_to_bytes_array; @@ -210,6 +210,81 @@ fn test_exec_log1_read_only_context() { ); } +#[test] +#[available_gas(20000000)] +fn test_exec_log1_size_0_offset_0() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x00); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_ok(), 'should have succeeded'); + assert(ctx.stack.len() == 0, 'stack should be empty'); + + let mut events = ctx.events(); + assert(events.len() == 1, 'context should have one event'); + + let event = events.pop_front().unwrap(); + assert(event.keys.len() == 1, 'event should have one key'); + assert(*event.keys[0] == 0x0123456789ABCDEF, 'event key is not correct'); + + assert(event.data.len() == 0, 'event data should be empty'); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log1_size_too_big() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(BoundedInt::::max()); + ctx.stack.push(0x00); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_err(), 'should return an error'); + assert( + result.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR), + 'err != TypeConversionError' + ); +} + +#[test] +#[available_gas(20000000)] +fn test_exec_log1_offset_too_big() { + // Given + let mut ctx = setup_execution_context(); + + ctx.memory.store(BoundedInt::::max(), 0); + + ctx.stack.push(0x0123456789ABCDEF); + ctx.stack.push(0x20); + ctx.stack.push(BoundedInt::::max()); + + // When + let result = ctx.exec_log1(); + + // Then + assert(result.is_err(), 'should return an error'); + assert( + result.unwrap_err() == EVMError::TypeConversionError(TYPE_CONVERSION_ERROR), + 'err != TypeConversionError' + ); +} + #[test] #[available_gas(20000000)] fn test_exec_log_multiple_events() { From e3e79dc69ea16d01f22c88ab520565a664a799e5 Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 27 Sep 2023 10:53:35 +0200 Subject: [PATCH 23/29] simplified expected array --- .../test_logging_operations.cairo | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index efd60fdfe..6a2eddf2b 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -136,11 +136,7 @@ fn test_exec_log3() { assert(event.data.len() == 40, 'event should have 40 bytes'); let data_expected = u256_to_bytes_array(BoundedInt::::max()).span(); assert(event.data.span().slice(0, 32) == data_expected, 'event data are incorrect'); - let data_expected = u256_to_bytes_array( - 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 - ) - .span() - .slice(0, 8); + let data_expected = array![0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF].span(); assert(event.data.span().slice(32, 8) == data_expected, 'event data are incorrect'); } @@ -178,11 +174,7 @@ fn test_exec_log4() { assert(*event.keys[3] == BoundedInt::::max(), 'event key is not correct'); assert(event.data.len() == 10, 'event should have 10 bytes'); - let data_expected = u256_to_bytes_array( - 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 - ) - .span() - .slice(0, 10); + let data_expected = array![0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x00, 0x00].span(); assert(event.data.span() == data_expected, 'event data are incorrect'); } @@ -326,11 +318,7 @@ fn test_exec_log_multiple_events() { assert(event1.data.len() == 40, 'event1 should have 40 bytes'); let data_expected = u256_to_bytes_array(BoundedInt::::max()).span(); assert(event1.data.span().slice(0, 32) == data_expected, 'event1 data are incorrect'); - let data_expected = u256_to_bytes_array( - 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 - ) - .span() - .slice(0, 8); + let data_expected = array![0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF].span(); assert(event1.data.span().slice(32, 8) == data_expected, 'event1 data are incorrect'); let event2 = events.pop_front().unwrap(); @@ -341,10 +329,6 @@ fn test_exec_log_multiple_events() { assert(*event2.keys[3] == BoundedInt::::max(), 'event2 key is not correct'); assert(event2.data.len() == 10, 'event2 should have 10 bytes'); - let data_expected = u256_to_bytes_array( - 0x0123456789ABCDEF000000000000000000000000000000000000000000000000 - ) - .span() - .slice(0, 10); + let data_expected = array![0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x00, 0x00].span(); assert(event2.data.span() == data_expected, 'event2 data are incorrect'); } From eec78543b0448c3597ebac8e1c606802320083fd Mon Sep 17 00:00:00 2001 From: Quentash Date: Tue, 3 Oct 2023 11:02:04 +0200 Subject: [PATCH 24/29] merge correction --- crates/evm/src/context.cairo | 21 -- .../src/instructions/logging_operations.cairo | 16 +- crates/evm/src/machine.cairo | 16 ++ .../test_logging_operations.cairo | 195 +++++++++--------- 4 files changed, 122 insertions(+), 126 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index e15e0dbe5..52c00a6eb 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -309,27 +309,6 @@ impl ExecutionContextImpl of ExecutionContextTrait { self.program_counter = value; } - #[inline(always)] - fn set_read_only(ref self: ExecutionContext, value: bool) { - let mut static_ctx = self.static_context.unbox(); - static_ctx.read_only = value; - self.static_context = BoxTrait::new(static_ctx); - } - - /// Store a new event in the dynamic context. - /// The dynamic context has to be recreated to be modified. - /// - /// # Arguments - /// - /// * `self` - The context to which the event will be added - /// * `event` - Event to append - #[inline(always)] - fn append_event(ref self: ExecutionContext, event: Event) { - let mut dyn_ctx = self.dynamic_context.unbox(); - dyn_ctx.events.append(event); - self.dynamic_context = BoxTrait::new(dyn_ctx); - } - #[inline(always)] fn pc(self: @ExecutionContext) -> u32 { *self.program_counter diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index b16a352d2..ff4791bc1 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -1,6 +1,7 @@ //! Logging Operations. // Internal imports +use evm::machine::{Machine, MachineCurrentContext}; use evm::context::ExecutionContext; use evm::errors::EVMError; @@ -9,41 +10,42 @@ impl LoggingOperations of LoggingOperationsTrait { /// 0xA0 - LOG0 operation /// Append log record with no topic. /// # Specification: https://www.evm.codes/#a0?fork=shanghai - fn exec_log0(ref self: ExecutionContext) -> Result<(), EVMError> { + fn exec_log0(ref self: Machine) -> Result<(), EVMError> { internal::exec_log_i(ref self, 0) } /// 0xA1 - LOG1 /// Append log record with one topic. /// # Specification: https://www.evm.codes/#a1?fork=shanghai - fn exec_log1(ref self: ExecutionContext) -> Result<(), EVMError> { + fn exec_log1(ref self: Machine) -> Result<(), EVMError> { internal::exec_log_i(ref self, 1) } /// 0xA2 - LOG2 /// Append log record with two topics. /// # Specification: https://www.evm.codes/#a2?fork=shanghai - fn exec_log2(ref self: ExecutionContext) -> Result<(), EVMError> { + fn exec_log2(ref self: Machine) -> Result<(), EVMError> { internal::exec_log_i(ref self, 2) } /// 0xA3 - LOG3 /// Append log record with three topics. /// # Specification: https://www.evm.codes/#a3?fork=shanghai - fn exec_log3(ref self: ExecutionContext) -> Result<(), EVMError> { + fn exec_log3(ref self: Machine) -> Result<(), EVMError> { internal::exec_log_i(ref self, 3) } /// 0xA4 - LOG4 /// Append log record with four topics. /// # Specification: https://www.evm.codes/#a4?fork=shanghai - fn exec_log4(ref self: ExecutionContext) -> Result<(), EVMError> { + fn exec_log4(ref self: Machine) -> Result<(), EVMError> { internal::exec_log_i(ref self, 4) } } mod internal { - use evm::context::{ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct}; + use evm::context::{ExecutionContext, ExecutionContextTrait}; + use evm::machine::{Machine, MachineCurrentContext}; use evm::stack::StackTrait; use evm::memory::MemoryTrait; use evm::model::Event; @@ -59,7 +61,7 @@ mod internal { /// /// * `self` - The context to which the event will be added /// * `topics_len` - The amount of topics to pop from the stack - fn exec_log_i(ref self: ExecutionContext, topics_len: u8) -> Result<(), EVMError> { + fn exec_log_i(ref self: Machine, topics_len: u8) -> Result<(), EVMError> { // Revert if the transaction is in a read only context if self.read_only() { return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); diff --git a/crates/evm/src/machine.cairo b/crates/evm/src/machine.cairo index 37a3d606d..ea32d2e0b 100644 --- a/crates/evm/src/machine.cairo +++ b/crates/evm/src/machine.cairo @@ -172,6 +172,22 @@ impl MachineCurrentContextImpl of MachineCurrentContext { current_call_ctx.read_only() } + #[inline(always)] + fn set_read_only(ref self: Machine, value: bool) { + let mut current_call_ctx = self.call_context(); + let mut current_execution_ctx = self.current_context.unbox(); + current_call_ctx.read_only = value; + current_execution_ctx.call_context = BoxTrait::new(current_call_ctx); + self.current_context = BoxTrait::new(current_execution_ctx); + } + + #[inline(always)] + fn append_event(ref self: Machine, event: Event) { + let mut current_execution_ctx = self.current_context.unbox(); + current_execution_ctx.events.append(event); + self.current_context = BoxTrait::new(current_execution_ctx); + } + #[inline(always)] fn gas_limit(ref self: Machine) -> u64 { let current_call_ctx = self.call_context(); diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 6a2eddf2b..72d7b9877 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -1,9 +1,8 @@ -use evm::context::{ - ExecutionContext, ExecutionContextTrait, BoxDynamicExecutionContextDestruct, CallContextTrait, -}; +use evm::context::{ExecutionContext, ExecutionContextTrait, CallContextTrait,}; +use evm::machine::{Machine, MachineCurrentContext}; use evm::stack::StackTrait; use evm::memory::MemoryTrait; -use evm::tests::test_utils::setup_execution_context; +use evm::tests::test_utils::setup_machine; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR, TYPE_CONVERSION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; @@ -13,21 +12,21 @@ use utils::helpers::u256_to_bytes_array; #[available_gas(20000000)] fn test_exec_log0() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x1F); - ctx.stack.push(0x00); + machine.stack.push(0x1F); + machine.stack.push(0x00); // When - let result = ctx.exec_log0(); + let result = machine.exec_log0(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -42,22 +41,22 @@ fn test_exec_log0() { #[available_gas(20000000)] fn test_exec_log1() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x20); - ctx.stack.push(0x00); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x20); + machine.stack.push(0x00); // When - let result = ctx.exec_log1(); + let result = machine.exec_log1(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -73,23 +72,23 @@ fn test_exec_log1() { #[available_gas(20000000)] fn test_exec_log2() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x05); - ctx.stack.push(0x05); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x05); + machine.stack.push(0x05); // When - let result = ctx.exec_log2(); + let result = machine.exec_log2(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -106,25 +105,25 @@ fn test_exec_log2() { #[available_gas(20000000)] fn test_exec_log3() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); - ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); + machine.memory.store(BoundedInt::::max(), 0); + machine.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x28); - ctx.stack.push(0x00); + machine.stack.push(0x00); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x28); + machine.stack.push(0x00); // When - let result = ctx.exec_log3(); + let result = machine.exec_log3(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -144,26 +143,26 @@ fn test_exec_log3() { #[available_gas(20000000)] fn test_exec_log4() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); - ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); + machine.memory.store(BoundedInt::::max(), 0); + machine.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x0A); - ctx.stack.push(0x20); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x00); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x0A); + machine.stack.push(0x20); // When - let result = ctx.exec_log4(); + let result = machine.exec_log4(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -182,17 +181,17 @@ fn test_exec_log4() { #[available_gas(20000000)] fn test_exec_log1_read_only_context() { // Given - let mut ctx = setup_execution_context(); - ctx.set_read_only(true); + let mut machine = setup_machine(); + machine.set_read_only(true); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x20); - ctx.stack.push(0x00); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x20); + machine.stack.push(0x00); // When - let result = ctx.exec_log1(); + let result = machine.exec_log1(); // Then assert(result.is_err(), 'should have returned an error'); @@ -206,22 +205,22 @@ fn test_exec_log1_read_only_context() { #[available_gas(20000000)] fn test_exec_log1_size_0_offset_0() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x00); - ctx.stack.push(0x00); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x00); + machine.stack.push(0x00); // When - let result = ctx.exec_log1(); + let result = machine.exec_log1(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack should be empty'); + assert(machine.stack.len() == 0, 'stack should be empty'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 1, 'context should have one event'); let event = events.pop_front().unwrap(); @@ -235,16 +234,16 @@ fn test_exec_log1_size_0_offset_0() { #[available_gas(20000000)] fn test_exec_log1_size_too_big() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x00); // When - let result = ctx.exec_log1(); + let result = machine.exec_log1(); // Then assert(result.is_err(), 'should return an error'); @@ -258,16 +257,16 @@ fn test_exec_log1_size_too_big() { #[available_gas(20000000)] fn test_exec_log1_offset_too_big() { // Given - let mut ctx = setup_execution_context(); + let mut machine = setup_machine(); - ctx.memory.store(BoundedInt::::max(), 0); + machine.memory.store(BoundedInt::::max(), 0); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x20); - ctx.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x20); + machine.stack.push(BoundedInt::::max()); // When - let result = ctx.exec_log1(); + let result = machine.exec_log1(); // Then assert(result.is_err(), 'should return an error'); @@ -281,32 +280,32 @@ fn test_exec_log1_offset_too_big() { #[available_gas(20000000)] fn test_exec_log_multiple_events() { // Given - let mut ctx = setup_execution_context(); - - ctx.memory.store(BoundedInt::::max(), 0); - ctx.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); - - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x0A); - ctx.stack.push(0x20); - ctx.stack.push(0x00); - ctx.stack.push(BoundedInt::::max()); - ctx.stack.push(0x0123456789ABCDEF); - ctx.stack.push(0x28); - ctx.stack.push(0x00); + let mut machine = setup_machine(); + + machine.memory.store(BoundedInt::::max(), 0); + machine.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20); + + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x00); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x0A); + machine.stack.push(0x20); + machine.stack.push(0x00); + machine.stack.push(BoundedInt::::max()); + machine.stack.push(0x0123456789ABCDEF); + machine.stack.push(0x28); + machine.stack.push(0x00); // When - let result = ctx.exec_log3(); - let result = ctx.exec_log4(); + let result = machine.exec_log3(); + let result = machine.exec_log4(); // Then assert(result.is_ok(), 'should have succeeded'); - assert(ctx.stack.len() == 0, 'stack size should be 0'); + assert(machine.stack.len() == 0, 'stack size should be 0'); - let mut events = ctx.events(); + let mut events = machine.events(); assert(events.len() == 2, 'context should have 2 events'); let event1 = events.pop_front().unwrap(); From 8be21f61a2d6a2b1215884719fc583b56269e9bd Mon Sep 17 00:00:00 2001 From: Quentash Date: Tue, 3 Oct 2023 11:13:26 +0200 Subject: [PATCH 25/29] cleaned imports --- crates/evm/src/instructions/logging_operations.cairo | 7 +------ .../tests/test_instructions/test_logging_operations.cairo | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index ff4791bc1..d410c430c 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -1,8 +1,7 @@ //! Logging Operations. // Internal imports -use evm::machine::{Machine, MachineCurrentContext}; -use evm::context::ExecutionContext; +use evm::machine::Machine; use evm::errors::EVMError; #[generate_trait] @@ -44,15 +43,11 @@ impl LoggingOperations of LoggingOperationsTrait { } mod internal { - use evm::context::{ExecutionContext, ExecutionContextTrait}; use evm::machine::{Machine, MachineCurrentContext}; use evm::stack::StackTrait; use evm::memory::MemoryTrait; use evm::model::Event; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; - use evm::helpers::U256IntoResultU32; - use utils::helpers::u256_to_bytes_array; - use utils::helpers::ArrayExtensionTrait; /// Store a new event in the dynamic context using topics /// popped from the stack and data from the memory. diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 72d7b9877..b536d723c 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -1,8 +1,7 @@ -use evm::context::{ExecutionContext, ExecutionContextTrait, CallContextTrait,}; use evm::machine::{Machine, MachineCurrentContext}; +use evm::tests::test_utils::setup_machine; use evm::stack::StackTrait; use evm::memory::MemoryTrait; -use evm::tests::test_utils::setup_machine; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR, TYPE_CONVERSION_ERROR}; use evm::instructions::LoggingOperationsTrait; use integer::BoundedInt; From 2d7917c962cb2bb783c8be05ab5ddfb423b7f578 Mon Sep 17 00:00:00 2001 From: Quentash Date: Tue, 3 Oct 2023 11:32:11 +0200 Subject: [PATCH 26/29] scarb fmt did that --- crates/evm/src/instructions.cairo | 2 +- crates/evm/src/instructions/logging_operations.cairo | 8 ++++---- crates/evm/src/instructions/memory_operations.cairo | 4 +--- crates/evm/src/tests/test_instructions.cairo | 2 +- .../tests/test_instructions/test_logging_operations.cairo | 8 ++++---- crates/utils/src/helpers.cairo | 4 +--- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/crates/evm/src/instructions.cairo b/crates/evm/src/instructions.cairo index 9e906f9c9..a6b0192f9 100644 --- a/crates/evm/src/instructions.cairo +++ b/crates/evm/src/instructions.cairo @@ -6,7 +6,6 @@ mod duplication_operations; mod environmental_information; mod exchange_operations; mod logging_operations; -use logging_operations::LoggingOperationsTrait; mod memory_operations; mod push_operations; mod sha3; @@ -18,6 +17,7 @@ use comparison_operations::ComparisonAndBitwiseOperationsTrait; use duplication_operations::DuplicationOperationsTrait; use environmental_information::EnvironmentInformationTrait; use exchange_operations::ExchangeOperationsTrait; +use logging_operations::LoggingOperationsTrait; use memory_operations::MemoryOperationTrait; use push_operations::PushOperationsTrait; use sha3::Sha3Trait; diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index d410c430c..d793c0dad 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -1,8 +1,8 @@ +use evm::errors::EVMError; //! Logging Operations. // Internal imports use evm::machine::Machine; -use evm::errors::EVMError; #[generate_trait] impl LoggingOperations of LoggingOperationsTrait { @@ -43,11 +43,11 @@ impl LoggingOperations of LoggingOperationsTrait { } mod internal { - use evm::machine::{Machine, MachineCurrentContext}; - use evm::stack::StackTrait; + use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; + use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; use evm::model::Event; - use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; + use evm::stack::StackTrait; /// Store a new event in the dynamic context using topics /// popped from the stack and data from the memory. diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 6477f0f69..c48493a79 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -70,9 +70,7 @@ impl MemoryOperation of MemoryOperationTrait { return Result::Err(EVMError::JumpError(INVALID_DESTINATION)); } }, - Option::None => { - return Result::Err(EVMError::JumpError(INVALID_DESTINATION)); - } + Option::None => { return Result::Err(EVMError::JumpError(INVALID_DESTINATION)); } } self.set_pc(index); Result::Ok(()) diff --git a/crates/evm/src/tests/test_instructions.cairo b/crates/evm/src/tests/test_instructions.cairo index 4591b04e4..e4ef87860 100644 --- a/crates/evm/src/tests/test_instructions.cairo +++ b/crates/evm/src/tests/test_instructions.cairo @@ -3,9 +3,9 @@ mod test_comparison_operations; mod test_duplication_operations; mod test_environment_information; mod test_exchange_operations; +mod test_logging_operations; mod test_memory_operations; mod test_push_operations; mod test_sha3; mod test_stop_and_arithmetic_operations; mod test_system_operations; -mod test_logging_operations; diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index b536d723c..4aa5a97c2 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -1,9 +1,9 @@ -use evm::machine::{Machine, MachineCurrentContext}; -use evm::tests::test_utils::setup_machine; -use evm::stack::StackTrait; -use evm::memory::MemoryTrait; use evm::errors::{EVMError, STATE_MODIFICATION_ERROR, TYPE_CONVERSION_ERROR}; use evm::instructions::LoggingOperationsTrait; +use evm::machine::{Machine, MachineCurrentContextTrait}; +use evm::memory::MemoryTrait; +use evm::stack::StackTrait; +use evm::tests::test_utils::setup_machine; use integer::BoundedInt; use utils::helpers::u256_to_bytes_array; diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index 406a8194a..09468e64e 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -448,9 +448,7 @@ impl ArrayExtension, impl TDrop: Drop> of ArrayExtensi loop { match arr2.pop_front() { Option::Some(elem) => self.append(*elem), - Option::None => { - break; - } + Option::None => { break; } }; } } From bffc21ecc4d1adbeea6aadc7d59820bc1aa59772 Mon Sep 17 00:00:00 2001 From: Quentash Date: Tue, 3 Oct 2023 13:34:20 +0200 Subject: [PATCH 27/29] dupl app_event + setup_readonly --- crates/evm/src/context.cairo | 5 +++ crates/evm/src/machine.cairo | 11 +---- .../test_logging_operations.cairo | 5 +-- crates/evm/src/tests/test_utils.cairo | 40 +++++++++++++++++++ 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/crates/evm/src/context.cairo b/crates/evm/src/context.cairo index b9b442c29..29165432a 100644 --- a/crates/evm/src/context.cairo +++ b/crates/evm/src/context.cairo @@ -314,4 +314,9 @@ impl ExecutionContextImpl of ExecutionContextTrait { fn pc(self: @ExecutionContext) -> u32 { *self.program_counter } + + #[inline(always)] + fn append_event(ref self: ExecutionContext, event: Event) { + self.events.append(event); + } } diff --git a/crates/evm/src/machine.cairo b/crates/evm/src/machine.cairo index fbc08f9ba..3c2008a72 100644 --- a/crates/evm/src/machine.cairo +++ b/crates/evm/src/machine.cairo @@ -189,19 +189,10 @@ impl MachineCurrentContextImpl of MachineCurrentContextTrait { current_call_ctx.read_only() } - #[inline(always)] - fn set_read_only(ref self: Machine, value: bool) { - let mut current_call_ctx = self.call_context(); - let mut current_execution_ctx = self.current_context.unbox(); - current_call_ctx.read_only = value; - current_execution_ctx.call_context = BoxTrait::new(current_call_ctx); - self.current_context = BoxTrait::new(current_execution_ctx); - } - #[inline(always)] fn append_event(ref self: Machine, event: Event) { let mut current_execution_ctx = self.current_context.unbox(); - current_execution_ctx.events.append(event); + current_execution_ctx.append_event(event); self.current_context = BoxTrait::new(current_execution_ctx); } diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index 4aa5a97c2..d99ef5843 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -3,7 +3,7 @@ use evm::instructions::LoggingOperationsTrait; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; use evm::stack::StackTrait; -use evm::tests::test_utils::setup_machine; +use evm::tests::test_utils::{setup_machine, setup_machine_with_read_only}; use integer::BoundedInt; use utils::helpers::u256_to_bytes_array; @@ -180,8 +180,7 @@ fn test_exec_log4() { #[available_gas(20000000)] fn test_exec_log1_read_only_context() { // Given - let mut machine = setup_machine(); - machine.set_read_only(true); + let mut machine = setup_machine_with_read_only(); machine.memory.store(BoundedInt::::max(), 0); diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index c79b432c3..c6116db92 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -148,3 +148,43 @@ fn setup_machine_with_calldata(calldata: Span) -> Machine { storage_journal: Default::default(), } } + +fn setup_machine_with_read_only() -> Machine { + Machine { + current_context: BoxTrait::new(setup_execution_context_with_read_only()), + ctx_count: 1, + stack: Default::default(), + memory: Default::default(), + storage_journal: Default::default(), + } +} + +fn setup_execution_context_with_read_only() -> ExecutionContext { + let context_id = 0; + let call_context = setup_call_context_with_read_only(); + let starknet_address: ContractAddress = starknet_address(); + let evm_address: EthAddress = evm_address(); + let return_data = Default::default(); + + ExecutionContextTrait::new( + context_id, + evm_address, + starknet_address, + call_context, + Default::default(), + Default::default(), + return_data, + ) +} + +fn setup_call_context_with_read_only() -> CallContext { + let bytecode: Span = array![1, 2, 3].span(); + let calldata: Span = array![4, 5, 6].span(); + let value: u256 = callvalue(); + let address = evm_address(); + let read_only = true; + let gas_price = 0xaaaaaa; + let gas_limit = 0xffffff; + + CallContextTrait::new(address, bytecode, calldata, value, read_only, gas_limit, gas_price) +} From 54cc27e2cbf105fb2d5cd91fa1d17636641c588d Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 4 Oct 2023 12:07:23 +0200 Subject: [PATCH 28/29] change error message + setup with readonly --- crates/evm/src/errors.cairo | 6 +-- .../src/instructions/logging_operations.cairo | 4 +- .../test_logging_operations.cairo | 6 +-- crates/evm/src/tests/test_utils.cairo | 44 +++---------------- 4 files changed, 15 insertions(+), 45 deletions(-) diff --git a/crates/evm/src/errors.cairo b/crates/evm/src/errors.cairo index f15122a38..00d13775e 100644 --- a/crates/evm/src/errors.cairo +++ b/crates/evm/src/errors.cairo @@ -15,7 +15,7 @@ const RETURNDATA_OUT_OF_BOUNDS_ERROR: felt252 = 'KKT: ReturnDataOutOfBounds'; const INVALID_DESTINATION: felt252 = 'KKT: invalid JUMP destination'; // EVM STATE -const STATE_MODIFICATION_ERROR: felt252 = 'KKT: StateModificationError'; +const WRITE_IN_STATIC_CONTEXT: felt252 = 'KKT: WriteInStaticContext'; #[derive(Drop, Copy, PartialEq)] enum EVMError { @@ -26,7 +26,7 @@ enum EVMError { JumpError: felt252, NotImplemented, UnknownOpcode: u8, - StateModificationError: felt252 + WriteInStaticContext: felt252 } @@ -41,7 +41,7 @@ impl EVMErrorIntoU256 of Into { EVMError::NotImplemented => 'NotImplemented'.into(), // TODO: refactor with dynamic strings once supported EVMError::UnknownOpcode => 'UnknownOpcode'.into(), - EVMError::StateModificationError(error_message) => error_message.into(), + EVMError::WriteInStaticContext(error_message) => error_message.into(), } } } diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index d793c0dad..700b5eaca 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -43,7 +43,7 @@ impl LoggingOperations of LoggingOperationsTrait { } mod internal { - use evm::errors::{EVMError, STATE_MODIFICATION_ERROR}; + use evm::errors::{EVMError, WRITE_IN_STATIC_CONTEXT}; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; use evm::model::Event; @@ -59,7 +59,7 @@ mod internal { fn exec_log_i(ref self: Machine, topics_len: u8) -> Result<(), EVMError> { // Revert if the transaction is in a read only context if self.read_only() { - return Result::Err(EVMError::StateModificationError(STATE_MODIFICATION_ERROR)); + return Result::Err(EVMError::WriteInStaticContext(WRITE_IN_STATIC_CONTEXT)); } let offset = self.stack.pop_usize()?; diff --git a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo index d99ef5843..14a78d6dd 100644 --- a/crates/evm/src/tests/test_instructions/test_logging_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_logging_operations.cairo @@ -1,4 +1,4 @@ -use evm::errors::{EVMError, STATE_MODIFICATION_ERROR, TYPE_CONVERSION_ERROR}; +use evm::errors::{EVMError, WRITE_IN_STATIC_CONTEXT, TYPE_CONVERSION_ERROR}; use evm::instructions::LoggingOperationsTrait; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; @@ -194,8 +194,8 @@ fn test_exec_log1_read_only_context() { // Then assert(result.is_err(), 'should have returned an error'); assert( - result.unwrap_err() == EVMError::StateModificationError(STATE_MODIFICATION_ERROR), - 'err != StateModificationError' + result.unwrap_err() == EVMError::WriteInStaticContext(WRITE_IN_STATIC_CONTEXT), + 'err != WriteInStaticContext' ); } diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index c6116db92..157b325c9 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -150,41 +150,11 @@ fn setup_machine_with_calldata(calldata: Span) -> Machine { } fn setup_machine_with_read_only() -> Machine { - Machine { - current_context: BoxTrait::new(setup_execution_context_with_read_only()), - ctx_count: 1, - stack: Default::default(), - memory: Default::default(), - storage_journal: Default::default(), - } -} - -fn setup_execution_context_with_read_only() -> ExecutionContext { - let context_id = 0; - let call_context = setup_call_context_with_read_only(); - let starknet_address: ContractAddress = starknet_address(); - let evm_address: EthAddress = evm_address(); - let return_data = Default::default(); - - ExecutionContextTrait::new( - context_id, - evm_address, - starknet_address, - call_context, - Default::default(), - Default::default(), - return_data, - ) -} - -fn setup_call_context_with_read_only() -> CallContext { - let bytecode: Span = array![1, 2, 3].span(); - let calldata: Span = array![4, 5, 6].span(); - let value: u256 = callvalue(); - let address = evm_address(); - let read_only = true; - let gas_price = 0xaaaaaa; - let gas_limit = 0xffffff; - - CallContextTrait::new(address, bytecode, calldata, value, read_only, gas_limit, gas_price) + let mut machine = setup_machine(); + let mut current_ctx = machine.current_context.unbox(); + let mut current_call_ctx = current_ctx.call_context.unbox(); + current_call_ctx.read_only = true; + current_ctx.call_context = BoxTrait::new(current_call_ctx); + machine.current_context = BoxTrait::new(current_ctx); + machine } From e2844c06ad78985b40e44cc0a2776da194cb4921 Mon Sep 17 00:00:00 2001 From: Quentash Date: Wed, 4 Oct 2023 12:26:34 +0200 Subject: [PATCH 29/29] correction rebase --- crates/evm/src/machine.cairo | 4 ++-- crates/evm/src/tests/test_utils.cairo | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/evm/src/machine.cairo b/crates/evm/src/machine.cairo index 7efbd13b6..01fb23cbb 100644 --- a/crates/evm/src/machine.cairo +++ b/crates/evm/src/machine.cairo @@ -191,9 +191,9 @@ impl MachineCurrentContextImpl of MachineCurrentContextTrait { #[inline(always)] fn append_event(ref self: Machine, event: Event) { - let mut current_execution_ctx = self.current_context.unbox(); + let mut current_execution_ctx = self.current_ctx.unbox(); current_execution_ctx.append_event(event); - self.current_context = BoxTrait::new(current_execution_ctx); + self.current_ctx = BoxTrait::new(current_execution_ctx); } #[inline(always)] diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index c125d4978..1bc97d047 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -154,10 +154,10 @@ fn setup_machine_with_calldata(calldata: Span) -> Machine { fn setup_machine_with_read_only() -> Machine { let mut machine = setup_machine(); - let mut current_ctx = machine.current_context.unbox(); - let mut current_call_ctx = current_ctx.call_context.unbox(); + let mut current_ctx = machine.current_ctx.unbox(); + let mut current_call_ctx = current_ctx.call_ctx.unbox(); current_call_ctx.read_only = true; - current_ctx.call_context = BoxTrait::new(current_call_ctx); - machine.current_context = BoxTrait::new(current_ctx); + current_ctx.call_ctx = BoxTrait::new(current_call_ctx); + machine.current_ctx = BoxTrait::new(current_ctx); machine }