fix(execution): forbid calling cairo0 contract with cairo1 only builtins

[meship-starkware]

Pull Request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build-related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

• Yes
• No

Other information

---

This change is 

[codecov]

The author of this PR, meship-starkware, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at [email protected] with any questions.

[avi-starkware]

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

---

crates/blockifier/src/execution/errors.rs line 45 at r1 (raw file):

    #[error("Requested contract address {:#064x} is not deployed.", .0.key())]
    UninitializedStorageAddress(ContractAddress),
    #[error("Called an unsupported builtin inside a Cairo0 contract.")]

The original phrasing is ambiguous (is the builtin unsupported in general? Or just inside a Cairo0 contract?)

Suggestion:

    #[error("Called a builtin that is unsupported in a Cairo0 contract.")]

---

crates/papyrus_monitoring_gateway/src/lib.rs line 316 at r1 (raw file):

    match prometheus_handle {
        Some(handle) => {
            Collector::new(PROCESS_METRICS_PREFIX).collect();

Why make this change in papyrus in this PR? Does it fail a test without the change?

Code quote:

 Collector::new(PROCESS_METRICS_PREFIX).collect();

[meship-starkware]

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/blockifier/src/execution/errors.rs line 45 at r1 (raw file):

Previously, avi-starkware wrote…

The original phrasing is ambiguous (is the builtin unsupported in general? Or just inside a Cairo0 contract?)

Done.

---

crates/papyrus_monitoring_gateway/src/lib.rs line 316 at r1 (raw file):

Previously, avi-starkware wrote…

Why make this change in papyrus in this PR? Does it fail a test without the change?

The function is deprecated, and it does not build otherwise.

[noaov1]

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

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 92 at r2 (raw file):

    if !program_builtins_set.is_subset(&cairo0_builtins_set) {
        return Err(PreExecutionError::UnsupportedCairo0Builtin);
    }

Is it possible to know the unsupported builtin that is used?

Suggestion:

    let program_builtins: HashSet<&BuiltinName> =
        HashSet::from_iter(contract_class.program.iter_builtins();
    if !program_builtins.is_subset(&HashSet::from_iter(CAIRO0_BUILTINS_NAMES.iter()) {
        return Err(PreExecutionError::UnsupportedCairo0Builtin);
    }
    
    // Resolve initial PC from EP indicator.
    let entry_point_pc = resolve_entry_point_pc(call, &contract_class)?;

[meship-starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @noaov1)

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 92 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Is it possible to know the unsupported builtin that is used?

I subtract the sets instead of using the is_subset. The downside is that if the length of program_builtins_set is longer than cairo0_builtin_names, we still have to do the calculation, but there is no way around it if we want the builtins_names.
Assuming that the length of program_builtins_set is bounded by the number of our builtins, I think it's ok.

[meship-starkware]

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

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 90 at r3 (raw file):

    let subtract_set = &program_builtins_set - &HashSet::from_iter(CAIRO0_BUILTINS_NAMES.iter());
    if !subtract_set.is_empty() {
        return Err(PreExecutionError::UnsupportedCairo0Builtin(

I used it so I could send the builtins set without using lifetime.
I think this would be ok, assuming the subtract set length is bounded by our builtins number.

[avi-starkware]

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

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 88 at r3 (raw file):

    let program_builtins_set: HashSet<&BuiltinName> =
        HashSet::from_iter(contract_class.program.iter_builtins());
    let subtract_set = &program_builtins_set - &HashSet::from_iter(CAIRO0_BUILTINS_NAMES.iter());

Suggestion:

unsupported_builtins_set

---

crates/blockifier/src/execution/errors.rs line 48 at r3 (raw file):

    #[error("Requested contract address {:#064x} is not deployed.", .0.key())]
    UninitializedStorageAddress(ContractAddress),
    #[error("Called a builtin that is unsupported in a Cairo0 contract. Builtins: {0:?}.")]

Can you please paste an example error?

Suggestion:

"Called builtins: {0} are unsupported in a Cairo0 contract."

[meship-starkware]

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

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 88 at r3 (raw file):

    let program_builtins_set: HashSet<&BuiltinName> =
        HashSet::from_iter(contract_class.program.iter_builtins());
    let subtract_set = &program_builtins_set - &HashSet::from_iter(CAIRO0_BUILTINS_NAMES.iter());

Done.

---

crates/blockifier/src/execution/errors.rs line 48 at r3 (raw file):

Previously, avi-starkware wrote…

Can you please paste an example error?

Called builtins: {range_check96, segment_arena, keccak} are unsupported in a Cairo0 contract