RPX hash function #201

Al-Kindi-0 · 2023-10-24T13:56:17Z

Describe your changes

This is a first attempt at implementing a variation, called RPX, of the xHash12 hash function. There is a lot of code overlap with RPO and this PR tries to reduce that but it would probably make more sense to create a trait in order to simplify even more.

Checklist before requesting a review

Repo forked and branch created from next according to naming convention.
Commit messages and codestyle follow conventions.
Relevant issues are linked in the PR description.
Tests added for new functionality.
Documentation/comments updated according to changes.

bobbinth

Thank you! Looks good! Not a full review yet - but I had a few comments which may consolidate things a bit more and reduce the amount of changes.

In terms of the overall organization, I think we could group things a bit more since things constants, MDS matrix etc. some common procedures can be shared. On the other hand, I would probably keep the types for RpoDigest and RpxDigest separate - mostly for type-safety reasons.

Overall, I am wondering if the following structure would work:

Put the root module under src/hash/rescue/mod.rs In this module we can put definitions of common routines and constants (round constants but also things like STATE_WIDTH etc.).
Create a separate module for MDS matrix with the following structure:
- src/hash/rescue/mds/mod.rs with the MDS matrix and also apply_mds() function.
- src/hash/rescue/mds/freq.rs with the contents of the current mds_freq.rs file.
For RPO, we'd have a separate module with the following files:
- src/hash/rescue/rpo/mod.rs
- src/hash/rescue/rpo/digest.rs
- src/hash/rescue/rpo/tests.rs
And similar structure for RPX
- src/hash/rescue/rpx/mod.rs
- src/hash/rescue/rpx/digest.rs
- src/hash/rescue/rpx/tests.rs (though, not sure if we'll have any at this point)

src/hash/mod.rs

bobbinth

Looks good! Thank you! I pushed a commit which moves some more common functionality to the root rescue module - let me know what you think.

A couple of notes:

I'm not sure if we actually need to derive a different set of round constants for RPX - unless there is a good reason to do that?
I've changed apply_permutation() function RPX so that each round gets its own constants. This will make it much easier to define round constants a periodic columns inside the STARK (we basically stick to how we define them now).

Al-Kindi-0 · 2023-10-26T05:37:38Z

Looks good!
Using the round constants of RPO should definitely be fine. I was thinking more of the scenario when one has a specification of how to generate them starting from something generic e.g., RPX Hash. We can comeback to this once we have constant generation in a specification.