refactor(papyrus_rpc): get compiled class

[AvivYossef-starkware]

No description provided.

[AvivYossef-starkware]

• chore(blockifier): add erc20 sierra contract #2668 : 2 dependent PRs (#2641 , #2678 )
• refactor(blockifier): use get versioned contract class in py state reader #2614
• refactor(papyrus_rpc): get compiled class #2493 👈 (View in Graphite)
• feat(papyrus_storage): add sierra version to papyrus cache #2438
• feat(papyrus_storage): get versioned casm #2420 : 1 other dependent PR (#2421 )
• refactor(starknet_integration_tests): add sierra contract class to dummy state #2467
• feat(blockifier): get sierra contract class from feature contract #2466
• main

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

[reviewable-StarkWare]

This change is 

[dorimedini-starkware]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware and @Yael-Starkware)

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1478 at r1 (raw file):

                    .sierra_program,
            )
            .unwrap();

why panic here? shouldn't you return an RpcError if the sierra version is unavailable?

Code quote:

.unwrap();

[Yael-Starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @dorimedini-starkware)

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1478 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why panic here? shouldn't you return an RpcError if the sierra version is unavailable?

Doesn't this error fails the declare transaction in the gateway?

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1475 at r1 (raw file):

                    .get_class(&class_hash)
                    .map_err(internal_server_error)?
                    .ok_or_else(|| ErrorObjectOwned::from(CLASS_HASH_NOT_FOUND))?

does it make sense that the casm exists in storage and the sierra doesn't?
I think this scenario indicates a bug.

Code quote:

ErrorObjectOwned::from(CLASS_HASH_NOT_FOUND)

[AvivYossef-starkware]

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

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1475 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

does it make sense that the casm exists in storage and the sierra doesn't?
I think this scenario indicates a bug.

Right, Now the get_sierra_and_casm returns an error if Sierra Xor Casm exist in the server`s storage

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1478 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

Doesn't this error fails the declare transaction in the gateway?

It should fail because it indicates that the Sierra program is invalid. I think that the internal server error is ok since it indicates we have saved an invalid Sierra program on our server.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (aviv/add_sierra_version_to_papyrus_cache@9e86221). Learn more about missing BASE report.

Additional details and impacted files

@@                             Coverage Diff                             @@
##             aviv/add_sierra_version_to_papyrus_cache    #2493   +/-   ##
===========================================================================
  Coverage                                            ?   47.89%           
===========================================================================
  Files                                               ?      187           
  Lines                                               ?    23007           
  Branches                                            ?    23007           
===========================================================================
  Hits                                                ?    11020           
  Misses                                              ?    10886           
  Partials                                            ?     1101           

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

[dorimedini-starkware]

Reviewed 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware)

[Yael-Starkware]

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

[dorimedini-starkware]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)

---

crates/papyrus_rpc/src/v0_8/api/api_impl.rs line 1473 at r4 (raw file):

            // Check if both options are `Some`.
            let (casm, sierra) = option_casm
                .zip(option_sierra)

it's logic like this that makes me think returning Option<(X, Y)> is better than returning (Option<X>, Option<Y>). this is not possible, since the storage layer can return one None and one Some?

Code quote:

            let (option_casm, option_sierra) = storage_txn
                .get_casm_and_sierra(&class_hash)
                .map_err(internal_server_error_with_msg)?;

            // Check if both options are `Some`.
            let (casm, sierra) = option_casm
                .zip(option_sierra)

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-15T19:33:52Z

[AvivYossef-starkware]

Merge activity

• Dec 15, 3:06 PM EST: A user merged this pull request with Graphite.