feat(levm): opcodes revert invalid selfdestruct

crates/vm/levm/src/constants.rs

@@ -88,6 +88,9 @@ pub mod gas_cost {
     pub const CODECOPY_DYNAMIC_BASE: U256 = U256([3, 0, 0, 0]);
     pub const GASPRICE: U256 = U256([2, 0, 0, 0]);
     pub const EXTCODECOPY_DYNAMIC_BASE: U256 = U256([3, 0, 0, 0]);
+    pub const SELFDESTRUCT_STATIC: U256 = U256([5000, 0, 0, 0]);
+    pub const SELFDESTRUCT_DYNAMIC: U256 = U256([25000, 0, 0, 0]);
+    pub const COLD_ADDRESS_ACCESS_COST: U256 = U256([2600, 0, 0, 0]);
 }
 
 // Costs in gas for call opcodes (in wei)

crates/vm/levm/src/errors.rs

@@ -19,9 +19,10 @@ pub enum VMError {
     OverflowInArithmeticOp,
     FatalError,
     InvalidTransaction,
+    RevertOpcode,
+    InvalidOpcode,
     MissingBlobHashes,
     BlobHashIndexOutOfBounds,
-    RevertOpcode,
     SenderAccountDoesNotExist,
     SenderAccountShouldNotHaveBytecode,
     SenderBalanceShouldContainTransferValue,
@@ -38,6 +39,7 @@ pub enum ResultReason {
     Stop,
     Revert,
     Return,
+    SelfDestruct,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/vm/levm/src/opcode_handlers/system.rs

@@ -1,6 +1,7 @@
 use crate::{
     constants::{call_opcode, SUCCESS_FOR_RETURN},
     errors::ResultReason,
+    vm::Account,
 };
 
 use super::*;
@@ -254,4 +255,116 @@ impl VM {
             current_call_frame,
         )
     }
+
+    // REVERT operation
+    pub fn op_revert(
+        &mut self,
+        current_call_frame: &mut CallFrame,
+    ) -> Result<OpcodeSuccess, VMError> {
+        // Description: Gets values from stack, calculates gas cost and sets return data.
+        // Returns: VMError RevertOpcode if executed correctly.
+        // Notes:
+        //      The reversion of changes is made in the generic_call().
+        //      Changes are not "reverted" if it is the first callframe, they are just not commited.
+
+        let offset = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_| VMError::VeryLargeNumber)?;
+
+        let size = current_call_frame
+            .stack
+            .pop()?
+            .try_into()
+            .map_err(|_| VMError::VeryLargeNumber)?;
+
+        let gas_cost = current_call_frame.memory.expansion_cost(offset + size)?;
+
+        self.increase_consumed_gas(current_call_frame, gas_cost)?;
+
+        current_call_frame.returndata = current_call_frame.memory.load_range(offset, size).into();
+
+        Err(VMError::RevertOpcode)
+    }
+
+    /// ### INVALID operation
+    /// Reverts consuming all gas, no return data.
+    pub fn op_invalid(&mut self) -> Result<OpcodeSuccess, VMError> {
+        Err(VMError::InvalidOpcode)
+    }
+
+    // SELFDESTRUCT operation
+    pub fn op_selfdestruct(
+        &mut self,
+        current_call_frame: &mut CallFrame,
+    ) -> Result<OpcodeSuccess, VMError> {
+        // Sends all ether in the account to the target address
+        // Steps:
+        // 1. Pop the target address from the stack
+        // 2. Get current account and: Store the balance in a variable, set it's balance to 0
+        // 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
+        if current_call_frame.is_static {
+            return Err(VMError::OpcodeNotAllowedInStaticContext);
+        }
+
+        // Gas costs variables
+        let static_gas_cost = gas_cost::SELFDESTRUCT_STATIC;
+        let dynamic_gas_cost = gas_cost::SELFDESTRUCT_DYNAMIC;
+        let _cold_gas_cost = gas_cost::COLD_ADDRESS_ACCESS_COST;
+        let mut gas_cost = static_gas_cost; // This will be updated later
+
+        // 1. Pop the target address from the stack
+        let target_address = Address::from_low_u64_be(current_call_frame.stack.pop()?.low_u64());
+
+        // 2. Get current account and: Store the balance in a variable, set it's balance to 0
+        let current_account_balance = self
+            .db
+            .accounts
+            .get(&current_call_frame.to)
+            .unwrap()
+            .balance;
+        self.db
+            .accounts
+            .get_mut(&current_call_frame.to)
+            .unwrap()
+            .balance = U256::zero();
+
+        // 3 & 4. Get target account and add the balance of the current account to it
+        // TODO: If address is cold, there is an additional cost of 2600. AFAIK accessList has not been implemented yet.
+        if self.db.account_is_empty(&target_address) {
+            gas_cost += dynamic_gas_cost;
+            self.db.accounts.insert(
+                target_address,
+                Account::default().with_balance(current_account_balance),
+            );
+        } else {
+            let target_account = self.db.accounts.get_mut(&target_address).unwrap();
+
+            target_account.balance += current_account_balance;
+        }
+
+        // 5. Register account to be destroyed in accrued substate IF executed in the same transaction a contract was created
+        // This is temporary because it is not necessarily right but we should just check if the current account was created in this transaction.
+        if self
+            .accrued_substate
+            .created_contract_addresses
+            .contains(&current_call_frame.to)
+        {
+            self.accrued_substate
+                .self_destruct_set
+                .insert(current_call_frame.to);
+        }
+        // Those accounts should be destroyed at the end of the transaction.
+
+        self.increase_consumed_gas(current_call_frame, gas_cost)?;
+
+        Ok(OpcodeSuccess::Result(ResultReason::SelfDestruct))
+    }
 }

crates/vm/levm/src/opcodes.rs

@@ -165,9 +165,9 @@ pub enum Opcode {
     DELEGATECALL = 0xF4,
     CREATE2 = 0xF5,
     STATICCALL = 0xFA,
-    // REVERT = 0xFD,
-    // INVALID = 0xFE,
-    // SELFDESTRUCT = 0xFF,
+    REVERT = 0xFD,
+    INVALID = 0xFE,
+    SELFDESTRUCT = 0xFF,
 }
 
 impl Copy for Opcode {}
@@ -321,6 +321,9 @@ impl From<u8> for Opcode {
             0xF5 => Opcode::CREATE2,
             0xF4 => Opcode::DELEGATECALL,
             0xFA => Opcode::STATICCALL,
+            0xFD => Opcode::REVERT,
+            0xFE => Opcode::INVALID,
+            0xFF => Opcode::SELFDESTRUCT,
             _ => panic!("Unknown opcode: 0x{:02X}", byte),
         }
     }

crates/vm/levm/src/operations.rs

@@ -86,9 +86,9 @@ pub enum Operation {
     DelegateCall,
     Create2,
     StaticCall,
-    // Revert,
-    // Invalid,
-    // SelfDestruct,
+    Revert,
+    Invalid,
+    SelfDestruct,
 }
 
 impl Operation {
@@ -202,9 +202,9 @@ impl Operation {
             Operation::DelegateCall => Bytes::copy_from_slice(&[Opcode::DELEGATECALL as u8]),
             Operation::Create2 => Bytes::copy_from_slice(&[Opcode::CREATE2 as u8]),
             Operation::StaticCall => Bytes::copy_from_slice(&[Opcode::STATICCALL as u8]),
-            // Operation::Revert => Bytes::copy_from_slice(&[Opcode::REVERT as u8]),
-            // Operation::Invalid => Bytes::copy_from_slice(&[Opcode::INVALID as u8]),
-            // Operation::SelfDestruct => Bytes::copy_from_slice(&[Opcode::SELFDESTRUCT as u8]),
+            Operation::Revert => Bytes::copy_from_slice(&[Opcode::REVERT as u8]),
+            Operation::Invalid => Bytes::copy_from_slice(&[Opcode::INVALID as u8]),
+            Operation::SelfDestruct => Bytes::copy_from_slice(&[Opcode::SELFDESTRUCT as u8]),
         }
     }
 }

crates/vm/levm/src/vm.rs

@@ -88,7 +88,7 @@ impl Account {
 
 pub type Storage = HashMap<U256, H256>;
 
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
 pub struct Db {
     pub accounts: HashMap<Address, Account>,
     // contracts: HashMap<B256, Bytecode>,
@@ -149,12 +149,18 @@ impl Db {
 
         Ok(self.accounts.get(address).unwrap())
     }
+
+    pub fn account_is_empty(&self, address: &Address) -> bool {
+        !self.accounts.contains_key(address)
+    }
 }
 
 #[derive(Debug, Clone, Default)]
 // TODO: https://github.com/lambdaclass/ethereum_rust/issues/604
 pub struct Substate {
     pub warm_addresses: HashSet<Address>,
+    pub created_contract_addresses: HashSet<Address>, // Maybe not necessary
+    pub self_destruct_set: HashSet<Address>,
 }
 
 #[derive(Debug, Default, Clone)]
@@ -273,10 +279,13 @@ impl VM {
     }
 
     pub fn execute(&mut self, current_call_frame: &mut CallFrame) -> TransactionReport {
-        // let mut current_call_frame = self
-        //     .call_frames
-        //     .pop()
-        //     .expect("Fatal Error: This should not happen"); // if this happens during execution, we are cooked 💀
+        // Backup of Database, Substate and Gas Refunds if sub-context is reverted
+        let (backup_db, backup_substate, backup_refunded_gas) = (
+            self.db.clone(),
+            self.accrued_substate.clone(),
+            self.env.refunded_gas,
+        );
+
         loop {
             let opcode = current_call_frame.next_opcode().unwrap_or(Opcode::STOP);
             let op_result: Result<OpcodeSuccess, VMError> = match opcode {
@@ -372,6 +381,10 @@ impl VM {
                 Opcode::EXTCODESIZE => self.op_extcodesize(current_call_frame),
                 Opcode::EXTCODECOPY => self.op_extcodecopy(current_call_frame),
                 Opcode::EXTCODEHASH => self.op_extcodehash(current_call_frame),
+                Opcode::REVERT => self.op_revert(current_call_frame),
+                Opcode::INVALID => self.op_invalid(),
+                Opcode::SELFDESTRUCT => self.op_selfdestruct(current_call_frame),
+
                 _ => Err(VMError::OpcodeNotFound),
             };
 
@@ -394,19 +407,23 @@ impl VM {
                 Err(error) => {
                     self.call_frames.push(current_call_frame.clone());
 
-                    // CONSUME ALL GAS UNLESS THE ERROR IS FROM REVERT OPCODE
+                    // Unless error is from Revert opcode, all gas is consumed
                     if error != VMError::RevertOpcode {
                         let left_gas = current_call_frame.gas_limit - current_call_frame.gas_used;
                         current_call_frame.gas_used += left_gas;
                         self.env.consumed_gas += left_gas;
                     }
 
+                    // Restore previous state
+                    (self.accrued_substate, self.db, self.env.refunded_gas) =
+                        (backup_substate, backup_db, backup_refunded_gas);
+
                     return TransactionReport {
                         result: TxResult::Revert(error),
                         new_state: self.db.accounts.clone(),
                         gas_used: current_call_frame.gas_used.low_u64(),
                         gas_refunded: self.env.refunded_gas.low_u64(),
-                        output: current_call_frame.returndata.clone(),
+                        output: current_call_frame.returndata.clone(), // Bytes::new() if error is not RevertOpcode
                         logs: current_call_frame.logs.clone(),
                         created_address: None,
                     };
@@ -536,7 +553,7 @@ impl VM {
         // self.call_frames.push(new_call_frame.clone());
         let tx_report = self.execute(&mut new_call_frame);
 
-        current_call_frame.gas_used += tx_report.gas_used.into(); // We add the gas used by the sub-context to the current one after it's execution.
+        current_call_frame.gas_used += tx_report.gas_used.into(); // Add gas used by the sub-context to the current one after it's execution.
         current_call_frame.logs.extend(tx_report.logs);
         current_call_frame
             .memory
@@ -550,10 +567,8 @@ impl VM {
                     .stack
                     .push(U256::from(SUCCESS_FOR_CALL))?;
             }
-            TxResult::Revert(_error) => {
-                // Behavior for revert between contexts goes here if necessary
-                // It is also possible to differentiate between RevertOpcode error and other kinds of revert.
-
+            TxResult::Revert(_) => {
+                // Push 0 to stack
                 current_call_frame.stack.push(U256::from(REVERT_FOR_CALL))?;
             }
         }

crates/vm/levm/tests/tests.rs

@@ -4340,3 +4340,69 @@ fn extcodehash_non_existing_account() {
     );
     assert_eq!(vm.env.consumed_gas, 23603.into());
 }
+
+#[test]
+fn invalid_opcode() {
+    let operations = [Operation::Invalid, Operation::Stop];
+
+    let mut vm = new_vm_with_ops(&operations);
+
+    let mut current_call_frame = vm.call_frames.pop().unwrap();
+    let tx_report = vm.execute(&mut current_call_frame);
+
+    assert!(matches!(
+        tx_report.result,
+        TxResult::Revert(VMError::InvalidOpcode)
+    ));
+}
+
+// Revert Opcode has correct output and result
+#[test]
+fn revert_opcode() {
+    let ops = vec![
+        Operation::Push((32, U256::from(0xA))),  // value
+        Operation::Push((32, U256::from(0xFF))), // offset
+        Operation::Mstore,
+        Operation::Push((32, U256::from(32))),   // size
+        Operation::Push((32, U256::from(0xFF))), // offset
+        Operation::Revert,
+    ];
+
+    let mut vm = new_vm_with_ops(&ops);
+
+    let mut current_call_frame = vm.call_frames.pop().unwrap();
+    let tx_report = vm.execute(&mut current_call_frame);
+
+    assert_eq!(U256::from_big_endian(&tx_report.output), U256::from(0xA));
+    assert!(matches!(
+        tx_report.result,
+        TxResult::Revert(VMError::RevertOpcode)
+    ));
+}
+
+// Store something in the database, then revert. Database should be like it was before the store.
+#[test]
+fn revert_sstore() {
+    let key = U256::from(80);
+    let value = U256::from(100);
+    let sender_address = Address::from_low_u64_be(3000);
+    let operations = vec![
+        Operation::Push((1, value)),
+        Operation::Push((1, key)),
+        Operation::Sstore,
+        Operation::Revert,
+    ];
+
+    let mut vm = new_vm_with_ops(&operations);
+    vm.current_call_frame_mut().code_address = sender_address;
+    vm.db.accounts.insert(sender_address, Account::default());
+
+    let mut current_call_frame = vm.call_frames.pop().unwrap();
+
+    // Db state before the SSTORE
+    let db_backup = vm.db.clone();
+
+    vm.execute(&mut current_call_frame);
+
+    assert_eq!(vm.db, db_backup);
+}