From 04a22355b0c4e1efc1c6f98c92eeb98bf1751ba3 Mon Sep 17 00:00:00 2001
From: Eagle941 <8973725+Eagle941@users.noreply.github.com>
Date: Wed, 4 Sep 2024 22:35:34 +0100
Subject: [PATCH] chore: improved documentation for 'VisitedPcs'

---
 crates/blockifier/src/state/visited_pcs.rs | 43 +++++++++++++++++-----
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/crates/blockifier/src/state/visited_pcs.rs b/crates/blockifier/src/state/visited_pcs.rs
index 39b6820afa..306660ad0c 100644
--- a/crates/blockifier/src/state/visited_pcs.rs
+++ b/crates/blockifier/src/state/visited_pcs.rs
@@ -6,38 +6,63 @@ 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 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.
+///
+/// # Object Safety
+///
+/// This trait uses associated types instead of generics because only one implementation of the
+/// trait is required. Also, using associated types reduces the number of parameters required to be
+/// specified.
+/// The use of associated types makes the trait implementation not [object safe](https://doc.rust-lang.org/reference/items/traits.html#object-safety).
+///
+/// Self Bounds
+///
+/// - [`Default`] is required to allow a default instantiation of `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;
 
+    /// Constructs a concrete implementation of the trait.
     fn new() -> Self;
 
-    /// The function `insert` reads the program counters returned by the cairo vm trace.
+    /// This function records the program counters returned by the cairo vm trace.
     ///
     /// The elements of the vector `pcs` match the type of field `pc` in
     /// [`cairo_vm::vm::trace::trace_entry::RelocatedTraceEntry`]
     fn insert(&mut self, class_hash: &ClassHash, pcs: &[usize]);
 
-    /// The function `extend` is used to extend an instance of `VisitedPcs` with another one.
+    /// 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);
 
     /// 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)>;
 
     /// Get the recorded visited program counters for a specific `class_hash`.
     fn entry(&mut self, class_hash: ClassHash) -> Entry<'_, ClassHash, Self::Pcs>;
 
-    /// Marks the given PC values as visited for the given class hash.
+    /// 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);
 
-    /// This function returns the program counters in a set.
+    /// This function transforms the internal representation of program counters into a set.
     fn to_set(pcs: Self::Pcs) -> HashSet<usize>;
 }
 
+/// [`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>>);
 impl VisitedPcs for VisitedPcsSet {
@@ -51,6 +76,10 @@ impl VisitedPcs for VisitedPcsSet {
         self.0.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 iter(&self) -> impl Iterator<Item = (&ClassHash, &Self::Pcs)> {
         self.0.iter()
     }
@@ -63,10 +92,6 @@ impl VisitedPcs for VisitedPcsSet {
         state.add_visited_pcs(*class_hash, &Vec::from_iter(pcs));
     }
 
-    fn extend(&mut self, class_hash: &ClassHash, pcs: &Self::Pcs) {
-        self.0.entry(*class_hash).or_default().extend(pcs);
-    }
-
     fn to_set(pcs: Self::Pcs) -> HashSet<usize> {
         pcs
     }