feat: differentiate reverts from run exceptions (#902)

* feat: differentiate reverts from run exceptions

* feat: handle exceptions and reverts

* propagate returndata of revert/success call, not exceptional fail

* fix build

crates/contracts/src/kakarot_core/kakarot.cairo

@@ -397,7 +397,7 @@ pub mod KakarotCore {
             // };
 
             TransactionResult {
-                success: summary.success,
+                success: summary.is_success(),
                 return_data: summary.return_data,
                 gas_used: total_gas_used,
                 state: summary.state,

crates/evm/src/call_helpers.cairo

@@ -10,7 +10,7 @@ use evm::interpreter::EVMTrait;
 use evm::memory::MemoryTrait;
 use evm::model::account::{AccountTrait};
 use evm::model::vm::{VM, VMTrait};
-use evm::model::{Transfer, Address, Message};
+use evm::model::{Transfer, Address, Message, ExecutionResultTrait, ExecutionResultStatus};
 use evm::stack::StackTrait;
 use evm::state::StateTrait;
 use utils::constants;
@@ -100,10 +100,20 @@ impl CallHelpersImpl of CallHelpers {
         let result = EVMTrait::process_message(message, ref self.env);
         self.merge_child(@result);
 
-        if result.success {
-            self.stack.push(1)?;
-        } else {
-            self.stack.push(0)?;
+        match result.status {
+            ExecutionResultStatus::Success => {
+                self.return_data = result.return_data;
+                self.stack.push(1)?;
+            },
+            ExecutionResultStatus::Revert => {
+                self.return_data = result.return_data;
+                self.stack.push(0)?;
+            },
+            ExecutionResultStatus::Exception => {
+                // If the call has halted exceptionnaly, the return_data is emptied.
+                self.return_data = [].span();
+                self.stack.push(0)?;
+            },
         }
 
         // Get the min between len(return_data) and call_ctx.ret_size.

crates/evm/src/create_helpers.cairo

@@ -14,7 +14,9 @@ use evm::interpreter::EVMTrait;
 use evm::memory::MemoryTrait;
 use evm::model::account::{Account, AccountTrait};
 use evm::model::vm::{VM, VMTrait};
-use evm::model::{ExecutionResult, ExecutionResultTrait, ExecutionSummary, Environment};
+use evm::model::{
+    ExecutionResult, ExecutionResultTrait, ExecutionResultStatus, ExecutionSummary, Environment
+};
 use evm::model::{Message, Address, Transfer};
 use evm::stack::StackTrait;
 use evm::state::StateTrait;
@@ -139,12 +141,20 @@ impl CreateHelpersImpl of CreateHelpers {
         let result = EVMTrait::process_create_message(child_message, ref self.env);
         self.merge_child(@result);
 
-        if result.success {
-            self.return_data = [].span();
-            self.stack.push(target_address.evm.into())?;
-        } else {
-            self.return_data = result.return_data;
-            self.stack.push(0)?;
+        match result.status {
+            ExecutionResultStatus::Success => {
+                self.return_data = [].span();
+                self.stack.push(target_address.evm.into())?;
+            },
+            ExecutionResultStatus::Revert => {
+                self.return_data = result.return_data;
+                self.stack.push(0)?;
+            },
+            ExecutionResultStatus::Exception => {
+                // returndata is emptied in case of exception
+                self.return_data = [].span();
+                self.stack.push(0)?;
+            },
         }
         Result::Ok(())
     }

crates/evm/src/interpreter.cairo

@@ -14,7 +14,7 @@ use evm::model::account::{AccountTrait};
 use evm::model::vm::{VM, VMTrait};
 use evm::model::{
     Message, Environment, Address, Transfer, ExecutionSummary, ExecutionSummaryTrait,
-    ExecutionResult, ExecutionResultTrait, AddressTrait
+    ExecutionResult, ExecutionResultTrait, ExecutionResultStatus, AddressTrait
 };
 use evm::precompiles::Precompiles;
 use evm::stack::{Stack, StackTrait};
@@ -37,7 +37,7 @@ impl EVMImpl of EVMTrait {
             }
 
             let mut result = Self::process_create_message(message, ref env);
-            if result.success {
+            if result.is_success() {
                 result.return_data = message.target.evm.to_bytes().span();
             }
             result
@@ -47,7 +47,7 @@ impl EVMImpl of EVMTrait {
 
         // No need to take snapshot of state, as the state is still empty at this point.
         ExecutionSummary {
-            success: result.success,
+            status: result.status,
             state: env.state,
             return_data: result.return_data,
             gas_left: result.gas_left,
@@ -76,7 +76,7 @@ impl EVMImpl of EVMTrait {
         }
 
         let mut result = Self::process_message(message, ref env);
-        if result.success {
+        if result.is_success() {
             // Write the return_data of the initcode
             // as the deployed contract's bytecode and charge gas
             let target_account = env.state.get_account(target_evm_address);
@@ -134,7 +134,7 @@ impl EVMImpl of EVMTrait {
         // The state in the environment has been modified by the VM.
         env = vm.env;
 
-        if !result.success {
+        if !result.is_success() {
             // The `process_message` function has mutated the environment state.
             // Revert state changes using the old snapshot as execution failed.
 
@@ -151,8 +151,13 @@ impl EVMImpl of EVMTrait {
 
             match result {
                 Result::Ok(_) => {
+                    let status = if vm.is_error() {
+                        ExecutionResultStatus::Revert
+                    } else {
+                        ExecutionResultStatus::Success
+                    };
                     return ExecutionResult {
-                        success: true,
+                        status,
                         return_data: vm.return_data(),
                         gas_left: vm.gas_left(),
                         accessed_addresses: vm.accessed_addresses(),
@@ -179,7 +184,7 @@ impl EVMImpl of EVMTrait {
             // REVERT opcode case
             if vm.is_error() {
                 return ExecutionResult {
-                    success: false,
+                    status: ExecutionResultStatus::Revert,
                     return_data: vm.return_data(),
                     gas_left: vm.gas_left(),
                     accessed_addresses: vm.accessed_addresses(),
@@ -189,7 +194,7 @@ impl EVMImpl of EVMTrait {
             };
             // Success case
             return ExecutionResult {
-                success: true,
+                status: ExecutionResultStatus::Success,
                 return_data: vm.return_data(),
                 gas_left: vm.gas_left(),
                 accessed_addresses: vm.accessed_addresses(),
@@ -210,7 +215,7 @@ impl EVMImpl of EVMTrait {
                 // REVERT opcode case
                 if vm.is_error() {
                     return ExecutionResult {
-                        success: false,
+                        status: ExecutionResultStatus::Revert,
                         return_data: vm.return_data(),
                         gas_left: vm.gas_left(),
                         accessed_addresses: vm.accessed_addresses(),
@@ -220,7 +225,7 @@ impl EVMImpl of EVMTrait {
                 };
                 // Success case
                 return ExecutionResult {
-                    success: true,
+                    status: ExecutionResultStatus::Success,
                     return_data: vm.return_data(),
                     gas_left: vm.gas_left(),
                     accessed_addresses: vm.accessed_addresses(),

crates/evm/src/model.cairo

@@ -48,14 +48,21 @@ struct Message {
 
 #[derive(Drop, Debug)]
 struct ExecutionResult {
-    success: bool,
+    status: ExecutionResultStatus,
     return_data: Span<u8>,
     gas_left: u128,
     accessed_addresses: SpanSet<EthAddress>,
     accessed_storage_keys: SpanSet<(EthAddress, u256)>,
     gas_refund: u128,
 }
 
+#[derive(Copy, Drop, PartialEq, Debug)]
+enum ExecutionResultStatus {
+    Success,
+    Revert,
+    Exception,
+}
+
 #[generate_trait]
 impl ExecutionResultImpl of ExecutionResultTrait {
     fn exceptional_failure(
@@ -64,7 +71,7 @@ impl ExecutionResultImpl of ExecutionResultTrait {
         accessed_storage_keys: SpanSet<(EthAddress, u256)>
     ) -> ExecutionResult {
         ExecutionResult {
-            success: false,
+            status: ExecutionResultStatus::Exception,
             return_data: error,
             gas_left: 0,
             accessed_addresses,
@@ -80,11 +87,23 @@ impl ExecutionResultImpl of ExecutionResultTrait {
         self.gas_left = self.gas_left.checked_sub(value).ok_or(EVMError::OutOfGas)?;
         Result::Ok(())
     }
+
+    fn is_success(self: @ExecutionResult) -> bool {
+        *self.status == ExecutionResultStatus::Success
+    }
+
+    fn is_exception(self: @ExecutionResult) -> bool {
+        *self.status == ExecutionResultStatus::Exception
+    }
+
+    fn is_revert(self: @ExecutionResult) -> bool {
+        *self.status == ExecutionResultStatus::Revert
+    }
 }
 
 #[derive(Destruct)]
 struct ExecutionSummary {
-    success: bool,
+    status: ExecutionResultStatus,
     return_data: Span<u8>,
     gas_left: u128,
     state: State,
@@ -95,13 +114,25 @@ struct ExecutionSummary {
 impl ExecutionSummaryImpl of ExecutionSummaryTrait {
     fn exceptional_failure(error: Span<u8>) -> ExecutionSummary {
         ExecutionSummary {
-            success: false,
+            status: ExecutionResultStatus::Exception,
             return_data: error,
             gas_left: 0,
             state: Default::default(),
             gas_refund: 0
         }
     }
+
+    fn is_success(self: @ExecutionSummary) -> bool {
+        *self.status == ExecutionResultStatus::Success
+    }
+
+    fn is_exception(self: @ExecutionSummary) -> bool {
+        *self.status == ExecutionResultStatus::Exception
+    }
+
+    fn is_revert(self: @ExecutionSummary) -> bool {
+        *self.status == ExecutionResultStatus::Revert
+    }
 }
 
 struct TransactionResult {

crates/evm/src/model/vm.cairo

@@ -2,7 +2,7 @@ use core::num::traits::CheckedSub;
 use core::starknet::EthAddress;
 use evm::errors::{EVMError, ensure};
 use evm::memory::{Memory, MemoryTrait};
-use evm::model::{Message, Environment, ExecutionResult, AccountTrait};
+use evm::model::{Message, Environment, ExecutionResult, ExecutionResultStatus, AccountTrait};
 use evm::stack::{Stack, StackTrait};
 use utils::helpers::{SpanExtTrait, ArrayExtTrait};
 use utils::set::{Set, SetTrait, SpanSet};
@@ -145,16 +145,18 @@ impl VMImpl of VMTrait {
 
     #[inline(always)]
     fn merge_child(ref self: VM, child: @ExecutionResult) {
-        if *child.success {
-            //TODO: merge accessed storage
-            self.accessed_addresses.extend(*child.accessed_addresses);
-            self.accessed_storage_keys.extend(*child.accessed_storage_keys);
-            self.gas_refund += *child.gas_refund;
-            self.return_data = *child.return_data;
-        }
-        //TODO(gas) handle error case
-
-        self.gas_left += *child.gas_left;
+        match child.status {
+            ExecutionResultStatus::Success => {
+                self.accessed_addresses.extend(*child.accessed_addresses);
+                self.accessed_storage_keys.extend(*child.accessed_storage_keys);
+                self.gas_refund += *child.gas_refund;
+                self.gas_left += *child.gas_left;
+                self.return_data = *child.return_data;
+            },
+            ExecutionResultStatus::Revert => { self.gas_left += *child.gas_left; },
+            // If the call has halted exceptionnaly, the gas is not returned.
+            ExecutionResultStatus::Exception => {}
+        };
     }
 }