chore(blockifier): save sierra to Feature contracts #2370

meship-starkware · 2024-12-01T11:01:07Z

No description provided.

reviewable-StarkWare · 2024-12-01T11:01:14Z

This change is

meship-starkware

Reviewed 7 of 11 files at r1.
Reviewable status: 7 of 11 files reviewed, all discussions resolved (waiting on @dorimedini-starkware and @Yoni-Starkware)

github-actions · 2024-12-01T11:10:15Z

Artifacts upload workflows:

meship-starkware

Reviewable status: 7 of 11 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/cairo_compile.rs line 128 at r1 (raw file):

    if fix {
        fs::write(&file_path, &sierra_output).unwrap();
    }

I am unsure if entering the fix inside and writing the files here is the best solution. The other solution I could think of is to return the Sierra data and write it in the test logic. Still, we have to write Sierra first to a temp file and then to the directory, and we'll also have to do something ugly with the return type since Cairo 0 does not return two data.

Code quote:

    if fix {
        fs::write(&file_path, &sierra_output).unwrap();
    }

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 127 at r1 (raw file):

        let file_name = path.file_stem().unwrap().to_string_lossy();
        let mut existing_sierra_path = "".to_string();

I don't like the empty string for cairo0 contracts. Should it be None, or should I get the Sierra from a different function?

Code quote:

 let mut existing_sierra_path = "".to_string();

crates/blockifier/src/execution/native/syscall_handler.rs line 35 at r1 (raw file):

