Skip to content

Commit

Permalink
fix: saturate gas taken from stack (#915)
Browse files Browse the repository at this point in the history 
* fix: saturate gas taken from stack
  • Loading branch information
@enitrat
enitrat authored Sep 6, 2024
1 parent 7cbfdae commit ad32fc0
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 41 deletions.
8 changes: 4 additions & 4 deletions crates/evm/src/instructions/system_operations.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl SystemOperations of SystemOperationsTrait {
/// CALL
/// # Specification: https://www.evm.codes/#f1?fork=shanghai
fn exec_call(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_u128()?;
let gas = self.stack.pop_saturating_u128()?;
let to = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_usize()?;
Expand Down Expand Up @@ -109,7 +109,7 @@ impl SystemOperations of SystemOperationsTrait {
/// CALLCODE
/// # Specification: https://www.evm.codes/#f2?fork=shanghai
fn exec_callcode(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_u128()?;
let gas = self.stack.pop_saturating_u128()?;
let code_address = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_usize()?;
Expand Down Expand Up @@ -194,7 +194,7 @@ impl SystemOperations of SystemOperationsTrait {
/// DELEGATECALL
/// # Specification: https://www.evm.codes/#f4?fork=shanghai
fn exec_delegatecall(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_u128()?;
let gas = self.stack.pop_saturating_u128()?;
let code_address = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?;
Expand Down Expand Up @@ -247,7 +247,7 @@ impl SystemOperations of SystemOperationsTrait {
/// STATICCALL
/// # Specification: https://www.evm.codes/#fa?fork=shanghai
fn exec_staticcall(ref self: VM) -> Result<(), EVMError> {
let gas = self.stack.pop_u128()?;
let gas = self.stack.pop_saturating_u128()?;
let to = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?;
Expand Down
111 changes: 74 additions & 37 deletions crates/evm/src/stack.cairo
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use core::fmt::{Debug, Formatter, Error, Display};
use core::nullable::{NullableTrait};
use core::num::traits::Bounded;
use core::starknet::{StorageBaseAddress, EthAddress};
//! Stack implementation.
//! # Example
Expand Down Expand Up @@ -37,6 +38,7 @@ trait StackTrait {
fn pop_usize(ref self: Stack) -> Result<usize, EVMError>;
fn pop_u64(ref self: Stack) -> Result<u64, EVMError>;
fn pop_u128(ref self: Stack) -> Result<u128, EVMError>;
fn pop_saturating_u128(ref self: Stack) -> Result<u128, EVMError>;
fn pop_i256(ref self: Stack) -> Result<i256, EVMError>;
fn pop_eth_address(ref self: Stack) -> Result<EthAddress, EVMError>;
fn pop_n(ref self: Stack, n: usize) -> Result<Array<u256>, EVMError>;
Expand Down Expand Up @@ -149,6 +151,17 @@ impl StackImpl of StackTrait {
Result::Ok(item)
}

/// Calls `Stack::pop` and saturates the result to u128
///
#[inline(always)]
fn pop_saturating_u128(ref self: Stack) -> Result<u128, EVMError> {
let item: u256 = self.pop()?;
if item.high != 0 {
return Result::Ok(Bounded::<u128>::MAX);
};
Result::Ok(item.low)
}

/// Calls `Stack::pop` and converts it to usize
///
/// # Errors
Expand Down Expand Up @@ -252,7 +265,7 @@ mod tests {
let mut stack = StackTrait::new();

// Then
assert(stack.len() == 0, 'stack length should be 0');
assert_eq!(stack.len(), 0);
}

#[test]
Expand All @@ -261,12 +274,12 @@ mod tests {
let mut stack = StackTrait::new();

// Then
assert(stack.is_empty() == true, 'stack should be empty');
assert!(stack.is_empty());

// When
stack.push(1).unwrap();
// Then
assert(stack.is_empty() == false, 'stack should not be empty');
assert!(!stack.is_empty());
}

#[test]
Expand All @@ -275,12 +288,12 @@ mod tests {
let mut stack = StackTrait::new();

// Then
assert(stack.len() == 0, 'stack length should be 0');
assert_eq!(stack.len(), 0);

// When
stack.push(1).unwrap();
// Then
assert(stack.len() == 1, 'stack length should be 1');
assert_eq!(stack.len(), 1);
}

mod push {
Expand All @@ -300,9 +313,9 @@ mod tests {
// Then
let res = stack.peek().unwrap();

assert(stack.is_empty() == false, 'stack should not be empty');
assert(stack.len() == 1, 'len should be 1');
assert(res == 1, 'wrong result');
assert_eq!(stack.is_empty(), false);
assert_eq!(stack.len(), 1);
assert_eq!(res, 1);
}

#[test]
Expand All @@ -319,13 +332,14 @@ mod tests {

// 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::StackOverflow, 'should return StackOverflow');
assert_eq!(stack.len(), constants::STACK_MAX_DEPTH);
assert!(res.is_err());
assert_eq!(res.unwrap_err(), EVMError::StackOverflow);
}
}

mod pop {
use core::num::traits::Bounded;
use core::starknet::storage_base_address_const;
use evm::errors::{EVMError, TYPE_CONVERSION_ERROR};
use super::StackTrait;
Expand All @@ -343,8 +357,8 @@ mod tests {
let last_item = stack.pop().unwrap();

// Then
assert(last_item == 3, 'wrong result');
assert(stack.len() == 2, 'wrong length');
assert_eq!(last_item, 3);
assert_eq!(stack.len(), 2);
}


Expand All @@ -360,11 +374,9 @@ mod tests {
let elements = stack.pop_n(3).unwrap();

// Then
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');
assert_eq!(stack.len(), 0);
assert_eq!(elements.len(), 3);
assert_eq!(elements.span(), [3, 2, 1].span())
}


Expand Down Expand Up @@ -394,6 +406,35 @@ mod tests {
result.unwrap_err() == EVMError::StackUnderflow, "should return StackUnderflow"
);
}


#[test]
fn test_pop_saturating_u128_should_return_max_when_overflow() {
// Given
let mut stack = StackTrait::new();
stack.push(Bounded::<u256>::MAX).unwrap();

// When
let result = stack.pop_saturating_u128();

// Then
assert!(result.is_ok());
assert_eq!(result.unwrap(), Bounded::<u128>::MAX);
}

#[test]
fn test_pop_saturating_u128_should_return_value_when_no_overflow() {
// Given
let mut stack = StackTrait::new();
stack.push(1234567890).unwrap();

// When
let result = stack.pop_saturating_u128();

// Then
assert!(result.is_ok());
assert_eq!(result.unwrap(), 1234567890);
}
}

mod peek {
Expand All @@ -411,7 +452,7 @@ mod tests {

// Then
let last_item = stack.peek().unwrap();
assert(last_item == 2, 'wrong result');
assert_eq!(last_item, 2);
}


Expand All @@ -427,7 +468,7 @@ mod tests {
let result = stack.peek_at(0).unwrap();

// Then
assert(result == 3, 'wrong result');
assert_eq!(result, 3);
}

#[test]
Expand All @@ -442,7 +483,7 @@ mod tests {
let result = stack.peek_at(1).unwrap();

// Then
assert(result == 2, 'wrong result');
assert_eq!(result, 2);
}

#[test]
Expand Down Expand Up @@ -473,26 +514,26 @@ mod tests {
stack.push(3).unwrap();
stack.push(4).unwrap();
let index3 = stack.peek_at(3).unwrap();
assert(index3 == 1, 'wrong index3');
assert_eq!(index3, 1);
let index2 = stack.peek_at(2).unwrap();
assert(index2 == 2, 'wrong index2');
assert_eq!(index2, 2);
let index1 = stack.peek_at(1).unwrap();
assert(index1 == 3, 'wrong index1');
assert_eq!(index1, 3);
let index0 = stack.peek_at(0).unwrap();
assert(index0 == 4, 'wrong index0');
assert_eq!(index0, 4);

// When
stack.swap_i(2).expect('swap failed');

// Then
let index3 = stack.peek_at(3).unwrap();
assert(index3 == 1, 'post-swap: wrong index3');
assert_eq!(index3, 1);
let index2 = stack.peek_at(2).unwrap();
assert(index2 == 4, 'post-swap: wrong index2');
assert_eq!(index2, 4);
let index1 = stack.peek_at(1).unwrap();
assert(index1 == 3, 'post-swap: wrong index1');
assert_eq!(index1, 3);
let index0 = stack.peek_at(0).unwrap();
assert(index0 == 2, 'post-swap: wrong index0');
assert_eq!(index0, 2);
}

#[test]
Expand All @@ -503,10 +544,8 @@ mod tests {
// When & Then
let result = stack.swap_i(1);

assert(result.is_err(), 'should return an EVMError');
assert!(
result.unwrap_err() == EVMError::StackUnderflow, "should return StackUnderflow"
);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::StackUnderflow);
}

#[test]
Expand All @@ -518,10 +557,8 @@ mod tests {
// When & Then
let result = stack.swap_i(2);

assert(result.is_err(), 'should return an EVMError');
assert!(
result.unwrap_err() == EVMError::StackUnderflow, "should return StackUnderflow"
);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::StackUnderflow);
}
}
}

0 comments on commit ad32fc0

Please sign in to comment.