Add is_infinity field to secp256 point struct #828

edg-l · 2024-10-03T10:56:35Z

Related to #817

This also mimics the syscall handler impl for secp256 methods from blockifier, so we are more equal to make future debugging easier.

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-03T11:24:58Z

Benchmark results Main vs HEAD.

Command	Mean [s]	Min [s]	Max [s]	Relative
`head factorial_2M.cairo (JIT)`	3.438 ± 0.011	3.426	3.456	1.01 ± 0.01
`base factorial_2M.cairo (JIT)`	3.477 ± 0.030	3.449	3.542	1.02 ± 0.02
`head factorial_2M.cairo (AOT)`	3.410 ± 0.030	3.347	3.463	1.00 ± 0.02
`base factorial_2M.cairo (AOT)`	3.393 ± 0.043	3.338	3.454	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`head fib_2M.cairo (JIT)`	2.979 ± 0.026	2.937	3.008	1.03 ± 0.03
`base fib_2M.cairo (JIT)`	2.996 ± 0.036	2.943	3.065	1.04 ± 0.04
`head fib_2M.cairo (AOT)`	2.914 ± 0.035	2.871	2.969	1.01 ± 0.03
`base fib_2M.cairo (AOT)`	2.879 ± 0.092	2.785	3.094	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`head logistic_map.cairo (JIT)`	3.336 ± 0.044	3.290	3.449	1.06 ± 0.02
`base logistic_map.cairo (JIT)`	3.355 ± 0.028	3.311	3.392	1.06 ± 0.01
`head logistic_map.cairo (AOT)`	3.161 ± 0.019	3.126	3.192	1.00
`base logistic_map.cairo (AOT)`	3.225 ± 0.037	3.162	3.281	1.02 ± 0.01

github-actions · 2024-10-03T11:25:14Z

Benchmarking results

Benchmark for program `factorial_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	11.070 ± 0.287	10.550	11.476	24.64 ± 0.67
`cairo-native (embedded AOT)`	3.269 ± 0.042	3.213	3.343	7.28 ± 0.11
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.286 ± 0.031	3.246	3.338	7.31 ± 0.09
`cairo-native (standalone AOT)`	0.676 ± 0.004	0.673	0.687	1.50 ± 0.01
`cairo-native (standalone AOT with -march=native)`	0.449 ± 0.003	0.448	0.459	1.00

Benchmark for program `fib_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	11.001 ± 0.256	10.470	11.263	1364.51 ± 33.73
`cairo-native (embedded AOT)`	2.833 ± 0.036	2.771	2.891	351.35 ± 5.34
`cairo-native (embedded JIT using LLVM's ORC Engine)`	2.830 ± 0.031	2.778	2.889	351.01 ± 4.80
`cairo-native (standalone AOT)`	0.008 ± 0.000	0.008	0.009	1.04 ± 0.01
`cairo-native (standalone AOT with -march=native)`	0.008 ± 0.000	0.008	0.009	1.00

Benchmark for program `logistic_map`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	4.399 ± 0.044	4.345	4.494	61.96 ± 0.64
`cairo-native (embedded AOT)`	3.110 ± 0.020	3.079	3.144	43.81 ± 0.30
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.282 ± 0.021	3.241	3.313	46.23 ± 0.33
`cairo-native (standalone AOT)`	0.115 ± 0.000	0.115	0.116	1.62 ± 0.01
`cairo-native (standalone AOT with -march=native)`	0.071 ± 0.000	0.071	0.072	1.00

src/starknet_stub.rs

…d muling points at infinity

codecov-commenter · 2024-10-04T14:22:28Z

Codecov Report

Attention: Patch coverage is 79.91266% with 46 lines in your changes missing coverage. Please review.

Project coverage is 88.08%. Comparing base (cff7a11) to head (8bd8e48).

Files with missing lines	Patch %	Lines
src/starknet_stub.rs	76.00%	42 Missing ⚠️
src/starknet.rs	82.35%	3 Missing ⚠️
src/arch/x86_64.rs	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #828      +/-   ##
==========================================
- Coverage   88.11%   88.08%   -0.04%     
==========================================
  Files         123      123              
  Lines       34973    35155     +182     
==========================================
+ Hits        30818    30965     +147     
- Misses       4155     4190      +35

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

src/starknet.rs

src/starknet_stub.rs

tests/tests/starknet/secp256.rs

edg-l · 2024-10-08T06:45:16Z

Fixed some bugs and addressed some comments

