chore(cairo_native): update native version

[meship-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2025-02-05T15:38:28Z
• Started at 2025-02-05T15:41:31Z
• Started at 2025-02-06T08:43:29Z
• Started at 2025-02-06T08:50:37Z
• Started at 2025-02-06T09:50:30Z
• Started at 2025-02-06T13:43:24Z
• Started at 2025-02-06T14:05:59Z
• Started at 2025-02-09T07:08:11Z
• Started at 2025-02-09T08:11:43Z
• Started at 2025-02-09T14:01:36Z
• Started at 2025-02-09T15:04:24Z

[github-actions]

Benchmark movements:
full_committer_flow performance improved 😺
full_committer_flow time: [31.319 ms 31.408 ms 31.509 ms]
change: [-2.4369% -1.8613% -1.3332%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
4 (4.00%) high mild
3 (3.00%) high severe

[noaov1]

Reviewed 6 of 9 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 7 of 9 files reviewed, 4 unresolved discussions (waiting on @avi-starkware and @meship-starkware)

---

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

        .into_iter()
        .map(|felt| felt.value)
        .collect::<Vec<_>>();

Isn't the sierra program of type Vec<BigUintAsHex>? 🤔
Consider sharing code with this.

Code quote:

    let raw_sierra_program = contract_class
        .sierra_program
        .clone()
        .into_iter()
        .map(|felt| felt.value)
        .collect::<Vec<_>>();

---

crates/bin/starknet-native-compile/src/main.rs line 35 at r2 (raw file):

        .map(|felt| felt.value)
        .collect::<Vec<_>>();
    let sierra_version = get_sierra_verizon_from_program(&raw_sierra_program);

Consider using extract_from_program and converting the result.

Code quote:

    let sierra_version = get_sierra_verizon_from_program(&raw_sierra_program);

---

crates/bin/starknet-native-compile/src/main.rs line 37 at r2 (raw file):

    let sierra_version = get_sierra_verizon_from_program(&raw_sierra_program);

    for _ in 0..N_RETRIES {

When do we expect it to fail?
I see in cairo native documentation that if it fails we should use AotContractExecutor::from_path.

Code quote:

for _ in 0..N_RETRIES {

---

crates/starknet_api/src/contract_class.rs line 36 at r2 (raw file):

fn u64_to_usize(val: u64) -> usize {
    val.try_into().expect("Failed to convert u64 version tag to usize.")
}

See here. Consider extracting it to a utils crate.

Code quote:

fn u64_to_usize(val: u64) -> usize {
    val.try_into().expect("Failed to convert u64 version tag to usize.")
}

[noaov1]

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

[meship-starkware]

Reviewed 2 of 9 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Isn't the sierra program of type Vec<BigUintAsHex>? 🤔
Consider sharing code with this.

I can, but again we dont want this create to be dependent on starknet_api and I think that for our dependency

---

crates/bin/starknet-native-compile/src/main.rs line 35 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Consider using extract_from_program and converting the result.

@avi-starkware does not want the compilation crate to be dependent on starknet API, and I think that for our dependency tree starknet API shouldn't be dependent on the compilation crate

---

crates/bin/starknet-native-compile/src/main.rs line 37 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

When do we expect it to fail?
I see in cairo native documentation that if it fails we should use AotContractExecutor::from_path.

That might make sense becuse it fails when we are trying to create a compiled native for the same file

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.584 ms 34.672 ms 34.808 ms]
change: [-4.0437% -2.4238% -1.0139%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
5 (5.00%) high mild
1 (1.00%) high severe

[meship-starkware]

Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/starknet_api/src/contract_class.rs line 36 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

See here. Consider extracting it to a utils crate.

Oh, the implementation I found returned an error I will extract it to a util in a separate PR

[meship-starkware]

Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I can, but again we dont want this create to be dependent on starknet_api and I think that for our dependency

it was cut in the middle I think that for our dependency tee straknet API shouldn't be dependent on our compilation crate

[avi-starkware]

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

---

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I can, but again we dont want this create to be dependent on starknet_api and I think that for our dependency

@noaov1 I suggested not sharing code here. I don't think we should introduce dependencies to crates in our workspace from this crate. This crate should be seen as an extension of the cairo_native repo.

---

crates/bin/starknet-native-compile/src/utils.rs line 8 at r3 (raw file):

use cairo_lang_starknet_classes::contract_class::ContractClass;

pub(crate) fn get_sierra_verizon_from_program<F>(sierra_program: &[F]) -> VersionId

Suggestion:

get_sierra_version_from_program

[avi-starkware]

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

@noaov1 I suggested not sharing code here. I don't think we should introduce dependencies to crates in our workspace from this crate. This crate should be seen as an extension of the cairo_native repo.

@meshi why not put this step inside the util function? (i.e., make it get_sierra_version_from_contract_class)

[avi-starkware]

crates/bin/starknet-native-compile/src/main.rs line 59 at r3 (raw file):

        });
    }
}

I discussed this with yoni yesterday and he said we shoul simpy panic if we get None, since we don't expect this behavior to happen and this indicates a bug. We shouldn't retry blindly.

Code quote:

    for _ in 0..N_RETRIES {
        if contract_executor.is_some() {
            break;
        }
        eprintln!("Failed to take lock on path {} . Retrying...", output.display());
        contract_executor = AotContractExecutor::from_path(&output).unwrap_or_else(|err| {
            eprintln!("Error deserializing Sierra file into contract class: {}.", err);
            process::exit(1);
        });
    }
}

[avi-starkware]

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

@meshi why not put this step inside the util function? (i.e., make it get_sierra_version_from_contract_class)

Actually, we can just make load_sierra_program_from_file return the sierra version in addition to the contract class and sierra program

[meship-starkware]

Reviewable status: 7 of 9 files reviewed, 6 unresolved discussions (waiting on @avi-starkware, @meshi, and @noaov1)

---

crates/bin/starknet-native-compile/src/main.rs line 34 at r2 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Actually, we can just make load_sierra_program_from_file return the sierra version in addition to the contract class and sierra program

Done.

---

crates/bin/starknet-native-compile/src/main.rs line 59 at r3 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

I discussed this with yoni yesterday and he said we shoul simpy panic if we get None, since we don't expect this behavior to happen and this indicates a bug. We shouldn't retry blindly.

Done.

---

crates/bin/starknet-native-compile/src/utils.rs line 8 at r3 (raw file):

use cairo_lang_starknet_classes::contract_class::ContractClass;

pub(crate) fn get_sierra_verizon_from_program<F>(sierra_program: &[F]) -> VersionId

Done.

[noaov1]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avi-starkware and @meship-starkware)

---

crates/bin/starknet-native-compile/src/main.rs line 59 at r3 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Done.

Yes..

---

crates/starknet_api/src/contract_class.rs line 36 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Oh, the implementation I found returned an error I will extract it to a util in a separate PR

I'm confused. The implantation I am referring to is:

Code snippet:

pub fn u64_from_usize(val: usize) -> u64 {
    val.try_into().expect("Conversion from usize to u64 should not fail.")
}

---

crates/bin/starknet-native-compile/src/utils.rs line 35 at r4 (raw file):

        .unwrap_or_else(|_| {
            eprintln!("Error parsing Sierra program to Sierra version");
            process::exit(1);

Do we have to explicitly call it? What will happen if we simply panic?

Code quote:

            process::exit(1);

---

crates/bin/starknet-native-compile/src/utils.rs line 60 at r4 (raw file):

        .into_iter()
        .map(|felt| felt.value)
        .collect::<Vec<_>>();

Can this done inside get_sierra_version_from_program?

Suggestion:

    let raw_sierra_program = contract_class
        .sierra_program
        .into_iter()
        .map(|big_uint_as_hex| big_uint_as_hex.value)
        .collect(),

[meship-starkware]

Reviewable status: 8 of 10 files reviewed, 5 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/bin/starknet-native-compile/src/utils.rs line 35 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Do we have to explicitly call it? What will happen if we simply panic?

it's a different process so I think the error might appear in the main process if we simply panic @avi-starkware might know better

---

crates/bin/starknet-native-compile/src/utils.rs line 60 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can this done inside get_sierra_version_from_program?

Done.

---

crates/starknet_api/src/contract_class.rs line 36 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I'm confused. The implantation I am referring to is:

Yes, I just think that if every place that uses u64_from_usize will use a different path, it might be a big change, so I rather do it in a different PR. I just saw a different implementation for u64_from_usize

[avi-starkware]

crates/bin/starknet-native-compile/src/utils.rs line 35 at r4 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

it's a different process so I think the error might appear in the main process if we simply panic @avi-starkware might know better

I don't think I checked this very thoroughly. panic! might be more useful in some of these cases.

The difference is that process::exit(1) terminates the process immediately without running destructors or printing a stack trace, while panic! causes unwinds the stack before terminating the process.

[meship-starkware]

Reviewable status: 7 of 10 files reviewed, 5 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/starknet_api/src/contract_class.rs line 36 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Yes, I just think that if every place that uses u64_from_usize will use a different path, it might be a big change, so I rather do it in a different PR. I just saw a different implementation for u64_from_usize

This is the other way around. I added a todo to share this code

[noaov1]

Reviewed 2 of 2 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @meship-starkware)

---

crates/bin/starknet-native-compile/src/utils.rs line 35 at r4 (raw file):
What do we do when compiling to casm?

panic! causes unwinds the stack before terminating the process.

Is it bad?

[avi-starkware]

crates/bin/starknet-native-compile/src/utils.rs line 35 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What do we do when compiling to casm?

panic! causes unwinds the stack before terminating the process.

Is it bad?

I looked into it. The idiomatic approach to handle errors in the CLI is to return result (rust knows how to convert the result to a nice error in a CLI). Moreover, using anyhow::Result we can provide context along with the error itself (e.g., "error compiling sierra program"). This is the way they do it in the casm-compiler cli and in the cli tools of cairo-native.

Using panic is not a best practice because it includes an unhelpful stack trace.