Skip to content

Commit

Permalink
feat: updates the emoji ID API to be more idiomatic (#6287)
Browse files Browse the repository at this point in the history
Description
---
Updates the emoji ID public API to be more idiomatic.

Motivation and Context
---
The emoji ID public API includes functionality to convert to and from
types like `String` and `PublicKey`, but not idiomatically. This PR
updates the API to use `From` and `FromStr` trait implementations where
possible. It also makes some minor changes to checksum function
signatures to avoid unnecessary vector cloning, and improves a test.

How Has This Been Tested?
---
Existing tests pass.

What process can a PR reviewer use to test or verify this change?
---
Check that the new API is, in fact, more idiomatic :)

While this doesn't affect consensus or existing databases, this public
API change is technically breaking. For example, existing
[documentation](https://www.tari.com/lessons/05_emoji_id) needs to be
updated.
  • Loading branch information
AaronFeickert authored Apr 22, 2024
1 parent b5b9360 commit f538714
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 51 deletions.
12 changes: 6 additions & 6 deletions applications/minotari_app_utilities/src/utilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ pub fn setup_runtime() -> Result<Runtime, ExitError> {

/// Returns a CommsPublicKey from either a emoji id or a public key
pub fn parse_emoji_id_or_public_key(key: &str) -> Option<CommsPublicKey> {
EmojiId::from_emoji_string(&key.trim().replace('|', ""))
.map(|emoji_id| emoji_id.to_public_key())
EmojiId::from_str(&key.trim().replace('|', ""))
.map(|emoji_id| PublicKey::from(&emoji_id))
.or_else(|_| CommsPublicKey::from_hex(key))
.ok()
}
Expand Down Expand Up @@ -79,8 +79,8 @@ impl FromStr for UniPublicKey {
type Err = UniIdError;

fn from_str(key: &str) -> Result<Self, Self::Err> {
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::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))
} else if let Ok(tari_address) = TariAddress::from_hex(key) {
Expand Down Expand Up @@ -116,8 +116,8 @@ impl FromStr for UniNodeId {
type Err = UniIdError;

fn from_str(key: &str) -> Result<Self, Self::Err> {
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::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))
} else if let Ok(node_id) = NodeId::from_hex(key) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions base_layer/common_types/src/dammsum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) -> u8 {
pub fn compute_checksum(data: &[u8]) -> u8 {
let mut mask = 1u8;

// Compute the bitmask (if possible)
Expand All @@ -71,7 +71,7 @@ pub fn compute_checksum(data: &Vec<u8>) -> u8 {
}

/// Determine whether the array ends with a valid checksum
pub fn validate_checksum(data: &Vec<u8>) -> 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);
Expand Down
88 changes: 50 additions & 38 deletions base_layer/common_types/src/emoji.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::{
convert::TryFrom,
fmt::{Display, Error, Formatter},
iter,
str::FromStr,
};

use once_cell::sync::Lazy;
Expand Down Expand Up @@ -121,16 +122,24 @@ pub enum EmojiIdError {
}

impl EmojiId {
/// Construct an emoji ID from an emoji string with checksum
pub fn from_emoji_string(emoji: &str) -> Result<Self, EmojiIdError> {
/// Get the public key from an emoji ID
pub fn as_public_key(&self) -> &PublicKey {
&self.0
}
}

impl FromStr for EmojiId {
type Err = EmojiIdError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
// The string must be the correct size, including the checksum
if emoji.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::<u8>::with_capacity(INTERNAL_SIZE + CHECKSUM_SIZE);
for c in emoji.chars() {
for c in s.chars() {
if let Some(i) = REVERSE_EMOJI.get(&c) {
bytes.push(*i);
} else {
Expand All @@ -152,40 +161,48 @@ 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::from(value.clone())
}
}

/// Convert the emoji ID to an emoji string with checksum
pub fn to_emoji_string(&self) -> String {
// Convert the public key to bytes and compute the checksum
let bytes = self.0.as_bytes().to_vec();
bytes
.iter()
.chain(iter::once(&compute_checksum(&bytes)))
.map(|b| EMOJI[*b as usize])
.collect::<String>()
impl From<PublicKey> for EmojiId {
fn from(value: PublicKey) -> Self {
Self(value)
}
}

/// 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())
// 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::<String>();

fmt.write_str(&emoji)
}
}

#[cfg(test)]
mod test {
use std::iter;
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,
Expand All @@ -201,49 +218,43 @@ 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::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
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]
/// Test invalid size
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::from_str(emoji_string), Err(EmojiIdError::InvalidSize));
}

#[test]
/// Test invalid emoji
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::from_str(emoji_string), Err(EmojiIdError::InvalidEmoji));
}

#[test]
/// Test invalid checksum
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::from_str(emoji_string), Err(EmojiIdError::InvalidChecksum));
}

#[test]
Expand All @@ -252,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();
Expand All @@ -262,7 +274,7 @@ mod test {
.collect::<String>();

assert_eq!(
EmojiId::from_emoji_string(&emoji_string),
EmojiId::from_str(&emoji_string),
Err(EmojiIdError::CannotRecoverPublicKey)
);
}
Expand Down
6 changes: 3 additions & 3 deletions base_layer/common_types/src/tari_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand All @@ -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
}
Expand Down Expand Up @@ -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::<String>();

Expand Down

0 comments on commit f538714

Please sign in to comment.