Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

fix: state error unavailable contract address format #2078

Open
wants to merge 1 commit into
base: main-v0.13.2
Choose a base branch
from

Conversation

AvivYossef-starkware
Copy link
Collaborator

@AvivYossef-starkware AvivYossef-starkware commented Jul 14, 2024

This change is Reviewable

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 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)


.gitignore line 12 at r1 (raw file):

tmp_venv/*

.spr.yml

hmmm... shame to ignore this, there are settings here that depend on the repo, not the user. right? for merge queue, for example?
is it possible to "split" .spr.yml to repo/user settings?
or maybe possible to use environment variables in .spr.yml, so only each user's .bashrc needs to add metadata for the specific user in environment variables, and then you can commit .spr.yml?
out of scope, please add a TODO here in a comment to investigate this

Code quote:

.spr.yml

crates/blockifier/src/state/error_format_test.rs line 18 at r1 (raw file):

#[test]
fn test_error_unavailable_contract_address_format() {
    let error = StateError::UnavailableContractAddress(ContractAddress::from(10_u128));

(non-blocking FYI: this should work too)

Suggestion:

let error = StateError::UnavailableContractAddress(10_u128.into());

crates/blockifier/src/state/errors.rs line 22 at r1 (raw file):

    #[error(transparent)]
    ProgramError(#[from] ProgramError),
    #[error("Requested {:#064x} is unavailable for deployment.",***.0)]

no auto-formatting in macros

Suggestion:

, ***.0)]

@AvivYossef-starkware AvivYossef-starkware changed the base branch from spr/main-v0.13.2/85a8a843 to main-v0.13.2 July 15, 2024 08:41
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.

2 participants