chore(blockifier): add cairo-native feature

[noaov1]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.

Project coverage is 68.96%. Comparing base (e3165c4) to head (6c4010e).
Report is 38 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/transaction/transactions.rs	27.27%	8 Missing ⚠️
crates/blockifier/src/execution/contract_class.rs	50.00%	1 Missing ⚠️
crates/blockifier/src/execution/syscalls/mod.rs	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1558       +/-   ##
===========================================
+ Coverage   40.10%   68.96%   +28.86%     
===========================================
  Files          26      100       +74     
  Lines        1895    13380    +11485     
  Branches     1895    13380    +11485     
===========================================
+ Hits          760     9228     +8468     
- Misses       1100     3751     +2651     
- Partials       35      401      +366     

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

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[avi-starkware]

Reviewed 7 of 15 files at r1, all commit messages.
Reviewable status: 7 of 15 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

-- commits line 2 at r1:
This is weird... Did you amend an existing commit instead of creating a new one?

Code quote:

7c9710f: feat(ci): small QoL improvements to publish_crates.py (#1542)

[avi-starkware]

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

---

crates/blockifier/src/execution/syscalls/mod.rs line 532 at r1 (raw file):

        }
        #[cfg(feature = "cairo_native")]
        ContractClass::V1Native(_) => {

This method introduces code duplication in many places.
Is there a way to avoid it?
Non-blocking

Code quote:

        #[cfg(feature = "cairo_native")]
        ContractClass::V1Native(_) => {

[github-actions]

Artifacts upload triggered. View details here

[noaov1]

Reviewable status: 14 of 15 files reviewed, 2 unresolved discussions (waiting on @avi-starkware and @dorimedini-starkware)

---

-- commits line 2 at r1:

Previously, avi-starkware wrote…

This is weird... Did you amend an existing commit instead of creating a new one?

Indeed weird. Nice catch!

[dorimedini-starkware]

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

a discussion (no related file):
there are two options as I see it

1. what you did here
2. duplicate all code that has a variation into separate modules - one for native and one without. for example:

// Module gated behind #[cfg(feature = "cairo_native")]
pub enum ContractClass {
    V0(ContractClassV0),
    V1(ContractClassV1),
    V1Native(NativeContractClassV1),
}
...
// Module gated behind #[cfg(not(feature = "cairo_native"))]
pub enum ContractClass {
    V0(ContractClassV0),
    V1(ContractClassV1),
}

which one is better?

---

---

.github/workflows/blockifier_ci.yml line 54 at r2 (raw file):

      - run: cargo build -p blockifier --features transaction_serde
      - run: cargo test -p blockifier --features transaction_serde
        # native-blckifier is not activated by any workspace crate; test the build.

Suggestion:

# cairo-native is not activated by any workspace crate; test the build.

---

crates/blockifier/src/transaction/account_transaction.rs line 972 at r2 (raw file):

    }

    false

does this work?

Suggestion:

    match contract_class {
        ContractClass::V0(_) => false,
        ContractClass::V1(_) => true,
        #[cfg(feature = "cairo_native")]
        ContractClass::V1Native(_) => true,
    }

[avi-starkware]

Previously, dorimedini-starkware wrote…

there are two options as I see it

1. what you did here
2. duplicate all code that has a variation into separate modules - one for native and one without. for example:

// Module gated behind #[cfg(feature = "cairo_native")]
pub enum ContractClass {
    V0(ContractClassV0),
    V1(ContractClassV1),
    V1Native(NativeContractClassV1),
}
...
// Module gated behind #[cfg(not(feature = "cairo_native"))]
pub enum ContractClass {
    V0(ContractClassV0),
    V1(ContractClassV1),
}

which one is better?

Solution 2 adds much more code duplication. This means any change to this module will have to be made twice.

[avi-starkware]

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

---

crates/blockifier/src/transaction/account_transaction.rs line 972 at r2 (raw file):

Previously, dorimedini-starkware wrote…

does this work?

It should work. The same pattern is used in test_utils.rs in this PR (and in many other places...)

[avi-starkware]

Reviewed all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @noaov1)

[github-actions]

Artifacts upload triggered. View details here

[noaov1]

Reviewable status: 12 of 15 files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @dorimedini-starkware)

a discussion (no related file):

Previously, avi-starkware wrote…

Solution 2 adds much more code duplication. This means any change to this module will have to be made twice.

I tend to agree with Avi.

---

---

crates/blockifier/src/execution/syscalls/mod.rs line 532 at r1 (raw file):

Previously, avi-starkware wrote…

This method introduces code duplication in many places.
Is there a way to avoid it?
Non-blocking

Good question. Changed. WDYT?

---

crates/blockifier/src/transaction/account_transaction.rs line 972 at r2 (raw file):

Previously, avi-starkware wrote…

It should work. The same pattern is used in test_utils.rs in this PR (and in many other places...)

Like. Done.

[noaov1]

Reviewable status: 12 of 15 files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @dorimedini-starkware)

---

.github/workflows/blockifier_ci.yml line 54 at r2 (raw file):

      - run: cargo build -p blockifier --features transaction_serde
      - run: cargo test -p blockifier --features transaction_serde
        # native-blckifier is not activated by any workspace crate; test the build.

Opps. Done

[github-actions]

Artifacts upload triggered. View details here

[avi-starkware]

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

---

crates/blockifier/src/execution/syscalls/mod.rs line 532 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Good question. Changed. WDYT?

Why not just:

if !is_cairo1(&class) {
    return Err(....)
}

The function is_cairo1 covers all the cases. If it is false, it has to be Cairo 0.

[avi-starkware]

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

---

crates/blockifier/src/transaction/transactions.rs line 156 at r4 (raw file):

        let declare_version = declare_tx.version();
        // Verify contract class version.
        if matches!(class_info.contract_class(), ContractClass::V0(_)) {

As I suggested above

Suggestion:

        if !is_cairo1(class_info.contract_class()) {

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)