Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
refactor: change variable and function names for gas_vector
Browse files Browse the repository at this point in the history
  • Loading branch information
aner-starkware committed Jan 30, 2024
1 parent 2e445ae commit 1b8b9c2
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 49 deletions.
6 changes: 3 additions & 3 deletions crates/blockifier/src/fee/fee_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use thiserror::Error;
use crate::block_context::{BlockContext, BlockInfo, ChainInfo};
use crate::fee::actual_cost::ActualCost;
use crate::fee::fee_utils::{
calculate_tx_l1_gas_usages, get_balance_and_if_covers_fee, get_fee_by_l1_gas_usages,
calculate_tx_gas_vector, get_balance_and_if_covers_fee, get_fee_by_gas_usage_vector,
};
use crate::state::state_api::StateReader;
use crate::transaction::errors::TransactionExecutionError;
Expand Down Expand Up @@ -72,7 +72,7 @@ impl FeeCheckReport {
// resource bounds), the sender should be able to pay this fee.
FeeCheckError::MaxFeeExceeded { .. } | FeeCheckError::MaxL1GasAmountExceeded { .. } => {
match account_tx_context {
AccountTransactionContext::Current(context) => get_fee_by_l1_gas_usages(
AccountTransactionContext::Current(context) => get_fee_by_gas_usage_vector(
block_info,
GasVector {
l1_gas: context.l1_resource_bounds()?.max_amount.into(),
Expand Down Expand Up @@ -104,7 +104,7 @@ impl FeeCheckReport {
let max_l1_gas = context.l1_resource_bounds()?.max_amount.into();

let GasVector { l1_gas: gas_usage, blob_gas: blob_gas_usage } =
calculate_tx_l1_gas_usages(actual_resources, block_context)?;
calculate_tx_gas_vector(actual_resources, block_context)?;

// TODO(Dori, 1/7/2024): When data gas limit is added (and enforced) in resource
// bounds, check it here as well (separately, with a different error variant if
Expand Down
15 changes: 8 additions & 7 deletions crates/blockifier/src/fee/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn calculate_l1_gas_by_vm_usage(
/// Computes and returns the total L1 gas consumption.
/// We add the l1_gas_usage (which may include, for example, the direct cost of L2-to-L1 messages)
/// to the gas consumed by Cairo VM resource.
pub fn calculate_tx_l1_gas_usages(
pub fn calculate_tx_gas_vector(
resources: &ResourcesMapping,
block_context: &BlockContext,
) -> TransactionFeeResult<GasVector> {
Expand All @@ -77,13 +77,14 @@ pub fn calculate_tx_l1_gas_usages(
})
}

pub fn get_fee_by_l1_gas_usages(
pub fn get_fee_by_gas_usage_vector(
block_info: &BlockInfo,
l1_gas_usages: GasVector,
l1_gas_usage_vector: GasVector,
fee_type: &FeeType,
) -> Fee {
Fee(l1_gas_usages.l1_gas * block_info.gas_prices.get_gas_price_by_fee_type(fee_type)
+ l1_gas_usages.blob_gas * block_info.gas_prices.get_data_gas_price_by_fee_type(fee_type))
Fee(l1_gas_usage_vector.l1_gas * block_info.gas_prices.get_gas_price_by_fee_type(fee_type)
+ l1_gas_usage_vector.blob_gas
* block_info.gas_prices.get_data_gas_price_by_fee_type(fee_type))
}

/// Calculates the fee that should be charged, given execution resources.
Expand All @@ -92,8 +93,8 @@ pub fn calculate_tx_fee(
block_context: &BlockContext,
fee_type: &FeeType,
) -> TransactionFeeResult<Fee> {
let l1_gas_usage_vector = calculate_tx_l1_gas_usages(resources, block_context)?;
Ok(get_fee_by_l1_gas_usages(&block_context.block_info, l1_gas_usage_vector, fee_type))
let l1_gas_usage_vector = calculate_tx_gas_vector(resources, block_context)?;
Ok(get_fee_by_gas_usage_vector(&block_context.block_info, l1_gas_usage_vector, fee_type))
}

/// Returns the current fee balance and a boolean indicating whether the balance covers the fee.
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/fee/gas_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use starknet_api::transaction::Fee;

use super::fee_utils::{calculate_tx_l1_gas_usages, get_fee_by_l1_gas_usages};
use super::fee_utils::{calculate_tx_gas_vector, get_fee_by_gas_usage_vector};
use crate::abi::constants;
use crate::block_context::BlockContext;
use crate::execution::call_info::{CallInfo, MessageL1CostInfo};
Expand Down Expand Up @@ -244,15 +244,15 @@ pub fn estimate_minimal_l1_gas(
(constants::N_STEPS_RESOURCE.to_string(), os_steps_for_type),
]));

Ok(calculate_tx_l1_gas_usages(&resources, block_context)?)
Ok(calculate_tx_gas_vector(&resources, block_context)?)
}

pub fn estimate_minimal_fee(
block_context: &BlockContext,
tx: &AccountTransaction,
) -> TransactionExecutionResult<Fee> {
let estimated_minimal_l1_gas = estimate_minimal_l1_gas(block_context, tx)?;
Ok(get_fee_by_l1_gas_usages(
Ok(get_fee_by_gas_usage_vector(
&block_context.block_info,
estimated_minimal_l1_gas,
&tx.fee_type(),
Expand Down
12 changes: 6 additions & 6 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use crate::execution::entry_point::{
};
use crate::fee::actual_cost::{ActualCost, ActualCostBuilder};
use crate::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport};
use crate::fee::fee_utils::{get_fee_by_l1_gas_usages, verify_can_pay_committed_bounds};
use crate::fee::gas_usage::estimate_minimal_l1_gas;
use crate::fee::fee_utils::{get_fee_by_gas_usage_vector, verify_can_pay_committed_bounds};
use crate::fee::gas_usage::estimate_minimal_gas_vector;
use crate::retdata;
use crate::state::cached_state::{CachedState, TransactionalState};
use crate::state::state_api::{State, StateReader};
Expand Down Expand Up @@ -178,8 +178,8 @@ impl AccountTransaction {
block_context: &BlockContext,
) -> TransactionPreValidationResult<()> {
// TODO(Aner, 21/01/24) modify for 4844 (blob_gas).
let minimal_l1_gas_and_blob_gas_amount = estimate_minimal_l1_gas(block_context, self)?;
let minimal_l1_gas_amount = minimal_l1_gas_and_blob_gas_amount.l1_gas;
let minimal_l1_gas_amount_vector = estimate_minimal_gas_vector(block_context, self)?;
let minimal_l1_gas_amount = minimal_l1_gas_amount_vector.l1_gas;
let block_info = &block_context.block_info;

match account_tx_context {
Expand Down Expand Up @@ -212,9 +212,9 @@ impl AccountTransaction {
}
AccountTransactionContext::Deprecated(context) => {
let max_fee = context.max_fee;
let min_fee = get_fee_by_l1_gas_usages(
let min_fee = get_fee_by_gas_usage_vector(
block_info,
minimal_l1_gas_and_blob_gas_amount,
minimal_l1_gas_amount_vector,
&account_tx_context.fee_type(),
);
if max_fee < min_fee {
Expand Down
26 changes: 13 additions & 13 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ use crate::execution::contract_class::{ContractClass, ContractClassV1};
use crate::execution::entry_point::EntryPointExecutionContext;
use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecutionError};
use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
use crate::fee::fee_utils::{calculate_tx_l1_gas_usages, get_fee_by_l1_gas_usages};
use crate::fee::gas_usage::estimate_minimal_l1_gas;
use crate::fee::fee_utils::{calculate_tx_gas_vector, get_fee_by_gas_usage_vector};
use crate::fee::gas_usage::estimate_minimal_gas_vector;
use crate::state::cached_state::CachedState;
use crate::state::state_api::{State, StateReader};
use crate::test_utils::contracts::FeatureContract;
Expand Down Expand Up @@ -331,12 +331,12 @@ fn test_max_fee_limit_validate(
resource_bounds: max_resource_bounds,
..tx_args.clone()
});
let estimated_min_l1_gas_and_blob_gas =
estimate_minimal_l1_gas(&block_context, &account_tx).unwrap();
let estimated_min_l1_gas = estimated_min_l1_gas_and_blob_gas.l1_gas;
let estimated_min_fee = get_fee_by_l1_gas_usages(
let estimated_min_gas_usage_vector =
estimate_minimal_gas_vector(&block_context, &account_tx).unwrap();
let estimated_min_l1_gas = estimated_min_gas_usage_vector.l1_gas;
let estimated_min_fee = get_fee_by_gas_usage_vector(
block_info,
estimated_min_l1_gas_and_blob_gas,
estimated_min_gas_usage_vector,
&account_tx.fee_type(),
);

Expand Down Expand Up @@ -863,8 +863,8 @@ fn test_max_fee_to_max_steps_conversion(
let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps();
let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap();
let n_steps1 = tx_execution_info1.actual_resources.n_steps();
let gas_and_blob_gas_used1 =
calculate_tx_l1_gas_usages(&tx_execution_info1.actual_resources, &block_context).unwrap();
let gas_used_vector1 =
calculate_tx_gas_vector(&tx_execution_info1.actual_resources, &block_context).unwrap();

// Second invocation of `with_arg` gets twice the pre-calculated actual fee as max_fee.
let account_tx2 = account_invoke_tx(invoke_tx_args! {
Expand All @@ -884,8 +884,8 @@ fn test_max_fee_to_max_steps_conversion(
let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps();
let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap();
let n_steps2 = tx_execution_info2.actual_resources.n_steps();
let gas_and_blob_gas_used2 =
calculate_tx_l1_gas_usages(&tx_execution_info2.actual_resources, &block_context).unwrap();
let gas_used_vector2 =
calculate_tx_gas_vector(&tx_execution_info2.actual_resources, &block_context).unwrap();

// Test that steps limit doubles as max_fee doubles, but actual consumed steps and fee remains.
assert_eq!(max_steps_limit2.unwrap(), 2 * max_steps_limit1.unwrap());
Expand All @@ -894,11 +894,11 @@ fn test_max_fee_to_max_steps_conversion(
// TODO(Aner, 21/01/24): verify test compliant with 4844 (or modify accordingly).
assert_eq!(
actual_gas_used,
u64::try_from(gas_and_blob_gas_used2.l1_gas).expect("Failed to convert u128 to u64.")
u64::try_from(gas_used_vector2.l1_gas).expect("Failed to convert u128 to u64.")
);
assert_eq!(actual_fee, tx_execution_info2.actual_fee.0);
assert_eq!(n_steps1, n_steps2);
assert_eq!(gas_and_blob_gas_used1, gas_and_blob_gas_used2);
assert_eq!(gas_used_vector1, gas_used_vector2);
}

#[rstest]
Expand Down
10 changes: 4 additions & 6 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::block_context::{BlockContext, ChainInfo};
use crate::execution::errors::EntryPointExecutionError;
use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
use crate::fee::fee_utils::{
calculate_tx_fee, calculate_tx_l1_gas_usages, get_fee_by_l1_gas_usages,
calculate_tx_fee, calculate_tx_gas_vector, get_fee_by_gas_usage_vector,
};
use crate::invoke_tx_args;
use crate::state::cached_state::CachedState;
Expand Down Expand Up @@ -88,7 +88,7 @@ fn gas_and_fee(base_gas: u64, validate_mode: bool, fee_type: &FeeType) -> (u64,
let gas = base_gas + if validate_mode { VALIDATE_GAS_OVERHEAD } else { 0 };
(
gas,
get_fee_by_l1_gas_usages(
get_fee_by_gas_usage_vector(
&BlockContext::create_for_account_testing().block_info,
GasVector { l1_gas: gas.into(), blob_gas: 0 },
fee_type,
Expand All @@ -107,9 +107,7 @@ fn check_gas_and_fee(
expected_cost_of_resources: Fee,
) {
assert_eq!(
calculate_tx_l1_gas_usages(&tx_execution_info.actual_resources, block_context)
.unwrap()
.l1_gas,
calculate_tx_gas_vector(&tx_execution_info.actual_resources, block_context).unwrap().l1_gas,
expected_actual_gas.into()
);
assert_eq!(tx_execution_info.actual_fee, expected_actual_fee);
Expand Down Expand Up @@ -486,7 +484,7 @@ fn test_simulate_validate_charge_fee_mid_execution(
let invoke_tx_max_n_steps_as_u64: u64 =
low_step_block_context.block_info.invoke_tx_max_n_steps.into();
let block_limit_gas = invoke_tx_max_n_steps_as_u64 + 1720;
let block_limit_fee = get_fee_by_l1_gas_usages(
let block_limit_fee = get_fee_by_gas_usage_vector(
&block_context.block_info,
GasVector { l1_gas: block_limit_gas.into(), blob_gas: 0 },
&fee_type,
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/transaction/post_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use starknet_crypto::FieldElement;

use crate::block_context::{BlockContext, ChainInfo};
use crate::fee::fee_checks::FeeCheckError;
use crate::fee::fee_utils::calculate_tx_l1_gas_usages;
use crate::fee::fee_utils::calculate_tx_gas_vector;
use crate::invoke_tx_args;
use crate::state::state_api::StateReader;
use crate::test_utils::contracts::FeatureContract;
Expand Down Expand Up @@ -258,7 +258,7 @@ fn test_revert_on_resource_overuse(
let actual_fee = execution_info_measure.actual_fee;
// TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works.
let actual_gas_usage: u64 =
calculate_tx_l1_gas_usages(&execution_info_measure.actual_resources, &block_context)
calculate_tx_gas_vector(&execution_info_measure.actual_resources, &block_context)
.unwrap()
.l1_gas
.try_into()
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/transaction/transaction_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ use crate::utils::{merge_hashmaps, usize_from_u128};
/// I.e., Cairo VM execution resources.
pub fn calculate_tx_resources(
execution_resources: &ExecutionResources,
l1_gas_usages: GasVector,
l1_gas_usage_vector: GasVector,
tx_type: TransactionType,
calldata_length: usize,
) -> TransactionExecutionResult<ResourcesMapping> {
let l1_gas_usage = usize_from_u128(l1_gas_usages.l1_gas)
let l1_gas_usage = usize_from_u128(l1_gas_usage_vector.l1_gas)
.expect("This conversion should not fail as the value is a converted usize.");
let l1_blob_gas_usage = usize_from_u128(l1_gas_usages.blob_gas)
let l1_blob_gas_usage = usize_from_u128(l1_gas_usage_vector.blob_gas)
.expect("This conversion should not fail as the value is a converted usize.");
// Add additional Cairo resources needed for the OS to run the transaction.
let total_vm_usage = &execution_resources.vm_resources
Expand Down
14 changes: 8 additions & 6 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::execution::errors::{EntryPointExecutionError, VirtualMachineExecution
use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
use crate::fee::fee_utils::calculate_tx_fee;
use crate::fee::gas_usage::{
calculate_tx_blob_gas_usage, calculate_tx_gas_usage_vector, estimate_minimal_l1_gas,
calculate_tx_blob_gas_usage, calculate_tx_gas_usage_vector, estimate_minimal_gas_vector,
get_onchain_data_cost,
};
use crate::state::cached_state::{CachedState, StateChangesCount};
Expand Down Expand Up @@ -809,10 +809,12 @@ fn test_insufficient_resource_bounds(account_cairo_version: CairoVersion) {
);

// The minimal gas estimate does not depend on tx version.
let minimal_l1_gas =
estimate_minimal_l1_gas(block_context, &account_invoke_tx(valid_invoke_tx_args.clone()))
.unwrap()
.l1_gas;
let minimal_l1_gas = estimate_minimal_gas_vector(
block_context,
&account_invoke_tx(valid_invoke_tx_args.clone()),
)
.unwrap()
.l1_gas;

// Test V1 transaction.

Expand Down Expand Up @@ -899,7 +901,7 @@ fn test_actual_fee_gt_resource_bounds(account_cairo_version: CairoVersion) {
);

let minimal_l1_gas =
estimate_minimal_l1_gas(block_context, &account_invoke_tx(invoke_tx_args.clone()))
estimate_minimal_gas_vector(block_context, &account_invoke_tx(invoke_tx_args.clone()))
.unwrap()
.l1_gas;
let minimal_fee = Fee(minimal_l1_gas * block_context.block_info.gas_prices.eth_l1_gas_price);
Expand Down

0 comments on commit 1b8b9c2

Please sign in to comment.