From f7372912b2a531aac44e4e31c2066623d80955df Mon Sep 17 00:00:00 2001 From: Tzahi Taub Date: Thu, 31 Oct 2024 14:02:02 +0200 Subject: [PATCH] feat(blockifier): define ChargedResources in CallInfo --- crates/blockifier/src/execution/call_info.rs | 19 ++++++++++- .../deprecated_entry_point_execution.rs | 6 ++-- .../deprecated_syscalls_test.rs | 34 ++++++++++++------- .../blockifier/src/execution/entry_point.rs | 4 +-- .../src/execution/entry_point_execution.rs | 10 ++++-- .../src/execution/entry_point_test.rs | 8 +++-- .../execution/native/entry_point_execution.rs | 8 +++-- .../syscalls/syscall_tests/library_call.rs | 14 +++++--- crates/blockifier/src/test_utils/prices.rs | 3 +- .../src/transaction/transactions_test.rs | 21 ++++++++---- crates/papyrus_execution/src/objects.rs | 2 +- 11 files changed, 91 insertions(+), 38 deletions(-) diff --git a/crates/blockifier/src/execution/call_info.rs b/crates/blockifier/src/execution/call_info.rs index e38eb29235..42398e82fe 100644 --- a/crates/blockifier/src/execution/call_info.rs +++ b/crates/blockifier/src/execution/call_info.rs @@ -5,6 +5,7 @@ use std::ops::Add; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use serde::Serialize; use starknet_api::core::{ClassHash, EthAddress}; +use starknet_api::execution_resources::GasAmount; use starknet_api::state::StorageKey; use starknet_api::transaction::{EventContent, L2ToL1Payload}; use starknet_types_core::felt::Felt; @@ -95,6 +96,22 @@ impl Sum for ExecutionSummary { } } +/// L2 resources counted for fee charge. +/// When all execution will be using gas (no VM mode), this should be removed, and the gas_consumed +/// field should be used for fee collection. +#[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))] +#[derive(Clone, Debug, Default, Serialize, Eq, PartialEq)] +pub struct ChargedResources { + pub vm_resources: ExecutionResources, // Counted in CairoSteps mode calls. + pub gas_for_fee: GasAmount, // Counted in SierraGas mode calls. +} + +impl ChargedResources { + pub fn from_execution_resources(resources: ExecutionResources) -> Self { + Self { vm_resources: resources, ..Default::default() } + } +} + /// Represents the full effects of executing an entry point, including the inner calls it invoked. #[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))] @@ -102,9 +119,9 @@ impl Sum for ExecutionSummary { pub struct CallInfo { pub call: CallEntryPoint, pub execution: CallExecution, - pub resources: ExecutionResources, pub inner_calls: Vec, pub tracked_resource: TrackedResource, + pub charged_resources: ChargedResources, // Additional information gathered during execution. pub storage_read_values: Vec, diff --git a/crates/blockifier/src/execution/deprecated_entry_point_execution.rs b/crates/blockifier/src/execution/deprecated_entry_point_execution.rs index 63aeeef433..becee618c7 100644 --- a/crates/blockifier/src/execution/deprecated_entry_point_execution.rs +++ b/crates/blockifier/src/execution/deprecated_entry_point_execution.rs @@ -12,7 +12,7 @@ use starknet_api::hash::StarkHash; use super::execution_utils::SEGMENT_ARENA_BUILTIN_SIZE; use crate::abi::abi_utils::selector_from_name; use crate::abi::constants::{CONSTRUCTOR_ENTRY_POINT_NAME, DEFAULT_ENTRY_POINT_SELECTOR}; -use crate::execution::call_info::{CallExecution, CallInfo}; +use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources}; use crate::execution::contract_class::{ContractClassV0, TrackedResource}; use crate::execution::deprecated_syscalls::hint_processor::DeprecatedSyscallHintProcessor; use crate::execution::entry_point::{ @@ -278,9 +278,11 @@ pub fn finalize_execution( failed: false, gas_consumed: 0, }, - resources: full_call_resources.filter_unused_builtins(), inner_calls: syscall_handler.inner_calls, tracked_resource: TrackedResource::CairoSteps, + charged_resources: ChargedResources::from_execution_resources( + full_call_resources.filter_unused_builtins(), + ), storage_read_values: syscall_handler.read_values, accessed_storage_keys: syscall_handler.accessed_keys, }) diff --git a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs index bc74cc201c..680ba36b94 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs @@ -24,7 +24,7 @@ use test_case::test_case; use crate::abi::abi_utils::selector_from_name; use crate::context::ChainInfo; -use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent}; +use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, OrderedEvent}; use crate::execution::common_hints::ExecutionMode; use crate::execution::deprecated_syscalls::DeprecatedSyscallSelector; use crate::execution::entry_point::{CallEntryPoint, CallType}; @@ -157,7 +157,9 @@ fn test_nested_library_call() { let nested_storage_call_info = CallInfo { call: nested_storage_entry_point, execution: CallExecution::from_retdata(retdata![felt!(value + 1)]), - resources: storage_entry_point_resources.clone(), + charged_resources: ChargedResources::from_execution_resources( + storage_entry_point_resources.clone(), + ), storage_read_values: vec![felt!(value + 1)], accessed_storage_keys: HashSet::from([storage_key!(key + 1)]), ..Default::default() @@ -172,14 +174,18 @@ fn test_nested_library_call() { let library_call_info = CallInfo { call: library_entry_point, execution: CallExecution::from_retdata(retdata![felt!(value + 1)]), - resources: library_call_resources.clone(), + charged_resources: ChargedResources::from_execution_resources( + library_call_resources.clone(), + ), inner_calls: vec![nested_storage_call_info], ..Default::default() }; let storage_call_info = CallInfo { call: storage_entry_point, execution: CallExecution::from_retdata(retdata![felt!(value)]), - resources: storage_entry_point_resources.clone(), + charged_resources: ChargedResources::from_execution_resources( + storage_entry_point_resources.clone(), + ), storage_read_values: vec![felt!(value)], accessed_storage_keys: HashSet::from([storage_key!(key)]), ..Default::default() @@ -196,7 +202,7 @@ fn test_nested_library_call() { let expected_call_info = CallInfo { call: main_entry_point.clone(), execution: CallExecution::from_retdata(retdata![felt!(0_u8)]), - resources: main_call_resources, + charged_resources: ChargedResources::from_execution_resources(main_call_resources), inner_calls: vec![library_call_info, storage_call_info], ..Default::default() }; @@ -241,11 +247,11 @@ fn test_call_contract() { ..trivial_external_entry_point }, execution: expected_execution.clone(), - resources: ExecutionResources { + charged_resources: ChargedResources::from_execution_resources(ExecutionResources { n_steps: 222, n_memory_holes: 0, builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 2)]), - }, + }), storage_read_values: vec![value], accessed_storage_keys: HashSet::from([storage_key!(key_int)]), ..Default::default() @@ -259,12 +265,14 @@ fn test_call_contract() { ..trivial_external_entry_point }, execution: expected_execution, - resources: &get_syscall_resources(DeprecatedSyscallSelector::CallContract) - + &ExecutionResources { - n_steps: 261, - n_memory_holes: 0, - builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 3)]), - }, + charged_resources: ChargedResources::from_execution_resources( + &get_syscall_resources(DeprecatedSyscallSelector::CallContract) + + &ExecutionResources { + n_steps: 261, + n_memory_holes: 0, + builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 3)]), + }, + ), ..Default::default() }; diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index e251e15bff..9bf38ebefa 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -360,7 +360,7 @@ impl EntryPointExecutionContext { } /// From the total amount of steps available for execution, deduct the steps consumed during - /// validation and the overhead steps required for fee transfer. + /// validation and the overhead steps required, among the rest, for fee transfer. /// Returns the remaining steps (after the subtraction). pub fn subtract_validation_and_overhead_steps( &mut self, @@ -370,7 +370,7 @@ impl EntryPointExecutionContext { ) -> usize { let validate_steps = validate_call_info .as_ref() - .map(|call_info| call_info.resources.n_steps) + .map(|call_info| call_info.charged_resources.vm_resources.n_steps) .unwrap_or_default(); let overhead_steps = diff --git a/crates/blockifier/src/execution/entry_point_execution.rs b/crates/blockifier/src/execution/entry_point_execution.rs index ada0e48d0f..c2a3404957 100644 --- a/crates/blockifier/src/execution/entry_point_execution.rs +++ b/crates/blockifier/src/execution/entry_point_execution.rs @@ -10,9 +10,10 @@ use cairo_vm::vm::runners::builtin_runner::BuiltinRunner; use cairo_vm::vm::runners::cairo_runner::{CairoArg, CairoRunner, ExecutionResources}; use cairo_vm::vm::security::verify_secure_runner; use num_traits::{ToPrimitive, Zero}; +use starknet_api::execution_resources::GasAmount; use starknet_types_core::felt::Felt; -use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; +use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata}; use crate::execution::contract_class::{ContractClassV1, EntryPointV1, TrackedResource}; use crate::execution::entry_point::{ CallEntryPoint, @@ -408,6 +409,11 @@ pub fn finalize_execution( syscall_handler.finalize(); let full_call_resources = &*syscall_handler.resources - &previous_resources; + let charged_resources = ChargedResources { + vm_resources: full_call_resources.filter_unused_builtins(), + // TODO(tzahi): Replace with a computed value. + gas_for_fee: GasAmount(0), + }; Ok(CallInfo { call: syscall_handler.call, execution: CallExecution { @@ -417,9 +423,9 @@ pub fn finalize_execution( failed: call_result.failed, gas_consumed: call_result.gas_consumed, }, - resources: full_call_resources.filter_unused_builtins(), inner_calls: syscall_handler.inner_calls, tracked_resource, + charged_resources, storage_read_values: syscall_handler.read_values, accessed_storage_keys: syscall_handler.accessed_keys, }) diff --git a/crates/blockifier/src/execution/entry_point_test.rs b/crates/blockifier/src/execution/entry_point_test.rs index 2bac50eb35..4309796136 100644 --- a/crates/blockifier/src/execution/entry_point_test.rs +++ b/crates/blockifier/src/execution/entry_point_test.rs @@ -525,8 +525,12 @@ fn test_cairo1_entry_point_segment_arena() { }; assert_eq!( - entry_point_call.execute_directly(&mut state).unwrap().resources.builtin_instance_counter - [&BuiltinName::segment_arena], + entry_point_call + .execute_directly(&mut state) + .unwrap() + .charged_resources + .vm_resources + .builtin_instance_counter[&BuiltinName::segment_arena], // Note: the number of segment_arena instances should not depend on the compiler or VM // version. Do not manually fix this then when upgrading them - it might be a bug. 2 diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs index b16cda293a..84f4c3b4d4 100644 --- a/crates/blockifier/src/execution/native/entry_point_execution.rs +++ b/crates/blockifier/src/execution/native/entry_point_execution.rs @@ -1,8 +1,9 @@ use cairo_native::execution_result::ContractExecutionResult; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use num_traits::ToPrimitive; +use starknet_api::execution_resources::GasAmount; -use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; +use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata}; use crate::execution::contract_class::TrackedResource; use crate::execution::entry_point::{ CallEntryPoint, @@ -72,7 +73,10 @@ fn create_callinfo( }, // todo(rodrigo): execution resources rely heavily on how the VM work, therefore // the dummy values - resources: ExecutionResources::default(), + charged_resources: ChargedResources { + vm_resources: ExecutionResources::default(), + gas_for_fee: GasAmount(gas_consumed), + }, inner_calls: syscall_handler.inner_calls, storage_read_values: syscall_handler.read_values, accessed_storage_keys: syscall_handler.accessed_keys, 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 1d41a94838..487bcb0e19 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs @@ -9,7 +9,7 @@ use test_case::test_case; use crate::abi::abi_utils::selector_from_name; use crate::context::ChainInfo; -use crate::execution::call_info::{CallExecution, CallInfo}; +use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources}; use crate::execution::entry_point::{CallEntryPoint, CallType}; use crate::execution::syscalls::syscall_tests::constants::{ REQUIRED_GAS_LIBRARY_CALL_TEST, @@ -158,7 +158,9 @@ fn test_nested_library_call(test_contract: FeatureContract, expected_gas: u64) { gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST, ..CallExecution::default() }, - resources: storage_entry_point_resources.clone(), + charged_resources: ChargedResources::from_execution_resources( + storage_entry_point_resources.clone(), + ), tracked_resource, storage_read_values: vec![felt!(value + 1)], accessed_storage_keys: HashSet::from([storage_key!(key + 1)]), @@ -179,7 +181,7 @@ fn test_nested_library_call(test_contract: FeatureContract, expected_gas: u64) { gas_consumed: REQUIRED_GAS_LIBRARY_CALL_TEST, ..CallExecution::default() }, - resources: library_call_resources, + charged_resources: ChargedResources::from_execution_resources(library_call_resources), inner_calls: vec![nested_storage_call_info], tracked_resource, ..Default::default() @@ -192,7 +194,9 @@ fn test_nested_library_call(test_contract: FeatureContract, expected_gas: u64) { gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST, ..CallExecution::default() }, - resources: storage_entry_point_resources, + charged_resources: ChargedResources::from_execution_resources( + storage_entry_point_resources, + ), storage_read_values: vec![felt!(value)], accessed_storage_keys: HashSet::from([storage_key!(key)]), tracked_resource, @@ -212,7 +216,7 @@ fn test_nested_library_call(test_contract: FeatureContract, expected_gas: u64) { gas_consumed: expected_gas, ..CallExecution::default() }, - resources: main_call_resources, + charged_resources: ChargedResources::from_execution_resources(main_call_resources), inner_calls: vec![library_call_info, storage_call_info], tracked_resource, ..Default::default() diff --git a/crates/blockifier/src/test_utils/prices.rs b/crates/blockifier/src/test_utils/prices.rs index 53686869fb..6ef4fee5f0 100644 --- a/crates/blockifier/src/test_utils/prices.rs +++ b/crates/blockifier/src/test_utils/prices.rs @@ -79,5 +79,6 @@ fn fee_transfer_resources( &mut remaining_gas, ) .unwrap() - .resources + .charged_resources + .vm_resources } diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 6de3e13f2a..95be2358e0 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -56,6 +56,7 @@ use crate::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionCont use crate::execution::call_info::{ CallExecution, CallInfo, + ChargedResources, ExecutionSummary, MessageToL1, OrderedEvent, @@ -237,7 +238,7 @@ fn expected_validate_call_info( initial_gas, }, // The account contract we use for testing has trivial `validate` functions. - resources, + charged_resources: ChargedResources::from_execution_resources(resources), execution: CallExecution { retdata, gas_consumed, ..Default::default() }, tracked_resource, ..Default::default() @@ -305,7 +306,9 @@ fn expected_fee_transfer_call_info( events: vec![expected_fee_transfer_event], ..Default::default() }, - resources: Prices::FeeTransfer(account_address, *fee_type).into(), + charged_resources: ChargedResources::from_execution_resources( + Prices::FeeTransfer(account_address, *fee_type).into(), + ), // We read sender and recipient balance - Uint256(BALANCE, 0) then Uint256(0, 0). storage_read_values: vec![felt!(BALANCE.0), felt!(0_u8), felt!(0_u8), felt!(0_u8)], accessed_storage_keys: HashSet::from_iter(vec![ @@ -328,7 +331,7 @@ fn get_expected_cairo_resources( versioned_constants.get_additional_os_tx_resources(tx_type, starknet_resources, false); for call_info in call_infos { if let Some(call_info) = &call_info { - expected_cairo_resources += &call_info.resources + expected_cairo_resources += &call_info.charged_resources.vm_resources }; } @@ -513,11 +516,15 @@ fn test_invoke_tx( gas_consumed: expected_arguments.execute_gas_consumed, ..Default::default() }, - resources: expected_arguments.resources, + charged_resources: ChargedResources::from_execution_resources(expected_arguments.resources), inner_calls: vec![CallInfo { call: expected_return_result_call, execution: CallExecution::from_retdata(expected_return_result_retdata), - resources: ExecutionResources { n_steps: 23, n_memory_holes: 0, ..Default::default() }, + charged_resources: ChargedResources::from_execution_resources(ExecutionResources { + n_steps: 23, + n_memory_holes: 0, + ..Default::default() + }), ..Default::default() }], tracked_resource, @@ -2246,11 +2253,11 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { gas_consumed: 6820, ..Default::default() }, - resources: ExecutionResources { + charged_resources: ChargedResources::from_execution_resources(ExecutionResources { n_steps: 158, n_memory_holes: 0, 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() diff --git a/crates/papyrus_execution/src/objects.rs b/crates/papyrus_execution/src/objects.rs index 2e095b8cbd..01c08e84ff 100644 --- a/crates/papyrus_execution/src/objects.rs +++ b/crates/papyrus_execution/src/objects.rs @@ -348,7 +348,7 @@ impl TryFrom<(CallInfo, GasVector)> for FunctionInvocation { }) .collect(), execution_resources: vm_resources_to_execution_resources( - call_info.resources, + call_info.charged_resources.vm_resources, gas_vector, )?, })