blockifier_reexecution: impl FetchedCompiledClass for test and offlin…

[Yonatan-Starkware]

…e state readers

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2025-11-23T06:16:37Z
• Started at 2025-11-23T06:52:14Z
• Started at 2025-11-23T21:56:43Z
• Started at 2025-11-23T22:11:08Z
• Started at 2025-11-24T06:08:34Z
• Started at 2025-11-26T08:21:58Z
• Started at 2025-11-26T08:30:56Z
• Started at 2025-11-26T08:30:56Z
• Started at 2025-11-26T11:57:44Z
• Started at 2025-11-26T12:42:58Z
• Started at 2025-11-26T12:54:34Z
• Started at 2025-11-27T13:06:00Z
• Started at 2025-11-27T14:02:22Z
• Started at 2025-11-27T16:19:05Z
• Started at 2025-11-27T18:09:16Z
• Started at 2025-12-01T08:05:44Z
• Started at 2025-12-02T16:57:58Z
• Started at 2025-12-05T06:02:22Z
• Started at 2025-12-07T09:30:42Z
• Started at 2025-12-08T08:27:18Z
• Started at 2025-12-08T11:16:08Z
• Started at 2025-12-08T11:23:31Z
• Started at 2025-12-10T08:19:15Z

[avivg-starkware]

@avivg-starkware reviewed 5 of 9 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: 8 of 9 files reviewed, 5 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/state/global_cache.rs line 42 at r2 (raw file):

    }

    pub fn from_contract_class(

Please add documentation (addressing the sierra_contract_class as well)

Code quote:

pub fn from_contract_class(

---

crates/blockifier/src/state/global_cache.rs line 61 at r2 (raw file):

        }
    }
}

please use an enum instead of having the "unwrap" possibility

Code quote:

    pub fn from_contract_class(
        contract_class: &ContractClass,
        sierra_contract_class: Option<SierraContractClass>,
    ) -> StateResult<CompiledClasses> {
        match contract_class {
            ContractClass::V0(deprecated_class) => {
                Ok(CompiledClasses::V0(CompiledClassV0::try_from(deprecated_class.clone())?))
            }
            ContractClass::V1(versioned_casm) => {
                let sierra_contract_class = sierra_contract_class.unwrap_or_else(|| {
                    panic!("Expected Sierra contract class, got None");
                });
                Ok(CompiledClasses::V1(
                    CompiledClassV1::try_from(versioned_casm.clone())?,
                    Arc::new(sierra_contract_class.clone()),
                ))
            }
        }
    }
}

---

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 126 at r2 (raw file):

        CompiledClasses::from_contract_class(&versioned_contract_class, sierra_contract_class)
    }

I find this slightly cleaner (but see enum comment above before)

Suggestion:

    /// Converts a `starknet_core::types::ContractClass` to `CompiledClasses`.
    fn starknet_core_contract_class_to_compiled_classes(
        &self,
        contract_class: &StarknetContractClass,
    ) -> StateResult<CompiledClasses> {
        match contract_class {
            StarknetContractClass::Sierra(flat_sierra) => {
                let (class_v1, _) = sierra_to_versioned_contract_class_v1(flat_sierra.clone())?;
                let sierra = SierraContractClass::from(flat_sierra.clone());

                Ok(CompiledClasses::from_contract_class(&class_v1, Some(sierra))?)
            }
            StarknetContractClass::Legacy(legacy) => {
                let class_v0 = legacy_to_contract_class_v0(legacy.clone())?;

                Ok(CompiledClasses::from_contract_class(&class_v0, None)?)
            }
        }
    }

---

crates/starknet_api/src/rpc_transaction.rs line 10 at r2 (raw file):

use cairo_lang_starknet_classes::contract_class::ContractEntryPoints as CairoLangContractEntryPoints;
use serde::{Deserialize, Serialize};
use starknet_core::types::EntryPointsByType;

Suggestion:

use starknet_core::types::EntryPointsByType as StarknetCoreEntryPointsByType;

---

