From de748a0876a1c13b50a2da3657b45b8823b6ae4f Mon Sep 17 00:00:00 2001 From: amos rothberg Date: Sun, 22 Sep 2024 12:25:35 +0300 Subject: [PATCH] chore(blockifier): added tests for TransactionResources::to_gas_vector --- crates/blockifier/src/fee/gas_usage_test.rs | 23 ++-- crates/blockifier/src/fee/receipt_test.rs | 105 +++++++++++------- .../blockifier/src/transaction/test_utils.rs | 41 ++++++- .../src/transaction/transactions_test.rs | 20 +++- 4 files changed, 135 insertions(+), 54 deletions(-) diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index 728e9daa7a..32c9422bb2 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -32,6 +32,8 @@ fn versioned_constants() -> &'static VersionedConstants { fn test_get_event_gas_cost( versioned_constants: &VersionedConstants, #[values(false, true)] use_kzg_da: bool, + #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] + gas_vector_computation_mode: GasVectorComputationMode, ) { let archival_data_gas_costs = &versioned_constants.archival_data_gas_costs; let (event_key_factor, data_word_cost) = @@ -45,7 +47,7 @@ fn test_get_event_gas_cost( starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ) ); @@ -80,21 +82,22 @@ fn test_get_event_gas_cost( }; let call_infos = vec![call_info_1, call_info_2, call_info_3]; let call_infos_iter = call_infos.iter(); - let expected = GasVector::from_l1_gas( - // 8 keys and 11 data words overall. - (data_word_cost - * versioned_constants.l1_to_l2_gas_price_ratio() - * (event_key_factor * 8_u128 + 11_u128)) - .to_integer(), - ); + // 8 keys and 11 data words overall. + let expected = (data_word_cost * (event_key_factor * 8_u128 + 11_u128)).to_integer(); + let expected_gas_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => { + GasVector::from_l1_gas(versioned_constants.convert_l2_to_l1_gas(expected)) + } + GasVectorComputationMode::All => GasVector::from_l2_gas(expected), + }; let starknet_resources = StarknetResources::new(0, 0, 0, StateChangesCount::default(), None, call_infos_iter); let gas_vector = starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); - assert_eq!(expected, gas_vector); + assert_eq!(expected_gas_vector, gas_vector); assert_ne!(GasVector::default(), gas_vector) } diff --git a/crates/blockifier/src/fee/receipt_test.rs b/crates/blockifier/src/fee/receipt_test.rs index 15a42e487e..282958a872 100644 --- a/crates/blockifier/src/fee/receipt_test.rs +++ b/crates/blockifier/src/fee/receipt_test.rs @@ -1,5 +1,5 @@ use rstest::{fixture, rstest}; -use starknet_api::transaction::{L2ToL1Payload, ValidResourceBounds}; +use starknet_api::transaction::L2ToL1Payload; use starknet_api::{invoke_tx_args, nonce}; use starknet_types_core::felt::Felt; @@ -25,7 +25,7 @@ use crate::transaction::objects::{ use crate::transaction::test_utils::{ account_invoke_tx, calculate_class_info_for_testing, - max_l1_resource_bounds, + get_resource_bounds, }; use crate::transaction::transactions::ExecutableTransaction; use crate::utils::{u128_from_usize, usize_from_u128}; @@ -49,14 +49,18 @@ fn versioned_constants() -> &'static VersionedConstants { // TODO(Aner, 29/01/24) Refactor with assert on GasVector objects. // TODO(Aner, 29/01/24) Refactor to replace match with if when formatting is nicer #[rstest] -fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool) { +fn test_calculate_tx_gas_usage_basic<'a>( + #[values(false, true)] use_kzg_da: bool, + #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] + gas_vector_computation_mode: GasVectorComputationMode, +) { // An empty transaction (a theoretical case for sanity check). let versioned_constants = VersionedConstants::create_for_account_testing(); let empty_tx_starknet_resources = StarknetResources::default(); let empty_tx_gas_usage_vector = empty_tx_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); assert_eq!(empty_tx_gas_usage_vector, GasVector::default()); @@ -72,19 +76,23 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool None, std::iter::empty(), ); - let code_gas_cost = versioned_constants.archival_data_gas_costs.gas_per_code_byte - * versioned_constants.l1_to_l2_gas_price_ratio() + let code_gas_cost = (versioned_constants.archival_data_gas_costs.gas_per_code_byte * u128_from_usize( (class_info.bytecode_length() + class_info.sierra_program_length()) * eth_gas_constants::WORD_WIDTH + class_info.abi_length(), - ); - let manual_gas_vector = - GasVector { l1_gas: code_gas_cost.to_integer(), ..Default::default() }; + )) + .to_integer(); + let manual_gas_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => { + GasVector::from_l1_gas(versioned_constants.convert_l2_to_l1_gas(code_gas_cost)) + } + GasVectorComputationMode::All => GasVector::from_l2_gas(code_gas_cost), + }; let declare_gas_usage_vector = declare_tx_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); assert_eq!(manual_gas_vector, declare_gas_usage_vector); } @@ -110,17 +118,22 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool std::iter::empty(), ); let calldata_and_signature_gas_cost = - versioned_constants.archival_data_gas_costs.gas_per_data_felt - * versioned_constants.l1_to_l2_gas_price_ratio() - * u128_from_usize(calldata_length + signature_length); - let manual_starknet_gas_usage = calldata_and_signature_gas_cost.to_integer(); - let manual_gas_vector = GasVector { l1_gas: manual_starknet_gas_usage, ..Default::default() } + (versioned_constants.archival_data_gas_costs.gas_per_data_felt + * u128_from_usize(calldata_length + signature_length)) + .to_integer(); + let manual_starknet_gas_usage_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas( + versioned_constants.convert_l2_to_l1_gas(calldata_and_signature_gas_cost), + ), + GasVectorComputationMode::All => GasVector::from_l2_gas(calldata_and_signature_gas_cost), + }; + let manual_gas_vector = manual_starknet_gas_usage_vector + deploy_account_tx_starknet_resources.get_state_changes_cost(use_kzg_da); let deploy_account_gas_usage_vector = deploy_account_tx_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); assert_eq!(manual_gas_vector, deploy_account_gas_usage_vector); @@ -138,26 +151,35 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let l1_handler_gas_usage_vector = l1_handler_tx_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); // Manual calculation. let message_segment_length = get_message_segment_length(&[], Some(l1_handler_payload_size)); let calldata_and_signature_gas_cost = - versioned_constants.archival_data_gas_costs.gas_per_data_felt - * versioned_constants.l1_to_l2_gas_price_ratio() - * u128_from_usize(l1_handler_payload_size + signature_length); - let manual_starknet_gas_usage = message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD + (versioned_constants.archival_data_gas_costs.gas_per_data_felt + * u128_from_usize(l1_handler_payload_size + signature_length)) + .to_integer(); + let calldata_and_signature_gas_cost_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas( + versioned_constants.convert_l2_to_l1_gas(calldata_and_signature_gas_cost), + ), + GasVectorComputationMode::All => GasVector::from_l2_gas(calldata_and_signature_gas_cost), + }; + let manual_starknet_l1_gas_usage = message_segment_length + * eth_gas_constants::GAS_PER_MEMORY_WORD + eth_gas_constants::GAS_PER_COUNTER_DECREASE + usize_from_u128( get_consumed_message_to_l2_emissions_cost(Some(l1_handler_payload_size)).l1_gas, ) - .unwrap() - + usize_from_u128(calldata_and_signature_gas_cost.to_integer()).unwrap(); + .unwrap(); + let manual_starknet_l1_gas_usage_vector = + GasVector::from_l1_gas(u128_from_usize(manual_starknet_l1_gas_usage)); let manual_sharp_gas_usage = message_segment_length * eth_gas_constants::SHARP_GAS_PER_MEMORY_WORD; - let manual_gas_computation = - GasVector::from_l1_gas(u128_from_usize(manual_starknet_gas_usage + manual_sharp_gas_usage)); + let manual_gas_computation = GasVector::from_l1_gas(u128_from_usize(manual_sharp_gas_usage)) + + manual_starknet_l1_gas_usage_vector + + calldata_and_signature_gas_cost_vector; assert_eq!(l1_handler_gas_usage_vector, manual_gas_computation); // Any transaction with L2-to-L1 messages. @@ -206,10 +228,11 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let l2_to_l1_messages_gas_usage_vector = l2_to_l1_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); // Manual calculation. + // No L2 gas is used, so gas amount does not depend on gas vector computation mode. let message_segment_length = get_message_segment_length(&l2_to_l1_payload_lengths, None); let n_l2_to_l1_messages = l2_to_l1_payload_lengths.len(); let manual_starknet_gas_usage = message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD @@ -252,10 +275,11 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let storage_writings_gas_usage_vector = storage_writes_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); // Manual calculation. + // No L2 gas is used, so gas amount does not depend on gas vector computation mode. let manual_gas_computation = storage_writes_starknet_resources.get_state_changes_cost(use_kzg_da); @@ -281,7 +305,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let gas_usage_vector = combined_cases_starknet_resources.to_gas_vector( &versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas, + &gas_vector_computation_mode, ); // Manual calculation. @@ -303,7 +327,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool l1_data_gas: combined_cases_starknet_resources .get_state_changes_cost(use_kzg_da) .l1_data_gas, - ..Default::default() + l2_gas: l1_handler_gas_usage_vector.l2_gas, }; assert_eq!(expected_gas_vector, gas_usage_vector); @@ -311,14 +335,15 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool // Test that we exclude the fee token contract modification and adds the account’s balance change // in the state changes. -// TODO(Aner, 21/01/24) modify for 4844 (taking blob_gas into account). // TODO(Nimrod, 1/5/2024): Test regression w.r.t. all resources (including VM). (Only starknet // resources are taken into account). #[rstest] fn test_calculate_tx_gas_usage( - max_l1_resource_bounds: ValidResourceBounds, #[values(false, true)] use_kzg_da: bool, + #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] + gas_vector_computation_mode: GasVectorComputationMode, ) { + let max_resource_bounds = get_resource_bounds(&gas_vector_computation_mode); let account_cairo_version = CairoVersion::Cairo0; let test_contract_cairo_version = CairoVersion::Cairo0; let block_context = &BlockContext::create_for_account_testing_with_kzg(use_kzg_da); @@ -332,7 +357,7 @@ fn test_calculate_tx_gas_usage( let account_tx = account_invoke_tx(invoke_tx_args! { sender_address: account_contract_address, calldata: create_trivial_calldata(test_contract.get_instance_address(0)), - resource_bounds: max_l1_resource_bounds, + resource_bounds: max_resource_bounds, }); let calldata_length = account_tx.calldata_length(); let signature_length = account_tx.signature_length(); @@ -360,12 +385,12 @@ fn test_calculate_tx_gas_usage( starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ), tx_execution_info.receipt.resources.starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ) ); @@ -382,7 +407,7 @@ fn test_calculate_tx_gas_usage( ); let account_tx = account_invoke_tx(invoke_tx_args! { - resource_bounds: max_l1_resource_bounds, + resource_bounds: max_resource_bounds, sender_address: account_contract_address, calldata: execute_calldata, nonce: nonce!(1_u8), @@ -401,6 +426,9 @@ fn test_calculate_tx_gas_usage( n_modified_contracts, n_compiled_class_hash_updates: 0, }; + let execution_call_info = + &tx_execution_info.execute_call_info.expect("Execution call info should exist."); + let execution_call_info_iter = vec![execution_call_info].into_iter(); let starknet_resources = StarknetResources::new( calldata_length, @@ -408,19 +436,20 @@ fn test_calculate_tx_gas_usage( 0, state_changes_count, None, - std::iter::empty(), + // The transfer entrypoint emits an event - pass the call info to count its resources. + execution_call_info_iter, ); assert_eq!( starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ), tx_execution_info.receipt.resources.starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ) ); } diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index bf40dcf832..21891f77f2 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -47,7 +47,12 @@ use crate::test_utils::{ }; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::constants; -use crate::transaction::objects::{FeeType, TransactionExecutionInfo, TransactionExecutionResult}; +use crate::transaction::objects::{ + FeeType, + GasVectorComputationMode, + TransactionExecutionInfo, + TransactionExecutionResult, +}; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{ExecutableTransaction, InvokeTransaction}; @@ -92,6 +97,22 @@ pub fn max_l1_resource_bounds() -> ValidResourceBounds { l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE) } +pub fn get_resource_bounds(computation_mode: &GasVectorComputationMode) -> ValidResourceBounds { + match computation_mode { + GasVectorComputationMode::NoL2Gas => { + l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE) + } + GasVectorComputationMode::All => get_all_resource_bounds( + MAX_L1_GAS_AMOUNT, + MAX_L1_GAS_PRICE, + DEFAULT_L2_GAS_MAX_AMOUNT, + DEFAULT_STRK_L2_GAS_PRICE, + DEFAULT_L1_DATA_GAS_MAX_AMOUNT, + DEFAULT_STRK_L1_DATA_GAS_PRICE, + ), + } +} + #[fixture] pub fn block_context() -> BlockContext { BlockContext::create_for_account_testing() @@ -315,6 +336,24 @@ pub fn all_resource_bounds( #[default(DEFAULT_STRK_L2_GAS_PRICE)] l2_max_price: u128, #[default(DEFAULT_L1_DATA_GAS_MAX_AMOUNT)] l1_data_max_amount: u64, #[default(DEFAULT_STRK_L1_DATA_GAS_PRICE)] l1_data_max_price: u128, +) -> ValidResourceBounds { + get_all_resource_bounds( + l1_max_amount, + l1_max_price, + l2_max_amount, + l2_max_price, + l1_data_max_amount, + l1_data_max_price, + ) +} + +fn get_all_resource_bounds( + l1_max_amount: u64, + l1_max_price: u128, + l2_max_amount: u64, + l2_max_price: u128, + l1_data_max_amount: u64, + l1_data_max_price: u128, ) -> ValidResourceBounds { ValidResourceBounds::AllResources(AllResourceBounds { l1_gas: ResourceBounds { max_amount: l1_max_amount, max_price_per_unit: l1_max_price }, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index fe96469f9b..ee20f08442 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1884,7 +1884,11 @@ fn test_only_query_flag( } #[rstest] -fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { +fn test_l1_handler( + #[values(false, true)] use_kzg_da: bool, + #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] + gas_vector_computation_mode: GasVectorComputationMode, +) { let cairo_version = CairoVersion::Cairo1; let test_contract = FeatureContract::TestContract(cairo_version); let chain_info = &ChainInfo::create_for_testing(); @@ -1931,9 +1935,15 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { // Build the expected resource mapping. // TODO(Nimrod, 1/5/2024): Change these hard coded values to match to the transaction resources // (currently matches only starknet resources). - let expected_gas = match use_kzg_da { - true => GasVector { l1_gas: 16023, l1_data_gas: 128, l2_gas: 0 }, - false => GasVector::from_l1_gas(17675), + let expected_gas = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => match use_kzg_da { + true => GasVector { l1_gas: 16023, l1_data_gas: 128, l2_gas: 0 }, + false => GasVector::from_l1_gas(17675), + }, + GasVectorComputationMode::All => match use_kzg_da { + true => GasVector { l1_gas: 16023, l1_data_gas: 128, l2_gas: 25 }, + false => GasVector { l1_gas: 17675, l1_data_gas: 0, l2_gas: 25 }, + }, }; let expected_da_gas = match use_kzg_da { true => GasVector::from_l1_data_gas(128), @@ -1978,7 +1988,7 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { actual_execution_info.receipt.resources.starknet_resources.to_gas_vector( versioned_constants, use_kzg_da, - &GasVectorComputationMode::NoL2Gas + &gas_vector_computation_mode ) );