Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Commit

Permalink
Fix ERC20 error due to newly deployed SELFDESTRUCT contracts (#805)
Browse files Browse the repository at this point in the history
Time spent on this PR: 0.2

## Pull request type

Please check the type of change your PR introduces:

- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

When a newly deployed contract calls SELFDESTRUCT, it's never deployed
on Starknet and consequently
Kakarot is not allowed to `transferFrom` its contract.

The Cairo VM hence raises in `state.commit`.

## What is the new behavior?

In `State.commit`, when executing transfer to a newly deployed
selfdestructed contract, uses
the Kakarot contract instead as temporary ETH holders.

Note: I have eventually realized that separating the balance from the
rest of the Account "to save on loading bytecode just to send ETH" is
indeed a nonsense because there is only two ways to send ETH without
requiring the bytecode:
- the initial `value` of a tx, hence the sender is an EOA and loading
its bytecode is just loading empty code so cheap
- the recipient of SELFDESTRUCT

Consequently, I think that the balance would be better re-integrated
into the Account, but I don't know if the refacto is worth it per se.
Maybe if we remove completely the ERC20, then we can do it in the
meantime.
  • Loading branch information
ClementWalter authored Nov 11, 2023
1 parent 71c140a commit b3b353b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 10 deletions.
10 changes: 5 additions & 5 deletions src/kakarot/account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ namespace Account {

// Case new Account
if (starknet_account_exists == 0) {
// If SELFDESTRUCT, just do nothing
if (self.selfdestruct != 0) {
return ();
}

// Just casting the Summary into an Account to apply has_code_or_nonce
// cf Summary note: like an Account, but frozen after squashing all dicts
// There is no reason to have has_code_or_nonce available in the public API
Expand All @@ -135,6 +130,11 @@ namespace Account {
// Deploy accounts
let (class_hash) = contract_account_class_hash.read();
deploy(class_hash, self.address);
// If SELFDESTRUCT, stops here to leave the account empty
if (self.selfdestruct != 0) {
return ();
}

// Write bytecode
IContractAccount.write_bytecode(starknet_address, self.code_len, self.code);
// Set nonce
Expand Down
7 changes: 3 additions & 4 deletions src/kakarot/library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,10 @@ namespace Kakarot {
let (state, success) = State.add_transfer(state, transfer);

// Check collision
let is_registered = Account.is_registered(address.evm);
let is_collision = is_registered * is_deploy_tx;

// Nonce is set to 1 in case of deploy_tx
let account = Account.fetch_or_create(address);
let code_or_nonce = Account.has_code_or_nonce(account);
let is_collision = code_or_nonce * is_deploy_tx;
// Nonce is set to 1 in case of deploy_tx
let nonce = account.nonce * (1 - is_deploy_tx) + is_deploy_tx;
let account = Account.set_nonce(account, nonce);
let state = State.set_account(state, address, account);
Expand Down
1 change: 0 additions & 1 deletion src/kakarot/state.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ namespace Internals {

// @notice Iterates through a list of Transfer and makes them
// @dev Transfers are made last so as to have all accounts created beforehand.
// Kakarot is not authorized for accounts that are created and SELDESTRUCT in the same transaction
// @param transfers_len The length of the transfers array.
// @param transfers The array of Transfer.
func _transfer_eth{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
Expand Down

0 comments on commit b3b353b

Please sign in to comment.