refactor(starknet_api): add contract class version to sierra contract

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

• refactor(starknet_api): add contract class version to sierra contract #2277 👈 (View in Graphite)
• refactor(starknet_api): rename contract class to sierra contract class #2276
• main

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

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.686 ms 35.151 ms 35.703 ms]
change: [+1.1239% +2.6890% +4.1730%] (p = 0.00 < 0.05)
Performance has regressed.
Found 15 outliers among 100 measurements (15.00%)
2 (2.00%) high mild
13 (13.00%) high severe

full_committer_flow performance regressed!
full_committer_flow time: [30.999 ms 31.042 ms 31.087 ms]
change: [+3.8548% +4.0589% +4.2750%] (p = 0.00 < 0.05)
Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [31.248 ms 31.311 ms 31.378 ms]
change: [+2.8388% +3.1636% +3.4451%] (p = 0.00 < 0.05)
Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.963 ms 31.041 ms 31.158 ms]
change: [+3.5667% +3.9101% +4.3478%] (p = 0.00 < 0.05)
Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
4 (4.00%) high mild
1 (1.00%) high severe

[Yoni-Starkware]

Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/papyrus_storage/src/serialization/serializers.rs line 986 at r1 (raw file):

            )?,
            abi: String::deserialize_from(&mut decompress_from_reader(bytes)?.as_slice())?,
            contract_class_version: String::deserialize_from(bytes)?,

Seems like you need to keep the order here

Code quote:

    fn deserialize_from(bytes: &mut impl std::io::Read) -> Option<Self> {
        Some(Self {
            sierra_program: Vec::<Felt>::deserialize_from(
                &mut decompress_from_reader(bytes)?.as_slice(),
            )?,
            entry_points_by_type: HashMap::<EntryPointType, Vec<EntryPoint>>::deserialize_from(
                bytes,
            )?,
            abi: String::deserialize_from(&mut decompress_from_reader(bytes)?.as_slice())?,
            contract_class_version: String::deserialize_from(bytes)?,

---

crates/starknet_api/src/state.rs line 218 at r1 (raw file):

    pub entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,
    pub abi: String,
    pub contract_class_version: String,

To be consistent with the compiler's struct

Suggestion:

    pub sierra_program: Vec<Felt>,
    pub contract_class_version: String,
    pub entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,
    pub abi: String,

---

crates/papyrus_storage/src/utils_test.rs line 34 at r1 (raw file):

            ClassHash(i_felt),
            SierraContractClass {
                contract_class_version: "".to_string(),

Let it be "0.1.0"

Code quote:

"".to_string(),

[AvivYossef-starkware]

Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/papyrus_storage/src/serialization/serializers.rs line 986 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Seems like you need to keep the order here

the order of the fields?

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.681 ms 35.153 ms 35.707 ms]
change: [+1.4530% +2.8518% +4.6847%] (p = 0.00 < 0.05)
Performance has regressed.
Found 17 outliers among 100 measurements (17.00%)
2 (2.00%) high mild
15 (15.00%) high severe

full_committer_flow performance regressed!
full_committer_flow time: [30.886 ms 30.924 ms 30.964 ms]
change: [+3.6661% +3.8536% +4.0388%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

[Yoni-Starkware]

Are you removing the duplicated object in a separate PR?

Reviewed all commit messages.
Reviewable status: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

[AvivYossef-starkware]

Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/papyrus_storage/src/utils_test.rs line 34 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Let it be "0.1.0"

Done.

---

crates/starknet_api/src/state.rs line 218 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

To be consistent with the compiler's struct

Done.

[codecov]

Codecov Report

Attention: Patch coverage is 8.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 61.56%. Comparing base (e3165c4) to head (3893a27).
Report is 582 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/papyrus_protobuf/src/converters/class.rs	0.00%	8 Missing ⚠️
...s/papyrus_storage/src/serialization/serializers.rs	0.00%	0 Missing and 2 partials ⚠️
crates/starknet_client/src/reader/objects/state.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2277       +/-   ##
===========================================
+ Coverage   40.10%   61.56%   +21.45%     
===========================================
  Files          26       85       +59     
  Lines        1895    10339     +8444     
  Branches     1895    10339     +8444     
===========================================
+ Hits          760     6365     +5605     
- Misses       1100     3047     +1947     
- Partials       35      927      +892     

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

[Yoni-Starkware]

Reviewed all commit messages.
Reviewable status: 0 of 7 files reviewed, 6 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/papyrus_storage/src/utils.rs line 26 at r2 (raw file):

#[derive(Serialize)]
struct DumpDeclaredClass {

Can you ask @DvirYo-starkware what is this class and why it's missing the string ABI?

Code quote:

#[derive(Serialize)]
struct DumpDeclaredClass {

---

crates/papyrus_storage/src/utils.rs line 31 at r2 (raw file):

    compiled_class_hash: CompiledClassHash,
    sierra_program: Vec<Felt>,
    entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,

Keep the order consistent

Suggestion:

struct DumpDeclaredClass {
    class_hash: ClassHash,
    compiled_class_hash: CompiledClassHash,
    sierra_program: Vec<Felt>,
    contract_class_version: String,
    entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,

---

crates/papyrus_storage/src/utils.rs line 82 at r2 (raw file):

                            compiled_class_hash: *compiled_class_hash,
                            sierra_program: contract_class.sierra_program.clone(),
                            entry_points_by_type: contract_class.entry_points_by_type.clone(),

Suggestion:

                        &DumpDeclaredClass {
                            class_hash: *class_hash,
                            compiled_class_hash: *compiled_class_hash,
                            sierra_program: contract_class.sierra_program.clone(),
                            contract_class_version: contract_class.contract_class_version.clone(),
                            entry_points_by_type: contract_class.entry_points_by_type.clone(),

[Yoni-Starkware]

Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/papyrus_storage/src/serialization/serializers.rs line 986 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

the order of the fields?

Yes

---

crates/starknet_client/src/reader/objects/state.rs line 71 at r2 (raw file):

            contract_class_version: class.contract_class_version,
            sierra_program: class.sierra_program,
            entry_points_by_type: class.entry_points_by_type,

Suggestion:

        Self {
            sierra_program: class.sierra_program,
            contract_class_version: class.contract_class_version,
            entry_points_by_type: class.entry_points_by_type,

---

crates/papyrus_test_utils/src/lib.rs line 503 at r2 (raw file):

        pub entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,
        pub abi: String,
        pub contract_class_version: String,

Suggestion:

    pub struct SierraContractClass {
        pub sierra_program: Vec<Felt>,
        pub contract_class_version: String,
        pub entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,
        pub abi: String,

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [31.296 ms 31.388 ms 31.501 ms]
change: [+2.5449% +3.3163% +4.0026%] (p = 0.00 < 0.05)
Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severe

[AvivYossef-starkware]

Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/papyrus_storage/src/utils.rs line 31 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Keep the order consistent

Done.

---

crates/papyrus_storage/src/utils.rs line 82 at r2 (raw file):

                            compiled_class_hash: *compiled_class_hash,
                            sierra_program: contract_class.sierra_program.clone(),
                            entry_points_by_type: contract_class.entry_points_by_type.clone(),

Done.

---

crates/papyrus_test_utils/src/lib.rs line 503 at r2 (raw file):

        pub entry_points_by_type: HashMap<EntryPointType, Vec<EntryPoint>>,
        pub abi: String,
        pub contract_class_version: String,

Done.

---

crates/starknet_client/src/reader/objects/state.rs line 71 at r2 (raw file):

            contract_class_version: class.contract_class_version,
            sierra_program: class.sierra_program,
            entry_points_by_type: class.entry_points_by_type,

Done.

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [31.012 ms 31.150 ms 31.332 ms]
change: [+3.9397% +4.4521% +5.0547%] (p = 0.00 < 0.05)
Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe

[Yoni-Starkware]

Don't forget to delete RpcContractClass

Reviewed 1 of 6 files at r1, 2 of 4 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)

[Yoni-Starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware)

---

crates/papyrus_storage/src/utils.rs line 26 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Can you ask @DvirYo-starkware what is this class and why it's missing the string ABI?

Non blocking

[DvirYo-starkware]

Reviewed 1 of 6 files at r1, 3 of 4 files at r2, 3 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)

---

crates/papyrus_storage/src/utils.rs line 26 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Non blocking

The struct DumpDeclaredClass can be deleted. I will delete it in a different PR.

---

crates/papyrus_storage/src/serialization/serializers.rs line 972 at r3 (raw file):

    fn serialize_into(&self, res: &mut impl std::io::Write) -> Result<(), StorageSerdeError> {
        serialize_and_compress(&self.sierra_program)?.serialize_into(res)?;
        self.contract_class_version.serialize_into(res)?;

This is a breaking change to the storage. We should discuss this.
A few months ago, we decided to break the storage in version 13.3. I don't know what was agreed upon about the storage in 13.3 and 13.4, but it is fine if we break the storage in 13.4.

---

crates/papyrus_protobuf/src/converters/class.rs line 209 at r3 (raw file):

        let sierra_program =
            value.program.into_iter().map(Felt::try_from).collect::<Result<Vec<_>, _>>()?;

Make sure with @ShahakShama it is fine (I don't think should be a problem here)