Skip to content

Commit

Permalink
fix: effective gas price (#951)
Browse files Browse the repository at this point in the history
* fix: effective gas price

* fmt
  • Loading branch information
enitrat committed Sep 17, 2024
1 parent fed4554 commit d0cafae
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 59 deletions.
6 changes: 3 additions & 3 deletions crates/contracts/src/kakarot_core/eth_rpc.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ pub impl EthRPC<

let TransactionResult { success, return_data, gas_used, state: _state } =
EVMTrait::process_transaction(
ref kakarot_state, origin, tx, tx.effective_gas_price(Option::None), 0
ref kakarot_state, origin, tx, 0
);

(success, return_data, gas_used)
Expand All @@ -164,15 +164,15 @@ pub impl EthRPC<
ref self: TContractState, mut tx: Transaction
) -> (bool, Span<u8>, u64) {
let mut kakarot_state = KakarotState::get_state();
let (gas_price, intrinsic_gas) = validate_eth_tx(@kakarot_state, tx);
let intrinsic_gas = validate_eth_tx(@kakarot_state, tx);

let starknet_caller_address = get_caller_address();
let account = IAccountDispatcher { contract_address: starknet_caller_address };
let origin = Address { evm: account.get_evm_address(), starknet: starknet_caller_address };

let TransactionResult { success, return_data, gas_used, mut state } =
EVMTrait::process_transaction(
ref kakarot_state, origin, tx, gas_price, intrinsic_gas
ref kakarot_state, origin, tx, intrinsic_gas
);
starknet_backend::commit(ref state).expect('Committing state failed');
(success, return_data, gas_used)
Expand Down
21 changes: 11 additions & 10 deletions crates/evm/src/backend/validation.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ use evm::gas;
use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait};
use starknet::storage::StorageTrait;
use utils::constants::POW_2_32;
use utils::eth_transaction::get_effective_gas_price;
use utils::eth_transaction::check_gas_fee;
use utils::eth_transaction::transaction::{Transaction, TransactionTrait};

/// Validates the ethereum transaction by checking adherence to Ethereum rules regarding
/// Gas logic, nonce, chainId and required balance.
/// Returns
/// - Effective gas price of the transaction
/// - Intrinsic Gas Cost of the transcation
pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transaction) -> (u128, u64) {
///
/// # Returns
///
/// * The intrinsic gas cost of the transaction
pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transaction) -> u64 {
let kakarot_storage = kakarot_state.snapshot_deref().storage();
// Validate transaction

Expand All @@ -42,10 +43,11 @@ pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transacti
let gas_limit = tx.gas_limit();
assert(gas_limit <= kakarot_storage.Kakarot_block_gas_limit.read(), 'Tx gas > Block gas');
let block_base_fee = kakarot_storage.Kakarot_base_fee.read();
let effective_gas_price = get_effective_gas_price(
let gas_fee_check = check_gas_fee(
tx.max_fee_per_gas(), tx.max_priority_fee_per_gas(), block_base_fee.into()
);
assert!(effective_gas_price.is_ok(), "{:?}", effective_gas_price.unwrap_err());
assert!(gas_fee_check.is_ok(), "{:?}", gas_fee_check.unwrap_err());

// Intrinsic Gas
let intrinsic_gas = gas::calculate_intrinsic_gas_cost(@tx);
assert(gas_limit >= intrinsic_gas, 'Intrinsic gas > gas limit');
Expand All @@ -58,7 +60,7 @@ pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transacti
let max_gas_fee = tx.gas_limit().into() * tx.max_fee_per_gas();
let tx_cost = tx.value() + max_gas_fee.into();
assert(tx_cost <= balance, 'Not enough ETH');
(effective_gas_price.unwrap(), intrinsic_gas)
intrinsic_gas
}

#[cfg(test)]
Expand Down Expand Up @@ -129,9 +131,8 @@ mod tests {
);

// Test that the function performs validation and assert expected results
let (effective_gas_price, intrinsic_gas) = validate_eth_tx(@kakarot_state, tx);
let intrinsic_gas = validate_eth_tx(@kakarot_state, tx);

assert_eq!(effective_gas_price, 2_000_000_000); // max_fee_per_gas
assert_eq!(intrinsic_gas, 21000); // Standard intrinsic gas for a simple transfer
}

Expand Down
10 changes: 5 additions & 5 deletions crates/evm/src/interpreter.cairo
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use contracts::kakarot_core::KakarotCore;
use contracts::kakarot_core::interface::IKakarotCore;
use core::num::traits::Zero;
use core::ops::SnapshotDeref;
use core::starknet::EthAddress;
use core::starknet::get_tx_info;
use core::starknet::storage::{StoragePointerReadAccess};
use evm::backend::starknet_backend;
use evm::create_helpers::CreateHelpers;
use evm::errors::{EVMError, EVMErrorTrait, CONTRACT_ACCOUNT_EXISTS};
Expand Down Expand Up @@ -34,13 +36,11 @@ use utils::set::{Set, SetTrait};
#[generate_trait]
pub impl EVMImpl of EVMTrait {
fn process_transaction(
ref self: KakarotCore::ContractState,
origin: Address,
tx: Transaction,
gas_price: u128,
intrinsic_gas: u64
ref self: KakarotCore::ContractState, origin: Address, tx: Transaction, intrinsic_gas: u64
) -> TransactionResult {
// Charge the cost of intrinsic gas - which has been verified to be <= gas_limit.
let block_base_fee = self.snapshot_deref().Kakarot_base_fee.read();
let gas_price = tx.effective_gas_price(Option::Some(block_base_fee.into()));
let gas_left = tx.gas_limit() - intrinsic_gas;
let mut env = starknet_backend::get_env(origin.evm, gas_price);

Expand Down
57 changes: 16 additions & 41 deletions crates/utils/src/eth_transaction.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ pub struct TransactionMetadata {
pub signature: Signature,
}

/// Get the effective gas price of a transaction as specfified in EIP-1559 with relevant
/// Checks the effective gas price of a transaction as specfified in EIP-1559 with relevant
/// checks.
pub fn get_effective_gas_price(
pub fn check_gas_fee(
max_fee_per_gas: u128, max_priority_fee_per_gas: Option<u128>, block_base_fee: u128,
) -> Result<u128, EthTransactionError> {
) -> Result<(), EthTransactionError> {
let max_priority_fee_per_gas = max_priority_fee_per_gas.unwrap_or(0);

if max_fee_per_gas < block_base_fee {
Expand All @@ -35,61 +35,36 @@ pub fn get_effective_gas_price(
// `max_priority_fee_per_gas` is greater than the `max_fee_per_gas`
return Result::Err(EthTransactionError::TipAboveFeeCap);
}
Result::Ok(
min(
max_fee_per_gas,
block_base_fee
.checked_add(max_priority_fee_per_gas)
.ok_or(EthTransactionError::TipVeryHigh)?,
)
)

Result::Ok(())
}

#[cfg(test)]
mod tests {
use core::num::traits::Bounded;
use super::get_effective_gas_price;
use super::check_gas_fee;
use utils::errors::EthTransactionError;

#[test]
fn test_max_fee_superior_block_fee_should_return_effective_gas_price() {
let result = get_effective_gas_price(100, Option::Some(10), 50);
assert_eq!(result, Result::Ok(60));
fn test_happy_path() {
let result = check_gas_fee(100, Option::Some(10), 50);
assert!(result.is_ok());
}

#[test]
fn test_max_fee_equal_block_fee_plus_priority_fee_should_return_max_fee() {
let result = get_effective_gas_price(100, Option::Some(50), 50);
assert_eq!(result, Result::Ok(100));
}

#[test]
fn test_max_fee_inferior_block_fee_should_err() {
let result = get_effective_gas_price(40, Option::Some(10), 50);
fn test_fee_cap_too_low() {
let result = check_gas_fee(40, Option::Some(10), 50);
assert_eq!(result, Result::Err(EthTransactionError::FeeCapTooLow));
}

#[test]
fn test_max_fee_inferior_priority_fee_should_err() {
let result = get_effective_gas_price(100, Option::Some(110), 50);
fn test_tip_above_fee_cap() {
let result = check_gas_fee(100, Option::Some(110), 50);
assert_eq!(result, Result::Err(EthTransactionError::TipAboveFeeCap));
}

#[test]
fn test_block_fee_plus_priority_fee_overflow_should_err() {
let result = get_effective_gas_price(Bounded::MAX, Option::Some(1), Bounded::MAX);
assert_eq!(result, Result::Err(EthTransactionError::TipVeryHigh));
}

#[test]
fn test_priority_fee_none_should_use_zero() {
let result = get_effective_gas_price(100, Option::None, 50);
assert_eq!(result, Result::Ok(50));
}

#[test]
fn test_max_fee_equal_block_fee_less_than_total_should_return_max_fee() {
let result = get_effective_gas_price(50, Option::Some(10), 50);
assert_eq!(result, Result::Ok(50));
fn test_priority_fee_none() {
let result = check_gas_fee(100, Option::None, 50);
assert!(result.is_ok());
}
}

0 comments on commit d0cafae

Please sign in to comment.