refactor(starknet_integration_tests): move classes outside

[yair-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[yair-starkware]

• fix(starknet_sequencer_node): use the real class manager client #4040
• feat(starknet_integration_tests): init the class manage storage with classes #4015
• refactor(starknet_integration_tests): use test storage builder #4012
• refactor(starknet_class_manager): builder for test storage #4011
• refactor(starknet_integration_tests): move classes outside #4010 👈 (View in Graphite)
• refactor(starknet_integration_tests): extract test contracts #4009
• main

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

[graphite-app]

Graphite Automations

"Yair - Auto-assign" took an action on this PR • (02/06/25)

1 assignee was added to this PR based on Yair's automation.

[elintul]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 58 at r1 (raw file):

    pub class_manager_storage_handles: TempDirHandlePair,
}

Any way to prevent some of the clones? Those tests (and CI in general) are long as it is. 😅

[ArniStarkware]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @yair-starkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 172 at r1 (raw file):

    pub cairo0_contract_classes: Vec<(ClassHash, DeprecatedContractClass)>,
    pub sierra_vec: Vec<(ClassHash, SierraContractClass)>,
    pub cairo1_contract_classes: Vec<(ClassHash, CasmContractClass)>,

Why do we need both Sierra classes and Casm classes?
Are Sierra classes only used for declares and Casm for everything else?

I see this variable is later unpacked as cairo1_sierra. Keep the name consistent throughout (This part is non-blocking).

Suggestion:

    pub cairo1_sierra_classes: Vec<(ClassHash, SierraContractClass)>,
    pub cairo1_contract_classes: Vec<(ClassHash, CasmContractClass)>,

[elintul]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 172 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Why do we need both Sierra classes and Casm classes?
Are Sierra classes only used for declares and Casm for everything else?

I see this variable is later unpacked as cairo1_sierra. Keep the name consistent throughout (This part is non-blocking).

We need both in the sequencer's flow; we don't have declares yet (and even when we will, we need a set of precompiled classes to start from) so we need to feed both to the system. We could have compiled Sierra in the setup of the test, but wanted to save time; it's long as it is.

[elintul]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 172 at r3 (raw file):

    pub cairo0_contract_classes: Vec<(ClassHash, DeprecatedContractClass)>,
    pub sierra_vec: Vec<(ClassHash, SierraContractClass)>,
    pub cairo1_contract_classes: Vec<(ClassHash, CasmContractClass)>,

Either this, or cairo1_sierras, but they are coupled, so putting them together makes sense.

Suggestion:

    pub cairo0_casms: Vec<(ClassHash, DeprecatedContractClass)>,
    pub cairo1_classes: Vec<(ClassHash, (SierraContractClass, CasmContractClass))>,

---

crates/starknet_integration_tests/src/state_reader.rs line 177 at r3 (raw file):

impl TestClasses {
    pub fn new(
        test_defined_accounts: Vec<AccountTransactionGenerator>,

Then to .iter(), so save cloneing.

Suggestion:

&[AccountTransactionGenerator],

[elintul]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 197 at r3 (raw file):

    storage_writer: &mut StorageWriter,
    chain_info: &ChainInfo,
    test_defined_accounts: Vec<AccountTransactionGenerator>,

Suggestion:

test_defined_accounts: &[AccountTransactionGenerator],

[yair-starkware]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @elintul)

---

crates/starknet_integration_tests/src/state_reader.rs line 58 at r1 (raw file):

Previously, elintul (Elin) wrote…

Any way to prevent some of the clones? Those tests (and CI in general) are long as it is. 😅

Added todo

---

crates/starknet_integration_tests/src/state_reader.rs line 172 at r1 (raw file):

Previously, elintul (Elin) wrote…

We need both in the sequencer's flow; we don't have declares yet (and even when we will, we need a set of precompiled classes to start from) so we need to feed both to the system. We could have compiled Sierra in the setup of the test, but wanted to save time; it's long as it is.

merged the vecs

---

crates/starknet_integration_tests/src/state_reader.rs line 172 at r3 (raw file):

Previously, elintul (Elin) wrote…

Either this, or cairo1_sierras, but they are coupled, so putting them together makes sense.

Done

---

crates/starknet_integration_tests/src/state_reader.rs line 177 at r3 (raw file):

Previously, elintul (Elin) wrote…

Then to .iter(), so save cloneing.

Done

---

crates/starknet_integration_tests/src/state_reader.rs line 197 at r3 (raw file):

    storage_writer: &mut StorageWriter,
    chain_info: &ChainInfo,
    test_defined_accounts: Vec<AccountTransactionGenerator>,

Done

[elintul]

Reviewed 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ArniStarkware and @elintul)

---

crates/starknet_integration_tests/src/state_reader.rs line 261 at r4 (raw file):

                    contract.class_hash(),
                    (sierra, serde_json::from_str(&contract.raw_class()).unwrap()),
                );

Easier to read.

Suggestion:

                let sierra = contract.sierra();
                let casm = serde_json::from_str(&contract.raw_class()).unwrap());
                cairo1_contract_classes.insert(contract.class_hash(), (sierra, casm));

---

crates/starknet_integration_tests/src/state_reader.rs line 268 at r4 (raw file):

    (
        cairo0_contract_classes.into_iter().collect(),
        cairo1_contract_classes.into_iter().map(|(ch, (s, c))| (ch, s, c)).collect(),

Why flatten?

Code quote:

map(|(ch, (s, c))| (ch, s, c))

---

crates/starknet_integration_tests/src/state_reader.rs line 285 at r4 (raw file):

    let mut write_txn = storage_writer.begin_rw_txn().unwrap();

    let mut sierra_vec = vec![];

Suggestion:

let mut sierras = Vec::with_capacity(cairo1_contract_classes.len());

[elintul]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

[yair-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

---

crates/starknet_integration_tests/src/state_reader.rs line 261 at r4 (raw file):

Previously, elintul (Elin) wrote…

Easier to read.

Done

---

crates/starknet_integration_tests/src/state_reader.rs line 268 at r4 (raw file):

Previously, elintul (Elin) wrote…

Why flatten?

Removed

---

crates/starknet_integration_tests/src/state_reader.rs line 285 at r4 (raw file):

    let mut write_txn = storage_writer.begin_rw_txn().unwrap();

    let mut sierra_vec = vec![];

Done

[elintul]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

[ArniStarkware]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yair-starkware)

[elintul]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yair-starkware)