* 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 <git@edgarluque.com> * 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 <esteve.soler@lambdaclass.com> Co-authored-by: Esteve Soler Arderiu <soler.arderiu@gmail.com> * 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 <esteve.soler@lambdaclass.com> Co-authored-by: Esteve Soler Arderiu <soler.arderiu@gmail.com> * 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 <azteca1998@users.noreply.github.com> --------- Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com> Co-authored-by: Esteve Soler Arderiu <soler.arderiu@gmail.com> Co-authored-by: MrAzteca <azteca1998@users.noreply.github.com> * 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 <limposfeed@gmail.com> * Implement secp * Fix bytes31 * try to fix ci (#858) * update implementing libfuncs doc (#856) * Fix bytes31 bug * Remove unused dep --------- Co-authored-by: FrancoGiachetta <francogiachetta27@gmail.com> Co-authored-by: Edgar <git@edgarluque.com> Co-authored-by: MrAzteca <azteca1998@users.noreply.github.com> Co-authored-by: Esteve Soler Arderiu <esteve.soler@lambdaclass.com> Co-authored-by: Esteve Soler Arderiu <soler.arderiu@gmail.com> Co-authored-by: Rodrigo <rodrodpino@gmail.com> Co-authored-by: Bohdan Ohorodnii <limposfeed@gmail.com>

edg-l added 2 commits October 3, 2024 12:53

add is_infinity to secp point

0b31334

clean

1e2fda4

edg-l mentioned this pull request Oct 3, 2024

bug: error in secp256k1_add when adding point at infinity #817

Closed

xrvdg reviewed Oct 4, 2024

View reviewed changes

src/starknet_stub.rs Outdated Show resolved Hide resolved

xrvdg mentioned this pull request Oct 4, 2024

fix: handle infinity arithmetic NethermindEth/sequencer#25

Open

edg-l added 2 commits October 4, 2024 12:26

Merge remote-tracking branch 'origin/main' into add_is_infinity

fcabb02

fix

eb585ff

edg-l marked this pull request as ready for review October 4, 2024 10:34

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

edg-l marked this pull request as draft October 4, 2024 12:34

edg-l added 3 commits October 4, 2024 14:35

use same deps as blockifier

dc00b3e

fix

27f4efd

fix layout

ed1864a

edg-l force-pushed the add_is_infinity branch from 6816b8d to ed1864a Compare October 4, 2024 13:21

edg-l added 3 commits October 4, 2024 15:30

fix test

25558b8

remove some brittle asserts due to random initial data when adding an…

04a55d6

…d muling points at infinity

fix

943f93e

edg-l changed the title ~~Add is_infinity field to seckp point struct~~ Add is_infinity field to secp256 point struct Oct 4, 2024

edg-l marked this pull request as ready for review October 4, 2024 13:45

remove unused dep

6f48948

Merge branch 'main' into add_is_infinity

56ff3d3

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

edg-l linked an issue Oct 7, 2024 that may be closed by this pull request

bug: error in secp256k1_add when adding point at infinity #817

Closed

azteca1998 reviewed Oct 7, 2024

View reviewed changes

src/starknet.rs Show resolved Hide resolved

src/starknet.rs Show resolved Hide resolved

src/starknet_stub.rs Show resolved Hide resolved

src/starknet_stub.rs Show resolved Hide resolved

tests/tests/starknet/secp256.rs Outdated Show resolved Hide resolved

fixes, missed the value entry/output

8fa3671

edg-l requested a review from azteca1998 October 8, 2024 06:43

azteca1998 previously approved these changes Oct 8, 2024

View reviewed changes

JulianGCalderon previously approved these changes Oct 8, 2024

View reviewed changes

fix

8bd8e48

edg-l dismissed stale reviews from JulianGCalderon and azteca1998 via 8bd8e48 October 8, 2024 15:06

edg-l requested review from azteca1998 and JulianGCalderon October 8, 2024 15:06

JulianGCalderon approved these changes Oct 8, 2024

View reviewed changes

azteca1998 approved these changes Oct 8, 2024

View reviewed changes

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

Merged via the queue into main with commit 951a76b Oct 8, 2024
15 checks passed

edg-l deleted the add_is_infinity branch October 8, 2024 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add is_infinity field to secp256 point struct #828

Add is_infinity field to secp256 point struct #828

edg-l commented Oct 3, 2024 •

edited

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading

codecov-commenter commented Oct 4, 2024 •

edited

Loading

edg-l commented Oct 8, 2024

Add is_infinity field to secp256 point struct #828

Add is_infinity field to secp256 point struct #828

Conversation

edg-l commented Oct 3, 2024 • edited Loading

Checklist

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

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

Benchmarking results

Benchmark for program factorial_2M

Benchmark for program fib_2M

Benchmark for program logistic_map

codecov-commenter commented Oct 4, 2024 • edited Loading

Codecov Report

edg-l commented Oct 8, 2024

edg-l commented Oct 3, 2024 •

edited

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading

Benchmark for program `factorial_2M`

Benchmark for program `fib_2M`

Benchmark for program `logistic_map`

codecov-commenter commented Oct 4, 2024 •

edited

Loading