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

Rust bindings: add typical impls #12

Closed
paulhauner opened this issue Jul 18, 2020 · 4 comments
Closed

Rust bindings: add typical impls #12

paulhauner opened this issue Jul 18, 2020 · 4 comments

Comments

@paulhauner
Copy link
Contributor

Thanks again for this great library!

When implementing blst in Lighthouse (Eth2 impl) I noticed that the core structs (e.g., PublicKey, Signature, etc) are missing some typical and useful impls. In particular:

  • Clone
  • PartialEq
  • Eq (if it adheres to the definition here)

As you can see in our code, I've added some wrapper structs to Lighthouse and made my own custom implementations. It would be great if these were included in the library by default.

If the implementations I've used (see previous link) are good, I'm happy to make a PR to this repository. However I suspect that you might know faster or more elegant ways to achieve these things.

@paulhauner paulhauner changed the title Add typical impls for Rust library Rust bindings: Add typical impls Jul 19, 2020
@paulhauner paulhauner changed the title Rust bindings: Add typical impls Rust bindings: add typical impls Jul 19, 2020
@dot-asm
Copy link
Collaborator

dot-asm commented Jul 19, 2020

However I suspect that you might know faster or more elegant ways to achieve these things.

Don't be so sure:-) PR is welcomed!

@sean-sn
Copy link
Collaborator

sean-sn commented Nov 26, 2020

Closing the issue since the merged PR resolved it

@sean-sn sean-sn closed this as completed Nov 26, 2020
@tessico
Copy link

tessico commented Dec 7, 2022

Any reason why PartialEq was not added for SecretKey? Is this some security concern or just an omission?
I wonder what the best practice is, e.g. RustCrypto's signatures crates have explicit PartialEq impl for their dsa and ecdsa crates.

@dot-asm
Copy link
Collaborator

dot-asm commented Dec 7, 2022

I wonder what the best practice is, e.g. RustCrypto's signatures crates have explicit PartialEq impl for their dsa and ecdsa crates.

Formally speaking this is not exactly a right place to pop such a question. As we are not considering ourselves as a "Rust shop." We do exercise, and one might even say advocate for minimally-required principle though. Taking this question as an example. You are not supposed to use your secret key for anything but signing, so why would it require a comparison trait? One can make a legitimate case against attaching generic traits as they make no commitments to constant-time execution, while blst does...

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 a pull request may close this issue.

4 participants