Use an XOF like Shake128 #15
Conversation
|
I added another commit that shows what the shake128 version of create_discriminant.rs looks like. |
There was a problem hiding this comment.
Thank you for your contribution, and for the explanations!
I think I agree with this; the only thing that makes me uncomfortable is that we're disabling tests and losing the ability to compare the outputs with Inkfish. We should probably extend and update our unit tests instead.
I wouldn't worry about backward compatibility: It is currently published as version 0.1.0, and we'll definitely increment to 0.2.0 when we publish this.
vdf/src/create_discriminant.rs
Outdated
| debug_assert!(n >= Zero::zero()); | ||
| let rem = n.frem_u32(M); | ||
|
|
||
| let residue = { |
vdf/src/create_discriminant.rs
Outdated
| let extra: u8 = (bit_length as u8) & 7; | ||
|
|
||
| let byte_length = (usize::from(bit_length) + 7) >> 3; | ||
| let mut n = vec![0u8; byte_length]; |
There was a problem hiding this comment.
This project is indented with spaces, not tabs.
(I'd even suggest rustfmt, but I'm not sure whether it has been applied before, and how much it would change…)
vdf/src/proof_pietrzak.rs
Outdated
| } | ||
| let res = hasher.fixed_result(); | ||
| T::unsigned_deserialize_bignum(&res[..16]) | ||
| let mut res = [0u8; 16]; |
There was a problem hiding this comment.
There's still a few tabs in these files.
vdf/src/proof_wesolowski.rs
Outdated
| } | ||
|
|
||
| /// As on page 10 of Wesolowski's paper, we uniformly sample a prime | ||
| /// from amongt the first 2^129 primes. According to the prime number |
vdf/src/proof_wesolowski.rs
Outdated
| /// As on page 10 of Wesolowski's paper, we uniformly sample a prime | ||
| /// from amongt the first 2^129 primes. According to the prime number | ||
| /// theorem, the prime counting function `π(x)` can be approximated | ||
| /// by `x / log x` asymptoticly, so like `2^128` when `x = 2^134` or |
vdf/src/proof_wesolowski.rs
Outdated
| /// from amongt the first 2^129 primes. According to the prime number | ||
| /// theorem, the prime counting function `π(x)` can be approximated | ||
| /// by `x / log x` asymptoticly, so like `2^128` when `x = 2^134` or | ||
| /// `2^122` when `x = 2^128`, which still leaves enormous margine. |
There was a problem hiding this comment.
"an enormous margin"/"enormous margins"
vdf/src/proof_wesolowski.rs
Outdated
| hasher.input(i); | ||
| } | ||
| let n = T::from(&hasher.fixed_result()[..16]); | ||
| let mut b = [0u8; 16]; // Ideally 17 |
There was a problem hiding this comment.
I guess your comment refers to the 129 bits?
Or is 16 correct here, and should n = T::from(&b[..]) * 2 + 1 instead?
Shake128 provides an XoF which you need here. Blake2x works too. SHA3 is slow, due to NIST competition rules being silly, but Shake128 is fast.
|
I cleaned this up according to your comments, thanks! I also added a separate branch that includes the notes without the new hashing https://github.com/w3f/vdf/tree/l_size_notes so you could fork that off as an inkfish compatible branch. Actually the real change I want is ownership of output proofs, which alters the hashing anyways, so I'll do it as a descendant of this branch. |
vdf/src/lib.rs
Outdated
| @@ -43,7 +43,7 @@ | |||
| //! | |||
| //! ### To use the VDF library | |||
| //! | |||
| //! ```rust | |||
| //! ```rust,no_run | |||
There was a problem hiding this comment.
Would it be possible to extend the example instead, so that it validates the proof instead of comparing to some known binary value?
There was a problem hiding this comment.
Thanks! I guess the no_run can be removed again?
vdf/src/lib.rs
Outdated
| @@ -43,7 +43,7 @@ | |||
| //! | |||
| //! ### To use the VDF library | |||
| //! | |||
| //! ```rust | |||
| //! ```rust,no_run | |||
There was a problem hiding this comment.
Thanks! I guess the no_run can be removed again?
vdf/src/lib.rs
Outdated
| //! assert!(pietrzak_vdf.verify(b"\xaa", 100, CORRECT_SOLUTION).is_ok()); | ||
| //! } | ||
| //! let solution = pietrzak_vdf.solve(b"\xaa", 100).unwrap(); | ||
| //! assert!( pietrzak_vdf.verify(b"\xaa", 100, &solution[..]).is_ok() ); |
There was a problem hiding this comment.
I'd remove the spaces before and after the brackets. That's what rustfmt would do.
vdf/src/proof_pietrzak.rs
Outdated
| } | ||
| let res = hasher.fixed_result(); | ||
| T::unsigned_deserialize_bignum(&res[..16]) | ||
| let mut res = [0u8; 16]; |
There was a problem hiding this comment.
There's still a few tabs in these files.
I'd suggest using either shake128 or blake2x for the two places where you sample considerable data in create_discriminant.rs and proof_wesolowski.rs. It's only a negligible impact on performance, but it just looks ugly to roll you own stream cipher with sha2.
We could easily use sha2 for hashing and chacha20 for output, but this requires using another crate and algorithm. Shake128 is a keccak based hash function together with an extensible output function (XOF). Blake2x is a chacha based hash with an XOF. So either gives everything requires with only one dependency.
I selected shake128 over blake2x based on prevalence and the promise that keccak implemented easily in hardware, which matters if this ever gets made into an ASIC. As an aside, shake128/shake256 are always preferred over SHA3 because NIST senselessly conflated security level with output bits in the SHA3 competition.
In this PR, I've only switched the usage in proof_wesolowski.rs so far because altering create_discriminant.rs would change the test vectors. If you've nothing in production, then we should change create_discriminant.rs too, and maybe remove test vectors until things stabilize. If anyone has this in production, then we should create a feature or something because even this PR breaks the non-existent test vectors for the VDF itself.