crates/starknet_api/src/state.rs line 345 at r2 (raw file):

    fn from(entry_point: SierraEntryPoint) -> Self {
        Self {
            function_idx: FunctionIndex(usize::try_from(entry_point.function_idx).unwrap()),

Suggestion:

function_idx: FunctionIndex(
                usize::try_from(entry_point.function_idx)
                    .expect("Function index should fit in a usize"),
            ),

[avivg-starkware]

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 100 at r2 (raw file):

    fn is_contract_class_declared(&self, class_hash: ClassHash) -> StateResult<bool> {
        match self.get_contract_class(&class_hash) {

I'm not sure about this, I'm worried it's costly for no reason (see impl of get_contract_class by TestReader - it involves unnecessary steps of deserialization, which are pricey.

Take a look?

Code quote:

match self.get_contract_class(&class_hash) {

[Yonatan-Starkware]

Reviewable status: 8 of 9 files reviewed, 5 unresolved discussions (waiting on @avivg-starkware and @AvivYossef-starkware)

---

crates/blockifier/src/state/global_cache.rs line 42 at r2 (raw file):

Previously, avivg-starkware wrote…

Please add documentation (addressing the sierra_contract_class as well)

Done.

---

crates/blockifier/src/state/global_cache.rs line 61 at r2 (raw file):

Previously, avivg-starkware wrote…

please use an enum instead of having the "unwrap" possibility

Done.

---

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 126 at r2 (raw file):

Previously, avivg-starkware wrote…

I find this slightly cleaner (but see enum comment above before)

Done.

---

crates/starknet_api/src/state.rs line 345 at r2 (raw file):

    fn from(entry_point: SierraEntryPoint) -> Self {
        Self {
            function_idx: FunctionIndex(usize::try_from(entry_point.function_idx).unwrap()),

Done.

[avivg-starkware]

@avivg-starkware reviewed 1 of 9 files at r1, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware and @Yonatan-Starkware)

---

crates/blockifier/src/state/global_cache.rs line 61 at r2 (raw file):

Previously, Yonatan-Starkware wrote…

Done.

see comment above

---

crates/blockifier/src/state/global_cache.rs line 44 at r3 (raw file):

    /// Create a CompiledClasses object from a ContractClass.
    /// If the ContractClass is a Cairo 1 contract class, the sierra contract class is required
    /// as CompiledClasses::V1 objects contains a SierraContractClass.

Suggestion:

    /// Converts a [`starknet_api::contract_class::ContractClass`] into the corresponding
    /// [`CompiledClasses`].
    ///
    /// For `V1` (Cairo 1) classes, a matching `SierraContractClass` must be provided.
    /// For `V0` classes, this argument should be `None`.

---

crates/blockifier/src/state/global_cache.rs line 59 at r3 (raw file):

                    } else {
                        panic!("Expected Sierra contract class, got None");
                    };

Sorry, my intention about introducing an enum was different; I meant replacing the input type, so this failure wouldn’t even be possible, but that’s overdoing it for now. The main issue is the unwrap(): it should be replaced with proper error propagation.

Suggestion:

                let sierra_contract_class = sierra_contract_class.ok_or_else(|| {
                    StateError::MissingSierra(
                        "V1 contract class requires Sierra but got None".into()
                    )
                })?;

[Yonatan-Starkware]

Reviewable status: 8 of 10 files reviewed, 3 unresolved discussions (waiting on @avivg-starkware and @AvivYossef-starkware)

---

crates/blockifier/src/state/global_cache.rs line 59 at r3 (raw file):

Previously, avivg-starkware wrote…

Sorry, my intention about introducing an enum was different; I meant replacing the input type, so this failure wouldn’t even be possible, but that’s overdoing it for now. The main issue is the unwrap(): it should be replaced with proper error propagation.

Done.

---

crates/blockifier/src/state/global_cache.rs line 44 at r3 (raw file):

    /// Create a CompiledClasses object from a ContractClass.
    /// If the ContractClass is a Cairo 1 contract class, the sierra contract class is required
    /// as CompiledClasses::V1 objects contains a SierraContractClass.

Done.

[avivg-starkware]

crates/blockifier/src/state/errors.rs line 35 at r5 (raw file):

    #[error("Failed to read from state: {0}.")]
    StateReadError(String),
    #[error("Missing Sierra class.")]

revert

Code quote:

#[error("Missing Sierra class.")]

[avivg-starkware]

@avivg-starkware reviewed 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @Yonatan-Starkware)

[Yonatan-Starkware]

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

---

crates/blockifier/src/state/errors.rs line 35 at r5 (raw file):

Previously, avivg-starkware wrote…

revert

Done.

[Yonatan-Starkware]

Reviewable status: 4 of 10 files reviewed, 2 unresolved discussions (waiting on @avivg-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 100 at r2 (raw file):

Previously, avivg-starkware wrote…

I'm not sure about this, I'm worried it's costly for no reason (see impl of get_contract_class by TestReader - it involves unnecessary steps of deserialization, which are pricey.

Take a look?

Done.

[avivg-starkware]

@avivg-starkware reviewed 6 of 6 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 204 at r6 (raw file):

    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)

What about a comment explaining?

Code quote:

Ok(true)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 167 at r6 (raw file):

    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)

What about a comment explaining?

Code quote:

Ok(true)

[Yonatan-Starkware]

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

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 204 at r6 (raw file):

Previously, avivg-starkware wrote…

What about a comment explaining?

Done.

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 167 at r6 (raw file):

Previously, avivg-starkware wrote…

What about a comment explaining?

Done.

[avivg-starkware]

@avivg-starkware reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @Yonatan-Starkware)

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 168 at r7 (raw file):

    /// This check is not needed anymore do to a similar check in ApolloReader.
    /// TODO(Yonatank): Remove this after removing is_declared method from FetchedCompiledClasses
    /// trait.

Suggestion:

    /// This check is no longer needed due to a similar check in the ApolloReader.
    /// TODO(Yonatank): Remove this once `is_declared` is removed from the `FetchedCompiledClasses` 
    /// trait.

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 205 at r7 (raw file):

    /// This check is not needed anymore do to a similar check in ApolloReader.
    /// TODO(Yonatank): Remove this after removing is_declared method from FetchedCompiledClasses
    /// trait.

Small fix suggestion before merging

Suggestion:

    /// This check is no longer needed due to a similar check in the ApolloReader.
    /// TODO(Yonatank): Remove this once `is_declared` is removed from the `FetchedCompiledClasses` 
    /// trait.

[noaov1]

@noaov1 reviewed 2 of 4 files at r3, 4 of 6 files at r6, 2 of 2 files at r8, 3 of 3 files at r9, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware and @Yonatan-Starkware)

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 208 at r9 (raw file):

    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)
    }

What does it have to do with the apollo state reader? Do we use it here?
It is okay to return true here as we insert a class to the contract_class_mapping only if it was declared ( @AvivYossef-starkware to verify)

Code quote:

    /// This check is no longer needed due to a similar check in the ApolloReader.
    /// TODO(Yonatank): Remove this once 'is_declared'is removed from the FetchedCompiledClasses
    /// trait.
    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)
    }

---

crates/blockifier/src/state/global_cache.rs line 63 at r9 (raw file):

    /// For `V1` (Cairo 1) classes, a matching `SierraContractClass` must be provided.
    /// For `V0` classes, this argument should be `None`.
    pub fn from_contract_class(

Please move it to the re-execution crate (can be under utils.rs).
Can we avois the clone? I.e. make it a consuming method?

Code quote:

    pub fn from_contract_class(

---

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 99 at r9 (raw file):

    /// Converts a `starknet_core::types::ContractClass` to `CompiledClasses`.
    fn starknet_core_contract_class_to_compiled_classes(

Can this method be consuming? (to avoid the clone)

Suggestion:

    fn starknet_core_contract_class_to_compiled_class(

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 171 at r9 (raw file):

    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)
    }

Why do we "care" about the apollo state reader?

Code quote:

    /// This check is no longer needed due to a similar check in the ApolloReader.
    /// TODO(Yonatank): Remove this once 'is_declared'is removed from the FetchedCompiledClasses
    /// trait.
    fn is_declared(&self, _class_hash: ClassHash) -> StateResult<bool> {
        Ok(true)
    }

[AvivYossef-starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @noaov1 and @Yonatan-Starkware)

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 208 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What does it have to do with the apollo state reader? Do we use it here?
It is okay to return true here as we insert a class to the contract_class_mapping only if it was declared ( @AvivYossef-starkware to verify)

It's ok to return always true in the reexecution because we assume that it's impossible to revert block during the time that the reexecution running ( a reasonable assumptions )

[noaov1]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware and @Yonatan-Starkware)

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 208 at r9 (raw file):

Previously, AvivYossef-starkware wrote…

It's ok to return always true in the reexecution because we assume that it's impossible to revert block during the time that the reexecution running ( a reasonable assumptions )

Please make sure that we insert a class to the contract_class_mapping only if it was declared

[AvivYossef-starkware]

• blockifier_reexecution: split execute tx logic #10844 : 2 dependent PRs (#10849 , #10855 )
• starknet_os_runner: initialize the crate #10718
• blockifier_reexecution: move files location #10717
• blockifier_reexecution: rename test state reader #10716
• blockifier_reexecution: move reexecute block for testing #10709
• blockifier_reexecution: move write_block reexecution data to file #10708
• blockifier_reexecution: move execute single tx from utils #10707
• blockifier_reexecution: seperate reexecute block from verify correctness #10706
• blockifier_reexecution: move reexecute and verify correctness to trait #10705
• blockifier_reexecution: allow custom chain id #10643
• blockifier_reexecution: use StateReaderAndContractManager in reexecut… #10425
• blockifier_reexecution: impl FetchedCompiledClass for test and offlin… #10330 👈 (View in Graphite)
• main-v0.14.1

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

[AvivYossef-starkware]

Reviewable status: 3 of 11 files reviewed, 4 unresolved discussions (waiting on @avivg-starkware and @noaov1)

---

crates/blockifier_reexecution/src/state_reader/offline_state_reader.rs line 208 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Please make sure that we insert a class to the contract_class_mapping only if it was declared

what do you mean?

---

crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs line 99 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can this method be consuming? (to avoid the clone)

yes

---

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 171 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why do we "care" about the apollo state reader?

we dont

---

crates/blockifier/src/state/global_cache.rs line 63 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Please move it to the re-execution crate (can be under utils.rs).
Can we avois the clone? I.e. make it a consuming method?

yes

[noaov1]

@noaov1 reviewed 2 of 8 files at r10, 3 of 3 files at r11, 3 of 3 files at r12, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)

[noaov1]

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

[AvivYossef-starkware]

@AvivYossef-starkware reviewed 2 of 2 files at r14, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)