-
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
feat(blockifier): support Cairo1 local recompilation #184
feat(blockifier): support Cairo1 local recompilation #184
Conversation
d79d774
to
f152b74
Compare
Benchmark movements: |
e736fb7
to
0b4fda1
Compare
f152b74
to
0516f8b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 07-29-chore_blockifier_verify_cairo_compiler_repo_with_correct_tag_ #184 +/- ##
======================================================================================================
Coverage ? 76.24%
======================================================================================================
Files ? 330
Lines ? 35104
Branches ? 35104
======================================================================================================
Hits ? 26765
Misses ? 6045
Partials ? 2294 ☔ View full report in Codecov by Sentry. |
0b4fda1
to
d7806fc
Compare
0516f8b
to
249b887
Compare
d7806fc
to
b7301ad
Compare
249b887
to
d870356
Compare
b7301ad
to
8e54eec
Compare
d870356
to
13acfad
Compare
8e54eec
to
1927450
Compare
13acfad
to
aa0605e
Compare
1927450
to
5d0ac45
Compare
ef4ccf9
to
7ce475e
Compare
Benchmark movements: |
378e67b
to
e57ad1c
Compare
7ce475e
to
cbb6cfe
Compare
Benchmark movements: |
e57ad1c
to
1fd4711
Compare
cbb6cfe
to
28ecdcd
Compare
Benchmark movements: |
1fd4711
to
678e67e
Compare
28ecdcd
to
9b19d4e
Compare
Benchmark movements: |
678e67e
to
469c6a8
Compare
9b19d4e
to
aa4a115
Compare
Benchmark movements: |
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 5 of 5 files at r9, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware, @dorimedini-starkware, @meship-starkware, and @noaov1)
crates/blockifier/src/test_utils/cairo_compile.rs
line 174 at r9 (raw file):
} fn verify_cairo1_compiler_deps(git_tag_override: Option<String>) {
verify
sounds like a no-side-effect action.
Suggestion:
prepare_cairo1_compiler_deps
crates/blockifier/src/test_utils/contracts.rs
line 279 at r9 (raw file):
Self::LegacyTestContract => ( // Legacy contract requires specific compiler tag (which is the point of // the test contract), + to build the compiler an
I don't understand the sentence (== this is why the test contract exists?)
Suggestion:
which is the point of the test contract
crates/blockifier/src/test_utils/contracts.rs
line 282 at r9 (raw file):
// older rust version is required. Some(LEGACY_CONTRACT_COMPILER_TAG.into()), Some(String::from("2023-07-05")),
Why 1 is const and the other isn't?
Code quote:
Some(LEGACY_CONTRACT_COMPILER_TAG.into()),
Some(String::from("2023-07-05")),
469c6a8
to
b4e9649
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: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware, @meship-starkware, @noaov1, and @TzahiTaub)
crates/blockifier/src/test_utils/contracts.rs
line 279 at r9 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
I don't understand the sentence (== this is why the test contract exists?)
better?
aa4a115
to
cf43b14
Compare
Benchmark movements: |
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 2 of 2 files at r10, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @meship-starkware, and @noaov1)
crates/blockifier/src/test_utils/contracts.rs
line 279 at r9 (raw file):
Previously, dorimedini-starkware wrote…
better?
Much
Merge activity
|
Benchmark movements: |
cf43b14
to
8d3dae6
Compare
Benchmark movements: |
This change is