Support Secp256r1 #310
Merged
Support Secp256r1 #310
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Trivo25 can you get me the necessary reviewers for this? |
querolita
reviewed
Oct 29, 2024
| @@ -15,6 +15,18 @@ const secp256k1Params: CurveParams = { | |||
| }, | |||
| }; | |||
|
|
|||
| const secp256r1Params: CurveParams = { | |||
| @@ -26,6 +26,10 @@ let exampleFields = { | |||
| f25519: createField(p25519), | |||
| secp256k1: createField(pSecp256k1), | |||
| secq256k1: createField(pSecq256k1), | |||
| secp256r1: | |||
| createField( | |||
| 0xffffffff00000001000000000000000000000000ffffffffffffffffffffffffn | |||
| @@ -530,14 +623,12 @@ function createCurveAffine({ | |||
| endoScalar, | |||
| endoBase, | |||
| }: CurveParams) { | |||
| // TODO: lift this limitation by using other formulas (in projectiveScale) for a != 0 | |||
| if (a !== 0n) throw Error('createCurveAffine only supports a = 0'); | |||
There was a problem hiding this comment.
Removing this check implies you allow any a? In createCurveProjective you removed the check and called createProjectiveDouble where there's a check that a is 0 or -3. But what about here?
There was a problem hiding this comment.
that's a good point, I mean we do support general a now in all the purely affine methods like affineAdd, affineDouble. the only place which doesn't support general a is projectiveDouble, which is used here by methods like scale() and isInSubgroup()
so I think we can accept general a here at this point and only when using certain unsupported methods users would hit the error.
Trivo25
approved these changes
Oct 30, 2024
querolita
approved these changes
Oct 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
companion of o1-labs/o1js#1885
ain most elliptic curve methodsaparametera = -3in projective curve doublinga. thanks to the other changes here, it would now be very simple to add the generalacase as well, but for testing we would need a concrete curve with generala, and I'm skipping the work of finding one since I'm not interested in general curves here