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

remove sgn0_be, change sgn0 to output in {0,1} #230

Merged
merged 5 commits into from
Mar 29, 2020

Conversation

kwantam
Copy link
Collaborator

@kwantam kwantam commented Mar 27, 2020

This PR makes the changes we discussed in #225.

Can we hold off on merging this until we get comments from folks working to deploy BLS signatures?

Closes #225

@kwantam
Copy link
Collaborator Author

kwantam commented Mar 27, 2020

CC @JustinDrake @hoeteck @zhenfeizhang @sergeynog

For background, see #225. The goal is to significantly simplify the document by unifying the notion of "sign" for all curves. Unfortunately, this is a breaking change for BLS12-381, and we'd like feedback on how much of a killer this is.

@JustinDrake, I know I said that there were probably no more changes coming down the pipe. I'm sorry... 😖 (If it's any consolation, the change is extremely simple from an implementation point of view...)

@kwantam kwantam mentioned this pull request Mar 27, 2020
@kwantam kwantam requested review from armfazh and chris-wood and removed request for armfazh March 27, 2020 05:31
@JustinDrake
Copy link
Contributor

If it's a significant spec cleanup/simplification which is minimal for implementers then in theory it sounds good to me. Having said that I'll differ to @djrtwo who can provide much more informed feedback based on the development reality of Eth2 clients :)

cc @CarlBeek @kirk-baird @mratsim

@sergeynog
Copy link

sergeynog commented Mar 27, 2020 via email

@kirk-baird
Copy link
Contributor

From an implementation point of view I'm happy for removing sgn0_be and only using sgn0_le as it is simpler and faster (although the speed is negligible relative to the rest of hashing and pairing).

For Ethereum we still have not updated our specs to match hash-to-curve-06 and thus we are already introducing breaking changes so no issues on that front.

@chris-wood chris-wood merged commit 8b9c788 into cfrg:master Mar 29, 2020
@chris-wood
Copy link
Collaborator

Merged after hearing no pushback!

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.

feedback about sgn0
6 participants