starknet_os_flow_tests: use declared_contracts mapping to get all exe…

[Yoni-Starkware]

…cuted class hashes

[reviewable-StarkWare]

This change is 

[dorimedini-starkware]

@dorimedini-starkware reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware and @Yoni-Starkware).

---

a discussion (no related file):
looks like this PR could have been split (field rename at least)

---

crates/starknet_os_flow_tests/src/utils.rs line 464 at r1 (raw file):

    contracts_trie_root_hashes: RootHashes,
) -> StarknetForestProofs {
    let class_hashes: Vec<ClassHash> = state_maps.declared_contracts.keys().cloned().collect();

why is this enough now?
what about replaced classes?

Code quote:

let class_hashes: Vec<ClassHash> = state_maps.declared_contracts.keys().cloned().collect();

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @nimrod-starkware).

---

crates/starknet_os_flow_tests/src/utils.rs line 464 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why is this enough now?
what about replaced classes?

This was always enough. The compiled_class_hashes is not enough since it's not maintained by all state readers.

declared_contracts is updated upon every access to contract classes.

[dorimedini-starkware]

@dorimedini-starkware made 1 comment and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).

---

crates/starknet_os_flow_tests/src/utils.rs line 464 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

This was always enough. The compiled_class_hashes is not enough since it's not maintained by all state readers.

declared_contracts is updated upon every access to contract classes.

so it's a bad field name...?
maybe it was needed on 0.14.1 branch to support migration?

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).

---

crates/starknet_os_flow_tests/src/utils.rs line 464 at r1 (raw file):

Previously, dorimedini-starkware wrote…

so it's a bad field name...?
maybe it was needed on 0.14.1 branch to support migration?

compiled_class_hashes holds hashes of newly declared contracts, and potentially reads of existing one (depending on reader)

declared_contracts is a boolean mapping and is mantained by the blockifier, so every executed class should be there.

The issue with compiled_class_hashes is really annoying. We didn't want to obligate nodes to be familiar with compiled class hashes. That's why we needed to add the additional delcare mapping

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).

---

a discussion (no related file):

Previously, dorimedini-starkware wrote…

looks like this PR could have been split (field rename at least)

Sorry, noted for next PRs