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

Add support for hash to curve draft 11 #59

Merged
merged 14 commits into from
May 31, 2021

Conversation

andrewwhitehead
Copy link
Contributor

@andrewwhitehead andrewwhitehead commented Mar 31, 2021

This PR implements draft 11 of the hash-to-curve RFC, including the Simplified SWU method but not the Shallue-van de Woestijne method. It is largely based on the implementation in paired.

@andrewwhitehead
Copy link
Contributor Author

Is anybody able to look at this? It's sufficient for implementing the current BLS signatures draft. We are also hoping to use this for BBS+ signatures.

@andrewwhitehead andrewwhitehead changed the title Add support for hash to curve draft 10 Add support for hash to curve draft 11 Apr 30, 2021
@str4d
Copy link
Member

str4d commented May 1, 2021

We have an implementation of draft 10 in the pasta_curves crate for the (prime-order) Pallas and Vesta curves, so I'm at least somewhat familiar with the ID. I'll have a look at this PR over the weekend.

Copy link
Member

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Looking good overall!

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/fp.rs Outdated Show resolved Hide resolved
src/fp.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
}

/// Map from a point on the curve E-prime to curve E
fn iso_map(u: &G2Projective) -> G2Projective {
Copy link
Member

Choose a reason for hiding this comment

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

I have not checked this implementation.

src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
}

#[cfg(test)]
fn check_g2_prime(pt: &G2Projective) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

I have not checked any of the tests in this module.

…for G1 and G2

Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Copy link
Member

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Flushing more comments.

src/hash_to_curve/mod.rs Outdated Show resolved Hide resolved
src/hash_to_curve/mod.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g1.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
src/hash_to_curve/map_g1.rs Outdated Show resolved Hide resolved
src/hash_to_curve/map_g1.rs Outdated Show resolved Hide resolved
Comment on lines +415 to +418
let tmp = Fp2 {
c0: -sqrt_candidate.c1,
c1: sqrt_candidate.c0,
};
Copy link
Member

Choose a reason for hiding this comment

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

This took me a moment to spot, but it works because G2 is c0 + i*c1, and this step needs to multiply by sqrt(-1) = i, resulting in i*c0 + i^2 * c1 = -c1 + i*c0.

// compute g(x1(u)) = g(x0(u)) * XI^3 * u^6
let gx1_num = gx0_num * xi_usq * xisq_u4;
// compute g(x1(u)) * u^3
let sqrt_candidate = sqrt_candidate * usq * u;
Copy link
Member

Choose a reason for hiding this comment

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

This differs from Appendix G.2.3 of draft 11, which uses y * usq * u in steps 44-45. That has an effect on how steps 46-67 are implemented.

src/hash_to_curve/map_g2.rs Outdated Show resolved Hide resolved
Copy link
Member

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK. I have not checked absolutely everything in this PR, but I am happy enough in the test vector coverage for this to be merged as an experimental feature. It should be audited more thoroughly before being removed from behind the experimental feature flag.

@str4d str4d merged commit 6d6eea8 into zkcrypto:main May 31, 2021
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.

2 participants