From 2f7e5f2506e629bc069f4c32c22d40aa3eaaa655 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 18 Apr 2024 16:15:32 -0500 Subject: [PATCH 1/3] Updates the emoji ID API to be more idiomatic --- .../minotari_app_utilities/src/utilities.rs | 12 ++-- .../src/automation/commands.rs | 2 +- .../src/commands/command/get_peer.rs | 2 +- base_layer/common_types/src/dammsum.rs | 4 +- base_layer/common_types/src/emoji.rs | 69 ++++++++++--------- base_layer/common_types/src/tari_address.rs | 6 +- 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/applications/minotari_app_utilities/src/utilities.rs b/applications/minotari_app_utilities/src/utilities.rs index b6daff88d2..45c42c6d19 100644 --- a/applications/minotari_app_utilities/src/utilities.rs +++ b/applications/minotari_app_utilities/src/utilities.rs @@ -47,8 +47,8 @@ pub fn setup_runtime() -> Result { /// Returns a CommsPublicKey from either a emoji id or a public key pub fn parse_emoji_id_or_public_key(key: &str) -> Option { - EmojiId::from_emoji_string(&key.trim().replace('|', "")) - .map(|emoji_id| emoji_id.to_public_key()) + EmojiId::try_from(key.trim().replace('|', "").as_str()) + .map(|emoji_id| PublicKey::from(&emoji_id)) .or_else(|_| CommsPublicKey::from_hex(key)) .ok() } @@ -79,8 +79,8 @@ impl FromStr for UniPublicKey { type Err = UniIdError; fn from_str(key: &str) -> Result { - if let Ok(emoji_id) = EmojiId::from_emoji_string(&key.trim().replace('|', "")) { - Ok(Self(emoji_id.to_public_key())) + if let Ok(emoji_id) = EmojiId::try_from(key.trim().replace('|', "").as_str()) { + Ok(Self(PublicKey::from(&emoji_id))) } else if let Ok(public_key) = PublicKey::from_hex(key) { Ok(Self(public_key)) } else if let Ok(tari_address) = TariAddress::from_hex(key) { @@ -116,8 +116,8 @@ impl FromStr for UniNodeId { type Err = UniIdError; fn from_str(key: &str) -> Result { - if let Ok(emoji_id) = EmojiId::from_emoji_string(&key.trim().replace('|', "")) { - Ok(Self::PublicKey(emoji_id.to_public_key())) + if let Ok(emoji_id) = EmojiId::try_from(key.trim().replace('|', "").as_str()) { + Ok(Self::PublicKey(PublicKey::from(&emoji_id))) } else if let Ok(public_key) = PublicKey::from_hex(key) { Ok(Self::PublicKey(public_key)) } else if let Ok(node_id) = NodeId::from_hex(key) { diff --git a/applications/minotari_console_wallet/src/automation/commands.rs b/applications/minotari_console_wallet/src/automation/commands.rs index aa520caee2..a912c612ad 100644 --- a/applications/minotari_console_wallet/src/automation/commands.rs +++ b/applications/minotari_console_wallet/src/automation/commands.rs @@ -796,7 +796,7 @@ pub async fn command_runner( }, Whois(args) => { let public_key = args.public_key.into(); - let emoji_id = EmojiId::from_public_key(&public_key).to_emoji_string(); + let emoji_id = EmojiId::from(&public_key).to_string(); println!("Public Key: {}", public_key.to_hex()); println!("Emoji ID : {}", emoji_id); diff --git a/applications/minotari_node/src/commands/command/get_peer.rs b/applications/minotari_node/src/commands/command/get_peer.rs index f9242244f9..a3b2cd342a 100644 --- a/applications/minotari_node/src/commands/command/get_peer.rs +++ b/applications/minotari_node/src/commands/command/get_peer.rs @@ -81,7 +81,7 @@ impl CommandContext { }; for peer in peers { - let eid = EmojiId::from_public_key(&peer.public_key).to_emoji_string(); + let eid = EmojiId::from(&peer.public_key).to_string(); println!("Emoji ID: {}", eid); println!("Public Key: {}", peer.public_key); println!("NodeId: {}", peer.node_id); diff --git a/base_layer/common_types/src/dammsum.rs b/base_layer/common_types/src/dammsum.rs index 959b11387a..3879b7d870 100644 --- a/base_layer/common_types/src/dammsum.rs +++ b/base_layer/common_types/src/dammsum.rs @@ -46,7 +46,7 @@ pub enum ChecksumError { const COEFFICIENTS: [u8; 3] = [4, 3, 1]; /// Compute the DammSum checksum for an array, each of whose elements are in the range `[0, 2^8)` -pub fn compute_checksum(data: &Vec) -> u8 { +pub fn compute_checksum(data: &[u8]) -> u8 { let mut mask = 1u8; // Compute the bitmask (if possible) @@ -71,7 +71,7 @@ pub fn compute_checksum(data: &Vec) -> u8 { } /// Determine whether the array ends with a valid checksum -pub fn validate_checksum(data: &Vec) -> Result<(), ChecksumError> { +pub fn validate_checksum(data: &[u8]) -> Result<(), ChecksumError> { // Empty data is not allowed, nor data only consisting of a checksum if data.len() < 2 { return Err(ChecksumError::InputDataTooShort); diff --git a/base_layer/common_types/src/emoji.rs b/base_layer/common_types/src/emoji.rs index 403d6d1801..8544e272c7 100644 --- a/base_layer/common_types/src/emoji.rs +++ b/base_layer/common_types/src/emoji.rs @@ -121,16 +121,24 @@ pub enum EmojiIdError { } impl EmojiId { - /// Construct an emoji ID from an emoji string with checksum - pub fn from_emoji_string(emoji: &str) -> Result { + /// Get the public key from an emoji ID + pub fn as_public_key(&self) -> &PublicKey { + &self.0 + } +} + +impl TryFrom<&str> for EmojiId { + type Error = EmojiIdError; + + fn try_from(value: &str) -> Result { // The string must be the correct size, including the checksum - if emoji.chars().count() != INTERNAL_SIZE + CHECKSUM_SIZE { + if value.chars().count() != INTERNAL_SIZE + CHECKSUM_SIZE { return Err(EmojiIdError::InvalidSize); } // Convert the emoji string to a byte array let mut bytes = Vec::::with_capacity(INTERNAL_SIZE + CHECKSUM_SIZE); - for c in emoji.chars() { + for c in value.chars() { if let Some(i) = REVERSE_EMOJI.get(&c) { bytes.push(*i); } else { @@ -152,38 +160,41 @@ impl EmojiId { Err(_) => Err(EmojiIdError::CannotRecoverPublicKey), } } +} - /// Construct an emoji ID from a public key - pub fn from_public_key(public_key: &PublicKey) -> Self { - Self(public_key.clone()) +impl From<&PublicKey> for EmojiId { + fn from(value: &PublicKey) -> Self { + Self(value.clone()) } +} - /// Convert the emoji ID to an emoji string with checksum - pub fn to_emoji_string(&self) -> String { +impl From<&EmojiId> for String { + fn from(value: &EmojiId) -> Self { // Convert the public key to bytes and compute the checksum - let bytes = self.0.as_bytes().to_vec(); + let bytes = value.as_public_key().as_bytes(); bytes .iter() - .chain(iter::once(&compute_checksum(&bytes))) + .chain(iter::once(&compute_checksum(bytes))) .map(|b| EMOJI[*b as usize]) .collect::() } +} - /// Convert the emoji ID to a public key - pub fn to_public_key(&self) -> PublicKey { - self.0.clone() +impl From<&EmojiId> for PublicKey { + fn from(value: &EmojiId) -> Self { + value.as_public_key().clone() } } impl Display for EmojiId { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> { - fmt.write_str(&self.to_emoji_string()) + fmt.write_str(&String::from(self)) } } #[cfg(test)] mod test { - use std::iter; + use std::{convert::TryFrom, iter}; use tari_crypto::keys::{PublicKey as PublicKeyTrait, SecretKey}; @@ -201,19 +212,19 @@ mod test { let public_key = PublicKey::from_secret_key(&PrivateKey::random(&mut rng)); // Generate an emoji ID from the public key and ensure we recover it - let emoji_id_from_public_key = EmojiId::from_public_key(&public_key); - assert_eq!(emoji_id_from_public_key.to_public_key(), public_key); + let emoji_id_from_public_key = EmojiId::from(&public_key); + assert_eq!(emoji_id_from_public_key.as_public_key(), &public_key); // Check the size of the corresponding emoji string - let emoji_string = emoji_id_from_public_key.to_emoji_string(); + let emoji_string = emoji_id_from_public_key.to_string(); assert_eq!(emoji_string.chars().count(), INTERNAL_SIZE + CHECKSUM_SIZE); // Generate an emoji ID from the emoji string and ensure we recover it - let emoji_id_from_emoji_string = EmojiId::from_emoji_string(&emoji_string).unwrap(); - assert_eq!(emoji_id_from_emoji_string.to_emoji_string(), emoji_string); + let emoji_id_from_emoji_string = EmojiId::try_from(emoji_string.as_str()).unwrap(); + assert_eq!(emoji_id_from_emoji_string.to_string(), emoji_string); // Return to the original public key for good measure - assert_eq!(emoji_id_from_emoji_string.to_public_key(), public_key); + assert_eq!(emoji_id_from_emoji_string.as_public_key(), &public_key); } #[test] @@ -221,7 +232,7 @@ mod test { fn invalid_size() { // This emoji string is too short to be a valid emoji ID let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’"; - assert_eq!(EmojiId::from_emoji_string(emoji_string), Err(EmojiIdError::InvalidSize)); + assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidSize)); } #[test] @@ -229,10 +240,7 @@ mod test { fn invalid_emoji() { // This emoji string contains an invalid emoji character let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’πŸŽ…"; - assert_eq!( - EmojiId::from_emoji_string(emoji_string), - Err(EmojiIdError::InvalidEmoji) - ); + assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidEmoji)); } #[test] @@ -240,10 +248,7 @@ mod test { fn invalid_checksum() { // This emoji string contains an invalid checksum let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’πŸŽ’"; - assert_eq!( - EmojiId::from_emoji_string(emoji_string), - Err(EmojiIdError::InvalidChecksum) - ); + assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidChecksum)); } #[test] @@ -262,7 +267,7 @@ mod test { .collect::(); assert_eq!( - EmojiId::from_emoji_string(&emoji_string), + EmojiId::try_from(emoji_string.as_str()), Err(EmojiIdError::CannotRecoverPublicKey) ); } diff --git a/base_layer/common_types/src/tari_address.rs b/base_layer/common_types/src/tari_address.rs index b84403f9eb..78ef770901 100644 --- a/base_layer/common_types/src/tari_address.rs +++ b/base_layer/common_types/src/tari_address.rs @@ -148,7 +148,7 @@ impl TariAddress { if bytes.len() != INTERNAL_SIZE { return Err(TariAddressError::InvalidSize); } - let checksum = compute_checksum(&bytes[0..32].to_vec()); + let checksum = compute_checksum(&bytes[0..32]); // if the network is a valid network number, we can assume that the checksum as valid let network = Network::try_from(checksum ^ bytes[32]).map_err(|_| TariAddressError::InvalidNetworkOrChecksum)?; @@ -164,7 +164,7 @@ impl TariAddress { pub fn to_bytes(&self) -> [u8; INTERNAL_SIZE] { let mut buf = [0u8; INTERNAL_SIZE]; buf[0..32].copy_from_slice(self.public_key.as_bytes()); - let checksum = compute_checksum(&buf[0..32].to_vec()); + let checksum = compute_checksum(&buf[0..32]); buf[32] = self.network.as_byte() ^ checksum; buf } @@ -330,7 +330,7 @@ mod test { fn invalid_public_key() { let mut bytes = [0; 33].to_vec(); bytes[0] = 1; - let checksum = compute_checksum(&bytes[0..32].to_vec()); + let checksum = compute_checksum(&bytes[0..32]); bytes[32] = Network::Esmeralda.as_byte() ^ checksum; let emoji_string = bytes.iter().map(|b| EMOJI[*b as usize]).collect::(); From ae0cbd08dc3df3e1278cc43885f4a9b2e8b33bb7 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Fri, 19 Apr 2024 09:28:34 -0500 Subject: [PATCH 2/3] Review comments --- .../minotari_app_utilities/src/utilities.rs | 6 +-- base_layer/common_types/src/emoji.rs | 47 ++++++++++--------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/applications/minotari_app_utilities/src/utilities.rs b/applications/minotari_app_utilities/src/utilities.rs index 45c42c6d19..9201a17a5e 100644 --- a/applications/minotari_app_utilities/src/utilities.rs +++ b/applications/minotari_app_utilities/src/utilities.rs @@ -47,7 +47,7 @@ pub fn setup_runtime() -> Result { /// Returns a CommsPublicKey from either a emoji id or a public key pub fn parse_emoji_id_or_public_key(key: &str) -> Option { - EmojiId::try_from(key.trim().replace('|', "").as_str()) + EmojiId::from_str(&key.trim().replace('|', "")) .map(|emoji_id| PublicKey::from(&emoji_id)) .or_else(|_| CommsPublicKey::from_hex(key)) .ok() @@ -79,7 +79,7 @@ impl FromStr for UniPublicKey { type Err = UniIdError; fn from_str(key: &str) -> Result { - if let Ok(emoji_id) = EmojiId::try_from(key.trim().replace('|', "").as_str()) { + if let Ok(emoji_id) = EmojiId::from_str(&key.trim().replace('|', "")) { Ok(Self(PublicKey::from(&emoji_id))) } else if let Ok(public_key) = PublicKey::from_hex(key) { Ok(Self(public_key)) @@ -116,7 +116,7 @@ impl FromStr for UniNodeId { type Err = UniIdError; fn from_str(key: &str) -> Result { - if let Ok(emoji_id) = EmojiId::try_from(key.trim().replace('|', "").as_str()) { + if let Ok(emoji_id) = EmojiId::from_str(&key.trim().replace('|', "")) { Ok(Self::PublicKey(PublicKey::from(&emoji_id))) } else if let Ok(public_key) = PublicKey::from_hex(key) { Ok(Self::PublicKey(public_key)) diff --git a/base_layer/common_types/src/emoji.rs b/base_layer/common_types/src/emoji.rs index 8544e272c7..926330dbca 100644 --- a/base_layer/common_types/src/emoji.rs +++ b/base_layer/common_types/src/emoji.rs @@ -25,6 +25,7 @@ use std::{ convert::TryFrom, fmt::{Display, Error, Formatter}, iter, + str::FromStr, }; use once_cell::sync::Lazy; @@ -127,18 +128,18 @@ impl EmojiId { } } -impl TryFrom<&str> for EmojiId { - type Error = EmojiIdError; +impl FromStr for EmojiId { + type Err = EmojiIdError; - fn try_from(value: &str) -> Result { + fn from_str(s: &str) -> Result { // The string must be the correct size, including the checksum - if value.chars().count() != INTERNAL_SIZE + CHECKSUM_SIZE { + if s.chars().count() != INTERNAL_SIZE + CHECKSUM_SIZE { return Err(EmojiIdError::InvalidSize); } // Convert the emoji string to a byte array let mut bytes = Vec::::with_capacity(INTERNAL_SIZE + CHECKSUM_SIZE); - for c in value.chars() { + for c in s.chars() { if let Some(i) = REVERSE_EMOJI.get(&c) { bytes.push(*i); } else { @@ -164,19 +165,13 @@ impl TryFrom<&str> for EmojiId { impl From<&PublicKey> for EmojiId { fn from(value: &PublicKey) -> Self { - Self(value.clone()) + Self::from(value.clone()) } } -impl From<&EmojiId> for String { - fn from(value: &EmojiId) -> Self { - // Convert the public key to bytes and compute the checksum - let bytes = value.as_public_key().as_bytes(); - bytes - .iter() - .chain(iter::once(&compute_checksum(bytes))) - .map(|b| EMOJI[*b as usize]) - .collect::() +impl From for EmojiId { + fn from(value: PublicKey) -> Self { + Self(value) } } @@ -188,13 +183,21 @@ impl From<&EmojiId> for PublicKey { impl Display for EmojiId { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> { - fmt.write_str(&String::from(self)) + // Convert the public key to bytes and compute the checksum + let bytes = self.as_public_key().as_bytes(); + let emoji = bytes + .iter() + .chain(iter::once(&compute_checksum(bytes))) + .map(|b| EMOJI[*b as usize]) + .collect::(); + + fmt.write_str(&emoji) } } #[cfg(test)] mod test { - use std::{convert::TryFrom, iter}; + use std::{iter, str::FromStr}; use tari_crypto::keys::{PublicKey as PublicKeyTrait, SecretKey}; @@ -220,7 +223,7 @@ mod test { assert_eq!(emoji_string.chars().count(), INTERNAL_SIZE + CHECKSUM_SIZE); // Generate an emoji ID from the emoji string and ensure we recover it - let emoji_id_from_emoji_string = EmojiId::try_from(emoji_string.as_str()).unwrap(); + let emoji_id_from_emoji_string = EmojiId::from_str(&emoji_string).unwrap(); assert_eq!(emoji_id_from_emoji_string.to_string(), emoji_string); // Return to the original public key for good measure @@ -232,7 +235,7 @@ mod test { fn invalid_size() { // This emoji string is too short to be a valid emoji ID let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’"; - assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidSize)); + assert_eq!(EmojiId::from_str(emoji_string), Err(EmojiIdError::InvalidSize)); } #[test] @@ -240,7 +243,7 @@ mod test { fn invalid_emoji() { // This emoji string contains an invalid emoji character let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’πŸŽ…"; - assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidEmoji)); + assert_eq!(EmojiId::from_str(emoji_string), Err(EmojiIdError::InvalidEmoji)); } #[test] @@ -248,7 +251,7 @@ mod test { fn invalid_checksum() { // This emoji string contains an invalid checksum let emoji_string = "πŸŒ΄πŸ©πŸ”ŒπŸ“ŒπŸš‘πŸŒ°πŸŽ“πŸŒ΄πŸŠπŸŒπŸ’•πŸ’‘πŸœπŸ“‰πŸ‘›πŸ΅πŸ‘›πŸ½πŸŽ‚πŸ»πŸŒ€πŸ“πŸ˜ΏπŸ­πŸΌπŸ€πŸŽͺπŸ’”πŸ’ΈπŸ…πŸ”‹πŸŽ’πŸŽ’"; - assert_eq!(EmojiId::try_from(emoji_string), Err(EmojiIdError::InvalidChecksum)); + assert_eq!(EmojiId::from_str(emoji_string), Err(EmojiIdError::InvalidChecksum)); } #[test] @@ -267,7 +270,7 @@ mod test { .collect::(); assert_eq!( - EmojiId::try_from(emoji_string.as_str()), + EmojiId::from_str(&emoji_string), Err(EmojiIdError::CannotRecoverPublicKey) ); } From 6762f9e6ab2e78795dc34d744a9f8cfeea345e1f Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Fri, 19 Apr 2024 10:37:14 -0500 Subject: [PATCH 3/3] Update invalid public key test --- base_layer/common_types/src/emoji.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/base_layer/common_types/src/emoji.rs b/base_layer/common_types/src/emoji.rs index 926330dbca..ac54a95390 100644 --- a/base_layer/common_types/src/emoji.rs +++ b/base_layer/common_types/src/emoji.rs @@ -199,7 +199,10 @@ impl Display for EmojiId { mod test { use std::{iter, str::FromStr}; - use tari_crypto::keys::{PublicKey as PublicKeyTrait, SecretKey}; + use tari_crypto::{ + keys::{PublicKey as PublicKeyTrait, SecretKey}, + tari_utilities::ByteArray, + }; use crate::{ dammsum::compute_checksum, @@ -260,6 +263,7 @@ mod test { // This byte representation does not represent a valid public key let mut bytes = vec![0u8; INTERNAL_SIZE]; bytes[0] = 1; + assert!(PublicKey::from_canonical_bytes(&bytes).is_err()); // Convert to an emoji string and manually add a valid checksum let emoji_set = emoji_set();