Fix: Updates the cache with the initial values of cells that were onl…

…y accessed via write (affects the fee calculation and the state diff).

crates/blockifier/src/state/cached_state.rs

@@ -50,17 +50,59 @@ impl<S: StateReader> CachedState<S> {
     /// Returns the number of storage changes done through this state.
     /// Any change to the contract's state (storage, nonce, class hash) is considered.
     // TODO(Noa, 30/04/23): Add nonce count.
-    pub fn count_actual_state_changes(&self) -> (usize, usize, usize) {
+    pub fn count_actual_state_changes(&mut self) -> StateResult<(usize, usize, usize)> {
+        self.update_initial_values_of_write_only_access()?;
+
         // Storage Update.
         let storage_updates = &self.cache.get_storage_updates();
+
         let mut modified_contracts: HashSet<ContractAddress> =
             storage_updates.keys().map(|address_key_pair| address_key_pair.0).collect();
 
         // Class hash Update (deployed contracts + replace_class syscall).
         let class_hash_updates = &self.cache.get_class_hash_updates();
         modified_contracts.extend(class_hash_updates.keys());
 
-        (storage_updates.len(), modified_contracts.len(), class_hash_updates.len())
+        Ok((storage_updates.len(), modified_contracts.len(), class_hash_updates.len()))
+    }
+
+    /// Updates cache with initial cell values for write-only access.
+    /// If written values match the original, the cell is unchanged and not counted as a
+    /// storage-change for fee calculation.
+    /// Same for class hash and nonce writes.
+    // TODO(Noa, 30/07/23): Consider adding DB getters in bulk (via a DB read transaction).
+    fn update_initial_values_of_write_only_access(&mut self) -> StateResult<()> {
+        // Eliminate storage writes that are identical to the initial value (no change). Assumes
+        // that `set_storage_at` does not affect the state field.
+        for (contract_storage_key, _v) in self.cache.storage_writes.iter() {
+            if self.cache.storage_initial_values.get(contract_storage_key).is_none() {
+                // First access to this cell was write; cache initial value.
+                self.cache.storage_initial_values.insert(
+                    *contract_storage_key,
+                    self.state.get_storage_at(contract_storage_key.0, contract_storage_key.1)?,
+                );
+            }
+        }
+
+        for (contract_address, _v) in self.cache.class_hash_writes.iter() {
+            if self.cache.class_hash_initial_values.get(contract_address).is_none() {
+                // First access to this cell was write; cache initial value.
+                self.cache
+                    .class_hash_initial_values
+                    .insert(*contract_address, self.state.get_class_hash_at(*contract_address)?);
+            }
+        }
+
+        for (contract_address, _v) in self.cache.nonce_writes.iter() {
+            if self.cache.nonce_initial_values.get(contract_address).is_none() {
+                // First access to this cell was write; cache initial value.
+                self.cache
+                    .nonce_initial_values
+                    .insert(*contract_address, self.state.get_nonce_at(*contract_address)?);
+            }
+        }
+
+        Ok(())
     }
 }
 
@@ -188,6 +230,7 @@ impl<S: StateReader> State for CachedState<S> {
         Ok(())
     }
 
+    // Assumes calling to `count_actual_state_changes` before. See its documentation.
     fn to_state_diff(&self) -> CommitmentStateDiff {
         type StorageDiff = IndexMap<ContractAddress, IndexMap<StorageKey, StarkFelt>>;
 

crates/blockifier/src/state/cached_state_test.rs

@@ -258,6 +258,7 @@ fn cached_state_state_diff_conversion() {
 #[test]
 fn count_actual_state_changes() {
     let contract_address = ContractAddress(patricia_key!("0x100"));
+    let contract_address2 = ContractAddress(patricia_key!("0x101"));
     let class_hash = ClassHash(stark_felt!("0x10"));
     let key = StorageKey(patricia_key!("0x10"));
     let storage_val: StarkFelt = stark_felt!("0x1");
@@ -266,8 +267,14 @@ fn count_actual_state_changes() {
     state.set_class_hash_at(contract_address, class_hash).unwrap();
     state.set_storage_at(contract_address, key, storage_val);
 
+    // Set the storage with its already exists value (should't count as a change).
+    // As the first access:
+    state.set_storage_at(contract_address2, key, StarkFelt::default());
+    // As the second access:
+    state.set_storage_at(contract_address, key, storage_val);
+
     let (n_storage_updates, n_modified_contracts, n_class_updates) =
-        state.count_actual_state_changes();
+        state.count_actual_state_changes().unwrap();
 
     assert_eq!((n_storage_updates, n_modified_contracts, n_class_updates), (1, 1, 1));
 }

crates/blockifier/src/transaction/transaction_utils.rs

@@ -44,7 +44,7 @@ pub fn calculate_tx_resources<S: StateReader>(
     l1_handler_payload_size: Option<usize>,
 ) -> TransactionExecutionResult<ResourcesMapping> {
     let (n_storage_changes, n_modified_contracts, n_class_updates) =
-        state.count_actual_state_changes();
+        state.count_actual_state_changes()?;
 
     let mut l2_to_l1_payloads_length = vec![];
     for call_info in call_infos {