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

expose hash to group #887

Merged
merged 7 commits into from
May 4, 2023
Merged

expose hash to group #887

merged 7 commits into from
May 4, 2023

Conversation

Trivo25
Copy link
Member

@Trivo25 Trivo25 commented May 2, 2023

exposes hashToGroup to SnarkJS
closes #772 companion of o1-labs/o1js-bindings#5

@Trivo25 Trivo25 changed the base branch from main to feature/move-bindings-2 May 2, 2023 16:02
@Trivo25 Trivo25 marked this pull request as ready for review May 2, 2023 16:04
@Trivo25 Trivo25 changed the title implement snarky' hash to curve expose hash to group May 2, 2023
Base automatically changed from feature/move-bindings-2 to main May 3, 2023 10:11
Copy link
Contributor

@rbonichon rbonichon left a comment

Choose a reason for hiding this comment

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

Minor comment from @crypto_guardian.

src/lib/hash.ts Outdated
Comment on lines 60 to 61
// r is the second root of sqrt(y^2)
let r = y.mul(-1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we factor that computation, albeit not costly with the let y_ = y.mul(-1) below?

Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

just realized I never finish my review 😓 can you address this @Trivo25

src/lib/hash.ts Outdated Show resolved Hide resolved
src/lib/hash.ts Outdated Show resolved Hide resolved
@Trivo25 Trivo25 merged commit 4e4fd4d into main May 4, 2023
@Trivo25 Trivo25 deleted the feature/hashToCurve-bindings branch May 4, 2023 10:10
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.

Implement hashing to curve
3 participants