cairo-vm
cairo-vm copied to clipboard
feat: SECP related hints
Context: port of the Starknet OS to Rust.
We need an implementation of the hints used by the Starknet OS in the SECP syscalls. These hints rely on private primitives in cairo-vm and must be implemented in this repository.
Checklist
- [ ] Linked to Github Issue
- [x] Unit tests added
- [ ] Integration tests added.
- [ ] This change requires new documentation.
- [ ] Documentation has been added/updated.
- [x] CHANGELOG has been updated.
Hi @odesenfans !! Sorry the delay, we didn't saw this PR Can you merge to main and solve the conflicts?
Codecov Report
Attention: Patch coverage is 57.26950% with 241 lines in your changes missing coverage. Please review.
Project coverage is 94.31%. Comparing base (
63b4a55) to head (5337a25). Report is 3 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1829 +/- ##
==========================================
- Coverage 94.83% 94.31% -0.53%
==========================================
Files 101 102 +1
Lines 39579 40142 +563
==========================================
+ Hits 37536 37858 +322
- Misses 2043 2284 +241
| Flag | Coverage Δ | |
|---|---|---|
? |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@odesenfans some more points:
- Since the VM is only allowed to run whitelisted hints in production we can
t enable the VM to execute this hints in production. There are two ways to solve this, leaving the hints under a feature flag likecairo-0-secp-hints`, the other way is that you implement your own hint processor in your repo - There are some lints error in the CI
- There are some unwraps where you can handle the error, like the
pop().unwrap();and some more cases - Can you add some integration tests for the new hints?
@pefontana The requested changes and integration tests have been added. Please feel free to run the workflows and review the code.
@whichqua sorry for the delay, you can ping me on telegram if I don't respond here https://t.me/Pedrofontana5 Some CI checks are failing
@JulianGCalderon @pefontana Let me know if there is anything else needed. Would be nice to get the tests passing.
Hi @whichqua, two jobs seem to be failing:
- Compute memory and execution traces with cairo-lang. This job executes the programs with cairo-lang
- Compute memory and execution traces with cairo-vm. This job executes the programs with cairo-vm.
The second job fails because the hint is not found. This is because this job on the CI runs all the standalone files in cairo_programs and compares it's execution. You could try to move these files to another directory cairo_programs/cairo-0-secp-hints-feature and see if it fixes it. Keep in mind that this will probably fix the CI, but it will not fix the underlying error that is breaking the first job.
@JulianGCalderon ~Is there a reason we cant add the feature cairo-0-secp-hints flag to cairo-vm-cli?~
Seems like this is intentional and cairo-1 hints themselves are tested via cairo-1-run.
Hi @whichqua, the compute memory and execution traces with cairo-lang is working OK now. On the other hand, compute memory and execution traces with cairo-vm is still failing. To fix this, could you move the added programs to another directory cairo_programs/cairo-0-secp-hints-feature?
The reason you should do this is because the failing workflow compares the execution of all the standalone programs in the directory with the default features (not including the new added feature). There is another job in the workflow that already tests with other features, so there is no need to test them in this job also.