Skip to content

Commit

Permalink
fix(levm): remove accounts in selfdestruct (#1566)
Browse files Browse the repository at this point in the history
**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.
  • Loading branch information
damiramirez authored Dec 26, 2024
1 parent cdaa844 commit 4952d7b
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cmd/ef_tests/levm/runner/levm_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion crates/vm/levm/src/opcode_handlers/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
14 changes: 8 additions & 6 deletions crates/vm/levm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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]),
Expand Down Expand Up @@ -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(())
Expand Down

0 comments on commit 4952d7b

Please sign in to comment.