Skip to content

Commit

Permalink
feat(blockifier): remove ChargedResources from CallInfo (starkware-li…
Browse files Browse the repository at this point in the history
  • Loading branch information
TzahiTaub authored Jan 6, 2025
1 parent 0dd87a8 commit 790ade6
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 117 deletions.
23 changes: 13 additions & 10 deletions crates/blockifier/src/execution/call_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,16 @@ impl Sum for ExecutionSummary {
#[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.
pub gas_consumed: GasAmount, // Counted in SierraGas mode calls.
}

impl ChargedResources {
pub fn from_execution_resources(resources: ExecutionResources) -> Self {
Self { vm_resources: resources, ..Default::default() }
}

pub fn from_gas(gas_for_fee: GasAmount) -> Self {
Self { gas_for_fee, ..Default::default() }
pub fn from_gas(gas_consumed: GasAmount) -> Self {
Self { gas_consumed, ..Default::default() }
}
}

Expand All @@ -132,8 +132,8 @@ impl Add<&ChargedResources> for &ChargedResources {
impl AddAssign<&ChargedResources> for ChargedResources {
fn add_assign(&mut self, other: &Self) {
self.vm_resources += &other.vm_resources;
self.gas_for_fee =
self.gas_for_fee.checked_add(other.gas_for_fee).expect("Gas for fee overflowed.");
self.gas_consumed =
self.gas_consumed.checked_add(other.gas_consumed).expect("Gas for fee overflowed.");
}
}

Expand All @@ -145,8 +145,8 @@ pub struct CallInfo {
pub call: CallEntryPoint,
pub execution: CallExecution,
pub inner_calls: Vec<CallInfo>,
pub resources: ExecutionResources,
pub tracked_resource: TrackedResource,
pub charged_resources: ChargedResources,

// Additional information gathered during execution.
pub storage_read_values: Vec<Felt>,
Expand Down Expand Up @@ -212,9 +212,12 @@ impl CallInfo {
}

ExecutionSummary {
// Note: the charged resources of a call contains the inner call resources, unlike other
// fields such as events and messages.
charged_resources: self.charged_resources.clone(),
// Note: the vm_resources and gas_consumed of a call contains the inner call resources,
// unlike other fields such as events and messages.
charged_resources: ChargedResources {
vm_resources: self.resources.clone(),
gas_consumed: GasAmount(self.execution.gas_consumed),
},
executed_class_hashes,
visited_storage_entries,
l2_to_l1_payload_lengths,
Expand All @@ -235,7 +238,7 @@ impl CallInfo {
// Note: the vm resources (and entire charged resources) of a call contains the inner call
// resources, unlike other fields such as events and messages.
call_infos.fold(ExecutionResources::default(), |mut acc, inner_call| {
acc += &inner_call.charged_resources.vm_resources;
acc += &inner_call.resources;
acc
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ use starknet_api::abi::abi_utils::selector_from_name;
use starknet_api::abi::constants::{CONSTRUCTOR_ENTRY_POINT_NAME, DEFAULT_ENTRY_POINT_SELECTOR};
use starknet_api::contract_class::EntryPointType;
use starknet_api::core::EntryPointSelector;
use starknet_api::execution_resources::GasAmount;
use starknet_api::hash::StarkHash;

use super::execution_utils::SEGMENT_ARENA_BUILTIN_SIZE;
use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources};
use crate::execution::call_info::{CallExecution, CallInfo};
use crate::execution::contract_class::{CompiledClassV0, TrackedResource};
use crate::execution::deprecated_syscalls::hint_processor::DeprecatedSyscallHintProcessor;
use crate::execution::entry_point::{
Expand Down Expand Up @@ -256,7 +255,6 @@ pub fn finalize_execution(

let vm_resources = &vm_resources_without_inner_calls
+ &CallInfo::summarize_vm_resources(syscall_handler.inner_calls.iter());
let charged_resources = ChargedResources { vm_resources, gas_for_fee: GasAmount(0) };

Ok(CallInfo {
call,
Expand All @@ -269,7 +267,7 @@ pub fn finalize_execution(
},
inner_calls: syscall_handler.inner_calls,
tracked_resource: TrackedResource::CairoSteps,
charged_resources,
resources: vm_resources,
storage_read_values: syscall_handler.read_values,
accessed_storage_keys: syscall_handler.accessed_keys,
..Default::default()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use starknet_types_core::felt::Felt;
use test_case::test_case;

use crate::context::ChainInfo;
use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, OrderedEvent};
use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent};
use crate::execution::common_hints::ExecutionMode;
use crate::execution::deprecated_syscalls::DeprecatedSyscallSelector;
use crate::execution::entry_point::{CallEntryPoint, CallType};
Expand Down Expand Up @@ -155,9 +155,7 @@ fn test_nested_library_call() {
let nested_storage_call_info = CallInfo {
call: nested_storage_entry_point,
execution: CallExecution::from_retdata(retdata![felt!(value + 1)]),
charged_resources: ChargedResources::from_execution_resources(
storage_entry_point_resources.clone(),
),
resources: storage_entry_point_resources.clone(),
storage_read_values: vec![felt!(value + 1)],
accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
..Default::default()
Expand All @@ -172,18 +170,14 @@ fn test_nested_library_call() {
let library_call_info = CallInfo {
call: library_entry_point,
execution: CallExecution::from_retdata(retdata![felt!(value + 1)]),
charged_resources: ChargedResources::from_execution_resources(
library_call_resources.clone(),
),
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)]),
charged_resources: ChargedResources::from_execution_resources(
storage_entry_point_resources.clone(),
),
resources: storage_entry_point_resources.clone(),
storage_read_values: vec![felt!(value)],
accessed_storage_keys: HashSet::from([storage_key!(key)]),
..Default::default()
Expand All @@ -200,7 +194,7 @@ fn test_nested_library_call() {
let expected_call_info = CallInfo {
call: main_entry_point.clone(),
execution: CallExecution::from_retdata(retdata![felt!(0_u8)]),
charged_resources: ChargedResources::from_execution_resources(main_call_resources),
resources: main_call_resources,
inner_calls: vec![library_call_info, storage_call_info],
..Default::default()
};
Expand Down Expand Up @@ -245,11 +239,11 @@ fn test_call_contract() {
..trivial_external_entry_point
},
execution: expected_execution.clone(),
charged_resources: ChargedResources::from_execution_resources(ExecutionResources {
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()
Expand All @@ -263,14 +257,12 @@ fn test_call_contract() {
..trivial_external_entry_point
},
execution: expected_execution,
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)]),
},
),
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()
};

Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ impl EntryPointExecutionContext {
) -> usize {
let validate_steps = validate_call_info
.as_ref()
.map(|call_info| call_info.charged_resources.vm_resources.n_steps)
.map(|call_info| call_info.resources.n_steps)
.unwrap_or_default();

let overhead_steps =
Expand Down
7 changes: 2 additions & 5 deletions crates/blockifier/src/execution/entry_point_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ 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, ChargedResources, Retdata};
use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
use crate::execution::contract_class::{CompiledClassV1, EntryPointV1, TrackedResource};
use crate::execution::entry_point::{
CallEntryPoint,
Expand Down Expand Up @@ -375,8 +374,6 @@ pub fn finalize_execution(

let vm_resources = &vm_resources_without_inner_calls
+ &CallInfo::summarize_vm_resources(syscall_handler.base.inner_calls.iter());
let charged_resources =
ChargedResources { vm_resources, gas_for_fee: GasAmount(call_result.gas_consumed) };
let syscall_handler_base = syscall_handler.base;
Ok(CallInfo {
call: syscall_handler_base.call,
Expand All @@ -389,7 +386,7 @@ pub fn finalize_execution(
},
inner_calls: syscall_handler_base.inner_calls,
tracked_resource,
charged_resources,
resources: vm_resources,
storage_read_values: syscall_handler_base.read_values,
accessed_storage_keys: syscall_handler_base.accessed_keys,
read_class_hash_values: syscall_handler_base.read_class_hash_values,
Expand Down
14 changes: 8 additions & 6 deletions crates/blockifier/src/execution/entry_point_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use starknet_api::execution_resources::GasAmount;
use starknet_api::transaction::fields::Calldata;

use crate::context::ChainInfo;
use crate::execution::call_info::{CallInfo, ChargedResources};
use crate::execution::call_info::CallInfo;
use crate::execution::contract_class::TrackedResource;
use crate::execution::entry_point::CallEntryPoint;
use crate::test_utils::contracts::FeatureContract;
Expand All @@ -27,20 +27,22 @@ fn assert_charged_resource_as_expected_rec(call_info: &CallInfo) {
let mut children_vm_resources = ExecutionResources::default();
let mut children_gas = GasAmount(0);
for child_call_info in inner_calls.iter() {
let ChargedResources { gas_for_fee, vm_resources } = &child_call_info.charged_resources;
let gas_consumed = GasAmount(child_call_info.execution.gas_consumed);
let vm_resources = &child_call_info.resources;
children_vm_resources += vm_resources;
children_gas += *gas_for_fee;
children_gas += gas_consumed;
}

let ChargedResources { gas_for_fee, vm_resources } = &call_info.charged_resources;
let gas_consumed = GasAmount(call_info.execution.gas_consumed);
let vm_resources = &call_info.resources;

match call_info.tracked_resource {
TrackedResource::SierraGas => {
assert_eq!(vm_resources, &children_vm_resources);
assert!(gas_for_fee > &children_gas)
assert!(gas_consumed > children_gas)
}
TrackedResource::CairoSteps => {
assert_eq!(gas_for_fee, &children_gas);
assert_eq!(gas_consumed, children_gas);
assert!(vm_resources.n_steps > children_vm_resources.n_steps)
}
}
Expand Down
8 changes: 2 additions & 6 deletions crates/blockifier/src/execution/entry_point_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,12 +525,8 @@ fn test_old_cairo1_entry_point_segment_arena() {
};

assert_eq!(
entry_point_call
.execute_directly(&mut state)
.unwrap()
.charged_resources
.vm_resources
.builtin_instance_counter[&BuiltinName::segment_arena],
entry_point_call.execute_directly(&mut state).unwrap().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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use cairo_native::execution_result::ContractExecutionResult;
use cairo_native::utils::BuiltinCosts;
use num_rational::Ratio;
use stacker;
use starknet_api::execution_resources::GasAmount;

use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata};
use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
use crate::execution::contract_class::TrackedResource;
use crate::execution::entry_point::{
CallEntryPoint,
Expand Down Expand Up @@ -120,7 +119,6 @@ fn create_callinfo(

let gas_consumed = syscall_handler.base.call.initial_gas - remaining_gas;
let vm_resources = CallInfo::summarize_vm_resources(syscall_handler.base.inner_calls.iter());
let charged_resources = ChargedResources { vm_resources, gas_for_fee: GasAmount(gas_consumed) };

Ok(CallInfo {
call: syscall_handler.base.call,
Expand All @@ -131,7 +129,7 @@ fn create_callinfo(
failed: call_result.failure_flag,
gas_consumed,
},
charged_resources,
resources: vm_resources,
inner_calls: syscall_handler.base.inner_calls,
storage_read_values: syscall_handler.base.read_values,
accessed_storage_keys: syscall_handler.base.accessed_keys,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ use std::collections::HashSet;

use pretty_assertions::assert_eq;
use starknet_api::abi::abi_utils::selector_from_name;
use starknet_api::execution_resources::GasAmount;
use starknet_api::{calldata, felt, storage_key};
use test_case::test_case;

use crate::context::ChainInfo;
use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata};
use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
use crate::execution::entry_point::{CallEntryPoint, CallType};
use crate::execution::syscalls::syscall_tests::constants::{
REQUIRED_GAS_LIBRARY_CALL_TEST,
Expand Down Expand Up @@ -148,8 +147,6 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
..nested_storage_entry_point
};

let storage_entry_point_gas = GasAmount(26850);

// 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_sierra_version_for_sierra_gas,
Expand All @@ -163,7 +160,6 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
..CallExecution::default()
},
charged_resources: ChargedResources::from_gas(storage_entry_point_gas),
tracked_resource,
storage_read_values: vec![felt!(value + 1)],
accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
Expand All @@ -177,7 +173,6 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
gas_consumed: REQUIRED_GAS_LIBRARY_CALL_TEST,
..CallExecution::default()
},
charged_resources: ChargedResources::from_gas(GasAmount(128430)),
inner_calls: vec![nested_storage_call_info],
tracked_resource,
..Default::default()
Expand All @@ -190,7 +185,6 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
..CallExecution::default()
},
charged_resources: ChargedResources::from_gas(storage_entry_point_gas),
storage_read_values: vec![felt!(value)],
accessed_storage_keys: HashSet::from([storage_key!(key)]),
tracked_resource,
Expand All @@ -205,7 +199,6 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
gas_consumed: main_gas_consumed,
..CallExecution::default()
},
charged_resources: ChargedResources::from_gas(GasAmount(main_gas_consumed)),
inner_calls: vec![library_call_info, storage_call_info],
tracked_resource,
..Default::default()
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/fee/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl TransactionReceipt {
computation: ComputationResources {
vm_resources: total_vm_resources,
n_reverted_steps: reverted_steps,
sierra_gas: charged_resources.gas_for_fee,
sierra_gas: charged_resources.gas_consumed,
reverted_sierra_gas,
},
};
Expand Down
3 changes: 1 addition & 2 deletions crates/blockifier/src/test_utils/prices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,5 @@ fn fee_transfer_resources(
&mut remaining_gas,
)
.unwrap()
.charged_resources
.vm_resources
.resources
}
Loading

0 comments on commit 790ade6

Please sign in to comment.