[go: nahoru, domu]
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

impl H2G for TE curves #65

Merged
merged 2 commits into from
May 14, 2022
Merged

impl H2G for TE curves #65

merged 2 commits into from
May 14, 2022

Conversation

zhenfeizhang
Copy link
Contributor
@zhenfeizhang zhenfeizhang commented May 13, 2022 •
edited by alxiong
Loading

Description

Implement hash to group features for twisted edwards curves

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (main)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the GitHub PR explorer

alxiong
alxiong reviewed May 14, 2022
View reviewed changes
Copy link
Contributor
@alxiong alxiong left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Edward -> Edwards
  • add for jubjub over bls12-381
  • add to changelog

I will address my own comments in a following commit

another question is: so it seems that the algorithm for both SW and TE is the same? (you mentioned that there might be slight difference yesterday? where (what document) can we double check?)

@alxiong alxiong merged commit 889180b into main May 14, 2022
@alxiong alxiong deleted the te-hash-to-group branch May 14, 2022 04:36
@zhenfeizhang
Copy link
Contributor Author

Thanks for finishing this up.

On a related note:
Arkworks at the moment only have the trait definitions for Wahby-Boneh hash-to-group: https://github.com/arkworks-rs/algebra/tree/master/ec/src/hashing/curve_maps
We cannot really use those until the actual WBParams are defined.

@zhenfeizhang
Copy link
Contributor Author

another question is: so it seems that the algorithm for both SW and TE is the same? (you mentioned that there might be slight difference yesterday? where (what document) can we double check?)

I was wrong.
see https://safecurves.cr.yp.to/equation.html
For both SW and TE, y has a sign bit. So they can share a same API.

I was thinking about get_point_from_y() in which case x is unique for SW curve (thus does not require the sign bit) while non-unique for TE curve. And in that case, there is a subtle different in the code.

@alxiong
Copy link
Contributor
alxiong commented May 17, 2022

For both SW and TE, y has a sign bit. So they can share a same API.

so basically we can have HashToGroup: ModelParameters instead? and unify the two TEHashToGroup and SWHashToGroup, correct? @zhenfeizhang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alxiong alxiong alxiong approved these changes

@chancharles92 chancharles92 Awaiting requested review from chancharles92

Assignees

@zhenfeizhang zhenfeizhang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zhenfeizhang @alxiong