Skip to content

Commit

Permalink
refactor(identity): follow naming conventions for conversion methods
Browse files Browse the repository at this point in the history
This PR renames some method names that don't follow Rust naming conventions or behave differently from what the name suggests:
- Enforce "try" prefix on all methods that return `Result`.
- Enforce "encode" method name for methods that return encoded bytes.
- Enforce "to_bytes" method name for methods that return raw bytes.
- Enforce "decode" method name for methods that convert encoded key.
- Enforce "from_bytes" method name for methods that convert raw bytes.

Pull-Request: libp2p#3775.
  • Loading branch information
drHuangMHT authored and tcoratger committed Apr 14, 2023
1 parent af2fd3e commit fc44e0a
Show file tree
Hide file tree
Showing 23 changed files with 490 additions and 109 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions core/src/signed_envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl SignedEnvelope {
use quick_protobuf::MessageWrite;

let envelope = proto::Envelope {
public_key: self.key.to_protobuf_encoding(),
public_key: self.key.encode_protobuf(),
payload_type: self.payload_type,
payload: self.payload,
signature: self.signature,
Expand All @@ -101,7 +101,7 @@ impl SignedEnvelope {
proto::Envelope::from_reader(&mut reader, bytes).map_err(DecodeError::from)?;

Ok(Self {
key: PublicKey::from_protobuf_encoding(&envelope.public_key)?,
key: PublicKey::try_decode_protobuf(&envelope.public_key)?,
payload_type: envelope.payload_type.to_vec(),
payload: envelope.payload.to_vec(),
signature: envelope.signature.to_vec(),
Expand Down
7 changes: 7 additions & 0 deletions identity/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 0.1.2 - unreleased

- Follow Rust naming conventions for conversion methods.
See [PR 3775].

[PR 3775]: https://github.com/libp2p/rust-libp2p/pull/3775

## 0.1.1

- Add `From` impl for specific keypairs.
Expand Down
2 changes: 1 addition & 1 deletion identity/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "libp2p-identity"
version = "0.1.1"
version = "0.1.2"
edition = "2021"
description = "Data structures and algorithms for identifying peers in libp2p."
rust-version = "1.60.0"
Expand Down
43 changes: 35 additions & 8 deletions identity/src/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use p256::{
};
use void::Void;

/// An ECDSA keypair.
/// An ECDSA keypair generated using `secp256r1` curve.
#[derive(Clone)]
pub struct Keypair {
secret: SecretKey,
Expand Down Expand Up @@ -85,7 +85,7 @@ impl From<Keypair> for SecretKey {
}
}

/// An ECDSA secret key.
/// An ECDSA secret key generated using `secp256r1` curve.
#[derive(Clone)]
pub struct SecretKey(SigningKey);

Expand All @@ -102,14 +102,23 @@ impl SecretKey {
signature.as_bytes().to_owned()
}

/// Encode a secret key into a byte buffer.
/// Convert a secret key into a byte buffer containing raw scalar of the key.
pub fn to_bytes(&self) -> Vec<u8> {
self.0.to_bytes().to_vec()
}

/// Decode a secret key from a byte buffer.
/// Decode a secret key from a byte buffer containing raw scalar of the key.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `SecretKey::try_from_bytes` instead"
)]
pub fn from_bytes(buf: &[u8]) -> Result<Self, DecodingError> {
SigningKey::from_bytes(buf)
Self::try_from_bytes(buf)
}

/// Try to parse a secret key from a byte buffer containing raw scalar of the key.
pub fn try_from_bytes(buf: impl AsRef<[u8]>) -> Result<SecretKey, DecodingError> {
SigningKey::from_bytes(buf.as_ref())
.map_err(|err| DecodingError::failed_to_parse("ecdsa p256 secret key", err))
.map(SecretKey)
}
Expand All @@ -135,8 +144,17 @@ impl PublicKey {
self.0.verify(msg, &sig).is_ok()
}

/// Decode a public key from a byte buffer without compression.
/// Decode a public key from a byte buffer containing raw components of a key with or without compression.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_from_bytes` instead."
)]
pub fn from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
Self::try_from_bytes(k)
}

