-
Notifications
You must be signed in to change notification settings - Fork 25
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
chore(blockifier): add erc20 sierra contract #2668
base: aviv/refactor_py_state_reader
Are you sure you want to change the base?
chore(blockifier): add erc20 sierra contract #2668
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contact is taken from here
Reviewable status: 0 of 2 files reviewed, all discussions resolved
d5d55fb
to
57b45d3
Compare
b6de543
to
a38fb1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware and @dorimedini-starkware)
crates/blockifier/src/test_utils/contracts.rs
line 294 at r1 (raw file):
assert_ne!(self.cairo_version(), CairoVersion::Cairo0); // TODO (Meshi 01/01/2025): add a spacial case for ERC20 when ERC20 sierra is supported. assert!(!matches!(self, &Self::ERC20(CairoVersion::Cairo1(_))));
I am not sure about the comment phrasing but, I would like to have the fact that this is not the ERC20 compiled Sierra documented.
Suggestion:
// This is not the compiled Sierra file of the existing ERC20 contract,
// but a file that was taken from the compiler repo of another ERC20 contract.
if matches!(self, &Self::ERC20(CairoVersion::Cairo1(_))) {
return ERC20_SIERRA_CONTRACT_PATH;
}
crates/blockifier/src/test_utils/contracts.rs
line 83 at r1 (raw file):
erc20_contract_without_some_syscalls_compiled.json"; const ERC20_CAIRO1_CONTRACT_SOURCE_PATH: &str = "./ERC20/ERC20_Cairo1/ERC20.cairo"; const ERC20_CONTRACT_NAME: &str = "erc20_contract";
Suggestion:
const ERC20_SIERRA_CONTRACT_PATH: &str = "./ERC20/ERC20_Cairo1/erc20.sierra.json";
crates/blockifier/src/test_utils/contracts.rs
line 253 at r1 (raw file):
} fn get_non_erc20_base_name(&self) -> &str {
Change the name if it also returns the ERC20 contract
Code quote:
get_non_erc20_base_name
crates/blockifier/src/test_utils/contracts.rs
line 264 at r1 (raw file):
Self::CairoStepsTestContract => CAIRO_STEPS_TEST_CONTRACT_NAME, Self::SierraExecutionInfoV1Contract(_) => EXECUTION_INFO_V1_CONTRACT_NAME, Self::ERC20(_) => ERC20_CONTRACT_NAME,
Suggestion:
Self::ERC20(_) => unreachable!(),
57b45d3
to
29d2e1e
Compare
a38fb1d
to
df0d546
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware and @meship-starkware)
crates/blockifier/src/test_utils/contracts.rs
line 253 at r1 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
Change the name if it also returns the ERC20 contract
Done.
crates/blockifier/src/test_utils/contracts.rs
line 294 at r1 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
I am not sure about the comment phrasing but, I would like to have the fact that this is not the ERC20 compiled Sierra documented.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware and @dorimedini-starkware)
crates/blockifier/feature_contracts/cairo1/sierra/erc20_contract.sierra.json
line 49 at r2 (raw file):
"0x2ee1e2b1b89f8c495f200e4956278a4d47395fe262f27b52e5865c9524c08c3", "0x3288d594b9a45d15bb2fcb7903f06cdb06b27f0ba88186ec4cfaa98307cb972", "0x11",
Move this file to crates/blockifier/ERC20/ERC20_Cairo1
There was a problem hiding this 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 r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware and @dorimedini-starkware)
29d2e1e
to
3a292c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)
df0d546
to
c857512
Compare
2605638
to
56b7c70
Compare
c857512
to
69c3d44
Compare
56b7c70
to
aab30c9
Compare
69c3d44
to
c17bcea
Compare
c832388
to
e66c37b
Compare
c17bcea
to
e83ae75
Compare
e83ae75
to
f45b6e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @meship-starkware)
crates/blockifier/feature_contracts/cairo1/sierra/erc20_contract.sierra.json
line 49 at r2 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
Move this file to crates/blockifier/ERC20/ERC20_Cairo1
ops
No description provided.