fix(levm): fixing vm arithmetic tests changes (lambdaclass#1333)

**Motivation** The goal is to make pass the tests in the arithmetic subfolder of the VMTests folder. **Description** These changes are mostly local, so don't change much in the passed tests count. --------- Co-authored-by: ilitteri <[email protected]> Co-authored-by: Ivan Litteri <[email protected]>
emirongrr · Nov 29, 2024 · 9981422 · 9981422
1 parent 93847ca
commit 9981422
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 34 deletions.
diff --git a/crates/vm/levm/src/opcode_handlers/arithmetic.rs b/crates/vm/levm/src/opcode_handlers/arithmetic.rs
@@ -237,19 +237,14 @@ impl VM {
     ) -> Result<OpcodeSuccess, VMError> {
         self.increase_consumed_gas(current_call_frame, gas_cost::SIGNEXTEND)?;
 
-        let byte_size: usize = current_call_frame
-            .stack
-            .pop()?
-            .try_into()
-            .map_err(|_| VMError::VeryLargeNumber)?;
-
+        let byte_size = current_call_frame.stack.pop()?;
         let value_to_extend = current_call_frame.stack.pop()?;
 
-        let bits_per_byte: usize = 8;
-        let sign_bit_position_on_byte = 7;
+        let bits_per_byte = U256::from(8);
+        let sign_bit_position_on_byte = U256::from(7);
 
-        let max_byte_size: usize = 31;
-        let byte_size: usize = byte_size.min(max_byte_size);
+        let max_byte_size = U256::from(31);
+        let byte_size = byte_size.min(max_byte_size);
         let total_bits = bits_per_byte
             .checked_mul(byte_size)
             .ok_or(VMError::Internal(
@@ -261,9 +256,20 @@ impl VM {
                 .ok_or(VMError::Internal(
                     InternalError::ArithmeticOperationOverflow,
                 ))?;
+
+        let sign_bit_index: usize = match sign_bit_index.try_into() {
+            Ok(val) => val,
+            Err(_) => {
+                // this means the value_to_extend was too big to extend, so remains the same.
+                // Maybe this verification could be done before in this function
+                current_call_frame.stack.push(value_to_extend)?;
+                return Ok(OpcodeSuccess::Continue);
+            }
+        };
+
         let is_negative = value_to_extend.bit(sign_bit_index);
 
-        let sign_bit_mask = checked_shift_left(U256::one(), sign_bit_index)?
+        let sign_bit_mask = checked_shift_left(U256::one(), U256::from(sign_bit_index))?
             .checked_sub(U256::one())
             .ok_or(VMError::Internal(
                 InternalError::ArithmeticOperationUnderflow,

diff --git a/crates/vm/levm/src/opcode_handlers/bitwise_comparison.rs b/crates/vm/levm/src/opcode_handlers/bitwise_comparison.rs
@@ -202,12 +202,10 @@ impl VM {
         let shift = current_call_frame.stack.pop()?;
         let value = current_call_frame.stack.pop()?;
 
-        let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256
-
         if shift < U256::from(256) {
             current_call_frame
                 .stack
-                .push(checked_shift_left(value, shift_usize)?)?;
+                .push(checked_shift_left(value, shift)?)?;
         } else {
             current_call_frame.stack.push(U256::zero())?;
         }
@@ -221,12 +219,10 @@ impl VM {
         let shift = current_call_frame.stack.pop()?;
         let value = current_call_frame.stack.pop()?;
 
-        let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256
-
         if shift < U256::from(256) {
             current_call_frame
                 .stack
-                .push(checked_shift_right(value, shift_usize)?)?;
+                .push(checked_shift_right(value, shift)?)?;
         } else {
             current_call_frame.stack.push(U256::zero())?;
         }
@@ -253,32 +249,32 @@ impl VM {
 }
 
 pub fn arithmetic_shift_right(value: U256, shift: U256) -> Result<U256, VMError> {
-    let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256
-
     if value.bit(255) {
         // if negative fill with 1s
-        let shifted = checked_shift_right(value, shift_usize)?;
+        let shifted = checked_shift_right(value, shift)?;
         let mask = checked_shift_left(
             U256::MAX,
-            256_usize.checked_sub(shift_usize).ok_or(VMError::Internal(
-                InternalError::ArithmeticOperationUnderflow,
-            ))?, // Note that this is already checked in op_sar
+            (U256::from(256))
+                .checked_sub(shift)
+                .ok_or(VMError::Internal(
+                    InternalError::ArithmeticOperationUnderflow,
+                ))?, // Note that this is already checked in op_sar
         )?;
 
         Ok(shifted | mask)
     } else {
-        Ok(checked_shift_right(value, shift_usize)?)
+        Ok(checked_shift_right(value, shift)?)
     }
 }
 
 /// Instead of using unsafe <<, uses checked_mul n times, replicating n shifts.
 /// Note: These (checked_shift_left and checked_shift_right) are done because
 /// are not available in U256
-pub fn checked_shift_left(value: U256, shift: usize) -> Result<U256, VMError> {
+pub fn checked_shift_left(value: U256, shift: U256) -> Result<U256, VMError> {
     let mut result = value;
     let mut shifts_left = shift;
 
-    while shifts_left > 0 {
+    while !shifts_left.is_zero() {
         result = match result.checked_mul(U256::from(2)) {
             Some(num) => num,
             None => {
@@ -297,26 +293,30 @@ pub fn checked_shift_left(value: U256, shift: usize) -> Result<U256, VMError> {
                     ))?
             }
         };
-        shifts_left = shifts_left.checked_sub(1).ok_or(VMError::Internal(
-            InternalError::ArithmeticOperationUnderflow,
-        ))?; // Should not reach negative values
+        shifts_left = shifts_left
+            .checked_sub(U256::one())
+            .ok_or(VMError::Internal(
+                InternalError::ArithmeticOperationUnderflow,
+            ))?; // Should not reach negative values
     }
 
     Ok(result)
 }
 
 // Instead of using unsafe >>, uses checked_div n times, replicating n shifts
-fn checked_shift_right(value: U256, shift: usize) -> Result<U256, VMError> {
+fn checked_shift_right(value: U256, shift: U256) -> Result<U256, VMError> {
     let mut result = value;
     let mut shifts_left = shift;
 
-    while shifts_left > 0 {
+    while !shifts_left.is_zero() {
         result = result.checked_div(U256::from(2)).ok_or(VMError::Internal(
             InternalError::ArithmeticOperationDividedByZero,
         ))?; // '2' will never be zero
-        shifts_left = shifts_left.checked_sub(1).ok_or(VMError::Internal(
-            InternalError::ArithmeticOperationUnderflow,
-        ))?; // Should not reach negative values
+        shifts_left = shifts_left
+            .checked_sub(U256::one())
+            .ok_or(VMError::Internal(
+                InternalError::ArithmeticOperationUnderflow,
+            ))?; // Should not reach negative values
     }
 
     Ok(result)