Skip to content

Conversation

@orizi
Copy link
Collaborator

@orizi orizi commented Jan 4, 2026

Summary

Refactored EC point validation and curve operations by extracting common elliptic curve functionality into the EcPointType struct. Added utility methods for calculating curve points and validating if a point lies on the curve, replacing duplicated curve equation implementations across the codebase.


Type of change

Please check one:

  • Performance improvement

Why is this change needed?

The code had multiple implementations of the same elliptic curve operations scattered across different modules. This created maintenance issues and potential inconsistencies when working with EC points. By centralizing these operations in the EcPointType struct, we ensure consistent behavior and make the code more maintainable.


What was the behavior or documentation before?

Previously, the curve equation and validation logic was duplicated in multiple places, with hardcoded constants for the curve parameters appearing in both the runner and simulation code. This made it difficult to ensure consistent behavior across different parts of the codebase.


What is the behavior or documentation after?

Now all EC point operations use a centralized implementation in the EcPointType struct, which provides methods for:

  • Calculating the left-hand side of the curve equation
  • Calculating the right-hand side of the curve equation
  • Checking if a point is on the curve

This ensures consistent behavior across the codebase and simplifies maintenance.


Additional context

This refactoring improves code organization without changing functionality. The centralized implementation makes it easier to update curve operations if needed in the future.

SIERRA_UPDATE_PATCH_CHANGE_TAG=Refactoring unreleased function.
@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator Author

orizi commented Jan 4, 2026

@orizi orizi marked this pull request as ready for review January 4, 2026 15:20
@ilyalesokhin-starkware
Copy link
Contributor

crates/cairo-lang-sierra/src/extensions/modules/ec.rs line 34 at r1 (raw file):

    /// The beta parameter of the curve.
    const BETA: Felt252 = Felt252::from_hex_unchecked(
        "0x6f21413efbe40de150e596d72f7a8c5609ad26c15c915c1f4cdfcb99cee9e89",

Consider mentioning that alpha is 1.

Code quote:

    /// The beta parameter of the curve.
    const BETA: Felt252 = Felt252::from_hex_unchecked(
        "0x6f21413efbe40de150e596d72f7a8c5609ad26c15c915c1f4cdfcb99cee9e89",

Copy link
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ilyalesokhin-starkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @liorgold2 and @TomerStarkware).

Copy link
Collaborator

@TomerStarkware TomerStarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@TomerStarkware reviewed all commit messages and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @liorgold2).

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.

5 participants