Make Rollbackable fields private (#209)

* Derive partial-eq for VmState

* Add initial gas param for VmState new

* Update era-compiler-tester branch

* fix test bytecode_publishing

* Store changes for get_used_contracts tests

* adapt to zksync-era tests

* Remove hash_map from Storage trait

* Fix increment_tx_number not cleaning transient_storage

* Fix decommit opcode

Fixes decommit opcode, which was not account for if the code had already been decommitted

* Various far_call fixes

Adds mandated gass, handles decommit cost err, and fixes the params of mimic call

* Add stipend param to frame and decrease it in panics and reverts

* Add no refund storage_read for far_call decommit_code_hash

* Remove rollbacks from main context

* Reimplement vec snapshots

* Add external snapshot(whole vm)

* Add storage cache and util functio to get storage changes from initial

* Refactor state pub fields

* Add new function to get storage changes from snapshot or initial

* Update zksync-era submodule

* Update era-compiler-tester submodule

* Address clippy warnings

* More clippy warnings

* Update zksync-era submodule

* ci: add submodule step for era

* initial commit

* Address review comments

* Add cache to storage_read

* Update zksync-era submodule

* Add full tracers implementation

* Refactor how the vm handles tracers

* Update era-compiler-tester submodule

* Update zksync-era submodule

* Address clippy warnings

* Add default function in Tracer trait

* Address clippy warnings

* Address review comments

* Add docs to state

* Address review comments

* Remove optional from run with tracers

* Update era-compiler-tester submodule

* Update era-compiler-tester submodule

* Address clippy warnings

* Fix storage_read

* update zksync era submodule

* Implement statistics (#213)

* Add vm statistics

* Add tracking of precompiles cycles

* Add storage and decommit cycles

* Make opcode variant pub

* Add cycles counter

* Update zksync-era submodule

* Address clippy warnings

* Update era-compiler-tester submodule

* Update zksync-era submodule

* Fix initial value read in storage_changes

* Expose opcode variants

* Update zksync-era submodule

* Address review comments

* Update zksync-era submodule

* Move storage param to vm run

* Update zksync era submodule

* Update compiler tester submodule

* Lint

* Remove print

* Update zksync era submodule

* Add heap size optimization

* Update submodules

* Address comments

* Lint

* Update submodule

---------

Co-authored-by: Marcos Nicolau <[email protected]>
Co-authored-by: Fran <[email protected]>
Co-authored-by: Marcos Nicolau <[email protected]>
Co-authored-by: Gianbelinche <[email protected]>

src/rollbacks.rs

@@ -11,7 +11,7 @@ pub trait Rollbackable {
 
 #[derive(Debug, Default, Clone)]
 pub struct RollbackableHashMap<K: Clone + Hash, V: Clone> {
-    pub map: HashMap<K, V>,
+    map: HashMap<K, V>,
 }
 
 impl<K: Clone + Hash + Eq, V: Clone> RollbackableHashMap<K, V> {
@@ -21,6 +21,18 @@ impl<K: Clone + Hash + Eq, V: Clone> RollbackableHashMap<K, V> {
         }
     }
 
+    pub fn insert(&mut self, key: K, value: V) {
+        self.map.insert(key, value);
+    }
+
+    pub fn get(&self, key: &K) -> Option<&V> {
+        self.map.get(key)
+    }
+
+    pub fn inner_ref(&self) -> &HashMap<K, V> {
+        &self.map
+    }
+
     pub fn get_logs_after_snapshot(
         &self,
         snapshot: <RollbackableHashMap<K, V> as Rollbackable>::Snapshot,
@@ -55,7 +67,7 @@ impl<K: Clone + Hash, V: Clone> Iterator for RollbackableHashMap<K, V> {
 
 #[derive(Debug, Default, Clone)]
 pub struct RollbackableVec<T: Clone> {
-    pub entries: Vec<T>,
+    entries: Vec<T>,
 }
 
 impl<T: Clone> RollbackableVec<T> {
@@ -65,6 +77,14 @@ impl<T: Clone> RollbackableVec<T> {
         }
     }
 
+    pub fn push(&mut self, value: T) {
+        self.entries.push(value);
+    }
+
+    pub fn entries(&self) -> &[T] {
+        &self.entries
+    }
+
     pub fn get_logs_after_snapshot(
         &self,
         snapshot: <RollbackableVec<T> as Rollbackable>::Snapshot,
@@ -89,7 +109,7 @@ impl<T: Clone> Rollbackable for RollbackableVec<T> {
 
 #[derive(Debug, Default, Clone)]
 pub struct RollbackablePrimitive<T: Copy> {
-    pub value: T,
+    value: T,
 }
 
 impl<T: Copy> Rollbackable for RollbackablePrimitive<T> {
@@ -103,9 +123,23 @@ impl<T: Copy> Rollbackable for RollbackablePrimitive<T> {
     }
 }
 
+impl<T: Copy> RollbackablePrimitive<T> {
+    pub fn new(value: T) -> Self {
+        Self { value }
+    }
+
+    pub fn value(&self) -> T {
+        self.value
+    }
+
+    pub fn set(&mut self, value: T) {
+        self.value = value;
+    }
+}
+
 #[derive(Debug, Default, Clone)]
 pub struct RollbackableHashSet<K: Clone> {
-    pub map: HashSet<K>,
+    map: HashSet<K>,
 }
 
 impl<K: Clone> Rollbackable for RollbackableHashSet<K> {
@@ -118,3 +152,17 @@ impl<K: Clone> Rollbackable for RollbackableHashSet<K> {
         self.map.clone()
     }
 }
+
+impl<K: Clone + Eq + Hash> RollbackableHashSet<K> {
+    pub fn insert(&mut self, value: K) -> bool {
+        self.map.insert(value)
+    }
+
+    pub fn contains(&self, value: &K) -> bool {
+        self.map.contains(value)
+    }
+
+    pub fn inner_ref(&self) -> &HashSet<K> {
+        &self.map
+    }
+}

src/state.rs

@@ -82,15 +82,15 @@ impl VMState {
     }
 
     pub fn storage_changes(&self) -> &HashMap<StorageKey, U256> {
-        &self.storage_changes.map
+        self.storage_changes.inner_ref()
     }
 
     pub fn transient_storage(&self) -> &HashMap<StorageKey, U256> {
-        &self.transient_storage.map
+        self.transient_storage.inner_ref()
     }
 
-    pub fn l2_to_l1_logs(&self) -> &Vec<L2ToL1Log> {
-        &self.l2_to_l1_logs.entries
+    pub fn l2_to_l1_logs(&self) -> &[L2ToL1Log] {
+        self.l2_to_l1_logs.entries()
     }
 
     pub fn get_l2_to_l1_logs_after_snapshot(
@@ -100,8 +100,8 @@ impl VMState {
         self.l2_to_l1_logs.get_logs_after_snapshot(snapshot)
     }
 
-    pub fn events(&self) -> &Vec<Event> {
-        &self.events.entries
+    pub fn events(&self) -> &[Event] {
+        self.events.entries()
     }
 
     pub fn get_events_after_snapshot(
@@ -111,28 +111,29 @@ impl VMState {
         self.events.get_logs_after_snapshot(snapshot)
     }
 
-    pub fn refunds(&self) -> &Vec<u32> {
-        &self.refunds.entries
+    pub fn refunds(&self) -> &[u32] {
+        self.refunds.entries()
     }
 
-    pub fn pubdata_costs(&self) -> &Vec<i32> {
-        &self.pubdata_costs.entries
+    pub fn pubdata_costs(&self) -> &[i32] {
+        self.pubdata_costs.entries()
     }
 
     pub fn pubdata(&self) -> i32 {
-        self.pubdata.value
+        self.pubdata.value()
     }
 
     pub fn add_pubdata(&mut self, to_add: i32) {
-        self.pubdata.value += to_add;
+        let previous = self.pubdata.value();
+        self.pubdata.set(previous + to_add);
     }
 
     pub fn read_storage_slots(&self) -> &HashSet<StorageKey> {
-        &self.read_storage_slots.map
+        self.read_storage_slots.inner_ref()
     }
 
     pub fn written_storage_slots(&self) -> &HashSet<StorageKey> {
-        &self.written_storage_slots.map
+        self.written_storage_slots.inner_ref()
     }
 
     // reads shouldn't be mutable, we should consider change it to a non-mutable reference
@@ -142,16 +143,16 @@ impl VMState {
             .storage_read_inner(&key, storage)
             .map_or_else(U256::zero, |val| val);
 
-        let refund =
-            if storage.is_free_storage_slot(&key) || self.read_storage_slots.map.contains(&key) {
-                WARM_READ_REFUND
-            } else {
-                self.read_storage_slots.map.insert(key);
-                0
-            };
+        let refund = if storage.is_free_storage_slot(&key) || self.read_storage_slots.contains(&key)
+        {
+            WARM_READ_REFUND
+        } else {
+            self.read_storage_slots.insert(key);
+            0
+        };
 
-        self.pubdata_costs.entries.push(0);
-        self.refunds.entries.push(refund);
+        self.pubdata_costs.push(0);
+        self.refunds.push(refund);
 
         (value, refund)
     }
@@ -165,17 +166,16 @@ impl VMState {
             .storage_read_inner(&key, storage)
             .map_or_else(U256::zero, |val| val);
 
-        if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) {
-            self.read_storage_slots.map.insert(key);
+        if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.contains(&key) {
+            self.read_storage_slots.insert(key);
         };
 
-        self.pubdata_costs.entries.push(0);
-
+        self.pubdata_costs.push(0);
         value
     }
 
     fn storage_read_inner(&self, key: &StorageKey, storage: &mut dyn Storage) -> Option<U256> {
-        match self.storage_changes.map.get(key) {
+        match self.storage_changes.get(key) {
             None => storage.storage_read(key),
             value => value.copied(),
         }
@@ -187,32 +187,32 @@ impl VMState {
         value: U256,
         storage: &mut dyn Storage,
     ) -> u32 {
-        self.storage_changes.map.insert(key, value);
+        self.storage_changes.insert(key, value);
 
         if storage.is_free_storage_slot(&key) {
-            self.written_storage_slots.map.insert(key);
+            self.written_storage_slots.insert(key);
             let refund = WARM_WRITE_REFUND;
-            self.refunds.entries.push(refund);
-            self.pubdata_costs.entries.push(0);
+            self.refunds.push(refund);
+            self.pubdata_costs.push(0);
             return refund;
         }
 
         // after every write, we store the current cost paid
         // on subsequent writes, we don't charge for what has already been paid
         // but for the newer price, which if it is lower might end up in a refund
         let current_cost = storage.cost_of_writing_storage(&key, value);
-        let prev_cost = *self.paid_changes.map.get(&key).unwrap_or(&0);
-        self.paid_changes.map.insert(key, current_cost);
+        let prev_cost = *self.paid_changes.get(&key).unwrap_or(&0);
+        self.paid_changes.insert(key, current_cost);
 
-        let refund = if self.written_storage_slots.map.contains(&key) {
+        let refund = if self.written_storage_slots.contains(&key) {
             WARM_WRITE_REFUND
         } else {
-            self.written_storage_slots.map.insert(key);
+            self.written_storage_slots.insert(key);
 
-            if self.read_storage_slots.map.contains(&key) {
+            if self.read_storage_slots.contains(&key) {
                 COLD_WRITE_AFTER_WARM_READ_REFUND
             } else {
-                self.read_storage_slots.map.insert(key);
+                self.read_storage_slots.insert(key);
                 0
             }
         };
@@ -222,37 +222,38 @@ impl VMState {
         // the slots to their final values.
         // The only case where users may overpay is when a previous transaction ends up with a negative pubdata total.
         let pubdata_cost = (current_cost as i32) - (prev_cost as i32);
-        self.pubdata.value += pubdata_cost;
-        self.refunds.entries.push(refund);
-        self.pubdata_costs.entries.push(pubdata_cost);
+        let previous_pubdata = self.pubdata.value();
+        self.pubdata.set(previous_pubdata + pubdata_cost);
+        self.refunds.push(refund);
+        self.pubdata_costs.push(pubdata_cost);
 
         refund
     }
 
     pub fn transient_storage_read(&mut self, key: StorageKey) -> U256 {
-        self.pubdata_costs.entries.push(0);
+        self.pubdata_costs.push(0);
         self.transient_storage
-            .map
+            .inner_ref()
             .get(&key)
             .copied()
             .unwrap_or_default()
     }
 
     pub fn transient_storage_write(&mut self, key: StorageKey, value: U256) {
-        self.pubdata_costs.entries.push(0);
-        self.transient_storage.map.insert(key, value);
+        self.pubdata_costs.push(0);
+        self.transient_storage.insert(key, value);
     }
 
     pub(crate) fn clear_transient_storage(&mut self) {
         self.transient_storage = RollbackableHashMap::default();
     }
 
     pub fn record_l2_to_l1_log(&mut self, msg: L2ToL1Log) {
-        self.l2_to_l1_logs.entries.push(msg);
+        self.l2_to_l1_logs.push(msg);
     }
 
     pub fn record_event(&mut self, event: Event) {
-        self.events.entries.push(event);
+        self.events.push(event);
     }
 
     /// Attempts to decommit the specified `hash` and retrieves any changes made since the initial storage state.
@@ -262,12 +263,12 @@ impl VMState {
     /// - `Option<Vec<U256>>`: the contract bytecode
     /// - `bool`: A boolean flag indicating whether the hash was decommitted (`true` if it was newly decommitted, `false` if it had already been decommitted).
     pub fn decommit(&mut self, hash: U256, storage: &mut dyn Storage) -> (Option<Vec<U256>>, bool) {
-        let was_decommitted = !self.decommitted_hashes.map.insert(hash);
+        let was_decommitted = !self.decommitted_hashes.insert(hash);
         (storage.decommit(hash), was_decommitted)
     }
 
     pub fn decommitted_hashes(&self) -> &HashSet<U256> {
-        &self.decommitted_hashes.map
+        self.decommitted_hashes.inner_ref()
     }
 
     /// Retrieves the values that have changed since the initial storage.
@@ -282,8 +283,7 @@ impl VMState {
         &mut self,
         storage: &mut dyn Storage,
     ) -> Vec<(StorageKey, Option<U256>, U256)> {
-        self.storage_changes
-            .map
+        self.storage_changes()
             .iter()
             .filter_map(|(key, value)| {
                 let initial_value = storage.storage_read(key);