From 4952d7bc821b8f5c4aa5273f38569cbd6dd410b1 Mon Sep 17 00:00:00 2001
From: Damian Ramirez <damian.ramirez@lambdaclass.com>
Date: Thu, 26 Dec 2024 17:31:53 -0300
Subject: [PATCH] fix(levm): remove accounts in selfdestruct (#1566)

**Motivation**

We were not handling the remove case well in `selfdestruct`. When
removing it directly from from the cache, it generated a discrepancy in
the root.

**Description**

- When we have addresses in `accrued_substate.selfdestruct_set`, we
assign the default value to the account, marking the account as empty.
- We check in `get_state_transitions` if `new_state_account.is_empty()`
and assign the value to the `removed` field.
- Handle the case when `target_account` is the same as
`current_call_frame.to`. In that case, we should burn the eth.
---
 cmd/ef_tests/levm/runner/levm_runner.rs      |  2 +-
 crates/vm/levm/src/opcode_handlers/system.rs |  6 +++++-
 crates/vm/levm/src/vm.rs                     | 14 ++++++++------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/cmd/ef_tests/levm/runner/levm_runner.rs b/cmd/ef_tests/levm/runner/levm_runner.rs
index fe361c141..1e0783377 100644
--- a/cmd/ef_tests/levm/runner/levm_runner.rs
+++ b/cmd/ef_tests/levm/runner/levm_runner.rs
@@ -381,7 +381,7 @@ pub fn get_state_transitions(
 
         let account_update = AccountUpdate {
             address: *new_state_account_address,
-            removed: false,
+            removed: new_state_account.is_empty(),
             info: Some(AccountInfo {
                 code_hash: code_hash(&new_state_account.info.bytecode),
                 balance: new_state_account.info.balance,
diff --git a/crates/vm/levm/src/opcode_handlers/system.rs b/crates/vm/levm/src/opcode_handlers/system.rs
index cfde778d0..07b5f5528 100644
--- a/crates/vm/levm/src/opcode_handlers/system.rs
+++ b/crates/vm/levm/src/opcode_handlers/system.rs
@@ -468,13 +468,17 @@ impl VM {
         self.increase_account_balance(target_address, balance_to_transfer)?;
         self.decrease_account_balance(current_call_frame.to, balance_to_transfer)?;
 
+        // Selfdestruct is executed in the same transaction as the contract was created
         if self
             .accrued_substate
             .created_accounts
             .contains(&current_call_frame.to)
         {
+            // If target is the same as the contract calling, Ether will be burnt.
+            self.get_account_mut(current_call_frame.to)?.info.balance = U256::zero();
+
             self.accrued_substate
-                .selfdestrutct_set
+                .selfdestruct_set
                 .insert(current_call_frame.to);
         }
 
diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs
index e713080b0..55da1e2f5 100644
--- a/crates/vm/levm/src/vm.rs
+++ b/crates/vm/levm/src/vm.rs
@@ -38,7 +38,7 @@ pub struct Substate {
     // accessed addresses and storage keys are considered WARM
     // pub accessed_addresses: HashSet<Address>,
     // pub accessed_storage_keys: HashSet<(Address, U256)>,
-    pub selfdestrutct_set: HashSet<Address>,
+    pub selfdestruct_set: HashSet<Address>,
     pub touched_accounts: HashSet<Address>,
     pub touched_storage_slots: HashMap<Address, HashSet<H256>>,
     pub created_accounts: HashSet<Address>,
@@ -176,7 +176,7 @@ impl VM {
                 );
 
                 let substate = Substate {
-                    selfdestrutct_set: HashSet::new(),
+                    selfdestruct_set: HashSet::new(),
                     touched_accounts: default_touched_accounts,
                     touched_storage_slots: default_touched_storage_slots,
                     created_accounts: HashSet::new(),
@@ -218,7 +218,7 @@ impl VM {
                 );
 
                 let substate = Substate {
-                    selfdestrutct_set: HashSet::new(),
+                    selfdestruct_set: HashSet::new(),
                     touched_accounts: default_touched_accounts,
                     touched_storage_slots: default_touched_storage_slots,
                     created_accounts: HashSet::from([new_contract_address]),
@@ -906,9 +906,11 @@ impl VM {
         };
 
         // 4. Destruct addresses in selfdestruct set.
-        // In Cancun the only addresses destroyed are contracts created in this transaction, so we 'destroy' them by just removing them from the cache, as if they never existed.
-        for address in &self.accrued_substate.selfdestrutct_set {
-            remove_account(&mut self.cache, address);
+        // In Cancun the only addresses destroyed are contracts created in this transaction
+        let selfdestruct_set = self.accrued_substate.selfdestruct_set.clone();
+        for address in selfdestruct_set {
+            let account_to_remove = self.get_account_mut(address)?;
+            *account_to_remove = Account::default();
         }
 
         Ok(())