starknet_os: parse os state diff for testing

[nimrod-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[nimrod-starkware]

• starknet_os: compare decompressed state diff #7818
• blockifier: externalize decompression utils #7846
• starknet_os: parse os state diff for testing #7792 👈 (View in Graphite)
• starknet_os: allocate and replace aliases test #7745 : 1 other dependent PR (#7765 )
• starknet_os: refactor cairo runner utils #7744
• starknet_os: pointer args wrap Vec<MaybeRelocatable> #7743
• main-v0.14.0

This stack of pull requests is managed by Graphite. Learn more about stacking.

[nimrod-starkware]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @amosStarkware and @dorimedini-starkware)

---

crates/starknet_os/src/test_utils/utils.rs line 163 at r1 (raw file):

pub(crate) type DictEntry = [Felt; DICT_ACCESS_SIZE];

@dorimedini-starkware this code should be also useful for the aggregator, right?
Maybe worth moving it somewhere else and use errors instead?

[dorimedini-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @amosStarkware and @TzahiTaub)

---

crates/starknet_os/src/test_utils/utils.rs line 163 at r1 (raw file):

Previously, nimrod-starkware wrote…

@dorimedini-starkware this code should be also useful for the aggregator, right?
Maybe worth moving it somewhere else and use errors instead?

WDYM by "use errors instead"?
can you ask @TzahiTaub if this code is useful in the aggregator area?

---

crates/starknet_os/src/test_utils/utils.rs line 0 at r1 (raw file):
please sync with @TzahiTaub , by a brief overview I see there may be code sharing potential with the aggregator code

---

crates/starknet_os/src/test_utils/utils.rs line 220 at r1 (raw file):

        let contracts = (0..n_contracts)
            .map(|_| FullOutputContractChanges::new_from_iter(os_output_iter))
            .collect();

since the order is crucial here, consider converting this to a for loop.
iterators can be easily parallelized, but doing so in this case would be incorrect.
(unless you can quote the rust docs for a guaranteed execution order :) )

Code quote:

        let contracts = (0..n_contracts)
            .map(|_| FullOutputContractChanges::new_from_iter(os_output_iter))
            .collect();

---

crates/starknet_os/src/tests/aliases.rs line 454 at r1 (raw file):

    #[case] initial_alias_storage: HashMap<u128, u128>,
    #[case] expected_alias_storage: HashMap<u128, u128>,
    #[case] contract_state_diff_len: usize,

can't this parameter be computed by the test input?

Code quote:

#[case] contract_state_diff_len: usize,

[dorimedini-starkware]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @amosStarkware, @nimrod-starkware, and @TzahiTaub)

[nimrod-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @amosStarkware, @dorimedini-starkware, and @TzahiTaub)

---

crates/starknet_os/src/test_utils/utils.rs line 220 at r1 (raw file):

Previously, dorimedini-starkware wrote…

since the order is crucial here, consider converting this to a for loop.
iterators can be easily parallelized, but doing so in this case would be incorrect.
(unless you can quote the rust docs for a guaranteed execution order :) )

Maybe this comment is relevant to Tzahi's implementation?

---

crates/starknet_os/src/test_utils/utils.rs line at r1 (raw file):

Previously, dorimedini-starkware wrote…

please sync with @TzahiTaub , by a brief overview I see there may be code sharing potential with the aggregator code

Talked to him and used his implementation.

---

crates/starknet_os/src/tests/aliases.rs line 454 at r1 (raw file):

Previously, dorimedini-starkware wrote…

can't this parameter be computed by the test input?

ATM, the cairo runner has a limitation that it needs to know in advance the length of an array. These numbers were calculated from python...
Maybe worth adding a TODO to extend the cairo runner to be able to load an entire segment without knowing in advance the length.

[dorimedini-starkware]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @amosStarkware and @nimrod-starkware)

---

crates/starknet_os/src/tests/aliases.rs line 454 at r1 (raw file):

Previously, nimrod-starkware wrote…

ATM, the cairo runner has a limitation that it needs to know in advance the length of an array. These numbers were calculated from python...
Maybe worth adding a TODO to extend the cairo runner to be able to load an entire segment without knowing in advance the length.

please do, and if you don't intend to do it please ask amos/rotem?

---

crates/starknet_os/src/tests/aliases.rs line 455 at r3 (raw file):

    #[case] contract_state_diff_len: usize,
) {
    use crate::io::os_output::OsStateDiff;

move to top

Code quote:

use crate::io::os_output::OsStateDiff;

[nimrod-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @amosStarkware and @dorimedini-starkware)

---

crates/starknet_os/src/tests/aliases.rs line 454 at r1 (raw file):

Previously, dorimedini-starkware wrote…

please do, and if you don't intend to do it please ask amos/rotem?

Done.

---

crates/starknet_os/src/tests/aliases.rs line 455 at r3 (raw file):

Previously, dorimedini-starkware wrote…

move to top

Done.

[dorimedini-starkware]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware)

[dorimedini-starkware]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware)

[dorimedini-starkware]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware)