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

Add Wahby-Boneh hash-to-curve #343

Merged
merged 154 commits into from
Feb 8, 2022
Merged

Add Wahby-Boneh hash-to-curve #343

merged 154 commits into from
Feb 8, 2022

Conversation

drskalman
Copy link
Contributor

@drskalman drskalman commented Oct 29, 2021 •
edited
Loading

Description

This PR uses #147 PR to implement WB hash to curve to it by essentially by translating https://github.com/zcash/pasta_curves/blob/main/src/hashtocurve.rs to arkworks language

We have also implemented the relevant parameters in our fork of arkworks-curves for bls12-381 and bls12-377 and now it works with our bls library.

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 (master)
  • 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

ValarDragon and others added 30 commits December 18, 2020 20:10
@ValarDragon
ec: Add HashToCurve trait and svw hash
377f4a8
@ValarDragon
Update traits
354fc96
@ValarDragon
Fix lint
73185bc
@ValarDragon
Update HashToField API to match draft
71b17cf
@ValarDragon
Update FieldHasher interfaces
809b1e1
@ValarDragon
Merge branch 'master' into hash_to_curve
7bcfdf1
@ValarDragon
Refactor file, progress on hash to field
7f7ef6d
@ValarDragon
Update hashing code structure
a03626d
@ValarDragon
Fix lint
6791b65
@ValarDragon
Merge branch 'master' into hash_to_curve
68c99ee
@ValarDragon
Merge branch 'master' into hash_to_curve
ac387cb
@ValarDragon
Add Hash to Field (untested)
50d5e33
@drskalman
First implementation for the simple swu map to curve
b0aa9d2
@drskalman
Merge remote-tracking branch 'upstream/master' into hash_to_curve_swu
9f228be
@drskalman
fixed some basic error in hash to curve module so it can be compiled.
6fc0fa3
@drskalman
removing mistakenly commited editor temps backup.
1c61897
@drskalman
Some syntax and dependancy fixing
65e45b8
@drskalman
Some more error and syntax clean up. Working toward a working test.
1c85dcd
@drskalman
fixes #2
11dc264
@drskalman
Fixes #1
095fa75
@drskalman
first attempt to add a simple finite field for a simple test.
05cc800
@ashutoshvarma
fix: trait not object in tests
8a9c0f6
@drskalman
Merge pull request #4 from ashutoshvarma/hash_to_curve_swu
90c67e8 
fix: trait not object in tests
@drskalman
First test running but failing because all field elements are equal to
2f20499 
zero for some reason.
@drskalman
fix the problem with field_new1 macro for F127.
480128d
@drskalman
added a test for checking parameters. Still panics at computing square
d50f8b5 
root.
@drskalman
Fixing Field parameters square root works but still it fails: `assert…
52c9bac 
…!(F127::new(BigInteger64([1])) == field_new!(F127, "1"));`
@drskalman
- use from instead of new to make field elements so we don't deal with
0075ba3 
montgomery form.
- added map_field_to_curve_swu
@drskalman
copy the structure of wb from sw
5a438c9
@drskalman
add curve map structure for of WB19 hash to curve method
5136fbe
Pratyush
Pratyush reviewed Jan 11, 2022
View reviewed changes
ff/src/fields/mod.rs Outdated Show resolved Hide resolved
Pratyush
Pratyush reviewed Jan 11, 2022
View reviewed changes
Copy link
Member

@Pratyush Pratyush 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 almost ready, modulo the small nits.

Pratyush
Pratyush reviewed Jan 11, 2022
View reviewed changes
ec/Cargo.toml Outdated Show resolved Hide resolved
Pratyush
Pratyush reviewed Jan 11, 2022
View reviewed changes
ec/Cargo.toml Outdated Show resolved Hide resolved
Marcin and others added 4 commits January 11, 2022 09:15
@Pratyush
Apply suggestions from code review
5f05861 
Co-authored-by: Pratyush Mishra <pratyushmishra@berkeley.edu>
@mmagician
Adapt the hash-to-curve codebase to digest v0.10
bfa1986
@drskalman
Merge pull request #32 from mmagician/adapt-to-digest-v0.10
712bcd0 
Adapt the hash-to-curve codebase to digest v0.10
@drskalman
- match definition of parity function to sgn0 of ietf draft in swu map
09057e2 
- adapt unit tests to the new definition
@drskalman drskalman mentioned this pull request Jan 12, 2022
Merged
6 tasks
drskalman and others added 3 commits January 12, 2022 13:12
@drskalman
non-zero not none-zero
dc9210f 
Co-authored-by: Marcin <marcin@web3.foundation>
@Pratyush
Compress parity impl
6644ce6
Merge pull request #33 from w3f/hash_to_curve_wb_with_ietf_parity
bcc53f5 
- match definition of parity function to sgn0 of ietf draft in swu map
@Pratyush
Copy link
Member

Pratyush commented Feb 1, 2022

Hi all, is there anything I can do to help push this along?

@drskalman
Copy link
Contributor Author

Hi all, is there anything I can do to help push this along?

I think it requires your judgement on @mmagician comment here:

https://github.com/arkworks-rs/algebra/pull/343/files/e18fd44106008cd3c885cf955556f805f24db9bc#diff-b13fb8649e8438bfe74b4b6f00cf6e78debf4df9bc5fae8d56775e3ee15966adR188

@mmagician
Copy link
Member

With the latest commits I have addressed the outstanding comments. Let us know if there's anything else @Pratyush.

daira
daira approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

Do you have any test vectors comparing against the Pasta implementation of hash-to-curve? I didn't see any (but this is a large PR and I might have missed them).

ec/src/hashing/curve_maps/swu/mod.rs Outdated Show resolved Hide resolved
ec/src/hashing/curve_maps/wb/mod.rs Outdated Show resolved Hide resolved
Pratyush and others added 2 commits February 7, 2022 18:14
@Pratyush Pratyush changed the title WB hash to curve Add Wahby-Boneh hash-to-curve Feb 8, 2022
@Pratyush Pratyush merged commit 548a3c4 into arkworks-rs:master Feb 8, 2022
@drskalman drskalman mentioned this pull request Feb 9, 2022
Closed
@ghost ghost mentioned this pull request Mar 14, 2022
Open
19 tasks
@NikZak NikZak mentioned this pull request Sep 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonmasson simonmasson simonmasson left review comments

@weikengchen weikengchen weikengchen left review comments

@mmagician mmagician mmagician left review comments

@Pratyush Pratyush Pratyush approved these changes

@daira daira daira approved these changes

@ValarDragon ValarDragon Awaiting requested review from ValarDragon

Assignees
No one assigned
Labels
T-design Type: discuss API design and/or research T-feature Type: new features T-performance Type: performance improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@drskalman @Pratyush @daira @mmagician @weikengchen @simonmasson @ValarDragon @ashutoshvarma @InaOana