feat: refactored 'VisitedPcs' trait to remove UFCS calls

crates/blockifier/src/blockifier/transaction_executor.rs

@@ -19,7 +19,7 @@ use crate::context::BlockContext;
 use crate::state::cached_state::{CachedState, CommitmentStateDiff, TransactionalState};
 use crate::state::errors::StateError;
 use crate::state::state_api::StateReader;
-use crate::state::visited_pcs::VisitedPcs;
+use crate::state::visited_pcs::{Pcs, VisitedPcs};
 use crate::transaction::errors::TransactionExecutionError;
 use crate::transaction::objects::TransactionExecutionInfo;
 use crate::transaction::transaction_execution::Transaction;
@@ -159,7 +159,7 @@ impl<S: StateReader, V: VisitedPcs> TransactionExecutor<S, V> {
                     .as_ref()
                     .expect(BLOCK_STATE_ACCESS_ERR)
                     .get_compiled_contract_class(*class_hash)?;
-                let class_visited_pcs = V::to_set(class_visited_pcs.clone());
+                let class_visited_pcs = class_visited_pcs.to_set();
                 Ok((*class_hash, contract_class.get_visited_segments(&class_visited_pcs)?))
             })
             .collect::<TransactionExecutorResult<_>>()?;

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -22,7 +22,7 @@ use crate::state::cached_state::{
 };
 use crate::state::errors::StateError;
 use crate::state::state_api::{State, StateReader, UpdatableState};
-use crate::state::visited_pcs::{VisitedPcs, VisitedPcsSet};
+use crate::state::visited_pcs::VisitedPcsSet;
 use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::deploy_account::deploy_account_tx;
 use crate::test_utils::dict_state_reader::DictStateReader;

crates/blockifier/src/state/cached_state.rs

@@ -13,6 +13,7 @@ use crate::context::TransactionContext;
 use crate::execution::contract_class::ContractClass;
 use crate::state::errors::StateError;
 use crate::state::state_api::{State, StateReader, StateResult, UpdatableState};
+use crate::state::visited_pcs::Pcs;
 use crate::transaction::objects::TransactionExecutionInfo;
 use crate::utils::{strict_subtract_mappings, subtract_mappings};
 
@@ -76,7 +77,7 @@ impl<S: StateReader, V: VisitedPcs> CachedState<S, V> {
 
     pub fn update_visited_pcs_cache(&mut self, visited_pcs: &V) {
         for (class_hash, class_visited_pcs) in visited_pcs.iter() {
-            V::add_visited_pcs(self, class_hash, class_visited_pcs.clone())
+            class_visited_pcs.add_visited_pcs(self, class_hash);
         }
     }
 

crates/blockifier/src/state/visited_pcs.rs

@@ -6,8 +6,18 @@ use starknet_api::core::ClassHash;
 
 use super::state_api::State;
 
-/// This trait is used in `CachedState` to record visited pcs of an entry point call. This allows
-/// flexible storage of program counters returned from cairo vm trace.
+/// This trait is used in [`VisitedPcs`]. It provides an interface to perform operations over the
+/// associated type of [`VisitedPcs`].
+pub trait Pcs {
+    /// Marks the given `pcs` values as visited for the given class hash.
+    fn add_visited_pcs(&self, state: &mut dyn State, class_hash: &ClassHash);
+
+    /// This function transforms the internal representation of program counters into a set.
+    fn to_set(&self) -> HashSet<usize>;
+}
+
+/// This trait is used in [`super::cached_state::CachedState`] to record visited pcs of an entry
+/// point call. This allows flexible storage of program counters returned from cairo vm trace.
 ///
 /// # Object Safety
 ///
@@ -18,17 +28,15 @@ use super::state_api::State;
 ///
 /// Self Bounds
 ///
-/// - [`Default`] is required to allow a default instantiation of `CachedState`.
+/// - [`Default`] is required to allow a default instantiation of
+///   [`super::cached_state::CachedState`].
 /// - [`Debug`] is required for compatibility with other structs which derive `Debug`.
 pub trait VisitedPcs
 where
     Self: Default + Debug,
 {
     /// This is the type which contains visited program counters.
-    ///
-    /// [`Clone`] is required to allow ownership of data throught cloning when receiving references
-    /// from one of the trait methods.
-    type Pcs: Clone;
+    type T: Pcs;
 
     /// Constructs a concrete implementation of the trait.
     fn new() -> Self;
@@ -42,57 +50,54 @@ where
     /// This function extends the program counters in `self` with those from another instance.
     ///
     /// It is used to transfer the visited program counters from one object to another.
-    fn extend(&mut self, class_hash: &ClassHash, pcs: &Self::Pcs);
+    fn extend(&mut self, class_hash: &ClassHash, pcs: &Self::T);
 
     /// This function returns an iterator of `VisitedPcs`.
     ///
     /// One tuple is returned for each class hash recorded in `self`.
-    fn iter(&self) -> impl Iterator<Item = (&ClassHash, &Self::Pcs)>;
+    fn iter(&self) -> impl Iterator<Item = (&ClassHash, &Self::T)>;
 
     /// Get the recorded visited program counters for a specific `class_hash`.
-    fn entry(&mut self, class_hash: ClassHash) -> Entry<'_, ClassHash, Self::Pcs>;
+    fn entry(&mut self, class_hash: ClassHash) -> Entry<'_, ClassHash, Self::T>;
+}
 
-    /// Marks the given `pcs` values as visited for the given class hash.
-    fn add_visited_pcs(state: &mut dyn State, class_hash: &ClassHash, pcs: Self::Pcs);
+/// [`PcsSet`] groups program counters in a [`std::collections::HashSet`].
+pub type PcsSet = HashSet<usize>;
+impl Pcs for PcsSet {
+    fn add_visited_pcs(&self, state: &mut dyn State, class_hash: &ClassHash) {
+        state.add_visited_pcs(*class_hash, &Vec::from_iter(self.clone()));
+    }
 
-    /// This function transforms the internal representation of program counters into a set.
-    fn to_set(pcs: Self::Pcs) -> HashSet<usize>;
+    fn to_set(&self) -> HashSet<usize> {
+        self.clone()
+    }
 }
 
 /// [`VisitedPcsSet`] is the default implementation of the trait [`VisiedPcs`]. All visited program
 /// counters are inserted in a set and grouped by class hash.
 ///
-/// This is also the structure used by the `native_blockifier`.
-#[derive(Debug, Default, PartialEq, Eq)]
-pub struct VisitedPcsSet(HashMap<ClassHash, HashSet<usize>>);
+/// This is also used by the `native_blockifier`.
+pub type VisitedPcsSet = HashMap<ClassHash, PcsSet>;
 impl VisitedPcs for VisitedPcsSet {
-    type Pcs = HashSet<usize>;
+    type T = PcsSet;
 
     fn new() -> Self {
-        VisitedPcsSet(HashMap::default())
+        VisitedPcsSet::default()
     }
 
     fn insert(&mut self, class_hash: &ClassHash, pcs: &[usize]) {
-        self.0.entry(*class_hash).or_default().extend(pcs);
+        self.entry(*class_hash).or_default().extend(pcs);
     }
 
-    fn extend(&mut self, class_hash: &ClassHash, pcs: &Self::Pcs) {
-        self.0.entry(*class_hash).or_default().extend(pcs);
+    fn extend(&mut self, class_hash: &ClassHash, pcs: &Self::T) {
+        self.entry(*class_hash).or_default().extend(pcs);
     }
 
-    fn iter(&self) -> impl Iterator<Item = (&ClassHash, &Self::Pcs)> {
-        self.0.iter()
+    fn iter(&self) -> impl Iterator<Item = (&ClassHash, &Self::T)> {
+        self.iter()
     }
 
     fn entry(&mut self, class_hash: ClassHash) -> Entry<'_, ClassHash, HashSet<usize>> {
-        self.0.entry(class_hash)
-    }
-
-    fn add_visited_pcs(state: &mut dyn State, class_hash: &ClassHash, pcs: Self::Pcs) {
-        state.add_visited_pcs(*class_hash, &Vec::from_iter(pcs));
-    }
-
-    fn to_set(pcs: Self::Pcs) -> HashSet<usize> {
-        pcs
+        self.entry(class_hash)
     }
 }