-
Notifications
You must be signed in to change notification settings - Fork 180
feat: implement VirtualMachine::is_accessed #2024
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: implement VirtualMachine::is_accessed #2024
Conversation
Signed-off-by: Dori Medini <[email protected]>
Signed-off-by: Dori Medini <[email protected]>
Signed-off-by: Dori Medini <[email protected]>
Hi @dorimedini-starkware, this looks good but it would be great if you add why is this change necessary in the description of the PR |
Sure thing, done |
Hi @dorimedini-starkware. Could you add some tests for the new methods? The coverage job is failing: https://github.com/lambdaclass/cairo-vm/pull/2028/checks?check_run_id=39449669998 |
Signed-off-by: Dori Medini <[email protected]>
ee8fe26
to
3060b6a
Compare
Done. Couldn't run locally (build errors due to missing JSON files...?), running CI |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2024 +/- ##
=======================================
Coverage 96.54% 96.54%
=======================================
Files 102 102
Lines 42755 42778 +23
=======================================
+ Hits 41276 41299 +23
Misses 1479 1479 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
LGTM
@JulianGCalderon can you update this? |
closing in favor of this |
Pull request was closed
Implement public API for
is_accessed
fromRelocatable
.Description
When using the VM to run the Starknet OS, we require a public API to check if a cell is accessed (for segmented contract loading).
Checklist