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.

Sign up for GitHub

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

Jump to bottom

Feature: Implement Secp Syscalls #279

Closed
wants to merge 9 commits into from
Closed

Feature: Implement Secp Syscalls #279

wants to merge 9 commits into from

Conversation

whichqua
Copy link
Collaborator

This PR handles all the Secp syscalls for both k1 and r1 curves. It includes:

  • secp_new
  • secp_add
  • secp_mul
  • secp_get_point_from_x
  • secp_get_xy

Issue Number: N/A

Type

  • feature
  • bugfix
  • dev (no functional changes, no API changes)
  • fmt (formatting, renaming)
  • build
  • docs
  • testing

Description

Breaking changes?

  • yes
  • no

whichqua
whichqua commented Jul 26, 2024
View reviewed changes
src/execution/helper.rs Outdated Show resolved Hide resolved
odesenfans
odesenfans suggested changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@odesenfans odesenfans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review for early changes, I need to review the implementation in depth.

src/cairo_types/new_syscalls.rs Outdated Show resolved Hide resolved
src/execution/helper.rs Outdated Show resolved Hide resolved
src/execution/helper.rs Outdated Show resolved Hide resolved
src/execution/syscall_handler.rs Outdated Show resolved Hide resolved
odesenfans
odesenfans suggested changes Jul 27, 2024
View reviewed changes
Copy link
Contributor

@odesenfans odesenfans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realizing there's a lot of duplication coming from Blockifier. Why not call the Blockifier code directly?

src/execution/syscall_handler.rs Outdated Show resolved Hide resolved
src/execution/syscall_handler.rs Outdated Show resolved Hide resolved
@whichqua whichqua requested a review from odesenfans July 29, 2024 13:52
Olivier Desenfans and others added 4 commits July 30, 2024 12:02
@whichqua
Internal: support giving names to declared contracts in tests (#270)
fdb17b4 
Problem: when declaring contracts with the state builder, there is no
easy way to find the declared contracts from the test state.

Solution: reproduce what is done for deployed contracts, give the user
the ability to define a name for each declared contract.
@whichqua @notlesh @odesenfans
Feature: Run a complete fixture testing all transactions (#261)
65ff3dd 
Problem: We currently only test specific transactions one at a time.

Solution:
Implement a comprehensive test case that covers all tx types.

---------

Co-authored-by: Stephen Shelton <[email protected]>
Co-authored-by: Olivier Desenfans <[email protected]>
@whichqua
Internal: remove dead code from the error module (#275)
74429b0
@whichqua
fixes: minor aesthetic changes
bfc3c5b
@whichqua whichqua closed this Jul 30, 2024
@whichqua
Copy link
Collaborator Author

Closed in favor of #286

@whichqua whichqua deleted the gm/secp-wip branch July 30, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@odesenfans odesenfans Awaiting requested review from odesenfans

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@whichqua @odesenfans