-
Notifications
You must be signed in to change notification settings - Fork 30
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
[WIP] VXEdDSA #167
base: master
Are you sure you want to change the base?
[WIP] VXEdDSA #167
Conversation
5dd17d8
to
509c439
Compare
…needs some cleanup: - [ ] increase code readability (and more golang idiomatic) - [ ] use cached public-key - [ ] cleanup ed25519 package (unexport funcs, keep only one elligator2 implementation, test-vectors for elligator) - [ ] adopt to google's keytransparency vrf interface (?) - [ ] asserts - [ ] TODOs, XXXs
3813426
to
95a8fd1
Compare
ErrGetPubKey = errors.New("[vrf] Couldn't get corresponding public-key from private-key") | ||
) | ||
// PrivateKey represents a Curve25519 private key. | ||
type PrivateKey [PrivateKeySize]byte |
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.
Could we keep using the old bytes slice type PrivateKey []byte
instead?
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.
Yes, thanks. It makes sense to keep independent from the actual implementation/key-size used.
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.
Cool!
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.
On the long run (or maybe even in this pull?) we should switch to using an interface which completely hides the underlying representation (similar to this: https://github.com/google/keytransparency/blob/master/core/vrf/vrf.go)
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.
Also the signer. It is tracked in #50.
Hey guys, This looks like great work. Any plans to merge this ? |
Hi, I'm afraid that we don't have any plan to merge this soon. In the meantime, you can merge it on your own fork if you want ;) |
Thanks vqhuy. May I ask why there are no plans to merge this ? Do you need a developer to help clean this up before it is merged ? |
@liamsi Thanks for the awesome work. I'd like to take this forward. Please can you elaborate a bit on these points of your checklist
Also, would you be willing to review this once it is done ? |
Unfortunately, I do not remember off the top of my head what those points on the checklist mean in detail. I'd need to look into the code (this implementation and the whispersystem C code) to understand what I was planning 🙈
Yes, of course! I'd be happy to review this! |
resolves #128
working version; still needs some cleanup: