-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add_option_to_simulate_builtins #1956
base: yaiv/add_all_cairo_stwo_layout
Are you sure you want to change the base?
Add_option_to_simulate_builtins #1956
Conversation
5509e52
to
bc25090
Compare
eb8a43c
to
1747d6a
Compare
Benchmark Results for unmodified programs 🚀
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## yaiv/add_all_cairo_stwo_layout #1956 +/- ##
===============================================================
Coverage 96.46% 96.46%
===============================================================
Files 102 102
Lines 41558 41564 +6
===============================================================
+ Hits 40087 40094 +7
+ Misses 1471 1470 -1 ☔ View full report in Codecov by Sentry. |
bc25090
to
6182e1f
Compare
1747d6a
to
7fd34a9
Compare
6182e1f
to
1b8e606
Compare
7fd34a9
to
22c9360
Compare
1b8e606
to
d2ab417
Compare
22c9360
to
2507fcb
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 4 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, and @yuvalsw)
vm/src/vm/runners/builtin_runner/ec_op.rs
line 26 at r2 (raw file):
impl EcOpBuiltinRunner { pub(crate) fn new(ratio: Option<u32>, included: bool) -> Self {
why is this change needed (x3)?
vm/src/vm/vm_core.rs
line 755 at r2 (raw file):
///Makes sure that all assigned memory cells are consistent with their auto deduction rules. pub fn verify_auto_deductions(&self) -> Result<(), VirtualMachineError> {
don't you need to take care of simulated_builtin_runners here?
other places?
TITLE
Adding option to simulate builtins
Description
This adds the option to use simulated runners that will later be verified by the cairo code that uses them (instead of proven later).
Checklist