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 functions to support nova repo #44

Conversation

chaosma
Copy link

@chaosma chaosma commented May 21, 2023

  • add hash_to_curve function
  • add from_slice function
  • fix minor issue of ysign in from_bytes

Copy link
Contributor

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but I know nothing about the hash to curve algorithms, I'll leave the core part for other reviewers to review.

Comment on lines +165 to +175
impl [< $name Compressed >] {
pub fn from_slice(slice: &[u8]) -> Result<Self, &'static str> {
if slice.len() != [< $name _COMPRESSED_SIZE >] {
return Err("Slice length not match");
}
let mut c = [0u8; [< $name _COMPRESSED_SIZE >]];
c.copy_from_slice(slice);
Ok(Self(c))
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the usecase for this function? I feel one can do this already by compressed.as_mut().copy_from_slice(slice), and copy_from_slice will panic if the length is uneuqal.

Copy link
Author

@chaosma chaosma May 24, 2023

Choose a reason for hiding this comment

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

In nova, the Group trait requires to define decompressed method to convert [u8;32] to G1Compressed. I didn't find there is a public method that allow me to do this conversion, so I added from_slice. What is compressed defined in this code?

use ff::{FromUniformBytes, PrimeField};
use static_assertions::const_assert;

// TODO: use simplified swu algorithm
Copy link
Contributor

Choose a reason for hiding this comment

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

So to use simplified swu method, we need to find the isogeny parameters right? Is there a deterministic script to do that? (this one might be it https://github.com/cfrg/draft-irtf-cfrg-hash-to-curve/blob/main/poc/iso_values.sage?)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we need first convert A=0 case to A*B != 0 curve before using simplified swu. en... Let me take a look at this code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a complete script to find Z and the SWU isogenies here as well: https://github.com/mratsim/constantine/blob/d996ccd/sage/derive_hash_to_curve.sage#L145-L275

@han0110 han0110 requested a review from kilic May 23, 2023 14:42
@kilic
Copy link
Collaborator

kilic commented Jun 5, 2023

is this PR overriden by #47?

@han0110
Copy link
Contributor

han0110 commented Jun 6, 2023

Yes I think we could replace this PR with #47, Chao also helped review #47 already.

@chaosma chaosma closed this Jun 21, 2023
@han0110 han0110 mentioned this pull request Jun 23, 2023
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.

4 participants