chore: streamline class info constructor

[ArniStarkware]

It contains the lion's share of what was once #427.

It was split as parts of the original PR are controversial - so let us deal with one idea at a time.

This change is 

[ArniStarkware]

• refactor: streamline class info object to match contract class #427
• chore: streamline class info constructor #1215 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ArniStarkware and the rest of your teammates on Graphite

[codecov]

Codecov Report

Attention: Patch coverage is 38.29787% with 29 lines in your changes missing coverage. Please review.

Project coverage is 62.88%. Comparing base (b0cfe82) to head (625fcaf).
Report is 314 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/native_blockifier/src/py_transaction.rs	0.00%	12 Missing ⚠️
crates/blockifier/src/execution/contract_class.rs	61.53%	9 Missing and 1 partial ⚠️
crates/papyrus_execution/src/lib.rs	22.22%	5 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (b0cfe82) and HEAD (625fcaf). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (b0cfe82)	HEAD (625fcaf)
	3	1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1215       +/-   ##
===========================================
- Coverage   74.18%   62.88%   -11.31%     
===========================================
  Files         359      144      -215     
  Lines       36240    18163    -18077     
  Branches    36240    18163    -18077     
===========================================
- Hits        26886    11422    -15464     
+ Misses       7220     5908     -1312     
+ Partials     2134      833     -1301     

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.

[noaov1]

Reviewed 2 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/blockifier/src/execution/contract_class.rs line 632 at r2 (raw file):

    pub fn new_v1(
        contract_class: ContractClassV1,
        sierra_program_length: usize,

Or make sure it is has a positive value.

Suggestion:

sierra_program_length: NonZeroUsize,

---

crates/native_blockifier/src/py_transaction.rs line 170 at r2 (raw file):

            | starknet_api::transaction::DeclareTransaction::V1(_) => {
                let contract_class =
                    ContractClassV0::try_from_json_string(&py_class_info.raw_contract_class)?;

Suggestion:

                    ContractClassV0::try_from_json_string(&py_class_info.raw_contract_class)?;
                    assert_eq!(py_class_info.sierra_program_length, 0);

[ArniStarkware]

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @noaov1 and @yair-starkware)

---

crates/blockifier/src/execution/contract_class.rs line 632 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Or make sure it is has a positive value.

Done.

---

crates/native_blockifier/src/py_transaction.rs line 170 at r2 (raw file):

            | starknet_api::transaction::DeclareTransaction::V1(_) => {
                let contract_class =
                    ContractClassV0::try_from_json_string(&py_class_info.raw_contract_class)?;

Done.

[noaov1]

Reviewed 4 of 5 files at r3, all commit messages.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @yair-starkware)

---

crates/papyrus_execution/src/lib.rs line 826 at r3 (raw file):

                        contract_class_version: 1,
                        sierra_program_length: 0,
                    },

This code is repeated throughout the repository.
Maybe it's better to have the method getting the sierra length as a usize and checking there that it's not zero.
WDYT?
(this will create inconsistency in the signature of the two new methods, which is not great).

Code quote:

                ExecutionError::BadDeclareTransaction {
                    tx: DeclareTransaction::V3(declare_tx.clone()),
                    err: ContractClassError::ContractClassVersionSierraProgramLengthMismatch {
                        contract_class_version: 1,
                        sierra_program_length: 0,
                    },

[noaov1]

Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @yair-starkware)

---

crates/papyrus_execution/src/lib.rs line 826 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

This code is repeated throughout the repository.
Maybe it's better to have the method getting the sierra length as a usize and checking there that it's not zero.
WDYT?
(this will create inconsistency in the signature of the two new methods, which is not great).

No need to return an error, it's enough to add an assert, as it is an invariant of a cairo 1 contract.

[noaov1]

Reviewed 3 of 4 files at r4, 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @yair-starkware)

---

crates/blockifier/src/execution/contract_class.rs line 588 at r5 (raw file):

            + self.abi_length()
    }

A suggestion:
Leave the new method, replacing the error with panic.
I agree that it is a bit ugly to ask for a sierra length for a Cairo 0 contract but, Cairo 0 is not here to stay and having one new method allow to avoid matches on the contract class.

WDYT?

---

crates/blockifier/src/execution/errors.rs line 133 at r5 (raw file):

// TODO(Arni): remove this error variant. Squash this error into other error types. At least
// simplify it.

Why is this error still needed after your changes?

Code quote:

// TODO(Arni): remove this error variant. Squash this error into other error types. At least
// simplify it.

[ArniStarkware]

This becomes harder now that there are three variants of contract class.