use crate::execution::syscalls::hint_processor::{
    SyscallExecutionError,
    INVALID_INPUT_LENGTH_ERROR,

Style change for unusable import: I can do it in a separate PR if it would be better.

Code quote:

   INVALID_INPUT_LENGTH_ERROR,

codecov · 2024-12-01T11:29:56Z

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.

Project coverage is 71.09%. Comparing base (e3165c4) to head (855b674).
Report is 693 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/test_utils/contracts.rs	0.00%	9 Missing ⚠️
crates/blockifier/src/test_utils/cairo_compile.rs	0.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2370       +/-   ##
===========================================
+ Coverage   40.10%   71.09%   +30.99%     
===========================================
  Files          26      102       +76     
  Lines        1895    13708    +11813     
  Branches     1895    13708    +11813     
===========================================
+ Hits          760     9746     +8986     
- Misses       1100     3548     +2448     
- Partials       35      414      +379

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dorimedini-starkware

Reviewed 1 of 11 files at r1, all commit messages.
Reviewable status: 8 of 11 files reviewed, 3 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/cairo_compile.rs line 128 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I am unsure if entering the fix inside and writing the files here is the best solution. The other solution I could think of is to return the Sierra data and write it in the test logic. Still, we have to write Sierra first to a temp file and then to the directory, and we'll also have to do something ugly with the return type since Cairo 0 does not return two data.

I think it would be better not to have a fix in this context - just in-memory data creation, input-output.
as for return type, how about

pub enum CompilationArtifacts {
    Cairo0 { casm: Vec<u8> },
    Cairo1 { casm: Vec<u8>, sierra: Vec<u8> },
}

?

dorimedini-starkware

Reviewed 2 of 11 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 127 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I don't like the empty string for cairo0 contracts. Should it be None, or should I get the Sierra from a different function?

see above: you should define a FeatureContractMetadata type and return it here.
this type can be an enum (cairo0 or cairo1 variants) with different data

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 73 at r2 (raw file):

    let existing_compiled_path = contract.get_compiled_path();

    if fix {

see above

Suggestion:

    let expected_compiled_output = contract.compile();
    let existing_compiled_path = contract.get_compiled_path();

    if fix {

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 90 at r2 (raw file):

/// Verifies that the feature contracts directory contains the expected contents, and returns a list
/// of pairs (source_path, base_filename, compiled_path) for each contract.
fn verify_and_get_files(cairo_version: CairoVersion) -> Vec<(String, String, String, String)> {

I think it's time you create a FeatureContractMetadata struct for these strings.
if you think it's out of scope, update the docstring (document the fourth return value)

Code quote:

/// of pairs (source_path, base_filename, compiled_path) for each contract.
fn verify_and_get_files(cairo_version: CairoVersion) -> Vec<(String, String, String, String)> {

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 164 at r2 (raw file):

    let mut sierra_paths_from_enum: Vec<String> = FeatureContract::all_feature_contracts()
        .filter(|contract| contract.cairo_version() == CairoVersion::Cairo1)

currently, cairo version also contains a Native variant (when compiled with the feature) -
this is more robust.
or, you could match the cairo version explicitly here, and the compiler will catch you :)

Suggestion:

contract.cairo_version() != CairoVersion::Cairo0

crates/blockifier/src/test_utils/contracts.rs line 294 at r2 (raw file):

    pub fn get_sierra_path(&self) -> String {
        format!("feature_contracts/cairo1/sierra/{}.sierra.json", self.get_non_erc20_base_name())

Suggestion:

"{CAIRO1_FEATURE_CONTRACTS_DIR}/{SIERRA_CONTRACTS_SUBDIR}/{}.sierra.json"

crates/blockifier/src/test_utils/contracts.rs line 295 at r2 (raw file):

    pub fn get_sierra_path(&self) -> String {
        format!("feature_contracts/cairo1/sierra/{}.sierra.json", self.get_non_erc20_base_name())
    }

add an assert that this is not a cairo0 contract, and not the ERC20 contract, with informative messages on failure

Code quote:

    pub fn get_sierra_path(&self) -> String {
        format!("feature_contracts/cairo1/sierra/{}.sierra.json", self.get_non_erc20_base_name())
    }

crates/blockifier/src/test_utils/contracts.rs line 330 at r2 (raw file):

    /// Compiles the feature contract and returns the compiled contract as a byte vector.
    /// Panics if the contract is ERC20, as ERC20 contract recompilation is not supported.
    pub fn compile(&self, fix: bool) -> Vec<u8> {

see above

Suggestion:

pub fn compile(&self) -> CompilationArtifacts

dorimedini-starkware

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

crates/blockifier/src/execution/native/syscall_handler.rs line 35 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Style change for unusable import: I can do it in a separate PR if it would be better.

lgtm

meship-starkware

Reviewable status: 7 of 11 files reviewed, 7 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/cairo_compile.rs line 128 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I think it would be better not to have a fix in this context - just in-memory data creation, input-output.
as for return type, how about
pub enum CompilationArtifacts {
    Cairo0 { casm: Vec<u8> },
    Cairo1 { casm: Vec<u8>, sierra: Vec<u8> },
}
?

Done.

crates/blockifier/src/test_utils/contracts.rs line 295 at r2 (raw file):

Previously, dorimedini-starkware wrote…

add an assert that this is not a cairo0 contract, and not the ERC20 contract, with informative messages on failure

Done.

crates/blockifier/src/test_utils/contracts.rs line 330 at r2 (raw file):

Previously, dorimedini-starkware wrote…

see above

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 127 at r1 (raw file):

Previously, dorimedini-starkware wrote…

see above: you should define a FeatureContractMetadata type and return it here.
this type can be an enum (cairo0 or cairo1 variants) with different data

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 73 at r2 (raw file):

Previously, dorimedini-starkware wrote…

see above

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 90 at r2 (raw file):

Previously, dorimedini-starkware wrote…

I think it's time you create a FeatureContractMetadata struct for these strings.
if you think it's out of scope, update the docstring (document the fourth return value)

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 164 at r2 (raw file):

Previously, dorimedini-starkware wrote…

currently, cairo version also contains a Native variant (when compiled with the feature) -
this is more robust.
or, you could match the cairo version explicitly here, and the compiler will catch you :)

Done.

crates/blockifier/src/test_utils/contracts.rs line 294 at r2 (raw file):

    pub fn get_sierra_path(&self) -> String {
        format!("feature_contracts/cairo1/sierra/{}.sierra.json", self.get_non_erc20_base_name())

Done.

dorimedini-starkware

Reviewed 1 of 4 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/cairo_compile.rs line 68 at r4 (raw file):

impl CompilationArtifacts {
    pub fn get_compiled_output(&self) -> Vec<u8> {

can you rename? I would expect the Cairo1 compiled output to include both casm and sierra

Code quote:

get_compiled_output

crates/blockifier/src/test_utils/cairo_compile.rs line 71 at r4 (raw file):

        match self {
            CompilationArtifacts::Cairo0 { casm } => casm.clone(),
            CompilationArtifacts::Cairo1 { casm, .. } => casm.clone(),

combine arms

Suggestion:

            CompilationArtifacts::Cairo0 { casm } | CompilationArtifacts::Cairo1 { casm, .. } => casm.clone(),

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 28 at r4 (raw file):

    Cairo1(InnerFeatureContractMetadata),
    #[cfg(feature = "cairo_native")]
    Native(InnerFeatureContractMetadata),

for metadata, why do you need to tell the difference between cairo1 and native?
can you delete the native variant?

Code quote:

    Cairo1(InnerFeatureContractMetadata),
    #[cfg(feature = "cairo_native")]
    Native(InnerFeatureContractMetadata),

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 55 at r4 (raw file):

    pub compiled_path: String,
}
pub struct InnerFeatureContractMetadata {

newlines

Suggestion:

}

pub struct DeprecatedFeatureContractMetadata {
    pub source_path: String,
    pub base_filename: String,
    pub compiled_path: String,
}

pub struct InnerFeatureContractMetadata {

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 55 at r4 (raw file):

    pub compiled_path: String,
}
pub struct InnerFeatureContractMetadata {

Suggestion:

pub struct Cairo0FeatureContractMetadata {
    pub source_path: String,
    pub base_filename: String,
    pub compiled_path: String,
}
pub struct Cairo1FeatureContractMetadata {

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 125 at r4 (raw file):

                fs::write(contract.get_sierra_path(), &sierra).unwrap();
            }
        }

I would put all writing logic in the match, and delete the get_compiled_output getter
why not fix the sierra contracts in native mode?

Suggestion:

    let existing_compiled_path = contract.get_compiled_path();

    if fix {
        match expected_compiled_raw_output {
            CompilationArtifacts::Cairo0 { casm } => {
                fs::write(&existing_compiled_path, &casm).unwrap();
            }
            #[cfg(feature = "cairo_native")]
            CompilationArtifacts::Cairo1Native { sierra } => {
                fs::write(contract.get_sierra_path(), &sierra).unwrap();
            }
            CompilationArtifacts::Cairo1 { sierra, casm } => {
                fs::write(&existing_compiled_path, &casm).unwrap();
                fs::write(contract.get_sierra_path(), &sierra).unwrap();
            }
        }

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 266 at r4 (raw file):

            sierra_paths_from_enum.sort();
            sierra_paths_on_filesystem.sort();
            assert_eq!(sierra_paths_from_enum, sierra_paths_on_filesystem);

duplicated... can you extract this into a function / lambda function?

Code quote:

        #[cfg(feature = "cairo_native")]
        CairoVersion::Native => {
            let mut sierra_paths_on_filesystem: Vec<String>;
            (compiled_paths_on_filesystem, sierra_paths_on_filesystem) =
                verify_and_get_files(cairo_version)
                    .into_iter()
                    .map(|metadata| {
                        (metadata.compiled_path().to_string(), metadata.sierra_path().to_string())
                    })
                    .collect();
            let mut sierra_paths_from_enum: Vec<String> = FeatureContract::all_feature_contracts()
                .filter(|contract| contract.cairo_version() == cairo_version)
                .map(|contract| contract.get_sierra_path())
                .collect();
            sierra_paths_from_enum.sort();
            sierra_paths_on_filesystem.sort();
            assert_eq!(sierra_paths_from_enum, sierra_paths_on_filesystem);

crates/blockifier/src/test_utils/contracts.rs line 298 at r4 (raw file):

    pub fn get_sierra_path(&self) -> String {
        assert_ne!(self.cairo_version(), CairoVersion::Cairo0);
        debug_assert_ne!(self, &Self::ERC20(CairoVersion::Cairo1));

why is this assert with debug_?

Code quote:

debug_assert_ne!

meship-starkware

Reviewable status: 9 of 11 files reviewed, 8 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/contracts.rs line 298 at r4 (raw file):

Previously, dorimedini-starkware wrote…

why is this assert with debug_?

Nice catch!
Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 55 at r4 (raw file):

Previously, dorimedini-starkware wrote…

newlines

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 125 at r4 (raw file):

Previously, dorimedini-starkware wrote…

I would put all writing logic in the match, and delete the get_compiled_output getter

why not fix the sierra contracts in native mode?

It might be confusing, but we fix the Sierra contracts in the native mod. It is just called expected_compiled_output. In a future PR, I would like for the native version to return both Casm and Sierra, but I did not want to make too many changes in this PR

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 266 at r4 (raw file):

Previously, dorimedini-starkware wrote…

duplicated... can you extract this into a function / lambda function?

I can, but it might become unnecessary when we remove the feature flag. Do you want me to add a TODO to remove it once we remove the feature flag?

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 55 at r4 (raw file):

    pub compiled_path: String,
}
pub struct InnerFeatureContractMetadata {

Done.

crates/blockifier/src/test_utils/contracts.rs line 373 at r5 (raw file):

                    &mut vec![],
                );
                CompilationArtifacts::Cairo1Native { sierra: sierra_output }

I think this should return the casm as well, but it will invoke changes that are beyond the scope of this PR.
Moreover, if we decide to save the compiled native file as well, this should return the Sierra the casm and the compiled native file

Code quote:

 CompilationArtifacts::Cairo1Native { sierra: sierra_output }

dorimedini-starkware

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 266 at r4 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I can, but it might become unnecessary when we remove the feature flag. Do you want me to add a TODO to remove it once we remove the feature flag?

yes please :)

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 33 at r5 (raw file):

        match self {
            FeatureContractMetadata::Cairo0(data) => data.compiled_path.clone(),
            FeatureContractMetadata::Cairo1(data) => data.compiled_path.clone(),

combine arms

Suggestion:

            FeatureContractMetadata::Cairo0(data) | FeatureContractMetadata::Cairo1(data) => data.compiled_path.clone(),

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 123 at r5 (raw file):

    // Compare output of cairo-file on file with existing compiled file.
    let expected_compiled_raw_output = contract.compile();
    // let expected_compiled_output = expected_compiled_raw_output.get_compiled_output();

delete

Code quote:

// let expected_compiled_output = expected_compiled_raw_output.get_compiled_output();

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 129 at r5 (raw file):

    if fix {
        match expected_compiled_raw_output {
            CompilationArtifacts::Cairo0 { ref casm, .. } => {

Suggestion:

CompilationArtifacts::Cairo0 { ref casm } =>

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 147 at r5 (raw file):

        .unwrap_or_else(|_| panic!("Cannot read {existing_compiled_path}."));
    match expected_compiled_raw_output {
        CompilationArtifacts::Cairo0 { casm, .. } => {

Suggestion:

{ casm }

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 183 at r5 (raw file):

            );
        }
    }

can you move all this logic (this match) into the compare_compilation_data function?
this function should inspect the type of compilation done and compare the correct things

Code quote:

    match expected_compiled_raw_output {
        CompilationArtifacts::Cairo0 { casm, .. } => {
            compare_compilation_data(
                casm,
                existing_compiled_contents,
                existing_compiled_path,
                contract.get_source_path(),
            );
        }
        CompilationArtifacts::Cairo1 { casm, sierra } => {
            compare_compilation_data(
                casm,
                existing_compiled_contents,
                existing_compiled_path,
                contract.get_source_path(),
            );

            let sierra_compiled_path = contract.get_sierra_path();
            let existing_sierra_contents = fs::read_to_string(&sierra_compiled_path)
                .unwrap_or_else(|_| panic!("Cannot read {sierra_compiled_path}."));
            compare_compilation_data(
                sierra,
                existing_sierra_contents,
                sierra_compiled_path,
                contract.get_source_path(),
            );
        }
        // TODO (Meshi 01/01/2025) Remove this once native folder is deleted.
        #[cfg(feature = "cairo_native")]
        CompilationArtifacts::Cairo1Native { sierra, .. } => {
            compare_compilation_data(
                sierra,
                existing_compiled_contents,
                existing_compiled_path,
                contract.get_source_path(),
            );
        }
    }

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 263 at r5 (raw file):

}

fn verify_feature_contracts_cairo1_logic(cairo_version: CairoVersion) -> Vec<String> {

why do you return something here?

Code quote:

 -> Vec<String>

meship-starkware

Reviewable status: 10 of 11 files reviewed, 10 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

crates/blockifier/src/test_utils/cairo_compile.rs line 68 at r4 (raw file):

Previously, dorimedini-starkware wrote…

can you rename? I would expect the Cairo1 compiled output to include both casm and sierra

Removed the impl.

crates/blockifier/src/test_utils/cairo_compile.rs line 71 at r4 (raw file):

Previously, dorimedini-starkware wrote…

combine arms

Removed the impl.

dorimedini-starkware

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

dorimedini-starkware

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

meship-starkware

Reviewable status: 10 of 11 files reviewed, 7 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 266 at r4 (raw file):

Previously, dorimedini-starkware wrote…

yes please :)

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 33 at r5 (raw file):

Previously, dorimedini-starkware wrote…

combine arms

I get a mismatched type error on the data inside.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 123 at r5 (raw file):

Previously, dorimedini-starkware wrote…

delete

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 183 at r5 (raw file):

Previously, dorimedini-starkware wrote…

can you move all this logic (this match) into the compare_compilation_data function?
this function should inspect the type of compilation done and compare the correct things

I don't think I understand your vision. I used the compare compilation data to avoid writing the panic message constantly.
As long as the message is the same for all contracts, I still see value in the small function. I have no problem moving the match to a different function, but I don't think that was your intention.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 263 at r5 (raw file):

Previously, dorimedini-starkware wrote…

why do you return something here?

So I won't have to write the comparison between the compiled files twice of the compiled file twice. I you think it will be more readable I will just separate the Cairo 0 and native + Cairo one cases completely

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 129 at r5 (raw file):

    if fix {
        match expected_compiled_raw_output {
            CompilationArtifacts::Cairo0 { ref casm, .. } => {

Done.

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 147 at r5 (raw file):

        .unwrap_or_else(|_| panic!("Cannot read {existing_compiled_path}."));
    match expected_compiled_raw_output {
        CompilationArtifacts::Cairo0 { casm, .. } => {

Done.

dorimedini-starkware

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

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 33 at r5 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I get a mismatched type error on the data inside.

ah, right... I got confused, this isn't the compiled data (Vec)

crates/blockifier/tests/feature_contracts_compatibility_test.rs line 183 at r5 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I don't think I understand your vision. I used the compare compilation data to avoid writing the panic message constantly.
As long as the message is the same for all contracts, I still see value in the small function. I have no problem moving the match to a different function, but I don't think that was your intention.

ok, i just meant to extract this to a function, but this function is small enough :)

dorimedini-starkware

Reviewed 2 of 2 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

Yoni-Starkware

Reviewed 7 of 11 files at r1, 1 of 4 files at r3, 1 of 1 files at r6, 2 of 2 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware)

dorimedini-starkware

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

meship-starkware requested review from dorimedini-starkware and Yoni-Starkware December 1, 2024 11:01

meship-starkware commented Dec 1, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from ba61e1e to a6ba283 Compare December 1, 2024 11:09

meship-starkware commented Dec 1, 2024

View reviewed changes

meship-starkware force-pushed the meship/split_the_FC_to_groups_by_type branch from 06c9560 to a3e3fc8 Compare December 1, 2024 12:23

meship-starkware force-pushed the meship/save_sierra_in_FC branch from a6ba283 to 52a09a0 Compare December 1, 2024 12:24

dorimedini-starkware reviewed Dec 1, 2024

View reviewed changes

dorimedini-starkware requested changes Dec 1, 2024

View reviewed changes

dorimedini-starkware reviewed Dec 1, 2024

View reviewed changes

meship-starkware force-pushed the meship/split_the_FC_to_groups_by_type branch from a3e3fc8 to 77c4183 Compare December 2, 2024 07:28

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 52a09a0 to ca6f335 Compare December 2, 2024 13:59

meship-starkware force-pushed the meship/split_the_FC_to_groups_by_type branch 3 times, most recently from a00bb03 to 51c0986 Compare December 2, 2024 14:08

meship-starkware force-pushed the meship/save_sierra_in_FC branch from ca6f335 to c45870f Compare December 2, 2024 14:08

meship-starkware commented Dec 2, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from c45870f to 5de51d1 Compare December 2, 2024 14:58

dorimedini-starkware requested changes Dec 3, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 5de51d1 to 0cd2266 Compare December 3, 2024 13:29

meship-starkware commented Dec 3, 2024

View reviewed changes

dorimedini-starkware requested changes Dec 3, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 0cd2266 to f49e536 Compare December 3, 2024 14:00

meship-starkware commented Dec 3, 2024

View reviewed changes

dorimedini-starkware reviewed Dec 3, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from f49e536 to 9a3e4d3 Compare December 3, 2024 14:34

dorimedini-starkware reviewed Dec 3, 2024

View reviewed changes

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 9a3e4d3 to 3ae3e78 Compare December 3, 2024 14:44

meship-starkware commented Dec 3, 2024

View reviewed changes

dorimedini-starkware approved these changes Dec 3, 2024

View reviewed changes

meship-starkware force-pushed the meship/split_the_FC_to_groups_by_type branch from 51c0986 to c82646b Compare December 4, 2024 07:49

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 3ae3e78 to 579008f Compare December 4, 2024 07:50

dorimedini-starkware approved these changes Dec 4, 2024

View reviewed changes

Base automatically changed from meship/split_the_FC_to_groups_by_type to main December 4, 2024 12:29

chore(blockifier): save sierra to Feature contracts

855b674

meship-starkware force-pushed the meship/save_sierra_in_FC branch from 579008f to 855b674 Compare December 4, 2024 12:30

Yoni-Starkware approved these changes Dec 4, 2024

View reviewed changes

dorimedini-starkware approved these changes Dec 4, 2024

View reviewed changes

meship-starkware merged commit abbecf9 into main Dec 4, 2024
14 checks passed

meship-starkware deleted the meship/save_sierra_in_FC branch December 4, 2024 13:24

github-actions bot locked and limited conversation to collaborators Dec 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(blockifier): save sierra to Feature contracts #2370

chore(blockifier): save sierra to Feature contracts #2370

meship-starkware commented Dec 1, 2024

reviewable-StarkWare commented Dec 1, 2024

meship-starkware left a comment

github-actions bot commented Dec 1, 2024 •

edited

Loading

meship-starkware left a comment

codecov bot commented Dec 1, 2024 •

edited

Loading

dorimedini-starkware left a comment

dorimedini-starkware left a comment

dorimedini-starkware left a comment

meship-starkware left a comment

dorimedini-starkware left a comment

meship-starkware left a comment

dorimedini-starkware left a comment

meship-starkware left a comment

dorimedini-starkware left a comment

dorimedini-starkware left a comment

meship-starkware left a comment

dorimedini-starkware left a comment

dorimedini-starkware left a comment

Yoni-Starkware left a comment

dorimedini-starkware left a comment

chore(blockifier): save sierra to Feature contracts #2370

chore(blockifier): save sierra to Feature contracts #2370

Conversation

meship-starkware commented Dec 1, 2024

reviewable-StarkWare commented Dec 1, 2024

meship-starkware left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 1, 2024 • edited Loading

meship-starkware left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 1, 2024 • edited Loading

Codecov Report

dorimedini-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

Yoni-Starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 1, 2024 •

edited

Loading

codecov bot commented Dec 1, 2024 •

edited

Loading