-
Notifications
You must be signed in to change notification settings - Fork 441
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
Dori/ec op builtin #1657
Dori/ec op builtin #1657
Conversation
250dc65
to
dbd8240
Compare
4661750
to
4a55e27
Compare
170e9de
to
5390428
Compare
4a55e27
to
bf4043f
Compare
5390428
to
009f0ff
Compare
bf4043f
to
0c50226
Compare
009f0ff
to
b47c62e
Compare
0c50226
to
904b7a1
Compare
b47c62e
to
ac0fb4a
Compare
4878b02
to
cd06fc3
Compare
0e7c5a8
to
143306b
Compare
cd06fc3
to
b48398a
Compare
091ae24
to
67329b0
Compare
8548774
to
1d3f217
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 13 of 13 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)
crates/cairo-lang-starknet/src/db.rs
line 19 at r1 (raw file):
// Override implicit precedence for compatibility with the StarkNet OS. db.set_implicit_precedence(Arc::new( ["Pedersen", "RangeCheck", "Bitwise", "GasBuiltin", "System", "EcOp"]
this is certainly not the correct order.
i think it is:
Suggestion:
["Pedersen", "RangeCheck", "Bitwise", "EcOp", "GasBuiltin", "System"]
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, 2 unresolved discussions (waiting on @dorimedini-starkware)
crates/cairo-lang-sierra/src/extensions/modules/builtin_cost.rs
line 49 at r1 (raw file):
CostTokenType::Step => panic!("offset_in_builtin_costs is not supported for 'Step'."), CostTokenType::Pedersen => 0, CostTokenType::EcOp => 2,
why did you decide on this number?
Code quote:
CostTokenType::EcOp => 2,
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, 2 unresolved discussions (waiting on @orizi)
crates/cairo-lang-sierra/src/extensions/modules/builtin_cost.rs
line 49 at r1 (raw file):
Previously, orizi wrote…
why did you decide on this number?
because I'm prepping for this to be merged first
crates/cairo-lang-starknet/src/db.rs
line 19 at r1 (raw file):
Previously, orizi wrote…
this is certainly not the correct order.
i think it is:
Done. Where can I see the correct order?
1d3f217
to
5858980
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: 11 of 13 files reviewed, 2 unresolved discussions (waiting on @orizi)
crates/cairo-lang-sierra/src/extensions/modules/builtin_cost.rs
line 49 at r1 (raw file):
Previously, dorimedini-starkware wrote…
because I'm prepping for this to be merged first
And actually, I just assumed sequential is fine. Where can I see the actual builtin cost buffer values?
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 r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware)
crates/cairo-lang-sierra/src/extensions/modules/builtin_cost.rs
line 49 at r1 (raw file):
Previously, dorimedini-starkware wrote…
And actually, I just assumed sequential is fine. Where can I see the actual builtin cost buffer values?
It will be created in the OS, so this is fine
crates/cairo-lang-starknet/src/db.rs
line 19 at r1 (raw file):
Previously, dorimedini-starkware wrote…
Done. Where can I see the correct order?
In the OS and in lamdaclass code
5858980
to
d2ca939
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: 9 of 13 files reviewed, 1 unresolved discussion (waiting on @orizi)
crates/cairo-lang-starknet/src/db.rs
line 19 at r1 (raw file):
Previously, orizi wrote…
In the OS and in lamdaclass code
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 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)
17699a8
to
88b7a87
Compare
Signed-off-by: Dori Medini <dori@starkware.co> commit-id:35a8c1e6
88b7a87
to
5810bd6
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 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)
This change is