test(blockifier): function to build calldata for recursive call_contract calls

[TzahiTaub]

No description provided.

[lotem-starkware]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.33%. Comparing base (e3165c4) to head (98ec680).
Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/test_utils/syscall.rs	95.45%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1449       +/-   ##
===========================================
+ Coverage   40.10%   67.33%   +27.22%     
===========================================
  Files          26      103       +77     
  Lines        1895    13723    +11828     
  Branches     1895    13723    +11828     
===========================================
+ Hits          760     9240     +8480     
- Misses       1100     4082     +2982     
- Partials       35      401      +366     

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

[dorimedini-starkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @TzahiTaub and @yoavGrs)

---

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

        Self::CairoVersion(version)
    }
}

I would delete this - it is not intuitive. conversion from CairoVersion to the new enum should be explicit.

The other way around can be implemented if you want

Code quote:

impl From<CairoVersion> for CompilerBasedVersion {
    fn from(version: CairoVersion) -> Self {
        Self::CairoVersion(version)
    }
}

---

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

    }
}
// Storage keys.

newline

Suggestion:

}

// Storage keys.

---

crates/blockifier/src/execution/syscalls/syscall_tests/call_contract.rs line 113 at r1 (raw file):

) {
    let outer_contract = FeatureContract::TestContract(outer_version);
    let inner_contract = FeatureContract::TestContract(inner_version);

consider making [inner|outer]_version params of type CompilerBasedVersion.
will be easy to parametrize a third variant later

Suggestion:

    #[values(
        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0),
        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1)
    ]
    outer_version: CairoVersion,
    #[values(
        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0),
        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1)
    ]
    inner_version: CairoVersion,
) {
    let outer_contract = outer_version.get_test_contract();
    let inner_contract = inner_version.get_test_contract();

[TzahiTaub]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)

---

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

Previously, dorimedini-starkware wrote…

I would delete this - it is not intuitive. conversion from CairoVersion to the new enum should be explicit.

The other way around can be implemented if you want

It's sort of backward compatability + convinience thing. The OldCairo1 should be explicit in tests, and anything that was Ciaro1 until now should run as a new Cairo1 contract (i.e., sierra gas mode). The other way around is not the thing I'm interested in.
I just hate writing CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1)

---

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

Previously, dorimedini-starkware wrote…

newline

Done.

---

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

    /// Returns the tracked resource for a contract execution with the current version, assuming no
    /// calls were made to other contracts prior to this execution.

Help with rewriting this will be appreciated.

Code quote:

    /// Returns the tracked resource for a contract execution with the current version, assuming no
    /// calls were made to other contracts prior to this execution.

---

crates/blockifier/src/execution/syscalls/syscall_tests/call_contract.rs line 113 at r1 (raw file):

Previously, dorimedini-starkware wrote…

consider making [inner|outer]_version params of type CompilerBasedVersion.
will be easy to parametrize a third variant later

Changed the test a little and added the other cases. How does it look?

[dorimedini-starkware]

Reviewed 2 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TzahiTaub and @yoavGrs)

---

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

Previously, TzahiTaub (Tzahi) wrote…

Help with rewriting this will be appreciated.

hmmm... maybe

/// Returns the context-free tracked resource of this contract (does not take caller contract into account).

?

[github-actions]

Artifacts upload triggered. View details here

[TzahiTaub]

Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @dorimedini-starkware and @yoavGrs)

---

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

Previously, dorimedini-starkware wrote…

hmmm... maybe

/// Returns the context-free tracked resource of this contract (does not take caller contract into account).

?

Bravo!

[dorimedini-starkware]

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