Add fuzz tests to show equivalence between stdlib cryptographic functions and rust implementations #6141
Assignees
Comments
|
compile from string snippet: Line 332 in be9dcfe |
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 7, 2024
# Description ## Problem\* Resolves #6141 ## Summary\* Adds property based testing for the following hash function in the standard library: * Keccak256 * Sha256 * Sha512 In a followup I will carry on with Poseidon and Poseidon 2, and Schnorr signatures. I also wanted to run the code with the [interpreter](https://github.com/noir-lang/noir/blob/master/compiler/noirc_frontend/src/hir/comptime/tests.rs) but felt like it would complicate things in this PR. ## Additional Context ```console ❯ cargo test -p nargo_cli --test stdlib-props Finished test [optimized + debuginfo] target(s) in 0.23s Running tests/stdlib-props.rs (target/debug/deps/stdlib_props-cbab5917b839ebf4) running 4 tests test test_basic ... ok test test_sha256 ... ok test test_keccak256 ... ok test test_sha512 ... ok test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 20.25s ``` ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: TomAFrench <tom@tomfren.ch> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 10, 2024
# Description ## Problem\* Related to #6141 ## Summary\* Adds a fuzz test for `poseidon2.nr` comparing the results to [bn254_blackbox_solver::poseidon_hash](https://github.com/noir-lang/noir/blob/70cbeb4322a0b11c1c167ab27bf0408d04fe7b7d/acvm-repo/bn254_blackbox_solver/src/poseidon2.rs#L547), which says it's `"Performs a poseidon hash with a sponge construction equivalent to the one in poseidon2.nr"` To pass the test the Rust implementation was given a new `is_variable_length` parameter to inform it whether it needs to append an extra 1 like Noir and Berratenberg do. ## Additional Context The test initially failed: ```console ❯ cargo test -p nargo_cli --test stdlib-props poseidon test fuzz_poseidon2_equivalence ... FAILED ---- fuzz_poseidon2_equivalence stdout ---- Test failed: assertion failed: `(left == right)` left: `Field(1187863985434533916290764679013201786939267142671550539990974992402592744116)`, right: `Field(11250791130336988991462250958918728798886439319225016858543557054782819955502)`: max_len = 1 at tooling/nargo_cli/tests/stdlib-props.rs:106. minimal failing input: io = SnippetInputOutput { description: "max_len = 1", inputs: { "input": Vec( [ Field( 0, ), ], ), "message_size": Field( 0, ), }, expected_output: Field( 11250791130336988991462250958918728798886439319225016858543557054782819955502, ), } ``` So we pass in `input = [0; 1]` with `message_size=0`. It fails because the Noir code treats the case where the `message_size` is different from the maximum length differently by [appending](https://github.com/noir-lang/noir/blob/70cbeb4322a0b11c1c167ab27bf0408d04fe7b7d/noir_stdlib/src/hash/poseidon2.nr#L75-L80) an extra 1, to keep variable and fixed length hashes distinct. The Rust implementation doesn't do this, nor did the other hashes tested so far. I'm not sure if it's worth noting that the hash will not depend on how much shorter the message is than the maximum, just that it's shorter. ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 10, 2024
# Description ## Problem\* Related to #6141 ## Summary\* Testing `std::hash::poseidon::bn254::hash_[1-12]` against https://github.com/Lightprotocol/light-poseidon/tree/v0.2.0 The library doesn't support inputs wider than 12 (doesn't have the parameters for it), so the Noir function `hash_[13-16]` are not covered. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 11, 2024
# Description ## Problem\* Related to #6141 ## Summary\* Assert that the `PoseidonHasher` calculates the same hash on dynamic input as the `hash_<lenght>` function we know it has to call. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We should add some fuzz tests which generates random inputs and feeds them into the stdlib implementations of
keccak256/sha256/etc. alongside an off-the-shelf rust implementation to give greater assurances about correctness.Note that this won't work with just a regular fuzz test but will need a rust harness.
The text was updated successfully, but these errors were encountered: