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

refactor: change variable and function names for gas_vector #1400

Merged
merged 1 commit into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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_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_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
14 changes: 7 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,13 @@ pub fn calculate_tx_l1_gas_usages(
})
}

pub fn get_fee_by_l1_gas_usages(
pub fn get_fee_by_gas_vector(
block_info: &BlockInfo,
l1_gas_usages: GasVector,
gas_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(gas_vector.l1_gas * block_info.gas_prices.get_gas_price_by_fee_type(fee_type)
+ gas_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 +92,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 gas_vector = calculate_tx_gas_vector(resources, block_context)?;
Ok(get_fee_by_gas_vector(&block_context.block_info, gas_vector, fee_type))
}

/// Returns the current fee balance and a boolean indicating whether the balance covers the fee.
Expand Down
10 changes: 3 additions & 7 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_vector};
use crate::abi::constants;
use crate::block_context::BlockContext;
use crate::execution::call_info::{CallInfo, MessageL1CostInfo};
Expand Down Expand Up @@ -244,17 +244,13 @@ 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(
&block_context.block_info,
estimated_minimal_l1_gas,
&tx.fee_type(),
))
Ok(get_fee_by_gas_vector(&block_context.block_info, estimated_minimal_l1_gas, &tx.fee_type()))
}
10 changes: 5 additions & 5 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ 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::fee_utils::{get_fee_by_gas_vector, verify_can_pay_committed_bounds};
use crate::fee::gas_usage::estimate_minimal_l1_gas;
use crate::retdata;
use crate::state::cached_state::{CachedState, TransactionalState};
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_l1_gas(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_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
25 changes: 11 additions & 14 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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::fee_utils::{calculate_tx_gas_vector, get_fee_by_gas_vector};
use crate::fee::gas_usage::estimate_minimal_l1_gas;
use crate::state::cached_state::CachedState;
use crate::state::state_api::{State, StateReader};
Expand Down Expand Up @@ -331,14 +331,11 @@ fn test_max_fee_limit_validate(
resource_bounds: max_resource_bounds,
..tx_args.clone()
});
let estimated_min_l1_gas_and_blob_gas =
let estimated_min_gas_usage_vector =
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(
block_info,
estimated_min_l1_gas_and_blob_gas,
&account_tx.fee_type(),
);
let estimated_min_l1_gas = estimated_min_gas_usage_vector.l1_gas;
let estimated_min_fee =
get_fee_by_gas_vector(block_info, estimated_min_gas_usage_vector, &account_tx.fee_type());

let error = run_invoke_tx(
&mut state,
Expand Down Expand Up @@ -863,8 +860,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 +881,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 +891,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
12 changes: 4 additions & 8 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ use starknet_api::transaction::{Calldata, Fee, TransactionSignature, Transaction
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,
};
use crate::fee::fee_utils::{calculate_tx_fee, calculate_tx_gas_vector, get_fee_by_gas_vector};
use crate::invoke_tx_args;
use crate::state::cached_state::CachedState;
use crate::state::state_api::StateReader;
Expand Down Expand Up @@ -88,7 +86,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_vector(
&BlockContext::create_for_account_testing().block_info,
GasVector { l1_gas: gas.into(), blob_gas: 0 },
fee_type,
Expand All @@ -107,9 +105,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 +482,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_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,
gas_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(gas_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(gas_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
15 changes: 6 additions & 9 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,10 +1047,7 @@ fn declare_validate_callinfo(

/// Returns the expected used L1 gas and blob gas (according to use_kzg_da flag) due to execution of
/// a declare transaction.
fn declare_expected_l1_gas_usage_vector(
version: TransactionVersion,
use_kzg_da: bool,
) -> GasVector {
fn declare_expected_gas_vector(version: TransactionVersion, use_kzg_da: bool) -> GasVector {
let state_changes_count = match version {
TransactionVersion::ZERO => StateChangesCount {
n_storage_updates: 1, // Sender balance.
Expand Down Expand Up @@ -1140,7 +1137,7 @@ fn test_declare_tx(
);

let GasVector { l1_gas: expected_gas_usage, blob_gas: expected_blob_gas_usage } =
declare_expected_l1_gas_usage_vector(tx_version, use_kzg_da);
declare_expected_gas_vector(tx_version, use_kzg_da);

let expected_execution_info = TransactionExecutionInfo {
validate_call_info: expected_validate_call_info,
Expand Down Expand Up @@ -1509,10 +1506,10 @@ fn test_calculate_tx_gas_usage(#[values(false, true)] use_kzg_da: bool) {
n_compiled_class_hash_updates: 0,
};

let l1_gas_usage_vector =
let gas_vector =
calculate_tx_gas_usage_vector(std::iter::empty(), state_changes_count, None, use_kzg_da)
.unwrap();
let GasVector { l1_gas: l1_gas_usage, blob_gas: l1_blob_gas_usage } = l1_gas_usage_vector;
let GasVector { l1_gas: l1_gas_usage, blob_gas: l1_blob_gas_usage } = gas_vector;
assert_eq!(
u128_from_usize(tx_execution_info.actual_resources.gas_usage()).unwrap(),
l1_gas_usage
Expand Down Expand Up @@ -1554,10 +1551,10 @@ fn test_calculate_tx_gas_usage(#[values(false, true)] use_kzg_da: bool) {
n_compiled_class_hash_updates: 0,
};

let l1_gas_usage_vector =
let gas_vector =
calculate_tx_gas_usage_vector(std::iter::empty(), state_changes_count, None, use_kzg_da)
.unwrap();
let GasVector { l1_gas: l1_gas_usage, blob_gas: l1_blob_gas_usage } = l1_gas_usage_vector;
let GasVector { l1_gas: l1_gas_usage, blob_gas: l1_blob_gas_usage } = gas_vector;
assert_eq!(
u128_from_usize(tx_execution_info.actual_resources.gas_usage()).unwrap(),
l1_gas_usage
Expand Down
Loading