From 080f3d6ca2920b473fabdd7374ced1b34cd838ce Mon Sep 17 00:00:00 2001 From: Nimrod Weiss Date: Wed, 14 Aug 2024 14:06:02 +0300 Subject: [PATCH] build(fee): sn_resources depend on resource bounds signature --- Cargo.lock | 12 +++ Cargo.toml | 1 + crates/blockifier/Cargo.toml | 1 + crates/blockifier/src/fee/actual_cost_test.rs | 49 ++++++------ crates/blockifier/src/fee/gas_usage_test.rs | 10 ++- crates/blockifier/src/test_utils.rs | 6 ++ crates/blockifier/src/transaction/objects.rs | 74 ++++++++++++------- .../src/transaction/transactions_test.rs | 10 +-- 8 files changed, 107 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78215043df1..11a41448dd7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1000,6 +1000,7 @@ dependencies = [ "rand 0.8.5", "regex", "rstest", + "rstest_reuse", "serde", "serde_json", "sha2", @@ -7933,6 +7934,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "rstest_reuse" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3a8fb4672e840a587a66fc577a5491375df51ddb88f2a2c2a792598c326fe14" +dependencies = [ + "quote", + "rand 0.8.5", + "syn 2.0.61", +] + [[package]] name = "rtnetlink" version = "0.10.1" diff --git a/Cargo.toml b/Cargo.toml index a7bd5c43fa8..724679e1ff9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -167,6 +167,7 @@ regex = "1.10.4" replace_with = "0.1.7" reqwest = { version = "0.11", features = ["blocking", "json"] } rstest = "0.17.0" +rstest_reuse = "0.7.0" rustc-hex = "2.1.0" schemars = "0.8.12" serde = "1.0.197" diff --git a/crates/blockifier/Cargo.toml b/crates/blockifier/Cargo.toml index 906b0056c58..c02b141416b 100644 --- a/crates/blockifier/Cargo.toml +++ b/crates/blockifier/Cargo.toml @@ -43,6 +43,7 @@ paste.workspace = true phf.workspace = true rand = { workspace = true, optional = true } rstest = { workspace = true, optional = true } +rstest_reuse.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision"] } sha2.workspace = true diff --git a/crates/blockifier/src/fee/actual_cost_test.rs b/crates/blockifier/src/fee/actual_cost_test.rs index 46d812d3597..e681d60889d 100644 --- a/crates/blockifier/src/fee/actual_cost_test.rs +++ b/crates/blockifier/src/fee/actual_cost_test.rs @@ -13,7 +13,13 @@ use crate::fee::gas_usage::{ use crate::state::cached_state::StateChangesCount; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::test_state; -use crate::test_utils::{create_calldata, create_trivial_calldata, CairoVersion, BALANCE}; +use crate::test_utils::{ + create_calldata, + create_trivial_calldata, + include_l2_gas, + CairoVersion, + BALANCE, +}; use crate::transaction::constants; use crate::transaction::objects::{GasVector, HasRelatedFeeType, StarknetResources}; use crate::transaction::test_utils::{ @@ -48,7 +54,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let versioned_constants = VersionedConstants::default(); 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); + empty_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); assert_eq!(empty_tx_gas_usage_vector, GasVector::default()); // Declare. @@ -72,7 +78,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool let manual_gas_vector = GasVector { l1_gas: code_gas_cost.to_integer(), ..Default::default() }; let declare_gas_usage_vector = - declare_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da); + declare_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); assert_eq!(manual_gas_vector, declare_gas_usage_vector); } @@ -104,7 +110,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool + 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); + deploy_account_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); assert_eq!(manual_gas_vector, deploy_account_gas_usage_vector); // L1 handler. @@ -119,7 +125,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool std::iter::empty(), ); let l1_handler_gas_usage_vector = - l1_handler_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da); + l1_handler_tx_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); // Manual calculation. let message_segment_length = get_message_segment_length(&[], Some(l1_handler_payload_size)); @@ -183,7 +189,7 @@ 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); + l2_to_l1_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); // Manual calculation. let message_segment_length = get_message_segment_length(&l2_to_l1_payload_lengths, None); @@ -226,7 +232,7 @@ 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); + storage_writes_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); // Manual calculation. let manual_gas_computation = @@ -252,7 +258,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); + combined_cases_starknet_resources.to_gas_vector(&versioned_constants, use_kzg_da, false); // Manual calculation. let fee_balance_discount = match use_kzg_da { @@ -284,10 +290,11 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool // 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] +#[rstest_reuse::apply(include_l2_gas)] fn test_calculate_tx_gas_usage( max_resource_bounds: ValidResourceBounds, #[values(false, true)] use_kzg_da: bool, + has_l2_gas: bool, ) { let account_cairo_version = CairoVersion::Cairo0; let test_contract_cairo_version = CairoVersion::Cairo0; @@ -327,12 +334,12 @@ fn test_calculate_tx_gas_usage( ); assert_eq!( - starknet_resources.to_gas_vector(versioned_constants, use_kzg_da), - tx_execution_info - .receipt - .resources - .starknet_resources - .to_gas_vector(versioned_constants, use_kzg_da) + starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, has_l2_gas), + tx_execution_info.receipt.resources.starknet_resources.to_gas_vector( + versioned_constants, + use_kzg_da, + has_l2_gas + ) ); // A tx that changes the account and some other balance in execute. @@ -378,11 +385,11 @@ fn test_calculate_tx_gas_usage( ); assert_eq!( - starknet_resources.to_gas_vector(versioned_constants, use_kzg_da), - tx_execution_info - .receipt - .resources - .starknet_resources - .to_gas_vector(versioned_constants, use_kzg_da) + starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, has_l2_gas), + tx_execution_info.receipt.resources.starknet_resources.to_gas_vector( + versioned_constants, + use_kzg_da, + has_l2_gas + ) ); } diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index 37c1c250f0f..abca5a0ac2e 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -17,7 +17,7 @@ use crate::fee::gas_usage::{ }; use crate::invoke_tx_args; use crate::state::cached_state::StateChangesCount; -use crate::test_utils::{DEFAULT_ETH_L1_DATA_GAS_PRICE, DEFAULT_ETH_L1_GAS_PRICE}; +use crate::test_utils::{include_l2_gas, DEFAULT_ETH_L1_DATA_GAS_PRICE, DEFAULT_ETH_L1_GAS_PRICE}; use crate::transaction::objects::{FeeType, GasVector, StarknetResources}; use crate::transaction::test_utils::account_invoke_tx; use crate::utils::{u128_div_ceil, u128_from_usize}; @@ -27,10 +27,11 @@ fn versioned_constants() -> &'static VersionedConstants { VersionedConstants::latest_constants() } -#[rstest] +#[rstest_reuse::apply(include_l2_gas)] fn test_get_event_gas_cost( versioned_constants: &VersionedConstants, #[values(false, true)] use_kzg_da: bool, + has_l2_gas: bool, ) { let l2_resource_gas_costs = &versioned_constants.l2_resource_gas_costs; let (event_key_factor, data_word_cost) = @@ -41,7 +42,7 @@ fn test_get_event_gas_cost( StarknetResources::new(0, 0, 0, StateChangesCount::default(), None, call_infos_iter); assert_eq!( GasVector::default(), - starknet_resources.to_gas_vector(versioned_constants, use_kzg_da) + starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, has_l2_gas) ); let create_event = |keys_size: usize, data_size: usize| OrderedEvent { @@ -75,13 +76,14 @@ 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(); + // TODO(Nimrod): Modify the expected gas vector l2-gas events cost is implemented. let expected = GasVector::from_l1_gas( // 8 keys and 11 data words overall. (data_word_cost * (event_key_factor * 8_u128 + 11_u128)).to_integer(), ); 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); + let gas_vector = starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, has_l2_gas); assert_eq!(expected, gas_vector); assert_ne!(GasVector::default(), gas_vector) } diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index a8d1f4eef16..762d692a6f1 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -436,3 +436,9 @@ pub fn update_json_value(base: &mut serde_json::Value, update: serde_json::Value _ => panic!("Both base and update should be of type serde_json::Value::Object."), } } + +#[cfg(test)] +#[rstest_reuse::template] +#[rstest] +// TODO(Nimrod): Add true as a case. +fn include_l2_gas(#[values(false)] has_l2_gas: bool) {} diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index a4253242696..95f35ede33c 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -328,12 +328,13 @@ impl StarknetResources { &self, versioned_constants: &VersionedConstants, use_kzg_da: bool, + include_l2_gas: bool, ) -> GasVector { - self.get_calldata_and_signature_cost(versioned_constants) - + self.get_code_cost(versioned_constants) + self.get_calldata_and_signature_cost(versioned_constants, include_l2_gas) + + self.get_code_cost(versioned_constants, include_l2_gas) + self.get_state_changes_cost(use_kzg_da) + self.get_messages_cost() - + self.get_events_cost(versioned_constants) + + self.get_events_cost(versioned_constants, include_l2_gas) } // Returns the gas cost for transaction calldata and transaction signature. Each felt costs a @@ -342,13 +343,18 @@ impl StarknetResources { pub fn get_calldata_and_signature_cost( &self, versioned_constants: &VersionedConstants, + include_l2_gas: bool, ) -> GasVector { - // TODO(Avi, 20/2/2024): Calculate the number of bytes instead of the number of felts. - let total_data_size = u128_from_usize(self.calldata_length + self.signature_length); - let l1_gas = (versioned_constants.l2_resource_gas_costs.gas_per_data_felt - * total_data_size) - .to_integer(); - GasVector::from_l1_gas(l1_gas) + if include_l2_gas { + todo!() + } else { + // TODO(Avi, 20/2/2024): Calculate the number of bytes instead of the number of felts. + let total_data_size = u128_from_usize(self.calldata_length + self.signature_length); + let l1_gas = (versioned_constants.l2_resource_gas_costs.gas_per_data_felt + * total_data_size) + .to_integer(); + GasVector::from_l1_gas(l1_gas) + } } /// Returns an estimation of the gas usage for processing L1<>L2 messages on L1. Accounts for @@ -397,12 +403,20 @@ impl StarknetResources { } /// Returns the gas cost of declared class codes. - pub fn get_code_cost(&self, versioned_constants: &VersionedConstants) -> GasVector { - GasVector::from_l1_gas( - (versioned_constants.l2_resource_gas_costs.gas_per_code_byte - * u128_from_usize(self.code_size)) - .to_integer(), - ) + pub fn get_code_cost( + &self, + versioned_constants: &VersionedConstants, + include_l2_gas: bool, + ) -> GasVector { + if include_l2_gas { + todo!() + } else { + GasVector::from_l1_gas( + (versioned_constants.l2_resource_gas_costs.gas_per_code_byte + * u128_from_usize(self.code_size)) + .to_integer(), + ) + } } /// Returns the gas cost of the transaction's state changes. @@ -412,15 +426,23 @@ impl StarknetResources { } /// Returns the gas cost of the transaction's emmited events. - pub fn get_events_cost(&self, versioned_constants: &VersionedConstants) -> GasVector { - let l2_resource_gas_costs = &versioned_constants.l2_resource_gas_costs; - let (event_key_factor, data_word_cost) = - (l2_resource_gas_costs.event_key_factor, l2_resource_gas_costs.gas_per_data_felt); - let l1_gas: u128 = (data_word_cost - * (event_key_factor * self.total_event_keys + self.total_event_data_size)) - .to_integer(); - - GasVector::from_l1_gas(l1_gas) + pub fn get_events_cost( + &self, + versioned_constants: &VersionedConstants, + include_l2_gas: bool, + ) -> GasVector { + if include_l2_gas { + todo!() + } else { + let l2_resource_gas_costs = &versioned_constants.l2_resource_gas_costs; + let (event_key_factor, data_word_cost) = + (l2_resource_gas_costs.event_key_factor, l2_resource_gas_costs.gas_per_data_felt); + let l1_gas: u128 = (data_word_cost + * (event_key_factor * self.total_event_keys + self.total_event_data_size)) + .to_integer(); + + GasVector::from_l1_gas(l1_gas) + } } pub fn get_onchain_data_segment_length(&self) -> usize { @@ -468,7 +490,7 @@ impl TransactionResources { versioned_constants: &VersionedConstants, use_kzg_da: bool, ) -> TransactionFeeResult { - Ok(self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da) + Ok(self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, false) + calculate_l1_gas_by_vm_usage( versioned_constants, &self.vm_resources, @@ -483,7 +505,7 @@ impl TransactionResources { with_reverted_steps: bool, ) -> ResourcesMapping { let GasVector { l1_gas, l1_data_gas, .. } = - self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da); + self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, false); let mut resources = self.vm_resources.to_resources_mapping(); resources.0.extend(HashMap::from([ ( diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index d8df0c0594d..b1c1921f846 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1912,11 +1912,11 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { }; assert_eq!( expected_gas, - actual_execution_info - .receipt - .resources - .starknet_resources - .to_gas_vector(versioned_constants, use_kzg_da) + actual_execution_info.receipt.resources.starknet_resources.to_gas_vector( + versioned_constants, + use_kzg_da, + false + ) ); let total_gas = expected_tx_resources