fix(levm): vm tests fix changes (#1281)

**Motivation** The goal is to make the tests of VMTests folder of EF tests. **Description** Currently, are passing 7/11 of the VMTests/vmTests folder. Closes #issue_number --------- Co-authored-by: ilitteri <[email protected]> Co-authored-by: Ivan Litteri <[email protected]>
lambdaclass · Nov 29, 2024 · 1208981 · 1208981
1 parent 18aae39
commit 1208981
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 86 deletions.
diff --git a/crates/vm/levm/src/gas_cost.rs b/crates/vm/levm/src/gas_cost.rs
@@ -219,11 +219,7 @@ fn copy_behavior(
         .saturating_sub(1))
         / WORD_SIZE;
 
-    let memory_expansion_cost = current_call_frame.memory.expansion_cost(
-        offset
-            .checked_add(size)
-            .ok_or(OutOfGasError::GasCostOverflow)?,
-    )?;
+    let memory_expansion_cost = current_call_frame.memory.expansion_cost(offset, size)?;
 
     let minimum_word_size_cost = dynamic_base
         .checked_mul(minimum_word_size.into())
@@ -255,11 +251,7 @@ pub fn log(
     offset: usize,
     number_of_topics: u8,
 ) -> Result<U256, OutOfGasError> {
-    let memory_expansion_cost = current_call_frame.memory.expansion_cost(
-        offset
-            .checked_add(size)
-            .ok_or(OutOfGasError::GasCostOverflow)?,
-    )?;
+    let memory_expansion_cost = current_call_frame.memory.expansion_cost(offset, size)?;
 
     let topics_cost = LOGN_DYNAMIC_BASE
         .checked_mul(number_of_topics.into())
@@ -294,11 +286,9 @@ fn mem_expansion_behavior(
     offset_add: usize,
     static_cost: U256,
 ) -> Result<U256, OutOfGasError> {
-    let memory_expansion_cost = current_call_frame.memory.expansion_cost(
-        offset
-            .checked_add(offset_add)
-            .ok_or(OutOfGasError::GasCostOverflow)?,
-    )?;
+    let memory_expansion_cost = current_call_frame
+        .memory
+        .expansion_cost(offset, offset_add)?;
     static_cost
         .checked_add(memory_expansion_cost)
         .ok_or(OutOfGasError::GasCostOverflow)
@@ -360,16 +350,21 @@ pub fn mcopy(
         .saturating_sub(1))
         / WORD_SIZE;
 
-    let memory_byte_size = src_offset
+    let src_sum = src_offset
+        .checked_add(size)
+        .ok_or(OutOfGasError::GasCostOverflow)?;
+    let dest_sum = dest_offset
         .checked_add(size)
-        .and_then(|src_sum| {
-            dest_offset
-                .checked_add(size)
-                .map(|dest_sum| src_sum.max(dest_sum))
-        })
         .ok_or(OutOfGasError::GasCostOverflow)?;
 
-    let memory_expansion_cost = current_call_frame.memory.expansion_cost(memory_byte_size)?;
+    let memory_expansion_cost = if src_sum > dest_sum {
+        current_call_frame.memory.expansion_cost(src_offset, size)?
+    } else {
+        current_call_frame
+            .memory
+            .expansion_cost(dest_offset, size)?
+    };
+
     let copied_words_cost = MCOPY_DYNAMIC_BASE
         .checked_mul(words_copied.into())
         .ok_or(OutOfGasError::GasCostOverflow)?;
@@ -381,9 +376,9 @@ pub fn mcopy(
 }
 
 pub fn create(
-    current_call_frame: &CallFrame,
-    code_offset_in_memory: U256,
-    code_size_in_memory: U256,
+    current_call_frame: &mut CallFrame,
+    code_offset_in_memory: usize,
+    code_size_in_memory: usize,
 ) -> Result<U256, OutOfGasError> {
     compute_gas_create(
         current_call_frame,
@@ -394,9 +389,9 @@ pub fn create(
 }
 
 pub fn create_2(
-    current_call_frame: &CallFrame,
-    code_offset_in_memory: U256,
-    code_size_in_memory: U256,
+    current_call_frame: &mut CallFrame,
+    code_offset_in_memory: usize,
+    code_size_in_memory: usize,
 ) -> Result<U256, OutOfGasError> {
     compute_gas_create(
         current_call_frame,
@@ -407,49 +402,45 @@ pub fn create_2(
 }
 
 fn compute_gas_create(
-    current_call_frame: &CallFrame,
-    code_offset_in_memory: U256,
-    code_size_in_memory: U256,
+    current_call_frame: &mut CallFrame,
+    code_offset_in_memory: usize,
+    code_size_in_memory: usize,
     is_create_2: bool,
 ) -> Result<U256, OutOfGasError> {
     let minimum_word_size = (code_size_in_memory
-        .checked_add(U256::from(31))
+        .checked_add(31)
         .ok_or(OutOfGasError::GasCostOverflow)?)
-    .checked_div(U256::from(32))
+    .checked_div(32)
     .ok_or(OutOfGasError::ArithmeticOperationDividedByZero)?; // '32' will never be zero
 
     let init_code_cost = minimum_word_size
-        .checked_mul(INIT_CODE_WORD_COST)
+        .checked_mul(INIT_CODE_WORD_COST.as_usize()) // will not panic since it's 2
         .ok_or(OutOfGasError::GasCostOverflow)?;
 
     let code_deposit_cost = code_size_in_memory
-        .checked_mul(CODE_DEPOSIT_COST)
+        .checked_mul(CODE_DEPOSIT_COST.as_usize()) // will not panic since it's 200
         .ok_or(OutOfGasError::GasCostOverflow)?;
 
-    let memory_expansion_cost = current_call_frame.memory.expansion_cost(
-        code_size_in_memory
-            .checked_add(code_offset_in_memory)
-            .ok_or(OutOfGasError::GasCostOverflow)?
-            .try_into()
-            .map_err(|_err| OutOfGasError::GasCostOverflow)?,
-    )?;
+    let memory_expansion_cost = current_call_frame
+        .memory
+        .expansion_cost(code_offset_in_memory, code_size_in_memory)?;
 
     let hash_cost = if is_create_2 {
         minimum_word_size
-            .checked_mul(KECCAK25_DYNAMIC_BASE)
+            .checked_mul(KECCAK25_DYNAMIC_BASE.as_usize()) // will not panic since it's 6
             .ok_or(OutOfGasError::GasCostOverflow)?
     } else {
-        U256::zero()
+        0
     };
 
-    init_code_cost
-        .checked_add(memory_expansion_cost)
+    memory_expansion_cost
+        .checked_add(init_code_cost.into())
         .ok_or(OutOfGasError::CreationCostIsTooHigh)?
-        .checked_add(code_deposit_cost)
+        .checked_add(code_deposit_cost.into())
         .ok_or(OutOfGasError::CreationCostIsTooHigh)?
         .checked_add(CREATE_BASE_COST)
         .ok_or(OutOfGasError::CreationCostIsTooHigh)?
-        .checked_add(hash_cost)
+        .checked_add(hash_cost.into())
         .ok_or(OutOfGasError::CreationCostIsTooHigh)
 }
 

diff --git a/crates/vm/levm/src/memory.rs b/crates/vm/levm/src/memory.rs
@@ -163,7 +163,14 @@ impl Memory {
         Ok(())
     }
 
-    pub fn expansion_cost(&self, memory_byte_size: usize) -> Result<U256, OutOfGasError> {
+    pub fn expansion_cost(&self, offset: usize, size: usize) -> Result<U256, OutOfGasError> {
+        if size == 0 {
+            return Ok(U256::zero());
+        }
+
+        let memory_byte_size = offset
+            .checked_add(size)
+            .ok_or(OutOfGasError::GasCostOverflow)?;
         if memory_byte_size <= self.data.len() {
             return Ok(U256::zero());
         }

diff --git a/crates/vm/levm/src/opcode_handlers/system.rs b/crates/vm/levm/src/opcode_handlers/system.rs
@@ -191,12 +191,7 @@ impl VM {
             .try_into()
             .map_err(|_| VMError::VeryLargeNumber)?;
 
-        let gas_cost =
-            current_call_frame
-                .memory
-                .expansion_cost(offset.checked_add(size).ok_or(VMError::Internal(
-                    InternalError::ArithmeticOperationOverflow,
-                ))?)?;
+        let gas_cost = current_call_frame.memory.expansion_cost(offset, size)?;
 
         self.increase_consumed_gas(current_call_frame, gas_cost)?;
 
@@ -368,8 +363,16 @@ impl VM {
         current_call_frame: &mut CallFrame,
     ) -> Result<OpcodeSuccess, VMError> {
         let value_in_wei_to_send = current_call_frame.stack.pop()?;
-        let code_offset_in_memory = current_call_frame.stack.pop()?;
-        let code_size_in_memory = current_call_frame.stack.pop()?;
+        let code_offset_in_memory = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_err| VMError::VeryLargeNumber)?;
+        let code_size_in_memory = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_err| VMError::VeryLargeNumber)?;
 
         // Gas Cost
         let gas_cost = gas_cost::create(
@@ -397,8 +400,16 @@ impl VM {
         current_call_frame: &mut CallFrame,
     ) -> Result<OpcodeSuccess, VMError> {
         let value_in_wei_to_send = current_call_frame.stack.pop()?;
-        let code_offset_in_memory = current_call_frame.stack.pop()?;
-        let code_size_in_memory = current_call_frame.stack.pop()?;
+        let code_offset_in_memory: usize = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_err| VMError::VeryLargeNumber)?;
+        let code_size_in_memory: usize = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_err| VMError::VeryLargeNumber)?;
         let salt = current_call_frame.stack.pop()?;
 
         // Gas Cost
@@ -442,12 +453,7 @@ impl VM {
             .try_into()
             .map_err(|_err| VMError::VeryLargeNumber)?;
 
-        let gas_cost =
-            current_call_frame
-                .memory
-                .expansion_cost(offset.checked_add(size).ok_or(VMError::Internal(
-                    InternalError::ArithmeticOperationOverflow,
-                ))?)?;
+        let gas_cost = current_call_frame.memory.expansion_cost(offset, size)?;
 
         self.increase_consumed_gas(current_call_frame, gas_cost)?;
 
@@ -474,7 +480,6 @@ impl VM {
         // 3. Get the target account, checking if it is empty and if it is cold. Update gas cost accordingly.
         // 4. Add the balance of the current account to the target account
         // 5. Register account to be destroyed in accrued substate.
-
         // Notes:
         //      If context is Static, return error.
         //      If executed in the same transaction a contract was created, the current account is registered to be destroyed

diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs
@@ -105,7 +105,7 @@ impl VM {
                     value,
                     calldata.clone(),
                     false,
-                    env.gas_limit.min(MAX_BLOCK_GAS_LIMIT),
+                    env.gas_limit,
                     TX_BASE_COST,
                     0,
                 );
@@ -117,7 +117,7 @@ impl VM {
                     accrued_substate: Substate::default(),
                     cache,
                     tx_kind: to,
-                    touched_accounts: HashSet::new(),
+                    touched_accounts: default_touched_accounts,
                     touched_storage_slots: HashMap::new(),
                 })
             }
@@ -148,7 +148,7 @@ impl VM {
                     value,
                     Bytes::new(),
                     false,
-                    env.gas_limit.min(MAX_BLOCK_GAS_LIMIT),
+                    env.gas_limit,
                     TX_BASE_COST,
                     0,
                 );
@@ -182,9 +182,6 @@ impl VM {
         loop {
             let opcode = current_call_frame.next_opcode()?.unwrap_or(Opcode::STOP); // This will execute opcode stop if there are no more opcodes, there are other ways of solving this but this is the simplest and doesn't change VM behavior.
 
-            // Note: This is commented because it's used for debugging purposes in development.
-            // dbg!(&current_call_frame.gas_used);
-            // dbg!(&opcode);
             let op_result: Result<OpcodeSuccess, VMError> = match opcode {
                 Opcode::STOP => Ok(OpcodeSuccess::Result(ResultReason::Stop)),
                 Opcode::ADD => self.op_add(current_call_frame),
@@ -625,7 +622,9 @@ impl VM {
             .checked_mul(priority_fee_per_gas)
             .ok_or(VMError::BalanceOverflow)?;
 
-        self.increase_account_balance(coinbase_address, coinbase_fee)?;
+        if !coinbase_fee.is_zero() {
+            self.increase_account_balance(coinbase_address, coinbase_fee)?;
+        }
 
         report.new_state.clone_from(&self.cache);
 
@@ -655,15 +654,14 @@ impl VM {
         ret_offset: usize,
         ret_size: usize,
     ) -> Result<OpcodeSuccess, VMError> {
-        let (sender_account_info, _address_was_cold) =
-            self.access_account(current_call_frame.msg_sender);
+        let (sender_account_info, _address_was_cold) = self.access_account(msg_sender);
 
         if sender_account_info.balance < value {
             current_call_frame.stack.push(U256::from(REVERT_FOR_CALL))?;
             return Ok(OpcodeSuccess::Continue);
         }
 
-        self.decrease_account_balance(current_call_frame.msg_sender, value)?;
+        self.decrease_account_balance(msg_sender, value)?;
         self.increase_account_balance(to, value)?;
 
         let (code_account_info, _address_was_cold) = self.access_account(code_address);
@@ -808,15 +806,11 @@ impl VM {
     pub fn create(
         &mut self,
         value_in_wei_to_send: U256,
-        code_offset_in_memory: U256,
-        code_size_in_memory: U256,
+        code_offset_in_memory: usize,
+        code_size_in_memory: usize,
         salt: Option<U256>,
         current_call_frame: &mut CallFrame,
     ) -> Result<OpcodeSuccess, VMError> {
-        let code_size_in_memory = code_size_in_memory
-            .try_into()
-            .map_err(|_err| VMError::VeryLargeNumber)?;
-
         if code_size_in_memory > MAX_CODE_SIZE * 2 {
             current_call_frame
                 .stack
@@ -850,10 +844,6 @@ impl VM {
             }
         };
 
-        let code_offset_in_memory = code_offset_in_memory
-            .try_into()
-            .map_err(|_err| VMError::VeryLargeNumber)?;
-
         let code = Bytes::from(
             current_call_frame
                 .memory