Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: storage commitment on selfdestruct accounts #953

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 125 additions & 2 deletions crates/evm/src/backend/starknet_backend.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn commit_storage(ref self: State) -> Result<(), EVMError> {
let (evm_address, key, value) = self.accounts_storage.changes.get(*state_key).deref();
let mut account = self.get_account(evm_address);
// @dev: EIP-6780 - If selfdestruct on an account created, dont commit data
if account.is_selfdestruct() {
if account.is_selfdestruct() && account.is_created() {
continue;
}
IAccountDispatcher { contract_address: account.starknet_address() }
Expand All @@ -249,15 +249,138 @@ mod tests {
use evm::model::Address;
use evm::model::account::Account;
use evm::state::{State, StateTrait};
use evm::test_utils::evm_address;
use evm::test_utils::{
setup_test_environment, uninitialized_account, account_contract, register_account
};
use evm::test_utils::{evm_address};
use snforge_std::{test_address, start_mock_call, get_class_hash};
use snforge_utils::snforge_utils::{assert_not_called, assert_called};
use super::commit_storage;
use utils::helpers::U8SpanExTrait;
use utils::helpers::compute_starknet_address;

// Helper function to create a test account
fn create_test_account(is_selfdestruct: bool, is_created: bool, id: felt252) -> Account {
let evm_address = (evm_address().into() + id).try_into().unwrap();
let starknet_address = (0x5678 + id).try_into().unwrap();
Account {
address: Address { evm: evm_address, starknet: starknet_address },
nonce: 0,
code: [].span(),
code_hash: 0,
balance: 0,
selfdestruct: is_selfdestruct,
is_created: is_created,
}
}

mod test_commit_storage {
use snforge_std::start_mock_call;
use snforge_utils::snforge_utils::{assert_called_with, assert_not_called};
use super::{create_test_account, StateTrait, commit_storage};

#[test]
fn test_commit_storage_normal_case() {
let mut state = Default::default();
let account = create_test_account(false, false, 0);
state.set_account(account);

let key = 0x100;
let value = 0x200;
state.write_state(account.address.evm, key, value);

// Mock the write_storage call
start_mock_call::<()>(account.address.starknet, selector!("write_storage"), ());

commit_storage(ref state).expect('commit storage failed');

//TODO(starknet-foundry): verify call args in assert_called
assert_called_with::<
(u256, u256)
>(account.address.starknet, selector!("write_storage"), (key, value));
}

#[test]
fn test_commit_storage_selfdestruct_and_created() {
let mut state = Default::default();
let account = create_test_account(true, true, 0);
state.set_account(account);

let key = 0x100;
let value = 0x200;
state.write_state(account.address.evm, key, value);

// Mock the write_storage call
start_mock_call::<()>(account.address.starknet, selector!("write_storage"), ());

commit_storage(ref state).expect('commit storage failed');

// Assert that write_storage was not called
assert_not_called(account.address.starknet, selector!("write_storage"));
}

#[test]
fn test_commit_storage_only_selfdestruct() {
let mut state = Default::default();
let account = create_test_account(true, false, 0);
state.set_account(account);

let key = 0x100;
let value = 0x200;
state.write_state(account.address.evm, key, value);

// Mock the write_storage call
start_mock_call::<()>(account.address.starknet, selector!("write_storage"), ());

commit_storage(ref state).expect('commit storage failed');

// Assert that write_storage was called
assert_called_with::<
(u256, u256)
>(account.address.starknet, selector!("write_storage"), (key, value));
}

#[test]
fn test_commit_storage_multiple_accounts() {
let mut state = Default::default();

// Account 0: Normal
let account0 = create_test_account(false, false, 0);
state.set_account(account0);

// Account 1: Selfdestruct and created
let account1 = create_test_account(true, true, 1);
state.set_account(account1);

// Account 2: Only selfdestruct
let account2 = create_test_account(true, false, 2);
state.set_account(account2);

// Set storage for all accounts
let key = 0x100;
let value = 0x200;
state.write_state(account0.address.evm, key, value);
state.write_state(account1.address.evm, key, value);
state.write_state(account2.address.evm, key, value);

// Mock the write_storage calls
start_mock_call::<()>(account0.address.starknet, selector!("write_storage"), ());
start_mock_call::<()>(account1.address.starknet, selector!("write_storage"), ());
start_mock_call::<()>(account2.address.starknet, selector!("write_storage"), ());

commit_storage(ref state).expect('commit storage failed');

// Assert that write_storage was called for accounts 1 and 3, but not for account 2
assert_called_with::<
(u256, u256)
>(account0.address.starknet, selector!("write_storage"), (key, value));
assert_not_called(account1.address.starknet, selector!("write_storage"));
assert_called_with::<
(u256, u256)
>(account2.address.starknet, selector!("write_storage"), (key, value));
}
}

#[test]
#[ignore]
//TODO(starknet-fonudry): it's impossible to deploy an un-declared class, nor is it possible to
Expand Down
4 changes: 2 additions & 2 deletions crates/snforge_utils/src/contracts.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ mod tests {
IHelloDispatcher { contract_address: hellocontract }.hello();

assert_called(hellocontract, selector!("hello"));
assert_called_with(hellocontract, selector!("hello"), [].span());
assert_called_with::<()>(hellocontract, selector!("hello"), ());
}

#[test]
Expand All @@ -46,6 +46,6 @@ mod tests {
IHelloDispatcher { contract_address: hellocontract }.bar();

assert_called(hellocontract, selector!("hello"));
assert_called_with(hellocontract, selector!("hello"), [].span());
assert_called_with::<()>(hellocontract, selector!("hello"), ());
}
}
8 changes: 5 additions & 3 deletions crates/snforge_utils/src/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,13 @@ pub mod snforge_utils {
}
}

pub fn assert_called_with(
contract_address: ContractAddress, selector: felt252, calldata: Span<felt252>
pub fn assert_called_with<C, +Serde<C>, +Drop<C>, +Copy<C>>(
contract_address: ContractAddress, selector: felt252, calldata: C
) {
let mut serialized_calldata = array![];
Serde::serialize(@calldata, ref serialized_calldata);
assert!(
is_called_with(contract_address, selector, calldata),
is_called_with(contract_address, selector, serialized_calldata.span()),
"Expected call with specific data not found in trace"
);
}
Expand Down
Loading