Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(blockifier_reexecution): fix bug in get contract class #1759

Conversation

AvivYossef-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link

codecov bot commented Nov 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.77%. Comparing base (e3165c4) to head (68a18f9).
Report is 215 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1759      +/-   ##
==========================================
+ Coverage   40.10%   41.77%   +1.66%     
==========================================
  Files          26      197     +171     
  Lines        1895    23304   +21409     
  Branches     1895    23304   +21409     
==========================================
+ Hits          760     9735    +8975     
- Misses       1100    13114   +12014     
- Partials       35      455     +420     

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

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from 3600295 to f532006 Compare November 3, 2024 13:47
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 7a8e174 to 96eeca3 Compare November 3, 2024 13:47
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)


crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 302 at r1 (raw file):

            "class_hash": class_hash.0.to_string(),
        });
        let contract_value =

Suggestion:

raw_contract_class

crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 311 at r1 (raw file):

                }
                Ok(contract) => contract,
            };

what happens if you try this? (note the ? at the end - i would expect a natural conversion from the send_rpc_request error to a variant of StateError)

Suggestion:

        let contract_value =
            match self.rpc_state_reader.send_rpc_request("starknet_getClass", params.clone()) {
                Err(RPCStateReaderError::ClassHashNotFound(_)) => {
                    return Err(StateError::UndeclaredClassHash(*class_hash));
                }
                other_result => other_result,
            }?;

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from f532006 to ef717ce Compare November 4, 2024 10:56
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 96eeca3 to f31706d Compare November 4, 2024 10:56
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aner-starkware)

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from ef717ce to dd06e06 Compare November 4, 2024 15:25
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from f31706d to d339e52 Compare November 4, 2024 15:25
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch 2 times, most recently from 4787c66 to bb0e710 Compare November 5, 2024 07:56
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from d339e52 to 12fe759 Compare November 5, 2024 07:57
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from bb0e710 to b366106 Compare November 5, 2024 08:00
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 12fe759 to 208e455 Compare November 5, 2024 08:09
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch 2 times, most recently from b4643dc to 0fe0464 Compare November 5, 2024 09:45
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 208e455 to 07d6e9e Compare November 5, 2024 09:45
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from 0fe0464 to bc3a4af Compare November 5, 2024 10:40
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 07d6e9e to 6458aac Compare November 5, 2024 10:40
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from bc3a4af to 9ceac3b Compare November 5, 2024 10:56
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 6458aac to a2eb905 Compare November 5, 2024 10:58
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from 9ceac3b to 49ea985 Compare November 5, 2024 14:18
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from a2eb905 to 625dd68 Compare November 5, 2024 14:18
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from 49ea985 to 0a1f2b4 Compare November 5, 2024 15:38
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 625dd68 to b93bc31 Compare November 5, 2024 15:40
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch 2 times, most recently from 6723e82 to 2b5af65 Compare November 6, 2024 07:45
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from b93bc31 to c728fca Compare November 6, 2024 07:47
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_refactor_test_result branch from 2b5af65 to f08222d Compare November 6, 2024 11:34
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from c728fca to 5993328 Compare November 6, 2024 11:36
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aner-starkware)

@AvivYossef-starkware AvivYossef-starkware changed the base branch from aviv/blockifier_reexecution_refactor_test_result to graphite-base/1759 November 6, 2024 11:59
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 5993328 to 97eee29 Compare November 6, 2024 11:59
@AvivYossef-starkware AvivYossef-starkware changed the base branch from graphite-base/1759 to main November 6, 2024 12:00
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/blockifier_reexecution_fix_bug_in_get_contract_class branch from 97eee29 to 68a18f9 Compare November 6, 2024 12:00
@AvivYossef-starkware AvivYossef-starkware merged commit a64cf6c into main Nov 6, 2024
10 checks passed
Copy link
Contributor Author

Merge activity

  • Nov 6, 7:16 AM EST: A user merged this pull request with Graphite.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants