Skip to content

Commit 058c2d8

Browse files
authored
refactor(identity): follow naming conventions for conversion methods
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: #3775.
1 parent 8ffcff9 commit 058c2d8

File tree

23 files changed

+490
-109
lines changed

23 files changed

+490
-109
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/src/signed_envelope.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl SignedEnvelope {
7676
use quick_protobuf::MessageWrite;
7777

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

103103
Ok(Self {
104-
key: PublicKey::from_protobuf_encoding(&envelope.public_key)?,
104+
key: PublicKey::try_decode_protobuf(&envelope.public_key)?,
105105
payload_type: envelope.payload_type.to_vec(),
106106
payload: envelope.payload.to_vec(),
107107
signature: envelope.signature.to_vec(),

identity/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
## 0.1.2 - unreleased
2+
3+
- Follow Rust naming conventions for conversion methods.
4+
See [PR 3775].
5+
6+
[PR 3775]: https://github.com/libp2p/rust-libp2p/pull/3775
7+
18
## 0.1.1
29

310
- Add `From` impl for specific keypairs.

identity/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "libp2p-identity"
3-
version = "0.1.1"
3+
version = "0.1.2"
44
edition = "2021"
55
description = "Data structures and algorithms for identifying peers in libp2p."
66
rust-version = "1.60.0"

identity/src/ecdsa.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use p256::{
3333
};
3434
use void::Void;
3535

36-
/// An ECDSA keypair.
36+
/// An ECDSA keypair generated using `secp256r1` curve.
3737
#[derive(Clone)]
3838
pub struct Keypair {
3939
secret: SecretKey,
@@ -85,7 +85,7 @@ impl From<Keypair> for SecretKey {
8585
}
8686
}
8787

88-
/// An ECDSA secret key.
88+
/// An ECDSA secret key generated using `secp256r1` curve.
8989
#[derive(Clone)]
9090
pub struct SecretKey(SigningKey);
9191

@@ -102,14 +102,23 @@ impl SecretKey {
102102
signature.as_bytes().to_owned()
103103
}
104104

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

110-
/// Decode a secret key from a byte buffer.
110+
/// Decode a secret key from a byte buffer containing raw scalar of the key.
111+
#[deprecated(
112+
since = "0.2.0",
113+
note = "This method name does not follow Rust naming conventions, use `SecretKey::try_from_bytes` instead"
114+
)]
111115
pub fn from_bytes(buf: &[u8]) -> Result<Self, DecodingError> {
112-
SigningKey::from_bytes(buf)
116+
Self::try_from_bytes(buf)
117+
}
118+
119+
/// Try to parse a secret key from a byte buffer containing raw scalar of the key.
120+
pub fn try_from_bytes(buf: impl AsRef<[u8]>) -> Result<SecretKey, DecodingError> {
121+
SigningKey::from_bytes(buf.as_ref())
113122
.map_err(|err| DecodingError::failed_to_parse("ecdsa p256 secret key", err))
114123
.map(SecretKey)
115124
}
@@ -135,8 +144,17 @@ impl PublicKey {
135144
self.0.verify(msg, &sig).is_ok()
136145
}
137146

138-
/// Decode a public key from a byte buffer without compression.
147+
/// Decode a public key from a byte buffer containing raw components of a key with or without compression.
148+
#[deprecated(
149+
since = "0.2.0",
150+
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_from_bytes` instead."
151+
)]
139152
pub fn from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
153+
Self::try_from_bytes(k)
154+
}
155+
156+
/// Try to parse a public key from a byte buffer containing raw components of a key with or without compression.
157+
pub fn try_from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
140158
let enc_pt = EncodedPoint::from_bytes(k)
141159
.map_err(|e| DecodingError::failed_to_parse("ecdsa p256 encoded point", e))?;
142160

@@ -145,7 +163,7 @@ impl PublicKey {
145163
.map(PublicKey)
146164
}
147165

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

159177
/// Decode a public key into a DER encoded byte buffer as defined by SEC1 standard.
178+
#[deprecated(
179+
since = "0.2.0",
180+
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_decode_der` instead."
181+
)]
160182
pub fn decode_der(k: &[u8]) -> Result<PublicKey, DecodingError> {
183+
Self::try_decode_der(k)
184+
}
185+
186+
/// Try to decode a public key from a DER encoded byte buffer as defined by SEC1 standard.
187+
pub fn try_decode_der(k: &[u8]) -> Result<PublicKey, DecodingError> {
161188
let buf = Self::del_asn1_header(k).ok_or_else(|| {
162189
DecodingError::failed_to_parse::<Void, _>("ASN.1-encoded ecdsa p256 public key", None)
163190
})?;
164-
Self::from_bytes(buf)
191+
Self::try_from_bytes(buf)
165192
}
166193

167194
// ecPublicKey (ANSI X9.62 public key type) OID: 1.2.840.10045.2.1

identity/src/ed25519.rs

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,35 @@ impl Keypair {
4141
/// Encode the keypair into a byte array by concatenating the bytes
4242
/// of the secret scalar and the compressed public point,
4343
/// an informal standard for encoding Ed25519 keypairs.
44+
#[deprecated(since = "0.2.0", note = "Renamed to `Keypair::to_bytes`")]
4445
pub fn encode(&self) -> [u8; 64] {
46+
self.to_bytes()
47+
}
48+
49+
/// Convert the keypair into a byte array by concatenating the bytes
50+
/// of the secret scalar and the compressed public point,
51+
/// an informal standard for encoding Ed25519 keypairs.
52+
pub fn to_bytes(&self) -> [u8; 64] {
4553
self.0.to_bytes()
4654
}
4755

4856
/// Decode a keypair from the [binary format](https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.5)
49-
/// produced by [`Keypair::encode`], zeroing the input on success.
57+
/// produced by [`Keypair::to_bytes`], zeroing the input on success.
5058
///
5159
/// Note that this binary format is the same as `ed25519_dalek`'s and `ed25519_zebra`'s.
60+
#[deprecated(
61+
since = "0.2.0",
62+
note = "This method name does not follow Rust naming conventions, use `Keypair::try_from_bytes` instead."
63+
)]
5264
pub fn decode(kp: &mut [u8]) -> Result<Keypair, DecodingError> {
65+
Self::try_from_bytes(kp)
66+
}
67+
68+
/// Try to parse a keypair from the [binary format](https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.5)
69+
/// produced by [`Keypair::to_bytes`], zeroing the input on success.
70+
///
71+
/// Note that this binary format is the same as `ed25519_dalek`'s and `ed25519_zebra`'s.
72+
pub fn try_from_bytes(kp: &mut [u8]) -> Result<Keypair, DecodingError> {
5373
ed25519::Keypair::from_bytes(kp)
5474
.map(|k| {
5575
kp.zeroize();
@@ -70,7 +90,7 @@ impl Keypair {
7090

7191
/// Get the secret key of this keypair.
7292
pub fn secret(&self) -> SecretKey {
73-
SecretKey::from_bytes(&mut self.0.secret.to_bytes())
93+
SecretKey::try_from_bytes(&mut self.0.secret.to_bytes())
7494
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
7595
}
7696
}
@@ -86,7 +106,7 @@ impl fmt::Debug for Keypair {
86106
impl Clone for Keypair {
87107
fn clone(&self) -> Keypair {
88108
let mut sk_bytes = self.0.secret.to_bytes();
89-
let secret = SecretKey::from_bytes(&mut sk_bytes)
109+
let secret = SecretKey::try_from_bytes(&mut sk_bytes)
90110
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
91111
.0;
92112

@@ -164,12 +184,31 @@ impl PublicKey {
164184

165185
/// Encode the public key into a byte array in compressed form, i.e.
166186
/// where one coordinate is represented by a single bit.
187+
#[deprecated(
188+
since = "0.2.0",
189+
note = "Renamed to `PublicKey::to_bytes` to reflect actual behaviour."
190+
)]
167191
pub fn encode(&self) -> [u8; 32] {
192+
self.to_bytes()
193+
}
194+
195+
/// Convert the public key to a byte array in compressed form, i.e.
196+
/// where one coordinate is represented by a single bit.
197+
pub fn to_bytes(&self) -> [u8; 32] {
168198
self.0.to_bytes()
169199
}
170200

171-
/// Decode a public key from a byte array as produced by `encode`.
201+
/// Decode a public key from a byte array as produced by `to_bytes`.
202+
#[deprecated(
203+
since = "0.2.0",
204+
note = "This method name does not follow Rust naming conventions, use `PublicKey::try_from_bytes` instead."
205+
)]
172206
pub fn decode(k: &[u8]) -> Result<PublicKey, DecodingError> {
207+
Self::try_from_bytes(k)
208+
}
209+
210+
/// Try to parse a public key from a byte array containing the actual key as produced by `to_bytes`.
211+
pub fn try_from_bytes(k: &[u8]) -> Result<PublicKey, DecodingError> {
173212
ed25519::PublicKey::from_bytes(k)
174213
.map_err(|e| DecodingError::failed_to_parse("Ed25519 public key", e))
175214
.map(PublicKey)
@@ -189,7 +228,8 @@ impl AsRef<[u8]> for SecretKey {
189228
impl Clone for SecretKey {
190229
fn clone(&self) -> SecretKey {
191230
let mut sk_bytes = self.0.to_bytes();
192-
Self::from_bytes(&mut sk_bytes).expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
231+
Self::try_from_bytes(&mut sk_bytes)
232+
.expect("ed25519::SecretKey::from_bytes(to_bytes(k)) != k")
193233
}
194234
}
195235

@@ -214,7 +254,20 @@ impl SecretKey {
214254
/// Create an Ed25519 secret key from a byte slice, zeroing the input on success.
215255
/// If the bytes do not constitute a valid Ed25519 secret key, an error is
216256
/// returned.
257+
#[deprecated(
258+
since = "0.2.0",
259+
note = "This method name does not follow Rust naming conventions, use `SecretKey::try_from_bytes` instead."
260+
)]
261+
#[allow(unused_mut)]
217262
pub fn from_bytes(mut sk_bytes: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
263+
Self::try_from_bytes(sk_bytes)
264+
}
265+
266+
/// Try to parse an Ed25519 secret key from a byte slice
267+
/// containing the actual key, zeroing the input on success.
268+
/// If the bytes do not constitute a valid Ed25519 secret key, an error is
269+
/// returned.
270+
pub fn try_from_bytes(mut sk_bytes: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
218271
let sk_bytes = sk_bytes.as_mut();
219272
let secret = ed25519::SecretKey::from_bytes(&*sk_bytes)
220273
.map_err(|e| DecodingError::failed_to_parse("Ed25519 secret key", e))?;
@@ -236,8 +289,8 @@ mod tests {
236289
fn ed25519_keypair_encode_decode() {
237290
fn prop() -> bool {
238291
let kp1 = Keypair::generate();
239-
let mut kp1_enc = kp1.encode();
240-
let kp2 = Keypair::decode(&mut kp1_enc).unwrap();
292+
let mut kp1_enc = kp1.to_bytes();
293+
let kp2 = Keypair::try_from_bytes(&mut kp1_enc).unwrap();
241294
eq_keypairs(&kp1, &kp2) && kp1_enc.iter().all(|b| *b == 0)
242295
}
243296
QuickCheck::new().tests(10).quickcheck(prop as fn() -> _);
@@ -248,7 +301,7 @@ mod tests {
248301
fn prop() -> bool {
249302
let kp1 = Keypair::generate();
250303
let mut sk = kp1.0.secret.to_bytes();
251-
let kp2 = Keypair::from(SecretKey::from_bytes(&mut sk).unwrap());
304+
let kp2 = Keypair::from(SecretKey::try_from_bytes(&mut sk).unwrap());
252305
eq_keypairs(&kp1, &kp2) && sk == [0u8; 32]
253306
}
254307
QuickCheck::new().tests(10).quickcheck(prop as fn() -> _);

identity/src/error.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
use std::error::Error;
2424
use std::fmt;
2525

26+
use crate::KeyType;
27+
2628
/// An error during decoding of key material.
2729
#[derive(Debug)]
2830
pub struct DecodingError {
@@ -156,3 +158,26 @@ impl Error for SigningError {
156158
self.source.as_ref().map(|s| &**s as &dyn Error)
157159
}
158160
}
161+
162+
/// Error produced when failing to convert [`Keypair`](crate::Keypair) to a more concrete keypair.
163+
#[derive(Debug)]
164+
pub struct OtherVariantError {
165+
actual: KeyType,
166+
}
167+
168+
impl OtherVariantError {
169+
pub(crate) fn new(actual: KeyType) -> OtherVariantError {
170+
OtherVariantError { actual }
171+
}
172+
}
173+
174+
impl fmt::Display for OtherVariantError {
175+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
176+
f.write_str(&format!(
177+
"Cannot convert to the given type, the actual key type inside is {}",
178+
self.actual
179+
))
180+
}
181+
}
182+
183+
impl Error for OtherVariantError {}

0 commit comments

Comments
 (0)