Fix felt252 and enum deserialization bugs. #844

azteca1998 · 2024-10-10T17:07:00Z

Some types, including at least felt252 and most enum's tags, have a deserialization bug where the bytes that are not part of the type in MLIR are also parsed as part of the value in Rust. This PR filters out those bits.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.

github-actions · 2024-10-10T17:07:29Z

✅ Code is now correctly formatted.

github-actions · 2024-10-11T14:41:22Z

Benchmark results Main vs HEAD.

Command	Mean [s]	Min [s]	Max [s]	Relative
`head factorial_2M.cairo (JIT)`	3.359 ± 0.032	3.309	3.399	1.01 ± 0.01
`base factorial_2M.cairo (JIT)`	3.351 ± 0.027	3.303	3.385	1.00 ± 0.01
`head factorial_2M.cairo (AOT)`	3.338 ± 0.031	3.307	3.406	1.00
`base factorial_2M.cairo (AOT)`	3.352 ± 0.025	3.320	3.401	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`head fib_2M.cairo (JIT)`	2.953 ± 0.054	2.879	3.045	1.03 ± 0.02
`base fib_2M.cairo (JIT)`	2.890 ± 0.038	2.839	2.943	1.01 ± 0.02
`head fib_2M.cairo (AOT)`	2.860 ± 0.041	2.777	2.937	1.00
`base fib_2M.cairo (AOT)`	2.874 ± 0.024	2.830	2.916	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`head logistic_map.cairo (JIT)`	3.383 ± 0.024	3.340	3.416	1.07 ± 0.01
`base logistic_map.cairo (JIT)`	3.364 ± 0.028	3.317	3.406	1.07 ± 0.01
`head logistic_map.cairo (AOT)`	3.182 ± 0.023	3.150	3.215	1.01 ± 0.01
`base logistic_map.cairo (AOT)`	3.159 ± 0.020	3.124	3.189	1.00

github-actions · 2024-10-11T14:43:54Z

Benchmarking results

Benchmark for program `factorial_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	11.418 ± 0.380	11.025	11.830	24.20 ± 0.81
`cairo-native (embedded AOT)`	3.345 ± 0.058	3.272	3.453	7.09 ± 0.12
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.341 ± 0.027	3.316	3.400	7.08 ± 0.06
`cairo-native (standalone AOT)`	0.681 ± 0.004	0.678	0.691	1.44 ± 0.01
`cairo-native (standalone AOT with -march=native)`	0.472 ± 0.001	0.471	0.473	1.00

Benchmark for program `fib_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	11.444 ± 0.295	10.868	11.705	1448.77 ± 112.68
`cairo-native (embedded AOT)`	2.817 ± 0.022	2.776	2.859	356.56 ± 26.32
`cairo-native (embedded JIT using LLVM's ORC Engine)`	2.859 ± 0.029	2.812	2.902	361.97 ± 26.81
`cairo-native (standalone AOT)`	0.009 ± 0.000	0.009	0.009	1.10 ± 0.08
`cairo-native (standalone AOT with -march=native)`	0.008 ± 0.001	0.008	0.014	1.00

Benchmark for program `logistic_map`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	4.449 ± 0.057	4.375	4.542	61.27 ± 0.82
`cairo-native (embedded AOT)`	3.239 ± 0.039	3.165	3.294	44.61 ± 0.56
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.347 ± 0.022	3.307	3.386	46.10 ± 0.36
`cairo-native (standalone AOT)`	0.114 ± 0.000	0.113	0.115	1.57 ± 0.01
`cairo-native (standalone AOT with -march=native)`	0.073 ± 0.000	0.072	0.074	1.00

codecov-commenter · 2024-10-11T14:44:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.43%. Comparing base (f862ec3) to head (6be70fc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #844      +/-   ##
==========================================
+ Coverage   82.40%   82.43%   +0.02%     
==========================================
  Files         120      120              
  Lines       34799    34856      +57     
==========================================
+ Hits        28676    28732      +56     
- Misses       6123     6124       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* Add logs * Implement Felt252Dict trace dump * Implement nullabel for trace dump * Print type id * Remove print * add convertion for Sints * fix mispelled types * forgot to add feature * change Sints value to correspond to sierra-emu * Readd feature * Add is_infinity field to secp256 point struct (#828) * add is_infinity to secp point * clean * fix * use same deps as blockifier * fix * fix layout * fix test * remove some brittle asserts due to random initial data when adding and muling points at infinity * fix * remove unused dep * fixes, missed the value entry/output * Reset src * Fix warnings * Make library public * Use debug runtime for trace dump * fix storage's values truncations (#839) * fix storage's values truncations * change implementation for i8 i16 i32 i64 too * format * Add ptr check to felt dict entry * Add malloc tracing and fix more memory leaks. (#833) * Add memory tracing. * Partial memory fixes. * Fix arrays. * Simplify dictionaries. Fix false positive memory leak in dictionary drops. * Fix zero-sized allocation. Fix warnings. More mem tracing checks. * Fix mem tracing. * Fix memory leak in `print` libfunc. * Fix array slice libfunc. * Fix starknet memory leaks. * Fix keccak syscall. * Fix dict get double free bug. * Lots of memory fixes. * More fixes. * Fix CI. * Remove old TODOs. * Fix CI (again). * Remove TODOs. * Fix test. * Maybe fix * Maybe fix * Maybe fix * Dont panic when building circuit partial outputs * trace dump for Sha256StateHandler * Builtin costs rework (#837) * Update to 2.8.4, release docs, alpha This PR updates cairo to 2.8.4, adds some release docs and updates the version to alpha.3 to prepare for another release. * try fix * cleanup ci, remove panic in link * rename from jit to from ptr, etc * crates.io badge * progress * progress * fmt2 * progress * progress * fix * fix bench * use struct * Fix felt252 and enum deserialization bugs. (#844) * Fix felt252 and enum deserialization bugs. * Fix formatting. * Also fix the runtime. * Fix errors. * try to fix ci * remove unused deps --------- Co-authored-by: Edgar Luque <[email protected]> * Fix trace dump type conv * fix aot contract executor not passing builtinstats (#849) * Fix felt252 and enum deserialization bugs. * Fix formatting. * Also fix the runtime. * Fix errors. * try to fix ci * remove unused deps * fix aot contract executor not passing builtinstats --------- Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> * Fix bug (#851) * Also fix felt bits in starknet syscall wrappers (#853) * Also fix felt bits in starknet syscall wrappers * style * missed * fix aot contract executor not passing builtinstats (#849) * Fix felt252 and enum deserialization bugs. * Fix formatting. * Also fix the runtime. * Fix errors. * try to fix ci * remove unused deps * fix aot contract executor not passing builtinstats --------- Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> * Fix bug (#851) * Also fix felt bits in starknet syscall wrappers (#853) * Also fix felt bits in starknet syscall wrappers * style * missed * update version to alpha 4 (#854) * bytes31 * Better function attributes and re-enable >O1 opt (#843) * Fix felt252 and enum deserialization bugs. * Fix formatting. * Also fix the runtime. * Fix errors. * try to fix ci * remove unused deps * proper function attributes * add proper function attrs to optimize better, add some passes, run tests with atleast some opts * dont use remi * oops * maybe with opt level 3 now it works * test * works * readd deleted bench * remove dbg * Update bench-hyperfine.sh * fixci * comment * Update src/ffi.rs Co-authored-by: MrAzteca <[email protected]> --------- Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: MrAzteca <[email protected]> * Resolve `CAIRO_NATIVE_RUNTIME_LIBRARY` relative path (#841) * feat(ffi): resolve runtime relative path using current dir * chore: remove mentions to old runtime variable * fix: typo --------- Co-authored-by: Bohdan Ohorodnii <[email protected]> * Implement secp * Fix bytes31 * try to fix ci (#858) * update implementing libfuncs doc (#856) * Fix bytes31 bug * Remove unused dep --------- Co-authored-by: FrancoGiachetta <[email protected]> Co-authored-by: Edgar <[email protected]> Co-authored-by: MrAzteca <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: Esteve Soler Arderiu <[email protected]> Co-authored-by: Rodrigo <[email protected]> Co-authored-by: Bohdan Ohorodnii <[email protected]>

Fix felt252 and enum deserialization bugs.

3c15f13

Fix formatting.

45be8c1

azteca1998 marked this pull request as ready for review October 10, 2024 17:14

azteca1998 requested review from edg-l, igaray, jrchatruc, entropidelic, fmoletta, Oppen and pefontana as code owners October 10, 2024 17:14

edg-l previously approved these changes Oct 10, 2024

View reviewed changes

Also fix the runtime.

524f76f

azteca1998 dismissed edg-l’s stale review via 524f76f October 10, 2024 17:34

azteca1998 and others added 2 commits October 11, 2024 15:56

Fix errors.

2bbd609

Merge branch 'main' into fix-felt252-deserialization

37e1faf

edg-l previously approved these changes Oct 14, 2024

View reviewed changes

edg-l added the review-ready A PR that is ready for review label Oct 14, 2024

FrancoGiachetta previously approved these changes Oct 14, 2024

View reviewed changes

edg-l added this pull request to the merge queue Oct 14, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2024

edg-l added 2 commits October 14, 2024 14:37

try to fix ci

7291191

remove unused deps

6be70fc

edg-l dismissed stale reviews from FrancoGiachetta and themself via 6be70fc October 14, 2024 12:38

edg-l approved these changes Oct 14, 2024

View reviewed changes

FrancoGiachetta approved these changes Oct 14, 2024

View reviewed changes

edg-l enabled auto-merge October 14, 2024 12:40

edg-l added this pull request to the merge queue Oct 14, 2024

Merged via the queue into main with commit 8175154 Oct 14, 2024
15 checks passed

edg-l deleted the fix-felt252-deserialization branch October 14, 2024 13:36

edg-l mentioned this pull request Oct 15, 2024

Also fix felt bits in starknet syscall wrappers #853

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix felt252 and enum deserialization bugs. #844

Fix felt252 and enum deserialization bugs. #844

azteca1998 commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

codecov-commenter commented Oct 11, 2024 •

edited

Loading

Fix felt252 and enum deserialization bugs. #844

Fix felt252 and enum deserialization bugs. #844

Conversation

azteca1998 commented Oct 10, 2024 • edited Loading

Checklist

github-actions bot commented Oct 10, 2024 • edited Loading

github-actions bot commented Oct 11, 2024 • edited Loading

github-actions bot commented Oct 11, 2024 • edited Loading

Benchmarking results

Benchmark for program factorial_2M

Benchmark for program fib_2M

Benchmark for program logistic_map

codecov-commenter commented Oct 11, 2024 • edited Loading

Codecov Report

azteca1998 commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

Benchmark for program `factorial_2M`

Benchmark for program `fib_2M`

Benchmark for program `logistic_map`

codecov-commenter commented Oct 11, 2024 •

edited

Loading