Skip to content

Commit

Permalink
fix: create restores correct state upon failure (#985)
Browse files Browse the repository at this point in the history
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->
**DEPENDS ON #982**
Time spent on this PR: 0.4d

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] 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?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Fixes restoration of the previous state upon CREATE failure. Previous
implementation did not take into account the possibliity that the
commitment of the code would fail.
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/985)
<!-- Reviewable:end -->

Co-authored-by: Clément Walter <clement0walter@gmail.com>
  • Loading branch information
enitrat and ClementWalter authored Feb 22, 2024
1 parent 1b86d8a commit a1f2456
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 15 deletions.
4 changes: 0 additions & 4 deletions blockchain-tests-skip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ testname:
- create2collisionStorage_d1g0v0_Shanghai
- create2collisionStorage_d2g0v0_Shanghai
stCreateTest:
- CreateAddressWarmAfterFail_d11g0v1_Shanghai
- CreateAddressWarmAfterFail_d12g0v1_Shanghai
- CreateAddressWarmAfterFail_d2g0v1_Shanghai
- CreateAddressWarmAfterFail_d3g0v1_Shanghai
- CreateOOGafterMaxCodesize_d2g0v0_Shanghai
- CreateOOGafterMaxCodesize_d3g0v0_Shanghai
- CreateOOGafterMaxCodesize_d5g0v0_Shanghai
Expand Down
24 changes: 18 additions & 6 deletions src/kakarot/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,14 @@ namespace SystemOperations {
let (bytecode: felt*) = alloc();
Memory.load_n(size.low, bytecode, offset.low);

// Get target address
let target_address = CreateHelper.get_evm_address(
evm.message.address.evm, popped_len, popped, size.low, bytecode
);

// @dev: performed before eventual subsequent early-returns of this function
// to mark the account as warm EIP-2929
let target_account = State.get_account(target_address);

// Get message call gas
let (gas_limit, _) = unsigned_div_rem(evm.gas_left, 64);
let gas_limit = evm.gas_left - gas_limit;
Expand All @@ -95,11 +98,6 @@ namespace SystemOperations {
return evm;
}

// TODO: Clear return data
// @dev: performed before eventual early-returns of this function
// to mark the account as warm
let target_account = State.get_account(target_address);

// Check sender balance and nonce
let sender = State.get_account(evm.message.address.evm);
let is_nonce_overflow = Helpers.is_zero(Constants.MAX_NONCE - sender.nonce);
Expand Down Expand Up @@ -878,6 +876,14 @@ namespace CallHelper {
let is_reverted = is_not_zero(evm.reverted);
Stack.push_uint128(1 - is_reverted);

// Restore parent state if the call has reverted
if (evm.reverted != FALSE) {
tempvar state = evm.message.parent.state;
} else {
tempvar state = state;
}
let state = cast([ap - 1], model.State*);

if (evm.reverted == Errors.EXCEPTIONAL_HALT) {
// If the call has halted exceptionnaly, the return_data is empty
// and nothing is copied to memory, and the gas is not returned;
Expand Down Expand Up @@ -1050,6 +1056,7 @@ namespace CreateHelper {
}

// @notice At the end of a sub-context initiated with CREATE or CREATE2, the calling context's stack is updated.
// @dev Restores the parent state if the sub-context has reverted.
// @param evm The pointer to the calling context.
// @return EVM The pointer to the updated calling context.
func finalize_parent{
Expand All @@ -1074,6 +1081,9 @@ namespace CreateHelper {
tempvar stack_code = new Uint256(low=0, high=0);
Stack.push(stack_code);
tempvar state = evm.message.parent.state;
tempvar evm = new model.EVM(
message=evm.message.parent.evm.message,
return_data_len=return_data_len,
Expand Down Expand Up @@ -1107,6 +1117,8 @@ namespace CreateHelper {
Stack.push(address);

if (success == FALSE) {
tempvar state = evm.message.parent.state;
tempvar evm = new model.EVM(
message=evm.message.parent.evm.message,
return_data_len=0,
Expand Down
5 changes: 0 additions & 5 deletions src/kakarot/interpreter.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -710,11 +710,6 @@ namespace Interpreter {

let stack = evm.message.parent.stack;
let memory = evm.message.parent.memory;
if (evm.reverted == FALSE) {
tempvar state = state;
} else {
tempvar state = evm.message.parent.state;
}

if (evm.message.is_create != FALSE) {
let evm = CreateHelper.finalize_parent(evm);
Expand Down

0 comments on commit a1f2456

Please sign in to comment.