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 70.21%. Comparing base (b0cfe82) to head (77410e1).
Report is 424 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   74.18%   70.21%   -3.98%     
==========================================
  Files         359       96     -263     
  Lines       36240    13068   -23172     
  Branches    36240    13068   -23172     
==========================================
- Hits        26886     9176   -17710     
+ Misses       7220     3495    -3725     
+ Partials     2134      397    -1737     

Flag	Coverage Δ
	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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).

?