refactor(blockifier): storage access tracker

Author: aner-starkware

crates/blockifier/src/concurrency/fee_utils.rs

@@ -69,7 +69,7 @@ pub fn fill_sequencer_balance_reads(
     sequencer_balance: (Felt, Felt),
 ) {
     let storage_read_values = if fee_transfer_call_info.inner_calls.is_empty() {
-        &mut fee_transfer_call_info.storage_read_values
+        &mut fee_transfer_call_info.storage_access_tracker.storage_read_values
     } else
     // Proxy pattern.
     {
@@ -78,7 +78,7 @@ pub fn fill_sequencer_balance_reads(
             1,
             "Proxy pattern should have one inner call"
         );
-        &mut fee_transfer_call_info.inner_calls[0].storage_read_values
+        &mut fee_transfer_call_info.inner_calls[0].storage_access_tracker.storage_read_values
     };
     assert_eq!(storage_read_values.len(), 4, "Storage read values should have 4 elements");
 

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -165,6 +165,7 @@ pub fn test_commit_tx() {
                     .fee_transfer_call_info
                     .as_ref()
                     .unwrap()
+                    .storage_access_tracker
                     .storage_read_values[read_storage_index];
                 assert_eq!(felt!(expected_sequencer_storage_read), actual_sequencer_storage_read,);
             }
@@ -221,7 +222,8 @@ fn test_commit_tx_when_sender_is_sequencer() {
     let execution_result = &execution_task_outputs.as_ref().unwrap().result;
     let fee_transfer_call_info =
         execution_result.as_ref().unwrap().fee_transfer_call_info.as_ref().unwrap();
-    let read_values_before_commit = fee_transfer_call_info.storage_read_values.clone();
+    let read_values_before_commit =
+        fee_transfer_call_info.storage_access_tracker.storage_read_values.clone();
     drop(execution_task_outputs);
 
     let tx_context = &executor.block_context.to_tx_context(&sequencer_tx[0]);
@@ -239,7 +241,10 @@ fn test_commit_tx_when_sender_is_sequencer() {
     let fee_transfer_call_info =
         commit_result.as_ref().unwrap().fee_transfer_call_info.as_ref().unwrap();
     // Check that the result call info is the same as before the commit.
-    assert_eq!(read_values_before_commit, fee_transfer_call_info.storage_read_values);
+    assert_eq!(
+        read_values_before_commit,
+        fee_transfer_call_info.storage_access_tracker.storage_read_values
+    );
 
     let sequencer_balance_low_after =
         tx_versioned_state.get_storage_at(fee_token_address, sequencer_balance_key_low).unwrap();

crates/blockifier/src/execution/call_info.rs

@@ -137,6 +137,16 @@ impl AddAssign<&ChargedResources> for ChargedResources {
     }
 }
 
+#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))]
+#[derive(Debug, Default, Eq, PartialEq, Serialize)]
+pub struct StorageAccessTracker {
+    pub storage_read_values: Vec<Felt>,
+    pub accessed_storage_keys: HashSet<StorageKey>,
+    pub read_class_hash_values: Vec<ClassHash>,
+    pub accessed_contract_addresses: HashSet<ContractAddress>,
+}
+
 /// 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))]
@@ -149,10 +159,11 @@ pub struct CallInfo {
     pub tracked_resource: TrackedResource,
 
     // Additional information gathered during execution.
-    pub storage_read_values: Vec<Felt>,
-    pub accessed_storage_keys: HashSet<StorageKey>,
-    pub read_class_hash_values: Vec<ClassHash>,
-    pub accessed_contract_addresses: HashSet<ContractAddress>,
+    pub storage_access_tracker: StorageAccessTracker,
+    // pub storage_read_values: Vec<Felt>,
+    // pub accessed_storage_keys: HashSet<StorageKey>,
+    // pub read_class_hash_values: Vec<ClassHash>,
+    // pub accessed_contract_addresses: HashSet<ContractAddress>,
 }
 
 impl CallInfo {
@@ -185,6 +196,7 @@ impl CallInfo {
 
             // Storage entries.
             let call_storage_entries = call_info
+                .storage_access_tracker
                 .accessed_storage_keys
                 .iter()
                 .map(|storage_key| (call_info.call.storage_address, *storage_key));

crates/blockifier/src/execution/deprecated_entry_point_execution.rs

@@ -11,6 +11,7 @@ use starknet_api::contract_class::EntryPointType;
 use starknet_api::core::EntryPointSelector;
 use starknet_api::hash::StarkHash;
 
+use super::call_info::StorageAccessTracker;
 use super::execution_utils::SEGMENT_ARENA_BUILTIN_SIZE;
 use crate::execution::call_info::{CallExecution, CallInfo};
 use crate::execution::contract_class::{CompiledClassV0, TrackedResource};
@@ -268,9 +269,11 @@ pub fn finalize_execution(
         inner_calls: syscall_handler.inner_calls,
         tracked_resource: TrackedResource::CairoSteps,
         resources: vm_resources,
-        storage_read_values: syscall_handler.read_values,
-        accessed_storage_keys: syscall_handler.accessed_keys,
-        ..Default::default()
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: syscall_handler.read_values,
+            accessed_storage_keys: syscall_handler.accessed_keys,
+            ..Default::default()
+        },
     })
 }
 

crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs

@@ -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, OrderedEvent};
+use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent, StorageAccessTracker};
 use crate::execution::common_hints::ExecutionMode;
 use crate::execution::deprecated_syscalls::DeprecatedSyscallSelector;
 use crate::execution::entry_point::{CallEntryPoint, CallType};
@@ -156,8 +156,11 @@ fn test_nested_library_call() {
         call: nested_storage_entry_point,
         execution: CallExecution::from_retdata(retdata![felt!(value + 1)]),
         resources: storage_entry_point_resources.clone(),
-        storage_read_values: vec![felt!(value + 1)],
-        accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: vec![felt!(value + 1)],
+            accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
+            ..Default::default()
+        },
         ..Default::default()
     };
     let mut library_call_resources = &get_syscall_resources(DeprecatedSyscallSelector::LibraryCall)
@@ -178,8 +181,11 @@ fn test_nested_library_call() {
         call: storage_entry_point,
         execution: CallExecution::from_retdata(retdata![felt!(value)]),
         resources: storage_entry_point_resources.clone(),
-        storage_read_values: vec![felt!(value)],
-        accessed_storage_keys: HashSet::from([storage_key!(key)]),
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: vec![felt!(value)],
+            accessed_storage_keys: HashSet::from([storage_key!(key)]),
+            ..Default::default()
+        },
         ..Default::default()
     };
 
@@ -244,8 +250,11 @@ fn test_call_contract() {
             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)]),
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: vec![value],
+            accessed_storage_keys: HashSet::from([storage_key!(key_int)]),
+            ..Default::default()
+        },
         ..Default::default()
     };
     let expected_call_info = CallInfo {

crates/blockifier/src/execution/entry_point_execution.rs

@@ -10,6 +10,7 @@ use cairo_vm::vm::security::verify_secure_runner;
 use num_traits::{ToPrimitive, Zero};
 use starknet_types_core::felt::Felt;
 
+use super::call_info::StorageAccessTracker;
 use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
 use crate::execution::contract_class::{CompiledClassV1, EntryPointV1, TrackedResource};
 use crate::execution::entry_point::{
@@ -387,10 +388,12 @@ pub fn finalize_execution(
         inner_calls: syscall_handler_base.inner_calls,
         tracked_resource,
         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,
-        accessed_contract_addresses: syscall_handler_base.accessed_contract_addresses,
+        storage_access_tracker: StorageAccessTracker {
+            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,
+            accessed_contract_addresses: syscall_handler_base.accessed_contract_addresses,
+        },
     })
 }
 

crates/blockifier/src/execution/entry_point_test.rs

@@ -491,9 +491,9 @@ fn test_storage_related_members() {
         ..trivial_external_entry_point_new(test_contract)
     };
     let actual_call_info = entry_point_call.execute_directly(&mut state).unwrap();
-    assert_eq!(actual_call_info.storage_read_values, vec![felt!(39_u8)]);
+    assert_eq!(actual_call_info.storage_access_tracker.storage_read_values, vec![felt!(39_u8)]);
     assert_eq!(
-        actual_call_info.accessed_storage_keys,
+        actual_call_info.storage_access_tracker.accessed_storage_keys,
         HashSet::from([get_storage_var_address("number_map", &[felt!(1_u8)])])
     );
 
@@ -508,8 +508,11 @@ fn test_storage_related_members() {
         ..trivial_external_entry_point_new(test_contract)
     };
     let actual_call_info = entry_point_call.execute_directly(&mut state).unwrap();
-    assert_eq!(actual_call_info.storage_read_values, vec![value]);
-    assert_eq!(actual_call_info.accessed_storage_keys, HashSet::from([storage_key!(key_int)]));
+    assert_eq!(actual_call_info.storage_access_tracker.storage_read_values, vec![value]);
+    assert_eq!(
+        actual_call_info.storage_access_tracker.accessed_storage_keys,
+        HashSet::from([storage_key!(key_int)])
+    );
 }
 
 #[test]

crates/blockifier/src/execution/native/entry_point_execution.rs

@@ -131,10 +131,12 @@ fn create_callinfo(
         },
         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,
-        accessed_contract_addresses: syscall_handler.base.accessed_contract_addresses,
-        read_class_hash_values: syscall_handler.base.read_class_hash_values,
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: syscall_handler.base.read_values,
+            accessed_storage_keys: syscall_handler.base.accessed_keys,
+            accessed_contract_addresses: syscall_handler.base.accessed_contract_addresses,
+            read_class_hash_values: syscall_handler.base.read_class_hash_values,
+        },
         tracked_resource: TrackedResource::SierraGas,
     })
 }

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs

@@ -36,8 +36,10 @@ fn test_get_class_hash_at(runnable_version: RunnableCairo1) {
     let positive_call_info =
         positive_entry_point_call.clone().execute_directly(&mut state).unwrap();
     let redeposit_gas = 300;
-    assert!(positive_call_info.accessed_contract_addresses.contains(&address));
-    assert!(positive_call_info.read_class_hash_values[0] == class_hash);
+    assert!(
+        positive_call_info.storage_access_tracker.accessed_contract_addresses.contains(&address)
+    );
+    assert!(positive_call_info.storage_access_tracker.read_class_hash_values[0] == class_hash);
     assert_eq!(
         positive_call_info.execution,
         CallExecution {

crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs

@@ -6,7 +6,7 @@ use starknet_api::{calldata, felt, storage_key};
 use test_case::test_case;
 
 use crate::context::ChainInfo;
-use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
+use crate::execution::call_info::{CallExecution, CallInfo, Retdata, StorageAccessTracker};
 use crate::execution::entry_point::{CallEntryPoint, CallType};
 use crate::execution::syscalls::syscall_tests::constants::{
     REQUIRED_GAS_LIBRARY_CALL_TEST,
@@ -161,8 +161,11 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
             ..CallExecution::default()
         },
         tracked_resource,
-        storage_read_values: vec![felt!(value + 1)],
-        accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: vec![felt!(value + 1)],
+            accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
+            ..Default::default()
+        },
         ..Default::default()
     };
 
@@ -185,8 +188,11 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) {
             gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
             ..CallExecution::default()
         },
-        storage_read_values: vec![felt!(value)],
-        accessed_storage_keys: HashSet::from([storage_key!(key)]),
+        storage_access_tracker: StorageAccessTracker {
+            storage_read_values: vec![felt!(value)],
+            accessed_storage_keys: HashSet::from([storage_key!(key)]),
+            ..Default::default()
+        },
         tracked_resource,
         ..Default::default()
     };