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

Added cofactors to non-edwardian curve interfaces #50

Merged
merged 3 commits into from
Aug 26, 2020

Conversation

AntoineRondelet
Copy link
Collaborator

@AntoineRondelet AntoineRondelet commented Jul 27, 2020

Proposal to address #45

Question: What is the security provided by the edward curve? It is documented that it provides 80 bits of security, but is that still the case today? Would there be a reason to remove this curve from the lib? (this is outside of the scope of this PR of course)

@AntoineRondelet AntoineRondelet changed the title [WIP] Curves cofactors Added cofactors to non-edwardian curve interfaces Jul 31, 2020
@ValarDragon ValarDragon changed the base branch from master to staging July 31, 2020 22:39
@AntoineRondelet AntoineRondelet changed the base branch from staging to master August 3, 2020 07:56
@AntoineRondelet
Copy link
Collaborator Author

@ValarDragon I changed the target branch back to master to keep the diffs of this PR clean (otherwise the diffs also contain some former commits etc which is more annoying to review).
I like the idea to merge to staging first though, and periodically merge staging to master to package new releases. Happy to go with this method if you are all happy with it.

Note: We'd need to update staging with master, as it does seem to be a few commits behind for now (we can just rebase it on top of master if necessary)

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

This looks great to me! Sorry for the delayed review. I think its good to merge into staging, and then we can make a new release

These sage scripts will probably be very useful!

/cc @Pratyush would it be useful to have code to generate C++ and Rust curve code based off this type of sage file?

@ValarDragon
Copy link
Member

I'm not sure what the edwards code is for, and 80 bits of security doesn't seem useful for anything anymore. Maybe the idea of this curve was to have an Edwards curve model, which can be constructed from the scalar field?

If so, we should probably deprecate the existing curve and demonstrate it on jub-jub. (And be sure to not have it implement the pairing interface)

@ValarDragon ValarDragon changed the base branch from master to staging August 26, 2020 02:19
@ValarDragon ValarDragon merged commit 219b83a into scipr-lab:staging Aug 26, 2020
@Pratyush
Copy link

I believe the special thing about these Edwards curves is that they’re pairing friendly

@ValarDragon
Copy link
Member

Do you have any thoughts about whether pairing friendly edwards curves are useful/should be maintained? I don't know of anyone using it, nor do I know of a concrete reason to not use them for BLS.

ValarDragon pushed a commit that referenced this pull request Sep 25, 2020
* Added cofactors to non-edwardian curve interfaces

* Added mnt sage script

* Added more sage and fixed mnt4 g2 cofactor
@AntoineRondelet AntoineRondelet deleted the curves-cofactors branch August 26, 2021 12:47
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.

3 participants