Skip to content
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: enable ec_add / ec_mul #1398

Merged
merged 15 commits into from
Sep 24, 2024
Merged

feat: enable ec_add / ec_mul #1398

merged 15 commits into from
Sep 24, 2024

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Sep 7, 2024

Time spent on this PR:

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Resolves #1230
Resolves #1231
Resolves #1365

What is the new behavior?

  • Adds ECMUL and ECADD execution from Cairo1Helpers
  • Fixes an issue where reverted precompile execution was not consuming all gas due to a mismanagement of error codes
  • note: reverted != execution fails silently (see ec_recover vs. ec_add, for example)
  • Fixes an issue where external_precompiles would not fail with error EXCEPTIONAL_HALT

This change is Reviewable

Makefile Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@ClementWalter
Copy link
Member

paused until katana update their blockifier fork

@enitrat enitrat force-pushed the feat/enable-ecadd-ecmud branch 4 times, most recently from 2d80d49 to f63d192 Compare September 17, 2024 11:55
@enitrat enitrat changed the title Feat/enable ecadd ecmud feat: enable ec_add / ec_mul Sep 23, 2024
@enitrat enitrat marked this pull request as ready for review September 23, 2024 12:42
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 10.52632% with 17 lines in your changes missing coverage. Please review.

Project coverage is 61.8%. Comparing base (c7896d0) to head (d9cae56).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/kakarot/interpreter.cairo 0.0% 11 Missing ⚠️
src/kakarot/precompiles/precompiles.cairo 14.2% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1398     +/-   ##
=======================================
- Coverage   63.1%   61.8%   -1.3%     
=======================================
  Files         49      44      -5     
  Lines       8332    8169    -163     
=======================================
- Hits        5260    5055    -205     
- Misses      3072    3114     +42     
Flag Coverage Δ
61.8% <10.5%> (-1.3%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@enitrat
Copy link
Collaborator Author

enitrat commented Sep 23, 2024

image
final commit this time i swear

Makefile Outdated Show resolved Hide resolved
@enitrat enitrat merged commit 414e4da into main Sep 24, 2024
8 of 10 checks passed
@enitrat enitrat deleted the feat/enable-ecadd-ecmud branch September 24, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dev: remove with_attr ecadd / ecmul ecMul ecAdd
3 participants