chore(blockifier): implement staknet_api class_info to match blockifier class_info

[avivg-starkware]

…er class_info

[reviewable-StarkWare]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.

Project coverage is 75.44%. Comparing base (e3165c4) to head (d6be674).
Report is 306 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_api/src/contract_class.rs	0.00%	35 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1702       +/-   ##
===========================================
+ Coverage   40.10%   75.44%   +35.33%     
===========================================
  Files          26      103       +77     
  Lines        1895    13907    +12012     
  Branches     1895    13907    +12012     
===========================================
+ Hits          760    10492     +9732     
- Misses       1100     2959     +1859     
- Partials       35      456      +421     

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

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.845 ms 33.977 ms 34.180 ms]
change: [-4.3743% -2.8665% -1.5399%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
4 (4.00%) high mild
5 (5.00%) high severe

[github-actions]

Benchmark movements:
full_committer_flow performance improved 😺
full_committer_flow time: [29.441 ms 29.486 ms 29.535 ms]
change: [-2.1544% -1.8472% -1.5661%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
5 (5.00%) high mild
2 (2.00%) high severe

[avivg-starkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @noaov1)

---

crates/starknet_api/src/contract_class.rs line 106 at r2 (raw file):

        }
    }
}

copied from blockifier::execution::contract_class::ClassInfo to be used through staknet_api executable_tx through AccountTransaction in following PR (1688)

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.555 ms 35.060 ms 35.654 ms]
change: [+1.1354% +2.8985% +4.8625%] (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: [29.904 ms 29.993 ms 30.116 ms]
change: [+1.3107% +1.6565% +2.0874%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high severe

[github-actions]

Artifacts upload triggered. View details here

[ArniStarkware]

Reviewed 1 of 3 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)

---

crates/starknet_api/src/contract_class.rs line 106 at r2 (raw file):

Previously, avivg-starkware wrote…

copied from blockifier::execution::contract_class::ClassInfo to be used through staknet_api executable_tx through AccountTransaction in following PR (1688)

#1688 (add a HashTag for PR number to become a link :) )

---

crates/starknet_api/src/contract_class.rs line 100 at r4 (raw file):

            })
        }
    }

Damn, do I hate this new method... (This is not a practical comment - just sharing).
It can only return an error if we have an internal error.

Out of scope for this PR - just personal grudges. And the upside is that with the old new method deleted - the refactor I planned can take effect.

Code quote:

    pub fn new(
        contract_class: &ContractClass,
        sierra_program_length: usize,
        abi_length: usize,
    ) -> Result<Self, StarknetApiError> {
        let (contract_class_version, condition) = match contract_class {
            ContractClass::V0(_) => (0, sierra_program_length == 0),
            ContractClass::V1(_) => (1, sierra_program_length > 0),
        };

        if condition {
            Ok(Self { contract_class: contract_class.clone(), sierra_program_length, abi_length })
        } else {
            Err(StarknetApiError::ContractClassVersionSierraProgramLengthMismatch {
                contract_class_version,
                sierra_program_length,
            })
        }
    }

[ArniStarkware]

crates/starknet_api/src/contract_class.rs line 48 at r4 (raw file):

/// and other parameters derived from the original declare transaction required for billing.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct ClassInfo {

Suggestion:

// TODO(Ayelet,10/02/2024): Change to bytes.
pub struct ClassInfo {

[avivg-starkware]

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

---

crates/starknet_api/src/contract_class.rs line 106 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

#1688 (add a HashTag for PR number to become a link :) )

thanks!

---

crates/starknet_api/src/contract_class.rs line 48 at r4 (raw file):

/// and other parameters derived from the original declare transaction required for billing.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct ClassInfo {

Done.

---

crates/starknet_api/src/contract_class.rs line 100 at r4 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Damn, do I hate this new method... (This is not a practical comment - just sharing).
It can only return an error if we have an internal error.

Out of scope for this PR - just personal grudges. And the upside is that with the old new method deleted - the refactor I planned can take effect.

I might ask you about it in person out of curiosity, for me to better understand your hate :)

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

Reviewable status: 7 of 9 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware, @meship-starkware, and @noaov1)

---

crates/starknet_api/src/core.rs line 468 at r10 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Constants should be at the start of the file.

Another issue is that the constant WORD_WIDTH 's name is non-informative in this context.

Would you change the name of the constant or just add the comment as you did?

As there were other constants along this file, I wasn't sure where best to place. I moved it just below the 'use'... should it be elsewhere?

---

crates/blockifier/src/fee/eth_gas_constants.rs line 4 at r10 (raw file):

pub const GAS_PER_MEMORY_ZERO_BYTE: usize = 4;
pub const GAS_PER_MEMORY_BYTE: usize = 16;
pub const WORD_WIDTH: usize = 32; //TODO(AvivG): use starknet_api::core::WORD_WIDTH instead.aig add -p

Done.

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.537 ms 30.579 ms 30.623 ms]
change: [+1.2640% +1.4847% +1.6929%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild

[ArniStarkware]

Reviewed 1 of 2 files at r11.
Reviewable status: 8 of 9 files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)

---

crates/starknet_api/src/core.rs line 468 at r10 (raw file):

Previously, avivg-starkware wrote…

Would you change the name of the constant or just add the comment as you did?

As there were other constants along this file, I wasn't sure where best to place. I moved it just below the 'use'... should it be elsewhere?

I think this is fine for now.

Maybe we want a starknet_api::constants.rs - but I think this is out of scope.

[ArniStarkware]

Reviewed 1 of 2 files at r11, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)

---

crates/blockifier/src/fee/eth_gas_constants.rs line 4 at r10 (raw file):

Previously, avivg-starkware wrote…

Done.

1. Great.
2. Non-blocking - how hard is it to follow through with this to-do in this PR? How many files are affected?

[meship-starkware]

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

[avivg-starkware]

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

---

crates/blockifier/src/fee/eth_gas_constants.rs line 4 at r10 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

1. Great.
2. Non-blocking - how hard is it to follow through with this to-do in this PR? How many files are affected?

there are only 4 files WORD_WIDTH appears in. I still prefer to handle this in a separate PR, will link in this thread when exists

[ArniStarkware]

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

[avivg-starkware]

• chore(blockifier): remove redundant blockifier transactions and related fns #1831
• refactor(blockifier): account_tx from enum to struct w api::executable_tx #1688
• refactor(blockifier): switch use of blockifier classinfo by starknet_api classinfo #1858
• chore(blockifier): implement staknet_api class_info to match blockifier class_info #1702 👈
• main

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

Join @avivg-starkware and the rest of your teammates on Graphite

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[ArniStarkware]

Reviewed 1 of 1 files at r12, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)