/// Try to parse a public key from a byte buffer containing raw components of a key with or without compression.
pub fn try_from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
let enc_pt = EncodedPoint::from_bytes(k)
.map_err(|e| DecodingError::failed_to_parse("ecdsa p256 encoded point", e))?;

Expand All @@ -145,7 +163,7 @@ impl PublicKey {
.map(PublicKey)
}

/// Encode a public key into a byte buffer without compression.
/// Convert a public key into a byte buffer containing raw components of the key without compression.
pub fn to_bytes(&self) -> Vec<u8> {
self.0.to_encoded_point(false).as_bytes().to_owned()
}
Expand All @@ -157,11 +175,20 @@ impl PublicKey {
}

/// Decode a public key into a DER encoded byte buffer as defined by SEC1 standard.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_decode_der` instead."
)]
pub fn decode_der(k: &[u8]) -> Result<PublicKey, DecodingError> {
Self::try_decode_der(k)
}

/// Try to decode a public key from a DER encoded byte buffer as defined by SEC1 standard.
pub fn try_decode_der(k: &[u8]) -> Result<PublicKey, DecodingError> {
let buf = Self::del_asn1_header(k).ok_or_else(|| {
DecodingError::failed_to_parse::<Void, _>("ASN.1-encoded ecdsa p256 public key", None)
})?;
Self::from_bytes(buf)
Self::try_from_bytes(buf)
}

// ecPublicKey (ANSI X9.62 public key type) OID: 1.2.840.10045.2.1
Expand Down
69 changes: 61 additions & 8 deletions identity/src/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,35 @@ impl Keypair {
/// Encode the keypair into a byte array by concatenating the bytes
/// of the secret scalar and the compressed public point,
/// an informal standard for encoding Ed25519 keypairs.
#[deprecated(since = "0.2.0", note = "Renamed to `Keypair::to_bytes`")]
pub fn encode(&self) -> [u8; 64] {
self.to_bytes()
}

/// Convert the keypair into a byte array by concatenating the bytes
/// of the secret scalar and the compressed public point,
/// an informal standard for encoding Ed25519 keypairs.
pub fn to_bytes(&self) -> [u8; 64] {
self.0.to_bytes()
}

/// Decode a keypair from the [binary format](https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.5)
/// produced by [`Keypair::encode`], zeroing the input on success.
/// produced by [`Keypair::to_bytes`], zeroing the input on success.
///
/// Note that this binary format is the same as `ed25519_dalek`'s and `ed25519_zebra`'s.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `Keypair::try_from_bytes` instead."
)]
pub fn decode(kp: &mut [u8]) -> Result<Keypair, DecodingError> {
Self::try_from_bytes(kp)
}

