feat: plume with poseidon2 hashes #8
Conversation
|
I also have no idea how to update the SAGE scripts to use poseidon, I suppose it's too much to ask for a SAGE implementation of it :D @NikitaMasych any idea how could we get around this? |
|
Hey, nice to see this PR; I'll take an extensive look a bit later, but at the first glance looks legit 🙂 |
I'll see that I can do; sage is merely an exemplary in this repository, so this shouldn't be a burden to deliver the feature of this PR |
crates/plume/src/lib.nr
Outdated
| @@ -109,7 +110,7 @@ fn sha256_12_coordinates( | |||
| } | |||
| } | |||
|
|
|||
| sha256(res) | |||
| hash([to_field(res)], res.len()) | |||
There was a problem hiding this comment.
This looks like we are missing on preimage data when doing conversion to field; res being 198 bytes of information is narrowed down to the prime order of underlying field of appr. 8 bytes for bb prior to hashing. Additionally, here is passed only one field element in array, but res.len() will give 198.
There was a problem hiding this comment.
I see, so in this case do you suggest I break it to limbs and pass it as an array maybe?
| @@ -52,7 +53,7 @@ fn msg_prime<let N: u32>(msg: [u8; N]) -> [u8; 32] { | |||
| preimage[64 + N + 2 + 1 + i] = DST_PRIME[i]; | |||
| } | |||
|
|
|||
| sha256(preimage) | |||
| hash([to_field(preimage)], preimage.len()).to_le_bytes() | |||
There was a problem hiding this comment.
Since this is not quite part of the PLUME, but rather of the hash-to-curve algorithm, it might be not conforming to requirements given in spec. Though, I am not particularly sure
There was a problem hiding this comment.
I think if I hash the whole thing by breaking it into limbs and hashing the limbs, it would conform to the spec. But I am not a cryptographer, so I may be wrong
| @@ -80,7 +81,7 @@ fn hash_b(b_idx: u8, b: [u8; 32]) -> [u8; 32] { | |||
| preimage[32 + 1 + i] = DST_PRIME[i]; | |||
| } | |||
|
|
|||
| sha256(preimage) | |||
| hash([to_field(preimage)], 32 + 1 + 50).to_le_bytes() | |||
There was a problem hiding this comment.
Same about conforming spec as in msg_prime
crates/use_v1/src/main.nr
Outdated
| @@ -3,7 +3,7 @@ use noir_bigcurve::BigCurve; | |||
| use noir_bigcurve::curves::secp256k1::{Secp256k1, Secp256k1Fr, Secp256k1Scalar}; | |||
| use noir_bigcurve::scalar_field::ScalarField; | |||
|
|
|||
| use plume::plume_v1; | |||
| use plume::{plume_v1, to_field}; | |||
There was a problem hiding this comment.
I guess here import shall be use plume::utils::to_field to resolve is private and not visible from the current module warning.
| @@ -109,7 +110,7 @@ fn sha256_12_coordinates( | |||
| } | |||
| } | |||
|
|
|||
| sha256(res) | |||
| hash([to_field(res)], res.len()) | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
Regarding all commented-out tests here: they need to be updated and passing nargo test, if uncommented. Currently, there are few data and compilation issues. Sorry for having to deal with commented code, I guess if noir'll support something akin to rust's #[ignore] macro in future, I'll update these
There was a problem hiding this comment.
No problem, feel free to update this branch too. I'm just an Aztec team member on my time off learning a bit about PLUME stuff 👍
There was a problem hiding this comment.
Sure, can you grant me write access to your fork?
crates/plume/src/utils.nr
Outdated
| as_field += (bytes[index] as Field) * offset; | ||
| offset *= 256; | ||
| } | ||
| std::as_witness(as_field); |
There was a problem hiding this comment.
Just curious, why need this line?
There was a problem hiding this comment.
This little guy shaves off 200k constraints out of the whole thing 😄 a team member found a bug in the compiler that was causing a huge blowout. Using as_witness forces the compiler to assign intermediate values, which makes it behave as it should
|
Thanks @NikitaMasych the reason I'm picking this up is mostly to learn, as I am playing around with Stealthdrop on the phone and thus felt the need to make it less costly. Stealthdrop is also one of the noir examples and the existent version is very outdated (and unsafe), so I'd like to update that too |
As discussed with @yush_g we wanted to try PLUME with poseidon hashes, this is the PR. Still pending: