chore(blockifier, native_blockifier, starknet_batcher): remove visite…

…d PCS (#2974)

crates/blockifier/src/blockifier/transaction_executor.rs

@@ -1,11 +1,9 @@
-use std::collections::{HashMap, HashSet};
 use std::panic::{self, catch_unwind, AssertUnwindSafe};
 use std::sync::{Arc, Mutex};
 
 use itertools::FoldWhile::{Continue, Done};
 use itertools::Itertools;
 use starknet_api::block::BlockHashAndNumber;
-use starknet_api::core::ClassHash;
 use thiserror::Error;
 
 use crate::blockifier::block::pre_process_block;
@@ -39,7 +37,6 @@ pub enum TransactionExecutorError {
 }
 
 pub type TransactionExecutorResult<T> = Result<T, TransactionExecutorError>;
-pub type VisitedSegmentsMapping = Vec<(ClassHash, Vec<usize>)>;
 
 /// A transaction executor, used for building a single block.
 pub struct TransactionExecutor<S: StateReader> {
@@ -141,32 +138,9 @@ impl<S: StateReader> TransactionExecutor<S> {
         results
     }
 
-    /// Returns the state diff, a list of contract class hash with the corresponding list of
-    /// visited segment values and the block weights.
+    /// Returns the state diff and the block weights.
     // TODO(Yoav): Consume "self".
-    pub fn finalize(
-        &mut self,
-    ) -> TransactionExecutorResult<(CommitmentStateDiff, VisitedSegmentsMapping, BouncerWeights)>
-    {
-        // Get the visited segments of each contract class.
-        // This is done by taking all the visited PCs of each contract, and compress them to one
-        // representative for each visited segment.
-        let visited_segments = self
-            .block_state
-            .as_ref()
-            .expect(BLOCK_STATE_ACCESS_ERR)
-            .visited_pcs
-            .iter()
-            .map(|(class_hash, class_visited_pcs)| -> TransactionExecutorResult<_> {
-                let contract_class = self
-                    .block_state
-                    .as_ref()
-                    .expect(BLOCK_STATE_ACCESS_ERR)
-                    .get_compiled_class(*class_hash)?;
-                Ok((*class_hash, contract_class.get_visited_segments(class_visited_pcs)?))
-            })
-            .collect::<TransactionExecutorResult<_>>()?;
-
+    pub fn finalize(&mut self) -> TransactionExecutorResult<(CommitmentStateDiff, BouncerWeights)> {
         log::debug!("Final block weights: {:?}.", self.bouncer.get_accumulated_weights());
         let mut block_state = self.block_state.take().expect(BLOCK_STATE_ACCESS_ERR);
         let state_diff = if self.block_context.versioned_constants.enable_stateful_compression {
@@ -181,7 +155,7 @@ impl<S: StateReader> TransactionExecutor<S> {
         } else {
             block_state.to_state_diff()?.state_maps
         };
-        Ok((state_diff.into(), visited_segments, *self.bouncer.get_accumulated_weights()))
+        Ok((state_diff.into(), *self.bouncer.get_accumulated_weights()))
     }
 }
 
@@ -276,7 +250,6 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
 
         let n_committed_txs = worker_executor.scheduler.get_n_committed_txs();
         let mut tx_execution_results = Vec::new();
-        let mut visited_pcs: HashMap<ClassHash, HashSet<usize>> = HashMap::new();
         for execution_output in worker_executor.execution_outputs.iter() {
             if tx_execution_results.len() >= n_committed_txs {
                 break;
@@ -288,9 +261,6 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
                 .expect("Output must be ready.");
             tx_execution_results
                 .push(locked_execution_output.result.map_err(TransactionExecutorError::from));
-            for (class_hash, class_visited_pcs) in locked_execution_output.visited_pcs {
-                visited_pcs.entry(class_hash).or_default().extend(class_visited_pcs);
-            }
         }
 
         let block_state_after_commit = Arc::try_unwrap(worker_executor)
@@ -301,7 +271,7 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
                      it."
                 )
             })
-            .commit_chunk_and_recover_block_state(n_committed_txs, visited_pcs);
+            .commit_chunk_and_recover_block_state(n_committed_txs);
         self.block_state.replace(block_state_after_commit);
 
         tx_execution_results

crates/blockifier/src/concurrency/fee_utils.rs

@@ -118,5 +118,5 @@ pub fn add_fee_to_sequencer_balance(
         ]),
         ..StateMaps::default()
     };
-    state.apply_writes(&writes, &ContractClassMapping::default(), &HashMap::default());
+    state.apply_writes(&writes, &ContractClassMapping::default());
 }

crates/blockifier/src/concurrency/flow_test.rs

@@ -48,11 +48,7 @@ fn scheduler_flow_test(
                                 Task::ExecutionTask(tx_index),
                                 &versioned_state,
                             );
-                            state_proxy.apply_writes(
-                                &new_writes,
-                                &ContractClassMapping::default(),
-                                &HashMap::default(),
-                            );
+                            state_proxy.apply_writes(&new_writes, &ContractClassMapping::default());
                             scheduler.finish_execution_during_commit(tx_index);
                         }
                     }
@@ -61,11 +57,9 @@ fn scheduler_flow_test(
                     Task::ExecutionTask(tx_index) => {
                         let (_, writes) =
                             get_reads_writes_for(Task::ExecutionTask(tx_index), &versioned_state);
-                        versioned_state.pin_version(tx_index).apply_writes(
-                            &writes,
-                            &ContractClassMapping::default(),
-                            &HashMap::default(),
-                        );
+                        versioned_state
+                            .pin_version(tx_index)
+                            .apply_writes(&writes, &ContractClassMapping::default());
                         scheduler.finish_execution(tx_index);
                         Task::AskForTask
                     }

crates/blockifier/src/concurrency/versioned_state.rs

@@ -1,4 +1,3 @@
-use std::collections::{HashMap, HashSet};
 use std::sync::{Arc, Mutex, MutexGuard};
 
 use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce};
@@ -199,11 +198,7 @@ impl<S: StateReader> VersionedState<S> {
 }
 
 impl<U: UpdatableState> VersionedState<U> {
-    pub fn commit_chunk_and_recover_block_state(
-        mut self,
-        n_committed_txs: usize,
-        visited_pcs: HashMap<ClassHash, HashSet<usize>>,
-    ) -> U {
+    pub fn commit_chunk_and_recover_block_state(mut self, n_committed_txs: usize) -> U {
         if n_committed_txs == 0 {
             return self.into_initial_state();
         }
@@ -212,7 +207,7 @@ impl<U: UpdatableState> VersionedState<U> {
         let class_hash_to_class =
             self.compiled_contract_classes.get_writes_up_to_index(commit_index);
         let mut state = self.into_initial_state();
-        state.apply_writes(&writes, &class_hash_to_class, &visited_pcs);
+        state.apply_writes(&writes, &class_hash_to_class);
         state
     }
 }
@@ -271,14 +266,8 @@ impl<S: StateReader> VersionedStateProxy<S> {
     }
 }
 
-// TODO(Noa, 15/5/24): Consider using visited_pcs.
 impl<S: StateReader> UpdatableState for VersionedStateProxy<S> {
-    fn apply_writes(
-        &mut self,
-        writes: &StateMaps,
-        class_hash_to_class: &ContractClassMapping,
-        _visited_pcs: &HashMap<ClassHash, HashSet<usize>>,
-    ) {
+    fn apply_writes(&mut self, writes: &StateMaps, class_hash_to_class: &ContractClassMapping) {
         self.state().apply_writes(self.tx_index, writes, class_hash_to_class)
     }
 }

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -441,7 +441,6 @@ fn test_apply_writes(
     safe_versioned_state.pin_version(0).apply_writes(
         &transactional_states[0].cache.borrow().writes,
         &transactional_states[0].class_hash_to_class.borrow().clone(),
-        &HashMap::default(),
     );
     assert!(transactional_states[1].get_class_hash_at(contract_address).unwrap() == class_hash_0);
     assert!(transactional_states[1].get_compiled_class(class_hash).unwrap() == contract_class_0);
@@ -470,7 +469,6 @@ fn test_apply_writes_reexecute_scenario(
     safe_versioned_state.pin_version(0).apply_writes(
         &transactional_states[0].cache.borrow().writes,
         &transactional_states[0].class_hash_to_class.borrow().clone(),
-        &HashMap::default(),
     );
     // Although transaction 0 wrote to the shared state, version 1 needs to be re-executed to see
     // the new value (its read value has already been cached).
@@ -516,11 +514,9 @@ fn test_delete_writes(
                 feature_contract.get_runnable_class(),
             )
             .unwrap();
-        safe_versioned_state.pin_version(i).apply_writes(
-            &tx_state.cache.borrow().writes,
-            &tx_state.class_hash_to_class.borrow(),
-            &HashMap::default(),
-        );
+        safe_versioned_state
+            .pin_version(i)
+            .apply_writes(&tx_state.cache.borrow().writes, &tx_state.class_hash_to_class.borrow());
     }
 
     safe_versioned_state.pin_version(tx_index_to_delete_writes).delete_writes(
@@ -579,11 +575,7 @@ fn test_delete_writes_completeness(
     let tx_index = 0;
     let mut versioned_state_proxy = safe_versioned_state.pin_version(tx_index);
 
-    versioned_state_proxy.apply_writes(
-        &state_maps_writes,
-        &class_hash_to_class_writes,
-        &HashMap::default(),
-    );
+    versioned_state_proxy.apply_writes(&state_maps_writes, &class_hash_to_class_writes);
     assert_eq!(
         safe_versioned_state.0.lock().unwrap().get_writes_of_index(tx_index),
         state_maps_writes
@@ -658,9 +650,8 @@ fn test_versioned_proxy_state_flow(
     for proxy in versioned_proxy_states {
         drop(proxy);
     }
-    let modified_block_state = safe_versioned_state
-        .into_inner_state()
-        .commit_chunk_and_recover_block_state(4, HashMap::new());
+    let modified_block_state =
+        safe_versioned_state.into_inner_state().commit_chunk_and_recover_block_state(4);
 
     assert!(modified_block_state.get_class_hash_at(contract_address).unwrap() == class_hash_3);
     assert!(modified_block_state.get_compiled_class(class_hash).unwrap() == contract_class_2);

crates/blockifier/src/concurrency/worker_logic.rs

@@ -1,11 +1,9 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::fmt::Debug;
 use std::sync::Mutex;
 use std::thread;
 use std::time::Duration;
 
-use starknet_api::core::ClassHash;
-
 use super::versioned_state::VersionedState;
 use crate::blockifier::transaction_executor::TransactionExecutorError;
 use crate::bouncer::Bouncer;
@@ -33,7 +31,6 @@ pub struct ExecutionTaskOutput {
     // TODO(Yoni): rename to state_diff.
     pub writes: StateMaps,
     pub contract_classes: ContractClassMapping,
-    pub visited_pcs: HashMap<ClassHash, HashSet<usize>>,
     pub result: TransactionExecutionResult<TransactionExecutionInfo>,
 }
 
@@ -123,6 +120,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
     fn execute_tx(&self, tx_index: TxIndex) {
         let mut tx_versioned_state = self.state.pin_version(tx_index);
         let tx = &self.chunk[tx_index];
+        // TODO(Yoni): is it necessary to use a transactional state here?
         let mut transactional_state =
             TransactionalState::create_transactional(&mut tx_versioned_state);
         let concurrency_mode = true;
@@ -135,23 +133,19 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
                 let tx_reads_writes = transactional_state.cache.take();
                 let writes = tx_reads_writes.to_state_diff().state_maps;
                 let contract_classes = transactional_state.class_hash_to_class.take();
-                let visited_pcs = transactional_state.visited_pcs;
-                // The versioned state does not carry the visited PCs.
-                tx_versioned_state.apply_writes(&writes, &contract_classes, &HashMap::default());
+                tx_versioned_state.apply_writes(&writes, &contract_classes);
                 ExecutionTaskOutput {
                     reads: tx_reads_writes.initial_reads,
                     writes,
                     contract_classes,
-                    visited_pcs,
                     result: execution_result,
                 }
             }
             Err(_) => ExecutionTaskOutput {
                 reads: transactional_state.cache.take().initial_reads,
-                // Failed transaction - ignore the writes and visited PCs.
+                // Failed transaction - ignore the writes.
                 writes: StateMaps::default(),
                 contract_classes: HashMap::default(),
-                visited_pcs: HashMap::default(),
                 result: execution_result,
             },
         };
@@ -261,13 +255,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
 }
 
 impl<U: UpdatableState> WorkerExecutor<'_, U> {
-    pub fn commit_chunk_and_recover_block_state(
-        self,
-        n_committed_txs: usize,
-        visited_pcs: HashMap<ClassHash, HashSet<usize>>,
-    ) -> U {
-        self.state
-            .into_inner_state()
-            .commit_chunk_and_recover_block_state(n_committed_txs, visited_pcs)
+    pub fn commit_chunk_and_recover_block_state(self, n_committed_txs: usize) -> U {
+        self.state.into_inner_state().commit_chunk_and_recover_block_state(n_committed_txs)
     }
 }

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -385,7 +385,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) {
 
     assert_eq!(execution_output.writes, writes.diff(&reads));
     assert_eq!(execution_output.reads, reads);
-    assert_ne!(execution_output.visited_pcs, HashMap::default());
 
     // Failed execution.
     let tx_index = 1;
@@ -404,7 +403,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) {
     };
     assert_eq!(execution_output.reads, reads);
     assert_eq!(execution_output.writes, StateMaps::default());
-    assert_eq!(execution_output.visited_pcs, HashMap::default());
 
     // Reverted execution.
     let tx_index = 2;
@@ -418,7 +416,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) {
     let execution_output = execution_output.as_ref().unwrap();
     assert!(execution_output.result.as_ref().unwrap().is_reverted());
     assert_ne!(execution_output.writes, StateMaps::default());
-    assert_ne!(execution_output.visited_pcs, HashMap::default());
 
     // Validate status change.
     for tx_index in 0..3 {