/// Try to parse a keypair from the [binary format](https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.5)
/// produced by [`Keypair::to_bytes`], zeroing the input on success.
///
/// Note that this binary format is the same as `ed25519_dalek`'s and `ed25519_zebra`'s.
pub fn try_from_bytes(kp: &mut [u8]) -> Result<Keypair, DecodingError> {
ed25519::Keypair::from_bytes(kp)
.map(|k| {
kp.zeroize();
Expand All @@ -70,7 +90,7 @@ impl Keypair {

/// Get the secret key of this keypair.
pub fn secret(&self) -> SecretKey {
SecretKey::from_bytes(&mut self.0.secret.to_bytes())
SecretKey::try_from_bytes(&mut self.0.secret.to_bytes())
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
}
}
Expand All @@ -86,7 +106,7 @@ impl fmt::Debug for Keypair {
impl Clone for Keypair {
fn clone(&self) -> Keypair {
let mut sk_bytes = self.0.secret.to_bytes();
let secret = SecretKey::from_bytes(&mut sk_bytes)
let secret = SecretKey::try_from_bytes(&mut sk_bytes)
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
.0;

Expand Down Expand Up @@ -164,12 +184,31 @@ impl PublicKey {

/// Encode the public key into a byte array in compressed form, i.e.
/// where one coordinate is represented by a single bit.
#[deprecated(
since = "0.2.0",
note = "Renamed to `PublicKey::to_bytes` to reflect actual behaviour."
)]
pub fn encode(&self) -> [u8; 32] {
self.to_bytes()
}

/// Convert the public key to a byte array in compressed form, i.e.
/// where one coordinate is represented by a single bit.
pub fn to_bytes(&self) -> [u8; 32] {
self.0.to_bytes()
}

/// Decode a public key from a byte array as produced by `encode`.
/// Decode a public key from a byte array as produced by `to_bytes`.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_from_bytes` instead."
)]
pub fn decode(k: &[u8]) -> Result<PublicKey, DecodingError> {
Self::try_from_bytes(k)
}

/// Try to parse a public key from a byte array containing the actual key as produced by `to_bytes`.
pub fn try_from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
ed25519::PublicKey::from_bytes(k)
.map_err(|e| DecodingError::failed_to_parse("Ed25519 public key", e))
.map(PublicKey)
Expand All @@ -189,7 +228,8 @@ impl AsRef<[u8]> for SecretKey {
impl Clone for SecretKey {
fn clone(&self) -> SecretKey {
let mut sk_bytes = self.0.to_bytes();
Self::from_bytes(&mut sk_bytes).expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
Self::try_from_bytes(&mut sk_bytes)
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
}
}

Expand All @@ -214,7 +254,20 @@ impl SecretKey {
/// Create an Ed25519 secret key from a byte slice, zeroing the input on success.
/// If the bytes do not constitute a valid Ed25519 secret key, an error is
/// returned.
#[deprecated(
since = "0.2.0",
note = "This method name does not follow Rust naming conventions, use `SecretKey::try_from_bytes` instead."
)]
#[allow(unused_mut)]
pub fn from_bytes(mut sk_bytes: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
Self::try_from_bytes(sk_bytes)
}

/// Try to parse an Ed25519 secret key from a byte slice
/// containing the actual key, zeroing the input on success.
/// If the bytes do not constitute a valid Ed25519 secret key, an error is
/// returned.
pub fn try_from_bytes(mut sk_bytes: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
let sk_bytes = sk_bytes.as_mut();
let secret = ed25519::SecretKey::from_bytes(&*sk_bytes)
.map_err(|e| DecodingError::failed_to_parse("Ed25519 secret key", e))?;
Expand All @@ -236,8 +289,8 @@ mod tests {
fn ed25519_keypair_encode_decode() {
fn prop() -> bool {
let kp1 = Keypair::generate();
let mut kp1_enc = kp1.encode();
let kp2 = Keypair::decode(&mut kp1_enc).unwrap();
let mut kp1_enc = kp1.to_bytes();
let kp2 = Keypair::try_from_bytes(&mut kp1_enc).unwrap();
eq_keypairs(&kp1, &kp2) && kp1_enc.iter().all(|b| *b == 0)
}
QuickCheck::new().tests(10).quickcheck(prop as fn() -> _);
Expand All @@ -248,7 +301,7 @@ mod tests {
fn prop() -> bool {
let kp1 = Keypair::generate();
let mut sk = kp1.0.secret.to_bytes();
let kp2 = Keypair::from(SecretKey::from_bytes(&mut sk).unwrap());
let kp2 = Keypair::from(SecretKey::try_from_bytes(&mut sk).unwrap());
eq_keypairs(&kp1, &kp2) && sk == [0u8; 32]
}
QuickCheck::new().tests(10).quickcheck(prop as fn() -> _);
Expand Down
25 changes: 25 additions & 0 deletions identity/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
use std::error::Error;
use std::fmt;

use crate::KeyType;

/// An error during decoding of key material.
#[derive(Debug)]
pub struct DecodingError {
Expand Down Expand Up @@ -156,3 +158,26 @@ impl Error for SigningError {
self.source.as_ref().map(|s| &**s as &dyn Error)
}
}

/// Error produced when failing to convert [`Keypair`](crate::Keypair) to a more concrete keypair.
#[derive(Debug)]
pub struct OtherVariantError {
actual: KeyType,
}

impl OtherVariantError {
pub(crate) fn new(actual: KeyType) -> OtherVariantError {
OtherVariantError { actual }
}
}

impl fmt::Display for OtherVariantError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(&format!(
"Cannot convert to the given type, the actual key type inside is {}",
self.actual
))
}
}

impl Error for OtherVariantError {}
Loading

0 comments on commit fc44e0a

Please sign in to comment.