Add Rescue Prime Optimized #930

jotabulacios · 2024-10-16T18:03:08Z

Add Rescue Prime Optimized

Description

This PR adds the Rescue Prime Optimized hash function.
The implementation offers three types of MDS transformations: Matrix Multiplication, Karatsuba, and NTT

Example:

let rescue = RescuePrimeOptimized::new(SecurityLevel::Sec128, MdsMethod::Ntt).unwrap();
let input = b"Hello there";
let hash_output = rescue.hash(input);

In this case 128 is the security level, the other available level is 160
To use other MdsMethod simply change MatrixMultiplication for Karatsuba or Ntt

Type of change

New feature

…bdaworks into add_rescue_prime

codecov-commenter · 2024-10-16T18:41:58Z

Codecov Report

Attention: Patch coverage is 92.80156% with 37 lines in your changes missing coverage. Please review.

Project coverage is 71.97%. Comparing base (8dfddac) to head (ea8267d).

Files with missing lines	Patch %	Lines
crypto/src/hash/rescue_prime/utils.rs	58.82%	28 Missing ⚠️
...to/src/hash/rescue_prime/rescue_prime_optimized.rs	98.07%	8 Missing ⚠️
crypto/src/hash/rescue_prime/parameters.rs	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #930      +/-   ##
==========================================
+ Coverage   71.62%   71.97%   +0.35%     
==========================================
  Files         149      152       +3     
  Lines       32560    33074     +514     
==========================================
+ Hits        23321    23805     +484     
- Misses       9239     9269      +30

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pablodeymo · 2024-10-16T20:05:12Z

crypto/src/hash/rescue_prime/mod.rs

+    Ntt,
+    Karatsuba,
+}
+#[warn(dead_code)]


why do you need this annotation?

Solved, it wasn’t necessary. I added it while trying to fix some Clippy issues, but it turned out to be incorrect

pablodeymo · 2024-10-16T20:07:15Z

crypto/src/hash/rescue_prime/mod.rs

