Skip to content

Commit

Permalink
fix(levm): change gas to return in post_execution_changes (#1464)
Browse files Browse the repository at this point in the history
**Motivation**

We have a problem when we need to refund the senders gas but it is too
high.

**Description**

This PR address an issue where we fail with `InternalState` error due to
an overflow when multiplying a very large number of `gas_to_return` by
`gas_price`. The overflow occur because the `u64` type cannot handle the
new value.

I change the name of the variable to something that is more related to
what it does: `gwei_return_amount`. Since this gas is what we are going
to add to the balance, it converts to `gwei` and we use `U256`

```rust
let gwei_return_amount = self
            .env
            .gas_price
            .checked_mul(U256::from(gas_to_return))
            .ok_or(VMError::Internal(InternalError::UndefinedState(1)))?;
self.increase_account_balance(sender_address, gwei_return_amount)?;
```
  • Loading branch information
damiramirez authored Dec 11, 2024
1 parent b585883 commit df63326
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
2 changes: 1 addition & 1 deletion crates/vm/levm/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub enum InternalError {
UtilsError,
#[error("PC out of bounds")]
PCOutOfBounds,
#[error("Undefined state")]
#[error("Undefined state: {0}")]
UndefinedState(i32), // This error is temporarily for things that cause an undefined state.
}

Expand Down
16 changes: 7 additions & 9 deletions crates/vm/levm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,14 +709,13 @@ impl VM {
.and_then(|gas| gas.checked_add(refunded_gas))
.ok_or(VMError::Internal(InternalError::UndefinedState(0)))?;

let gas_return_amount = self
let wei_return_amount = self
.env
.gas_price
.low_u64()
.checked_mul(gas_to_return)
.checked_mul(U256::from(gas_to_return))
.ok_or(VMError::Internal(InternalError::UndefinedState(1)))?;

self.increase_account_balance(sender_address, U256::from(gas_return_amount))?;
self.increase_account_balance(sender_address, wei_return_amount)?;

// 3. Pay coinbase fee
let coinbase_address = self.env.coinbase;
Expand All @@ -728,15 +727,14 @@ impl VM {
let priority_fee_per_gas = self
.env
.gas_price
.low_u64()
.checked_sub(self.env.base_fee_per_gas.low_u64())
.checked_sub(self.env.base_fee_per_gas)
.ok_or(VMError::GasPriceIsLowerThanBaseFee)?;
let coinbase_fee = gas_to_pay_coinbase
let coinbase_fee = U256::from(gas_to_pay_coinbase)
.checked_mul(priority_fee_per_gas)
.ok_or(VMError::BalanceOverflow)?;

if coinbase_fee != 0 {
self.increase_account_balance(coinbase_address, U256::from(coinbase_fee))?;
if coinbase_fee != U256::zero() {
self.increase_account_balance(coinbase_address, coinbase_fee)?;
};

// 4. Destruct addresses in selfdestruct set.
Expand Down

0 comments on commit df63326

Please sign in to comment.