-
Notifications
You must be signed in to change notification settings - Fork 47
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
Implement addition and multiplication operations for AffinePoint #112
Conversation
Multiply `AffinePoint` by `Felt`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain to me why is this something we want? Are are you sure there is no reason we could not want it to be implemented?
(I have no opinion, just trying to understand)
@@ -295,7 +295,7 @@ impl Felt { | |||
|
|||
/// Quotient and remainder between `self` and `rhs`. | |||
pub fn div_rem(&self, rhs: &NonZeroFelt) -> (Self, Self) { | |||
let (q, r) = self.0.representative().div_rem(&rhs.0.representative()); | |||
let (q, r) = self.representative().div_rem(&rhs.0.representative()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this change the behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the self.representative()
function returns self.0.representative()
so it's the same.
I had to add the public function representative
to get the representation of a Felt
for the operate_with_self
function in order to multiply the point with itself.
Adding representative()
function to Felt
and replace the old way of using it make the code cleaner.
Another missing functionality is the ability to uncompress EC points that are used in Cairo |
Implement `representative` function for `NonZeroFelt` and replace the `self.0.representative` with `self.representative` for all the places where it was used
Close this PR and open a new one with minimum changes and similar behavior as ProjectivePoint #114 |
What is the current behavior?
Addition or multiplication not allowed
What is the new behavior?
AffinePoints could now be added or multiplied by an exponent
Does this introduce a breaking change?
No