Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a wrapper for Word #59

Open
Tracked by #54 ...
vlopes11 opened this issue Feb 15, 2023 · 2 comments
Open
Tracked by #54 ...

Create a wrapper for Word #59

vlopes11 opened this issue Feb 15, 2023 · 2 comments
Labels
good first issue Good for newcomers

Comments

@vlopes11
Copy link
Contributor

vlopes11 commented Feb 15, 2023

Currently Word is a type alias

crypto/src/lib.rs

Lines 24 to 34 in 0c242d2

// TYPE ALIASES
// ================================================================================================
/// A group of four field elements in the Miden base field.
pub type Word = [Felt; WORD_SIZE];
// CONSTANTS
// ================================================================================================
/// Number of field elements in a word.
pub const WORD_SIZE: usize = 4;

However, it is a critical component of our stack as it is used everywhere, in different forms. One example is RpoDigest:

// DIGEST TRAIT IMPLEMENTATIONS
// ================================================================================================
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)]
pub struct RpoDigest([Felt; DIGEST_SIZE]);

They both have the same structure, but different concrete types. This leads to many cases where unsafe optimizations are required, such as in here:

#58

One of the problems highlighted by this PR is the need to convert between Word and RpoDigest efficiently (i.e. without copying/allocating) & safely.

We could achieve major simplification if:

  • Word was a struct
  • The digest of Rpo256 was a Word
@vlopes11
Copy link
Contributor Author

@bobbinth should we include this on 0.6? It will clean the code quite a bit as we won't have to convert between the types anymore

@bobbinth
Copy link
Contributor

Yes - let's do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants