feat(blockifier): get sierra contract class from feature contract

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

• feat(papyrus_storage): get versioned casm #2420 : 2 dependent PRs (#2421 , #2438 )
• refactor(starknet_integration_tests): add sierra contract class to dummy state #2467
• feat(blockifier): get sierra contract class from feature contract #2466 👈 (View in Graphite)
• main

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

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-05T09:40:21Z
• Started at 2024-12-05T10:53:33Z
• Started at 2024-12-05T13:05:36Z
• Started at 2024-12-05T14:02:24Z
• Started at 2024-12-05T14:31:31Z
• Started at 2024-12-08T09:17:08Z

[codecov]

Codecov Report

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

Project coverage is 80.25%. Comparing base (e3165c4) to head (cb2c53d).
Report is 736 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_api/src/state.rs	0.00%	20 Missing ⚠️
crates/blockifier/src/test_utils/contracts.rs	0.00%	13 Missing ⚠️
crates/starknet_api/src/rpc_transaction.rs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2466       +/-   ##
===========================================
+ Coverage   40.10%   80.25%   +40.15%     
===========================================
  Files          26       98       +72     
  Lines        1895    13528    +11633     
  Branches     1895    13528    +11633     
===========================================
+ Hits          760    10857    +10097     
- Misses       1100     2202     +1102     
- Partials       35      469      +434     

☔ 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, 4 unresolved discussions (waiting on @AvivYossef-starkware, @meship-starkware, and @Yoni-Starkware)

---

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

                    None => Default::default(),
                }
            },

Suggestion:

            abi: cairo_lang_contract_class.abi.map(|abi| abi.json()).unwrap_or_default(),

---

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

        }
    }
}

@Yoni-Starkware this conversion includes O(len(sierra_program)) Felt::from(biguint) operations...
how bad is it?
@AvivYossef-starkware how many times is this conversion invoked? is it cached?

Code quote:

impl From<CairoLangContractClass> for SierraContractClass {
    fn from(cairo_lang_contract_class: CairoLangContractClass) -> Self {
        Self {
            sierra_program: cairo_lang_contract_class
                .sierra_program
                .into_iter()
                .map(|big_uint_as_hex| Felt::from(big_uint_as_hex.value))
                .collect(),
            contract_class_version: cairo_lang_contract_class.contract_class_version,
            entry_points_by_type: cairo_lang_contract_class.entry_points_by_type.into(),
            abi: {
                match cairo_lang_contract_class.abi {
                    Some(abi) => abi.json(),
                    None => Default::default(),
                }
            },
        }
    }
}

---

crates/starknet_api/src/rpc_transaction.rs line 7 at r1 (raw file):

use std::collections::HashMap;

use cairo_lang_starknet_classes::contract_class::ContractEntryPoints as CairoLangContractEntryPoints;

why rename? I don't see a name collision

Suggestion:

ContractEntryPoints;

---

crates/starknet_api/src/rpc_transaction.rs line 287 at r1 (raw file):

// TODO(AVIV): Consider removing this conversion and using the one in the
// CairoLangContractEntryPoints

Suggestion:

// TODO(AVIV): Consider removing this conversion and using ContractEntryPoints instead of
//   defining the EntryPointByType struct.

[AvivYossef-starkware]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware, @meship-starkware, and @Yoni-Starkware)

---

crates/starknet_api/src/rpc_transaction.rs line 7 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why rename? I don't see a name collision

To make it clearer when looking at the function

---

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

Previously, dorimedini-starkware wrote…

@Yoni-Starkware this conversion includes O(len(sierra_program)) Felt::from(biguint) operations...
how bad is it?
@AvivYossef-starkware how many times is this conversion invoked? is it cached?

Currently, I only use it once for each feature contract in the end-to-end test.

---

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

                    None => Default::default(),
                }
            },

Done.

---

crates/starknet_api/src/rpc_transaction.rs line 287 at r1 (raw file):

// TODO(AVIV): Consider removing this conversion and using the one in the
// CairoLangContractEntryPoints

Done.

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.760 ms 34.806 ms 34.857 ms]
change: [-4.0048% -2.3800% -1.0120%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

[dorimedini-starkware]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @meship-starkware and @Yoni-Starkware)

---

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

Previously, AvivYossef-starkware wrote…

Currently, I only use it once for each feature contract in the end-to-end test.

this is not used in business logic? then please gate it behind #[cfg(any(test, feature = "testing"))]

[AvivYossef-starkware]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @meship-starkware, and @Yoni-Starkware)

---

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

Previously, dorimedini-starkware wrote…

this is not used in business logic? then please gate it behind #[cfg(any(test, feature = "testing"))]

No

[dorimedini-starkware]

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

[meship-starkware]

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

[dorimedini-starkware]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

[AvivYossef-starkware]

Merge activity

• Dec 8, 4:35 AM EST: A user merged this pull request with Graphite.