From 03ef939f1959d29bf6a451c706c17f6db288871c Mon Sep 17 00:00:00 2001 From: Tzahi Date: Wed, 6 Nov 2024 14:21:55 +0200 Subject: [PATCH] feat(blockifier): only All resource bounds txs can run in SierraGas mode (#1771) --- crates/blockifier/src/context.rs | 7 +--- .../src/execution/contract_class.rs | 20 +++++++---- .../src/execution/execution_utils.rs | 6 ++-- .../syscalls/syscall_tests/library_call.rs | 2 ++ crates/blockifier/src/test_utils.rs | 2 +- crates/blockifier/src/transaction/objects.rs | 10 ++++++ .../src/transaction/transactions_test.rs | 35 ++++++++++++------- 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/crates/blockifier/src/context.rs b/crates/blockifier/src/context.rs index 5f577ad1ad..b6a0333a86 100644 --- a/crates/blockifier/src/context.rs +++ b/crates/blockifier/src/context.rs @@ -32,12 +32,7 @@ impl TransactionContext { self.tx_info.sender_address() == self.block_context.block_info.sequencer_address } pub fn get_gas_vector_computation_mode(&self) -> GasVectorComputationMode { - match &self.tx_info { - TransactionInfo::Current(info) => { - info.resource_bounds.get_gas_vector_computation_mode() - } - TransactionInfo::Deprecated(_) => GasVectorComputationMode::NoL2Gas, - } + self.tx_info.gas_mode() } pub fn get_gas_prices(&self) -> &GasPriceVector { self.block_context diff --git a/crates/blockifier/src/execution/contract_class.rs b/crates/blockifier/src/execution/contract_class.rs index e65cd08787..777a03d601 100644 --- a/crates/blockifier/src/execution/contract_class.rs +++ b/crates/blockifier/src/execution/contract_class.rs @@ -29,6 +29,7 @@ use starknet_api::deprecated_contract_class::{ EntryPointV0, Program as DeprecatedProgram, }; +use starknet_api::transaction::GasVectorComputationMode; use starknet_types_core::felt::Felt; use crate::abi::constants::{self}; @@ -131,12 +132,19 @@ impl RunnableContractClass { } /// Returns whether this contract should run using Cairo steps or Sierra gas. - pub fn tracked_resource(&self, min_sierra_version: &CompilerVersion) -> TrackedResource { - match self { - Self::V0(_) => TrackedResource::CairoSteps, - Self::V1(contract_class) => contract_class.tracked_resource(min_sierra_version), - #[cfg(feature = "cairo_native")] - Self::V1Native(_) => TrackedResource::SierraGas, + pub fn tracked_resource( + &self, + min_sierra_version: &CompilerVersion, + gas_mode: GasVectorComputationMode, + ) -> TrackedResource { + match gas_mode { + GasVectorComputationMode::All => match self { + Self::V0(_) => TrackedResource::CairoSteps, + Self::V1(contract_class) => contract_class.tracked_resource(min_sierra_version), + #[cfg(feature = "cairo_native")] + Self::V1Native(_) => TrackedResource::SierraGas, + }, + GasVectorComputationMode::NoL2Gas => TrackedResource::CairoSteps, } } } diff --git a/crates/blockifier/src/execution/execution_utils.rs b/crates/blockifier/src/execution/execution_utils.rs index a17b2a71ba..7ee421d128 100644 --- a/crates/blockifier/src/execution/execution_utils.rs +++ b/crates/blockifier/src/execution/execution_utils.rs @@ -60,8 +60,10 @@ pub fn execute_entry_point_call_wrapper( context: &mut EntryPointExecutionContext, remaining_gas: &mut u64, ) -> EntryPointExecutionResult { - let contract_tracked_resource = contract_class - .tracked_resource(&context.versioned_constants().min_compiler_version_for_sierra_gas); + let contract_tracked_resource = contract_class.tracked_resource( + &context.versioned_constants().min_compiler_version_for_sierra_gas, + context.tx_context.tx_info.gas_mode(), + ); // Note: no return statements (explicit or implicit) should be added between the push and the // pop commands. diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs index 487bcb0e19..5fa6699102 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs @@ -4,6 +4,7 @@ use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use pretty_assertions::assert_eq; use starknet_api::execution_utils::format_panic_data; +use starknet_api::transaction::GasVectorComputationMode; use starknet_api::{calldata, felt, storage_key}; use test_case::test_case; @@ -149,6 +150,7 @@ fn test_nested_library_call(test_contract: FeatureContract, expected_gas: u64) { // The default VersionedConstants is used in the execute_directly call bellow. let tracked_resource = test_contract.get_runnable_class().tracked_resource( &VersionedConstants::create_for_testing().min_compiler_version_for_sierra_gas, + GasVectorComputationMode::All, ); let nested_storage_call_info = CallInfo { diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index 81043b50c4..9e658edc84 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -110,7 +110,7 @@ impl CompilerBasedVersion { } /// Returns the context-free tracked resource of this contract (does not take caller contract - /// into account). + /// and the transaction info into account). pub fn own_tracked_resource(&self) -> TrackedResource { match self { Self::CairoVersion(CairoVersion::Cairo0) | Self::OldCairo1 => { diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index aec7f63f59..bdd565bf1c 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -10,6 +10,7 @@ use starknet_api::transaction::{ AccountDeploymentData, AllResourceBounds, Fee, + GasVectorComputationMode, PaymasterData, ResourceBounds, Tip, @@ -85,6 +86,15 @@ impl TransactionInfo { TransactionInfo::Deprecated(context) => context.max_fee != Fee(0), } } + + pub fn gas_mode(&self) -> GasVectorComputationMode { + match self { + TransactionInfo::Current(info) => { + info.resource_bounds.get_gas_vector_computation_mode() + } + TransactionInfo::Deprecated(_) => GasVectorComputationMode::NoL2Gas, + } + } } impl HasRelatedFeeType for TransactionInfo { diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 23bb45dc5d..b3709ebe23 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -429,7 +429,7 @@ fn add_kzg_da_resources_to_resources_mapping( fn test_invoke_tx( #[values(default_l1_resource_bounds(), default_all_resource_bounds())] resource_bounds: ValidResourceBounds, - #[case] expected_arguments: ExpectedResultTestInvokeTx, + #[case] mut expected_arguments: ExpectedResultTestInvokeTx, #[case] account_cairo_version: CairoVersion, #[values(false, true)] use_kzg_da: bool, ) { @@ -472,9 +472,15 @@ fn test_invoke_tx( let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap(); - let tracked_resource = account_contract - .get_runnable_class() - .tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas); + let tracked_resource = account_contract.get_runnable_class().tracked_resource( + &versioned_constants.min_compiler_version_for_sierra_gas, + tx_context.tx_info.gas_mode(), + ); + if tracked_resource == TrackedResource::CairoSteps { + // In CairoSteps mode, the initial gas is set to the default once before the validate call. + expected_arguments.inner_call_initial_gas -= + expected_arguments.validate_gas_consumed + expected_arguments.execute_gas_consumed + } // Build expected validate call info. let expected_account_class_hash = account_contract.get_class_hash(); @@ -1525,9 +1531,10 @@ fn test_declare_tx( class_hash, account.get_class_hash(), sender_address, - account - .get_runnable_class() - .tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas), + account.get_runnable_class().tracked_resource( + &versioned_constants.min_compiler_version_for_sierra_gas, + tx_context.tx_info.gas_mode(), + ), if tx_version >= TransactionVersion::THREE { user_initial_gas_from_bounds(default_all_resource_bounds) } else { @@ -1725,9 +1732,10 @@ fn test_deploy_account_tx( Calldata(validate_calldata.into()), deployed_account_address, cairo_version, - account - .get_runnable_class() - .tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas), + account.get_runnable_class().tracked_resource( + &versioned_constants.min_compiler_version_for_sierra_gas, + tx_context.tx_info.gas_mode(), + ), user_initial_gas, ); @@ -2258,9 +2266,10 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 6)]), }), accessed_storage_keys: HashSet::from_iter(vec![accessed_storage_key]), - tracked_resource: test_contract - .get_runnable_class() - .tracked_resource(&versioned_constants.min_compiler_version_for_sierra_gas), + tracked_resource: test_contract.get_runnable_class().tracked_resource( + &versioned_constants.min_compiler_version_for_sierra_gas, + GasVectorComputationMode::NoL2Gas, + ), ..Default::default() };