+
+    fn get_mds_vector(m: usize) -> Vec<Fp> {
+        match m {
+            12 => vec![7, 23, 8, 26, 13, 10, 9, 7, 6, 22, 21, 8]


this can be an array instead of a vec

You're right, the mds_vector is now an array of Fp and is also precomputed

pablodeymo · 2024-10-16T20:07:47Z

crypto/src/hash/rescue_prime/mod.rs

+            .into_iter()
+            .map(Fp::from)
+            .collect(),
+            _ => panic!("Unsupported state size"),


I think this function shouldn't panic. It can return a Result indicating the error case.

This function isn't used anymore, but I acknowledged your comment about avoiding panics

pablodeymo · 2024-10-16T20:10:16Z

crypto/src/hash/rescue_prime/mod.rs

+        let m = self.m;
+        let round_constants = &self.round_constants;
+        for j in 0..m {
+            state[j] += round_constants[round * 2 * m + j];


instead of indexing, this can be improved by using get_mut and an iterator

updated the implementation as suggested

pablodeymo · 2024-10-16T20:12:15Z

crypto/src/hash/rescue_prime/utils.rs

+pub fn bytes_to_field_elements(input: &[u8]) -> Vec<Fp> {
+    input
+        .chunks(7)
+        .map(|chunk| {
+            let mut buf = [0u8; 8];
+            buf[..chunk.len()].copy_from_slice(chunk);
+            if chunk.len() < 7 {
+                buf[chunk.len()] = 1;
+            }
+            let value = u64::from_le_bytes(buf);
+            Fp::from(value)
+        })
+        .collect()
+}


Is this function just for rescue_prime or a function that can be used elsewhere?

Currently it 's only use in rescue_prime. Maybe later it can be added to fields

pablodeymo · 2024-10-16T20:12:59Z

crypto/src/hash/rescue_prime/mod.rs

@@ -0,0 +1,816 @@
+use crate::alloc::vec::Vec;


It's better not to have a giant mod.rs file.
Instead, it's better to have a separate file with each struct or enum.

Restructured the files as suggested. Now is more maintainable and clear

ilitteri

As a general rule, we won't want to panic anywhere. As this is a library meant for users, we must provide them Results of the operations to let them decide what to do with the errors in such cases. Commonly, panics can happen in places where unwraps, expects, as conversions, and slice indexing (and obviously when we explicitly use the panic macro). Unless it is planned for another PR to handle these cases I'd suggest doing it asap.

Unwraps and expects can be avoided returning Results. As conversions can be avoided mostly by using .into(), and .try_into() (we will prefer the first one as we'd not have to handle the error). Finally, slice indexing can be avoided using .get() and handling the Option.

To improve the code's readability we'd want to define descriptive constants for hardcoded structures.

crypto/src/hash/rescue_prime/mod.rs

crypto/src/hash/rescue_prime/utils.rs

crypto/src/hash/rescue_prime/mod.rs

crypto/src/hash/rescue_prime/parameters.rs

crypto/src/hash/rescue_prime/utils.rs

ilitteri · 2024-10-17T16:49:14Z

crypto/src/hash/rescue_prime/mod.rs

@@ -0,0 +1,816 @@
+use crate::alloc::vec::Vec;


…bdaworks into add_rescue_prime

ilitteri

The PR is looking good! Left some comments

crypto/src/hash/rescue_prime/rescue_prime_optimized.rs

ilitteri · 2024-10-29T22:11:46Z

crypto/src/hash/rescue_prime/rescue_prime_optimized.rs

+        let (m, capacity) = match security_level {
+            SecurityLevel::Sec128 => (12, 4),
+            SecurityLevel::Sec160 => (16, 6),
+        };


Let's do the same for these. get_state_size(security_level: SecurityLevel) and get_capacity(security_level: SecurityLevel).

…bdaworks into add_rescue_prime

ilitteri

Great work! LGTM

jotabulacios and others added 14 commits September 19, 2024 10:09

first commit

6c8fa67

first draft, save work

7b30c24

first working commit, passes all tests. Save work

6a9979c

save work

ccfcdca

save work

5174b11

fixed padding, now all test pass

6c4d06f

refactor

10878f2

more refactor

d5b9c91

added precomputed values for 160 security level

2a0338f

fix typos

ca9d36b

Merge branch 'main' into add_rescue_prime

37417c2

fix clippy and wasm

cabe504

Merge branch 'add_rescue_prime' of https://github.com/lambdaclass/lam…

29e0e08

…bdaworks into add_rescue_prime

add std feature when needed

fd293b2

jotabulacios marked this pull request as ready for review October 16, 2024 18:40

jotabulacios requested a review from a team as a code owner October 16, 2024 18:40

pablodeymo reviewed Oct 16, 2024

View reviewed changes

ilitteri reviewed Oct 17, 2024

View reviewed changes

jotabulacios and others added 6 commits October 20, 2024 22:08

save work, refactor and solved some comments

cbd20dd

save work

0e4c45f

Merge branch 'main' into add_rescue_prime

209a636

Merge branch 'main' into add_rescue_prime

855e277

move Mds enum and removed commented code

da7ab0e

Merge branch 'add_rescue_prime' of https://github.com/lambdaclass/lam…

1556450

…bdaworks into add_rescue_prime

Merge branch 'main' into add_rescue_prime

d50852d

jotabulacios requested review from pablodeymo and ilitteri October 28, 2024 15:51

ilitteri reviewed Oct 29, 2024

View reviewed changes

jotabulacios and others added 4 commits October 30, 2024 11:17

remove unnecessary match

8ee7116

Merge branch 'main' into add_rescue_prime

1c2db51

removed commented code

c385cbb

Merge branch 'add_rescue_prime' of https://github.com/lambdaclass/lam…

ea8267d

…bdaworks into add_rescue_prime

jotabulacios requested a review from ilitteri November 11, 2024 13:02

pablodeymo approved these changes Nov 11, 2024

View reviewed changes

ilitteri approved these changes Nov 11, 2024

View reviewed changes

jotabulacios added this pull request to the merge queue Nov 11, 2024

Merged via the queue into main with commit d35c96f Nov 11, 2024
8 checks passed

jotabulacios deleted the add_rescue_prime branch November 11, 2024 18:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rescue Prime Optimized #930

Add Rescue Prime Optimized #930

jotabulacios commented Oct 16, 2024 •

edited

Loading

codecov-commenter commented Oct 16, 2024 •

edited

Loading

pablodeymo Oct 16, 2024

jotabulacios Oct 28, 2024

pablodeymo Oct 16, 2024

jotabulacios Oct 28, 2024

pablodeymo Oct 16, 2024

jotabulacios Oct 28, 2024

pablodeymo Oct 16, 2024

jotabulacios Oct 28, 2024

pablodeymo Oct 16, 2024

jotabulacios Oct 28, 2024

pablodeymo Oct 16, 2024

ilitteri Oct 17, 2024

jotabulacios Oct 28, 2024

ilitteri left a comment •

edited

Loading

ilitteri Oct 17, 2024

ilitteri left a comment

ilitteri Oct 29, 2024

ilitteri left a comment

Add Rescue Prime Optimized #930

Add Rescue Prime Optimized #930

Conversation

jotabulacios commented Oct 16, 2024 • edited Loading

Add Rescue Prime Optimized

Description

Type of change

codecov-commenter commented Oct 16, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilitteri left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilitteri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilitteri left a comment

Choose a reason for hiding this comment

jotabulacios commented Oct 16, 2024 •

edited

Loading

codecov-commenter commented Oct 16, 2024 •

edited

Loading

ilitteri left a comment •

edited

Loading