Fix builtin security check. #714
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alonh5
requested review from
igaray,
Oppen,
fmoletta,
entropidelic,
HermanObst,
juanbono,
Jrigada,
pefontana and
Juan-M-V
as code owners
alonh5
force-pushed
the
starkware/alonh/fix_builtin_security_check
branch
2 times, most recently
from
69d08d6 to
c1f7e6b
Compare
Oppen
reviewed
Jan 12, 2023
src/vm/runners/builtin_runner/mod.rs
Outdated
| Some(_) => {} | ||
| None => { | ||
| let offset = offset_base + j; | ||
| match offsets.contains(&offset) { |
There was a problem hiding this comment.
This makes the complexity go from O(n_input_cells) to O(n_input_cells*offsets.len(). Could this be called often?
There was a problem hiding this comment.
I changed it back to O(n*n_input_cells).
alonh5
force-pushed
the
starkware/alonh/fix_builtin_security_check
branch
from
c1f7e6b to
322ad1b
Compare
Juan-M-V
approved these changes
Jan 16, 2023
Juan-M-V
reviewed
Jan 16, 2023
src/vm/runners/builtin_runner/mod.rs
Outdated
Comment on lines
292
to
295
| let n = match offsets.iter().max() { | ||
| Some(x) => div_floor(*x, cells_per_instance as usize) + 1, | ||
| None => 0, | ||
| }; |
There was a problem hiding this comment.
A map_or could shorten this match to offsets.iter().max().map_or(0, |v| div_floor(*x, cells_per_instance as usize) + 1)
Oppen
approved these changes
Jan 16, 2023
alonh5
force-pushed
the
starkware/alonh/fix_builtin_security_check
branch
from
322ad1b to
ecd5b3e
Compare
alonh5
force-pushed
the
starkware/alonh/fix_builtin_security_check
branch
from
ecd5b3e to
bb17218
Compare
Juan-M-V
added a commit
that referenced
this pull request
Jan 20, 2023
* DO NOT PUSH this commit * Add and AddAssign tests * Proptest for felt bitwise operations (#716) * DO NOT PUSH this commit * this works but a lot of the variables are 0 * add tests for the or and xor operations * fixed another test, where the prop_assume was in the wrong place to avoid dividing by zero * Update felt/src/lib.rs Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com> Co-authored-by: Martina <martina@martina> Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com> * Add proptest for felt subtraction (#722) * add test for subtraction * minor fix Co-authored-by: Martina <martina@martina> * Parse from string radix (#692) Co-authored-by: Juanma <juanma@Juanmas-MacBook-Air.local> * Neg prop test for felts (#721) * found a bug, when x is 0 the code breaks when applying the Neg operation * minor change * fixed bug for Neg operator Co-authored-by: Martina <martina@martina> * derive default for execution resources (#727) * Add Serialize trait to Felt (#724) Co-authored-by: Pedro Fontana <pedro.fontana@lamdaclass.com> * feat: enable the skip_instruction_exectution for hints (#645) * feat: enable the skip_instruction_exectution for hints * feat: add skip_next_instruction() as whitelisted hint * feat: update hint string to match python vm skip_instruction hint * feat: update hint text * feat: refactor skip_next_instruction into crate feature * fix: fix make compare command to ignore .noretrocompat.* files to allow for new features * feat: add mut to hintprocessor after main rebase * fix: fix clippy error -> reference instead of variable * fix: fix erros due to rebase * Whitespace Co-authored-by: Timothée Delabrouille <timothee.delabrouille@gmail.com> Co-authored-by: Mario Rugiero <mario.rugiero@nextroll.com> * Fix builtin security check. (#714) * verify signature hint (#702) * Add common signature test * Add hint to hint code * Add signature hint * Add signature test * Add test to signature-hint * Happy path test Co-authored-by: Juanma <juanma@Juanmas-MacBook-Air.local> * Mul assign felt proptest (#717) * DO NOT PUSH this commit * test for multiplication with assignment * minor fix Co-authored-by: Martina <martina@martina> Co-authored-by: Juan-M-V <102986292+Juan-M-V@users.noreply.github.com> * Fix: typos (#733) * Fix: typo Fix: typo * Fix: typos Fix: typos * Fix: typos Fix: typos * Fix: typos Fix: typos * Use CairoArg enum instead of Any in CairoRunner::run_from_entrypoint (#686) * Start get_traceback_entries + add convenience methos * Add fn is_call_instruction * add code * Refactor code * Clippy * Add get_traceback method * Fix get_error_attr_value * Add traceback to VmException * Make traceback non-optional * Add tests for is_call_instruction * Add traceback to error display * Add test + fix logic for get_traceback_entries * Code refactor * Add one more test for get_traceback_entries * Fix string format + add test for get_traceback * Improve fn * Add reference to is_call_instruction signature * Add reference to immediate in decode_instruction + remove clone * Fix hint_processor mutability in tests * Add Location::get_location_marks * Fix method to_string_with_contents * Fix string format * Fix string format * Update traceback tests * Add tests for Location::to_string_with_contents() * Fix intermediate string format * Fix test * Add tests for Location::get_location_marks() * Update VmException display * Fix string format * Fix string format * Remove debug print * Fix Display * Implement Display for MaybeRelocatable * Add real-case test for VmException Display * Remove debug format from erros containing MaybeRelocatable and Relocatable * Add tests for display implementation * Update Changelog * Clippy * Remove unnecessary & * Add hint location to InstructionLocation * Use InstructionLocation instead of Location in insruction_locations field of Program * Add hint location logic to get_location * Add rought version of VirtualMachineError::Hint * Add test for error display on HintError * Add test for get_location with hint_index * Start refactor * Update changelog * Finnish changing hint fns to HintError * Update custom hint example * Fix changelog format * Add changelog entry for this PR * Use CairoArg enum instead of Any in CairoRunner::run_from_entrypoint * Fix tests * Add tests + remove unused func * use slice instead of vec * Update changelog * Add non-typed helpers for ids variables * Swap BigInt for MaybeRelocatable in Dictionary * Update dict_hint_utils + dict macros in test_utils * Fix tests * Clippy * Add test for dict_write with relocatable * Add tests on hint_utils.rs * Add tetss on hint_processor_utils.rs * Add integration test for bug case * Update changelog * Fix eof * Remove debug print * Update readme (#729) * DO NOT PUSH this commit * Improve the About section * more changes * improve step by step instructions to use the repo * more changes * made requested changes in the hints section * removed (optional) as it was unnecessary Co-authored-by: Martina <martina@martina> * subtraction tests * delete unnecessary print Co-authored-by: Martina <martina@martina> Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com> Co-authored-by: Juan-M-V <102986292+Juan-M-V@users.noreply.github.com> Co-authored-by: Juanma <juanma@Juanmas-MacBook-Air.local> Co-authored-by: Juan Bono <juanbono94@gmail.com> Co-authored-by: Pedro Fontana <pedro.fontana@lambdaclass.com> Co-authored-by: Pedro Fontana <pedro.fontana@lamdaclass.com> Co-authored-by: Elias Tazartes <66871571+Eikix@users.noreply.github.com> Co-authored-by: Timothée Delabrouille <timothee.delabrouille@gmail.com> Co-authored-by: Mario Rugiero <mario.rugiero@nextroll.com> Co-authored-by: Alon Haramati <91828241+alonh5@users.noreply.github.com> Co-authored-by: omahs <73983677+omahs@users.noreply.github.com> Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix builtin security check
Description
Fix builtin security check to match python VM.
Checklist