-
Notifications
You must be signed in to change notification settings - Fork 83
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
Switches hash-to-field to new v06 hash system #87
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for keeping following the standardization updates, looks good 👍
Just some improvement suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Let me know if you think it's ready to merge.
@CarlBeek I pushed the fix into this branch, all the tests pass now. |
Thanks for pushing the changes. I'll check my local flake8/mypy config. Good to merge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evidently I didn't submit this comment... Probably a nice improvement to exception message if you find yourself back in this file.
raise ValueError('DST must be <= 255 bytes') | ||
ell = math.ceil(len_in_bytes / b_in_bytes) | ||
if ell > 255: | ||
raise ValueError('invalid len in bytes for hash function') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to include both the value of ell
and the limit value in this message.
What was wrong?
The BLS hash-to-curve spec is being changed from v05 -> v06. BLS hash-to-curve It is basically a change from repeated HKDF calls to direct SHA256 calls. This has the advantage of halving the number of SHA256 calls (and allows for other improvements for other curves in the hash-to-curve suite). See here for the full discussion: cfrg/draft-irtf-cfrg-hash-to-curve#202
The changes made are summarised in PRs 212 (specs update), 214 (proof of concept implementation) & 217 (test vectors).
How was it fixed?
The new specs & tests were implemented.
Please do not merge. This PR is for visibility for now until V06 is officially released.
To do:
Cute Animal Picture