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

Move Parse{Public|Private}Key and MarshalToPEM functions to their respective crypto package #122

Open
meling opened this issue Mar 10, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@meling
Copy link
Member

meling commented Mar 10, 2024

We have functions for parsing from []byte to key and marshaling key to PEM in the keygen package.

// ParsePrivateKey parses a PEM encoded private key.
func ParsePrivateKey(buf []byte) (key hotstuff.PrivateKey, err error)
// ParsePublicKey parses a PEM encoded public key
func ParsePublicKey(buf []byte) (key hotstuff.PublicKey, err error) 

// PrivateKeyToPEM encodes the private key in PEM format.
func PrivateKeyToPEM(key hotstuff.PrivateKey) ([]byte, error)
// PublicKeyToPEM encodes the public key in PEM format.
func PublicKeyToPEM(key hotstuff.PublicKey) ([]byte, error)

However, these belong in their respective packages {ecdsa, eddsa, bls12}, since we must add code to these methods whenever we add a new crypto scheme and expose parsing details outside the package, which could be kept internal to the specific package.

That is, we should add these to the different crypto packages:

  • hotstuff.{Private|Public}Key should require key types to provide a MarshalToPEM() ([]byte, error) method.
  • ParsePublicKey(buf []byte) (key hotstuff.PublicKey, err error)
  • ParsePrivateKey(buf []byte) (key hotstuff.PrivateKey, err error)

Have I've overlooked something that makes this approach less desirable?

@meling meling added the enhancement New feature or request label Mar 10, 2024
@meling
Copy link
Member Author

meling commented Mar 11, 2024

This rewrite should add tests to check that the marshaled keys and be unmarshaled with the corresponding Parse funcs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant