Skip to content

Commit

Permalink
chore: simplify checksum data extraction (#6338)
Browse files Browse the repository at this point in the history
Description
---
Updates checksum validation to return the data slice.

Motivation and Context
---
After
[validating](https://github.com/tari-project/tari/blob/9920916f891fbd26759f7cb4912701189b7579a1/base_layer/common_types/src/dammsum.rs#L73-L85)
a checksum, the caller is responsible for removing it to obtain the
underlying data. This is brittle, since it breaks the checksum
abstraction somewhat.

This PR updates checksum validation to return the underlying data slice,
making it easier for callers. It also exposes the checksum size for
callers that need to know it for encoding purposes, and adds an
unrelated `EmojiId` sanity check for a data size constant.

How Has This Been Tested?
---
Existing, new, and updated tests pass.

What process can a PR reviewer use to test or verify this change?
---
Check to make sure that the updated validation API operates as expected
and is properly tested.
  • Loading branch information
AaronFeickert committed May 16, 2024
1 parent 301ea00 commit 5db26f3
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 23 deletions.
18 changes: 12 additions & 6 deletions base_layer/common_types/src/dammsum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ pub enum ChecksumError {
InvalidChecksum,
}

/// The number of bytes used for the checksum
/// This is included for applications that need to know it for encodings
pub const CHECKSUM_BYTES: usize = 1;

// Fixed for a dictionary size of `2^8 == 256`
const COEFFICIENTS: [u8; 3] = [4, 3, 1];

Expand Down Expand Up @@ -71,15 +75,16 @@ pub fn compute_checksum(data: &[u8]) -> u8 {
}

/// Determine whether the array ends with a valid checksum
pub fn validate_checksum(data: &[u8]) -> Result<(), ChecksumError> {
/// If it is valid, returns the underlying data slice (without the checksum)
pub fn validate_checksum(data: &[u8]) -> Result<&[u8], ChecksumError> {
// Empty data is not allowed, nor data only consisting of a checksum
if data.len() < 2 {
return Err(ChecksumError::InputDataTooShort);
}

// It's sufficient to check the entire array against a zero checksum
match compute_checksum(data) {
0u8 => Ok(()),
0u8 => Ok(&data[..data.len() - 1]),
_ => Err(ChecksumError::InvalidChecksum),
}
}
Expand All @@ -97,13 +102,14 @@ mod test {

// Generate random data
let mut rng = rand::thread_rng();
let mut data: Vec<u8> = (0..SIZE).map(|_| rng.gen::<u8>()).collect();
let data: Vec<u8> = (0..SIZE).map(|_| rng.gen::<u8>()).collect();

// Compute and append the checksum
data.push(compute_checksum(&data));
let mut data_with_checksum = data.clone();
data_with_checksum.push(compute_checksum(&data));

// Validate
assert!(validate_checksum(&data).is_ok());
// Validate and ensure we get the same data back
assert_eq!(validate_checksum(&data_with_checksum).unwrap(), data);
}

#[test]
Expand Down
34 changes: 17 additions & 17 deletions base_layer/common_types/src/emoji.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use tari_crypto::tari_utilities::ByteArray;
use thiserror::Error;

use crate::{
dammsum::{compute_checksum, validate_checksum},
dammsum::{compute_checksum, validate_checksum, CHECKSUM_BYTES},
types::PublicKey,
};

Expand Down Expand Up @@ -75,8 +75,7 @@ use crate::{
pub struct EmojiId(PublicKey);

const DICT_SIZE: usize = 256; // number of elements in the symbol dictionary
const INTERNAL_SIZE: usize = 32; // number of bytes used for the internal representation (without checksum)
const CHECKSUM_SIZE: usize = 1; // number of bytes in the checksum
const DATA_BYTES: usize = 32; // number of bytes used for the key data

// The emoji table, mapping byte values to emoji characters
pub const EMOJI: [char; DICT_SIZE] = [
Expand Down Expand Up @@ -134,12 +133,12 @@ impl FromStr for EmojiId {

fn from_str(s: &str) -> Result<Self, Self::Err> {
// The string must be the correct size, including the checksum
if s.chars().count() != INTERNAL_SIZE + CHECKSUM_SIZE {
if s.chars().count() != DATA_BYTES + CHECKSUM_BYTES {
return Err(EmojiIdError::InvalidSize);
}

// Convert the emoji string to a byte array
let mut bytes = Vec::<u8>::with_capacity(INTERNAL_SIZE + CHECKSUM_SIZE);
let mut bytes = Vec::<u8>::with_capacity(DATA_BYTES + CHECKSUM_BYTES);
for c in s.chars() {
if let Some(i) = REVERSE_EMOJI.get(&c) {
bytes.push(*i);
Expand All @@ -148,16 +147,11 @@ impl FromStr for EmojiId {
}
}

// Assert the checksum is valid
if validate_checksum(&bytes).is_err() {
return Err(EmojiIdError::InvalidChecksum);
}

// Remove the checksum
bytes.pop();
// Assert the checksum is valid and get the underlying data
let data = validate_checksum(&bytes).map_err(|_| EmojiIdError::InvalidChecksum)?;

// Convert to a public key
match PublicKey::from_canonical_bytes(&bytes) {
match PublicKey::from_canonical_bytes(data) {
Ok(public_key) => Ok(Self(public_key)),
Err(_) => Err(EmojiIdError::CannotRecoverPublicKey),
}
Expand Down Expand Up @@ -206,8 +200,8 @@ mod test {
};

use crate::{
dammsum::compute_checksum,
emoji::{emoji_set, EmojiId, EmojiIdError, CHECKSUM_SIZE, INTERNAL_SIZE},
dammsum::{compute_checksum, CHECKSUM_BYTES},
emoji::{emoji_set, EmojiId, EmojiIdError, DATA_BYTES},
types::{PrivateKey, PublicKey},
};

Expand All @@ -224,7 +218,7 @@ mod test {

// Check the size of the corresponding emoji string
let emoji_string = emoji_id_from_public_key.to_string();
assert_eq!(emoji_string.chars().count(), INTERNAL_SIZE + CHECKSUM_SIZE);
assert_eq!(emoji_string.chars().count(), DATA_BYTES + CHECKSUM_BYTES);

// Generate an emoji ID from the emoji string and ensure we recover it
let emoji_id_from_emoji_string = EmojiId::from_str(&emoji_string).unwrap();
Expand Down Expand Up @@ -262,7 +256,7 @@ mod test {
/// Test invalid public key
fn invalid_public_key() {
// This byte representation does not represent a valid public key
let mut bytes = vec![0u8; INTERNAL_SIZE];
let mut bytes = vec![0u8; DATA_BYTES];
bytes[0] = 1;
assert!(PublicKey::from_canonical_bytes(&bytes).is_err());

Expand All @@ -279,4 +273,10 @@ mod test {
Err(EmojiIdError::CannotRecoverPublicKey)
);
}

#[test]
/// Test that the data size is correct for the underlying key type
fn data_size() {
assert_eq!(PublicKey::default().as_bytes().len(), DATA_BYTES);
}
}

0 comments on commit 5db26f3

Please sign in to comment.