Fix: Updates the cache with the initial values of cells that were only accessed via write (affects the fee calculation and the state diff).

[noaov1]

This change is 

[noaov1]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion

---

crates/blockifier/src/state/cached_state.rs line 215 at r1 (raw file):

    }

    // Assumes calling to `count_actual_state_changes` before. See its documentation.

I can call update_initial_values_of_write_only_access, but again it can return an error and that will change the state api.

Code quote:

 // Assumes calling to `count_actual_state_changes` before. See its documentation.

[giladchase]

Note for future reference: In Python we query the DB during each set_X command to get the initial value, here we do it in bulk towards the end of the tx, and keep the setters IO-free.

Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

crates/blockifier/src/state/cached_state.rs line 71 at r2 (raw file):

    /// is done so we can check if the written value is identical to the value previously held
    /// at that address. In this case, no change is made to that cell and it does not count as a
    /// storage-change in fee calculation.

Also plz add todo to do the DB getters in bulk (via a DB read transaction).

Suggestion:

/// 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.

[Yoni-Starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @giladchase and @noaov1)

---

crates/blockifier/src/state/cached_state.rs line 75 at r2 (raw file):

        // Eliminate storage writes that are identical to the initial value (no change). Assumes
        // that `set_storage_at` does not affect the state field.
        let mut storage_write_first_access: HashMap<ContractStorageKey, StarkFelt> = HashMap::new();

Why do you need this?

Code quote:

 storage_write_first_access

---

crates/blockifier/src/state/cached_state.rs line 83 at r2 (raw file):

                    self.state.get_storage_at(contract_storage_key.0, contract_storage_key.1)?,
                );
            }

Suggestion:

        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.
                storage_write_first_access.insert(
                    *contract_storage_key,
                    self.state.get_storage_at(contract_storage_key.0, contract_storage_key.1)?,
                );
            }

[noaov1]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @giladchase and @Yoni-Starkware)

---

crates/blockifier/src/state/cached_state.rs line 75 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why do you need this?

WDYM?

[Yoni-Starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @giladchase and @noaov1)

---

crates/blockifier/src/state/cached_state.rs line 75 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

WDYM?

Why not self.cache.storage_initial_values.insert(
instead of storage_write_first_access.insert(

[noaov1]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @giladchase and @Yoni-Starkware)

---

crates/blockifier/src/state/cached_state.rs line 75 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why not self.cache.storage_initial_values.insert(
instead of storage_write_first_access.insert(

For no good reason. Fixed.

---

crates/blockifier/src/state/cached_state.rs line 83 at r2 (raw file):

                    self.state.get_storage_at(contract_storage_key.0, contract_storage_key.1)?,
                );
            }

Done.

[giladchase]

Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 2 of 3 files reviewed, 6 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

crates/blockifier/src/state/cached_state.rs line 76 at r2 (raw file):

        // that `set_storage_at` does not affect the state field.
        let mut storage_write_first_access: HashMap<ContractStorageKey, StarkFelt> = HashMap::new();
        for (contract_storage_key, _v) in self.cache.storage_writes.iter() {

Suggestion:

for contract_storage_key in self.cache.storage_writes.keys()

---

crates/blockifier/src/state/cached_state.rs line 78 at r2 (raw file):

        for (contract_storage_key, _v) in self.cache.storage_writes.iter() {
            // First access to this cell was write; cache initial value.
            if self.cache.storage_initial_values.get(contract_storage_key).is_none() {

Suggestion:

if !self.cache.storage_initial_values.contains_key(contract_storage_key) {

---

crates/blockifier/src/state/cached_state_test.rs line 270 at r4 (raw file):

    state.set_storage_at(contract_address, key, storage_val);

    // Set the storage with its already exists value (should't count as a change).

Suggestion:

    // Assign the existing value to the storage (this shouldn't be considered a change).

[noaov1]

Reviewable status: 1 of 3 files reviewed, 6 unresolved discussions (waiting on @giladchase and @Yoni-Starkware)

---

crates/blockifier/src/state/cached_state.rs line 76 at r2 (raw file):

        // that `set_storage_at` does not affect the state field.
        let mut storage_write_first_access: HashMap<ContractStorageKey, StarkFelt> = HashMap::new();
        for (contract_storage_key, _v) in self.cache.storage_writes.iter() {

Done. Thanks!

---

crates/blockifier/src/state/cached_state.rs line 78 at r2 (raw file):

        for (contract_storage_key, _v) in self.cache.storage_writes.iter() {
            // First access to this cell was write; cache initial value.
            if self.cache.storage_initial_values.get(contract_storage_key).is_none() {

Done.

---

crates/blockifier/src/state/cached_state_test.rs line 270 at r4 (raw file):

    state.set_storage_at(contract_address, key, storage_val);

    // Set the storage with its already exists value (should't count as a change).

Done.

[Yoni-Starkware]

Reviewed 1 of 2 files at r5.
Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions (waiting on @giladchase)

[giladchase]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware)

[Yoni-Starkware]

Reviewed 1 of 3 files at r1, 1 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)