diff --git a/crates/evm/src/stack.cairo b/crates/evm/src/stack.cairo index 3e6ba15c0..67dbfa925 100644 --- a/crates/evm/src/stack.cairo +++ b/crates/evm/src/stack.cairo @@ -24,14 +24,17 @@ use evm::helpers::U256IntoResultU32; use starknet::EthAddress; use utils::i256::i256; + #[derive(Destruct, Default)] struct Stack { + active_segment: usize, items: Felt252Dict>, - len: usize, + len: Felt252Dict, } trait StackTrait { fn new() -> Stack; + fn set_active_segment(ref self: Stack, active_segment: usize); fn push(ref self: Stack, item: u256) -> Result<(), EVMError>; fn pop(ref self: Stack) -> Result; fn pop_usize(ref self: Stack) -> Result; @@ -41,8 +44,10 @@ trait StackTrait { fn peek(ref self: Stack) -> Option; fn peek_at(ref self: Stack, index: usize) -> Result; fn swap_i(ref self: Stack, index: usize) -> Result<(), EVMError>; - fn len(self: @Stack) -> usize; - fn is_empty(self: @Stack) -> bool; + fn len(ref self: Stack) -> usize; + fn is_empty(ref self: Stack) -> bool; + fn active_segment(self: @Stack) -> usize; + fn compute_active_segment_index(self: @Stack, index: usize) -> felt252; } impl StackImpl of StackTrait { @@ -51,19 +56,32 @@ impl StackImpl of StackTrait { Default::default() } + #[inline(always)] + fn set_active_segment(ref self: Stack, active_segment: usize) { + self.active_segment = active_segment; + } + /// Pushes a new bytes32 word onto the stack. /// + /// When pushing an item to the stack, we will compute + /// an index which corresponds to the index in the dict the item will be stored at. + /// The internal index is computed as follows: + /// + /// index = len(Stack_i) + i * STACK_SEGMENT_SIZE + /// /// # Errors /// /// If the stack is full, returns with a StackOverflow error. #[inline(always)] fn push(ref self: Stack, item: u256) -> Result<(), EVMError> { + let length = self.len(); // we can store at most 1024 256-bits words - if self.len() == constants::STACK_MAX_DEPTH { + if length == constants::STACK_MAX_DEPTH { return Result::Err(EVMError::StackError(STACK_OVERFLOW)); } - self.items.insert(self.len.into(), NullableTrait::new(item)); - self.len += 1; + let index = self.compute_active_segment_index(length); + self.items.insert(index, NullableTrait::new(item)); + self.len.insert(self.active_segment().into(), length + 1); Result::Ok(()) } @@ -74,12 +92,13 @@ impl StackImpl of StackTrait { /// If the stack is empty, returns with a StackOverflow error. #[inline(always)] fn pop(ref self: Stack) -> Result { - if self.len() == 0 { + let length = self.len(); + if length == 0 { return Result::Err(EVMError::StackError(STACK_UNDERFLOW)); } - let last_index = self.len() - 1; - self.len -= 1; - let item = self.items.get(last_index.into()); + self.len.insert(self.active_segment().into(), length - 1); + let internal_index = self.compute_active_segment_index(self.len()); + let item = self.items.get(internal_index); Result::Ok(item.deref()) } @@ -156,7 +175,7 @@ impl StackImpl of StackTrait { if self.len() == 0 { Option::None(()) } else { - let last_index = self.len() - 1; + let last_index = self.compute_active_segment_index(self.len() - 1); let item = self.items.get(last_index.into()); Option::Some(item.deref()) } @@ -174,7 +193,7 @@ impl StackImpl of StackTrait { return Result::Err(EVMError::StackError(STACK_UNDERFLOW)); } - let position = self.len() - 1 - index; + let position = self.compute_active_segment_index(self.len() - 1 - index); let item = self.items.get(position.into()); Result::Ok(item.deref()) @@ -187,7 +206,7 @@ impl StackImpl of StackTrait { if index >= self.len() { return Result::Err(EVMError::StackError(STACK_UNDERFLOW)); } - let position_0: felt252 = (self.len() - 1).into(); + let position_0: felt252 = self.compute_active_segment_index(self.len() - 1); let position_item: felt252 = position_0 - index.into(); let top_item = self.items.get(position_0); let swapped_item = self.items.get(position_item); @@ -198,13 +217,26 @@ impl StackImpl of StackTrait { /// Returns the length of the stack. #[inline(always)] - fn len(self: @Stack) -> usize { - *self.len + fn len(ref self: Stack) -> usize { + self.len.get(self.active_segment.into()) } /// Returns true if the stack is empty. #[inline(always)] - fn is_empty(self: @Stack) -> bool { - *self.len == 0 + fn is_empty(ref self: Stack) -> bool { + self.len() == 0 + } + + /// Returns the current active segment, i.e. the current active execution context's id + #[inline(always)] + fn active_segment(self: @Stack) -> usize { + *self.active_segment + } + + /// Computes the internal index to access the Stack of the current execution context + #[inline(always)] + fn compute_active_segment_index(self: @Stack, index: usize) -> felt252 { + let internal_index = index + self.active_segment() * constants::STACK_MAX_DEPTH; + internal_index.into() } } diff --git a/crates/evm/src/tests/test_instructions/test_comparison_operations.cairo b/crates/evm/src/tests/test_instructions/test_comparison_operations.cairo index ded46127f..86cecdec6 100644 --- a/crates/evm/src/tests/test_instructions/test_comparison_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_comparison_operations.cairo @@ -346,7 +346,7 @@ fn test_exec_gt_false_equal() { } #[test] -#[available_gas(22000000)] +#[available_gas(220000000)] fn test_exec_slt() { // https://github.com/ethereum/go-ethereum/blob/master/core/vm/testdata/testcases_slt.json assert_slt(0x0, 0x0, 0); @@ -591,7 +591,7 @@ fn assert_slt(b: u256, a: u256, expected: u256) { } #[test] -#[available_gas(22000000)] +#[available_gas(220000000)] fn test_exec_sgt() { // https://github.com/ethereum/go-ethereum/blob/master/core/vm/testdata/testcases_sgt.json assert_sgt(0x0, 0x0, 0); diff --git a/crates/evm/src/tests/test_instructions/test_stop_and_arithmetic_operations.cairo b/crates/evm/src/tests/test_instructions/test_stop_and_arithmetic_operations.cairo index 2a295fc1e..35c0b16fa 100644 --- a/crates/evm/src/tests/test_instructions/test_stop_and_arithmetic_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_stop_and_arithmetic_operations.cairo @@ -33,7 +33,7 @@ fn test_exec_add() { ctx.exec_add(); // Then - assert(ctx.stack.len == 2, 'stack should have two elems'); + assert(ctx.stack.len() == 2, 'stack should have two elems'); assert(ctx.stack.peek().unwrap() == 5, 'stack top should be 3+2'); assert(ctx.stack.peek_at(1).unwrap() == 1, 'stack[1] should be 1'); } diff --git a/crates/evm/src/tests/test_stack.cairo b/crates/evm/src/tests/test_stack.cairo index 35b72fe7e..1786dbcf6 100644 --- a/crates/evm/src/tests/test_stack.cairo +++ b/crates/evm/src/tests/test_stack.cairo @@ -5,17 +5,17 @@ use evm::stack::StackTrait; use utils::constants; #[test] -#[available_gas(12000)] +#[available_gas(120000)] fn test_stack_new_should_return_empty_stack() { // When let mut stack = StackTrait::new(); // Then - assert(stack.len == 0, 'stack length should be 0'); + assert(stack.len() == 0, 'stack length should be 0'); } #[test] -#[available_gas(40000)] +#[available_gas(400000)] fn test__empty__should_return_if_stack_is_empty() { // Given let mut stack = StackTrait::new(); @@ -30,7 +30,7 @@ fn test__empty__should_return_if_stack_is_empty() { } #[test] -#[available_gas(35000)] +#[available_gas(350000)] fn test__len__should_return_the_length_of_the_stack() { // Given let mut stack = StackTrait::new(); @@ -53,7 +53,7 @@ mod push { use evm::errors::{EVMError, STACK_OVERFLOW}; #[test] - #[available_gas(60000)] + #[available_gas(600000)] fn test_should_add_an_element_to_the_stack() { // Given let mut stack = StackTrait::new(); @@ -70,7 +70,25 @@ mod push { } #[test] - #[available_gas(30000000)] + #[available_gas(600000)] + fn test_should_add_an_element_to_the_stack_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xc1e3); + + // When + stack.push(1).unwrap(); + + // Then + let res = stack.peek().unwrap(); + + assert(stack.is_empty() == false, 'stack should not be empty'); + assert(stack.active_segment() == 0xc1e3, 'wrong stack active segment'); + assert(stack.len() == 1, 'len should be 1'); + assert(res == 1, 'wrong result'); + } + #[test] + #[available_gas(300000000)] fn test_should_fail_when_overflow() { // Given let mut stack = StackTrait::new(); @@ -86,6 +104,32 @@ mod push { stack.push(1).unwrap(); }; + // Then + let res = stack.push(1); + assert(stack.len() == constants::STACK_MAX_DEPTH, 'wrong length'); + assert(res.is_err(), 'should return error'); + assert( + res.unwrap_err() == EVMError::StackError(STACK_OVERFLOW), 'should return StackOverflow' + ); + } + #[test] + #[available_gas(300000000)] + fn test_should_fail_when_overflow_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xda3); + let mut i = 0; + + // When + loop { + if i == constants::STACK_MAX_DEPTH { + break; + } + i += 1; + + stack.push(1).unwrap(); + }; + // Then let res = stack.push(1); assert(stack.len() == constants::STACK_MAX_DEPTH, 'wrong length'); @@ -117,11 +161,30 @@ mod pop { // Then assert(last_item == 3, 'wrong result'); - assert(stack.len == 2, 'wrong length'); + assert(stack.len() == 2, 'wrong length'); } #[test] - #[available_gas(250000)] + #[available_gas(950000)] + fn test_should_pop_an_element_from_the_stack_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xe11a5); + stack.push(1).unwrap(); + stack.push(2).unwrap(); + stack.push(3).unwrap(); + + // When + let last_item = stack.pop().unwrap(); + + // Then + assert(stack.active_segment() == 0xe11a5, 'wrong active segment'); + assert(last_item == 3, 'wrong result'); + assert(stack.len() == 2, 'wrong length'); + } + + #[test] + #[available_gas(2500000)] fn test_should_pop_N_elements_from_the_stack() { // Given let mut stack = StackTrait::new(); @@ -133,7 +196,29 @@ mod pop { let elements = stack.pop_n(3).unwrap(); // Then - assert(stack.len == 0, 'wrong length'); + assert(stack.len() == 0, 'wrong length'); + assert(elements.len() == 3, 'wrong returned array length'); + assert(*elements[0] == 3, 'wrong result at index 0'); + assert(*elements[1] == 2, 'wrong result at index 1'); + assert(*elements[2] == 1, 'wrong result at index 2'); + } + + #[test] + #[available_gas(2500000)] + fn test_should_pop_N_elements_from_the_stack_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xabde1); + stack.push(1).unwrap(); + stack.push(2).unwrap(); + stack.push(3).unwrap(); + + // When + let elements = stack.pop_n(3).unwrap(); + + // Then + assert(stack.active_segment() == 0xabde1, 'wrong active segment'); + assert(stack.len() == 0, 'wrong length'); assert(elements.len() == 3, 'wrong returned array length'); assert(*elements[0] == 3, 'wrong result at index 0'); assert(*elements[1] == 2, 'wrong result at index 1'); @@ -141,7 +226,7 @@ mod pop { } #[test] - #[available_gas(55000)] + #[available_gas(550000)] fn test_pop_return_err_when_stack_underflow() { // Given let mut stack = StackTrait::new(); @@ -156,7 +241,23 @@ mod pop { } #[test] - #[available_gas(55000)] + #[available_gas(550000)] + fn test_pop_return_err_when_stack_underflow_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xabde1); + + // When & Then + let result = stack.pop(); + assert(result.is_err(), 'should return Err '); + assert( + result.unwrap_err() == EVMError::StackError(STACK_UNDERFLOW), + 'should return StackUnderflow' + ); + } + + #[test] + #[available_gas(550000)] fn test_pop_n_should_return_err_when_stack_underflow() { // Given let mut stack = StackTrait::new(); @@ -180,7 +281,7 @@ mod peek { use evm::errors::{EVMError, STACK_UNDERFLOW}; #[test] - #[available_gas(80000)] + #[available_gas(800000)] fn test_should_return_last_item() { // Given let mut stack = StackTrait::new(); @@ -194,6 +295,24 @@ mod peek { assert(last_item == 2, 'wrong result'); } + + #[test] + #[available_gas(800000)] + fn test_should_return_last_item_with_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xb00b); + + // When + stack.push(1).unwrap(); + stack.push(2).unwrap(); + + // Then + let last_item = stack.peek().unwrap(); + assert(stack.active_segment() == 0xb00b, 'wrong active segment'); + assert(last_item == 2, 'wrong result'); + } + #[test] #[available_gas(10000000)] fn test_should_return_stack_at_given_index_when_value_is_0() { @@ -210,6 +329,25 @@ mod peek { assert(result == 3, 'wrong result'); } + #[test] + #[available_gas(10000000)] + fn test_should_return_stack_at_given_index_when_value_is_0_with_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xdead); + + stack.push(1).unwrap(); + stack.push(2).unwrap(); + stack.push(3).unwrap(); + + // When + let result = stack.peek_at(0).unwrap(); + + // Then + assert(result == 3, 'wrong result'); + assert(stack.active_segment() == 0xdead, 'wrong active segment'); + } + #[test] #[available_gas(10000000)] fn test_should_return_stack_at_given_index_when_value_is_1() { @@ -241,6 +379,24 @@ mod peek { 'should return StackUnderflow' ); } + + + #[test] + #[available_gas(350000)] + fn test_should_return_err_when_underflow_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0x666); + + // When & Then + let result = stack.peek_at(1); + + assert(result.is_err(), 'should return an EVMError'); + assert( + result.unwrap_err() == EVMError::StackError(STACK_UNDERFLOW), + 'should return StackUnderflow' + ); + } } #[cfg(test)] @@ -250,7 +406,7 @@ mod swap { use evm::errors::{EVMError, STACK_UNDERFLOW}; #[test] - #[available_gas(400000)] + #[available_gas(4000000)] fn test_should_swap_2_stack_items() { // Given let mut stack = StackTrait::new(); @@ -282,7 +438,42 @@ mod swap { } #[test] - #[available_gas(50000)] + #[available_gas(4000000)] + fn test_should_swap_2_stack_items_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0x999); + + stack.push(1).unwrap(); + stack.push(2).unwrap(); + stack.push(3).unwrap(); + stack.push(4).unwrap(); + let index3 = stack.peek_at(3).unwrap(); + assert(index3 == 1, 'wrong index3'); + let index2 = stack.peek_at(2).unwrap(); + assert(index2 == 2, 'wrong index2'); + let index1 = stack.peek_at(1).unwrap(); + assert(index1 == 3, 'wrong index1'); + let index0 = stack.peek_at(0).unwrap(); + assert(index0 == 4, 'wrong index0'); + + // When + stack.swap_i(2); + + // Then + assert(stack.active_segment() == 0x999, 'wrong active segment'); + let index3 = stack.peek_at(3).unwrap(); + assert(index3 == 1, 'post-swap: wrong index3'); + let index2 = stack.peek_at(2).unwrap(); + assert(index2 == 4, 'post-swap: wrong index2'); + let index1 = stack.peek_at(1).unwrap(); + assert(index1 == 3, 'post-swap: wrong index1'); + let index0 = stack.peek_at(0).unwrap(); + assert(index0 == 2, 'post-swap: wront index0'); + } + + #[test] + #[available_gas(500000)] fn test_should_return_err_when_index_1_is_underflow() { // Given let mut stack = StackTrait::new(); @@ -298,7 +489,24 @@ mod swap { } #[test] - #[available_gas(600000)] + #[available_gas(500000)] + fn test_should_return_err_when_index_1_is_underflow_with_active_segment() { + // Given + let mut stack = StackTrait::new(); + stack.set_active_segment(0xabde1); + + // When & Then + let result = stack.swap_i(1); + + assert(result.is_err(), 'should return an EVMError'); + assert( + result.unwrap_err() == EVMError::StackError(STACK_UNDERFLOW), + 'should return StackUnderflow' + ); + } + + #[test] + #[available_gas(6000000)] fn test_should_return_err_when_index_2_is_underflow() { // Given let mut stack = StackTrait::new(); diff --git a/scripts/compare_snapshot.py b/scripts/compare_snapshot.py index 7610a3526..cf70254ad 100644 --- a/scripts/compare_snapshot.py +++ b/scripts/compare_snapshot.py @@ -114,7 +114,9 @@ def compare_snapshots(current, previous): for key in common_keys: prev = previous[key] cur = current[key] - log = f"{key:<{max_key_len + 5}} {prev:>10} {cur:>10} {cur / prev:>6.2%}" + log = ( + f"|{key:<{max_key_len + 5}} | {prev:>10} | {cur:>10} | {cur / prev:>6.2%}|" + ) if prev < cur: worsened.append(log) elif prev > cur: @@ -142,12 +144,18 @@ def main(): current_snapshots = get_current_gas_snapshot() improvements, worsened = compare_snapshots(current_snapshots, previous_snapshot) cur_gas, prev_gas = total_gas_used(current_snapshots, previous_snapshot) + header = [ + "| Test | Prev | Cur | Ratio |", + "| ---- | ---- | --- | ----- |", + ] if improvements: - logger.info("\n".join(["****BETTER****"] + improvements)) + logger.info("\n".join(["****BETTER****"] + header + improvements)) if worsened: - logger.info("\n".join(["****WORST****"] + worsened)) + logger.info("\n".join(["****WORST****"] + header + worsened)) - logger.info(f"Overall gas change: {(cur_gas - prev_gas) * 100 / prev_gas:.2%}") + logger.info( + f"\nTotal gas change: {prev_gas} -> {cur_gas} ({cur_gas / prev_gas:.2%})" + ) if worsened: logger.error("Gas usage increased") else: