Skip to content

Commit

Permalink
fix(levm): reserve gas for subcall in CREATE (#1532)
Browse files Browse the repository at this point in the history
**Motivation**

The gas for the subcall should be reserved before checking the validity
of the create, and shouldn't be returned in case we get an address
collision.

**Description**
- After doing the checks that can cause an OOG, we reserve the gas_limit
for the create subcall, if a check that shouldn't consume this reserved
gas fails, we return the gas, push a 0 and return. If the the address of
a new account has nonce or code, we don't return the gas, increment
nonce, push 0 and return.
- The address of the account to create should be inserted into the
"warm" account set, and remain even if we return because of a failed
check, so it is now done earlier and only removed from the set if the
new_call_frame results in a revert.

---------

Co-authored-by: Jeremías Salomón <[email protected]>
  • Loading branch information
LeanSerra and JereSalo authored Dec 19, 2024
1 parent f741744 commit fbf5f6d
Showing 1 changed file with 40 additions and 29 deletions.
69 changes: 40 additions & 29 deletions crates/vm/levm/src/opcode_handlers/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,32 +486,16 @@ impl VM {
return Err(VMError::OutOfGas(OutOfGasError::ConsumedGasOverflow));
}

// SECOND: Validations that push 0 to the stack
let deployer_address = current_call_frame.to;
// Reserve gas for subcall
let max_message_call_gas = max_message_call_gas(current_call_frame)?;
self.increase_consumed_gas(current_call_frame, max_message_call_gas.into())?;

let deployer_account_info = self.access_account(deployer_address).0;
// Clear callframe subreturn data
current_call_frame.sub_return_data = Bytes::new();

// 1. Sender doesn't have enough balance to send value.
if deployer_account_info.balance < value_in_wei_to_send {
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}

// 2. Depth limit has been reached
let new_depth = current_call_frame
.depth
.checked_add(1)
.ok_or(InternalError::ArithmeticOperationOverflow)?;
if new_depth > 1024 {
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}
let deployer_address = current_call_frame.to;

// 3. Sender nonce is max.
if deployer_account_info.nonce == u64::MAX {
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}
let deployer_account_info = self.access_account(deployer_address).0;

let code = Bytes::from(
memory::load_range(
Expand All @@ -527,15 +511,40 @@ impl VM {
None => Self::calculate_create_address(deployer_address, deployer_account_info.nonce)?,
};

// 3. Account has nonce or code.
// touch account
self.accrued_substate.touched_accounts.insert(new_address);

let new_depth = current_call_frame
.depth
.checked_add(1)
.ok_or(InternalError::ArithmeticOperationOverflow)?;
// SECOND: Validations that push 0 to the stack and return reserved_gas
// 1. Sender doesn't have enough balance to send value.
// 2. Depth limit has been reached
// 3. Sender nonce is max.
if deployer_account_info.balance < value_in_wei_to_send
|| new_depth > 1024
|| deployer_account_info.nonce == u64::MAX
{
// Return reserved gas
current_call_frame.gas_used = current_call_frame
.gas_used
.checked_sub(max_message_call_gas.into())
.ok_or(VMError::Internal(InternalError::GasOverflow))?;
// Push 0
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}

// THIRD: Validations that push 0 to the stack without returning reserved gas but incrementing deployer's nonce
let new_account = self.get_account(new_address);
if new_account.has_code_or_nonce() {
self.increment_account_nonce(deployer_address)?;
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}

// THIRD: Changes to the state
// FOURTH: Changes to the state
// 1. Creating contract.

// If the address has balance but there is no account associated with it, we need to add the value to it
Expand All @@ -552,7 +561,6 @@ impl VM {
// 3. Decrease sender's balance.
self.decrease_account_balance(deployer_address, value_in_wei_to_send)?;

let max_message_call_gas = max_message_call_gas(current_call_frame)?;
let mut new_call_frame = CallFrame::new(
deployer_address,
new_address,
Expand All @@ -568,14 +576,17 @@ impl VM {
);

self.accrued_substate.created_accounts.insert(new_address); // Mostly for SELFDESTRUCT during initcode.
self.accrued_substate.touched_accounts.insert(new_address);

let tx_report = self.execute(&mut new_call_frame)?;
let unused_gas = max_message_call_gas
.checked_sub(tx_report.gas_used)
.ok_or(InternalError::GasOverflow)?;

// Return reserved gas
current_call_frame.gas_used = current_call_frame
.gas_used
.checked_add(tx_report.gas_used.into())
.ok_or(VMError::OutOfGas(OutOfGasError::ConsumedGasOverflow))?;
.checked_sub(unused_gas.into())
.ok_or(InternalError::GasOverflow)?;
current_call_frame.logs.extend(tx_report.logs);

match tx_report.result {
Expand Down

0 comments on commit fbf5f6d

Please sign in to comment.