Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stdlib): Implement Poseidon hash #768

Merged
merged 32 commits into from
Apr 3, 2023
Merged

feat(stdlib): Implement Poseidon hash #768

merged 32 commits into from
Apr 3, 2023

Conversation

ax0
Copy link
Contributor

@ax0 ax0 commented Feb 8, 2023

Related issue(s)

Resolves #387

Description

This PR implements the Poseidon hash function. It is exposed in the form of a permutation function (hash::poseidon::permute) for fixed-length and an absorption function (hash::poseidon::absorb) for variable-length inputs. Both functions take the usual parameters as inputs in addition to the input state. Instantiations are provided for fixed length inputs of length 2 to 17 as hash::poseidon::bn254::perm::x5_2, ..., hash::poseidon::bn254::perm::x5_17 for the field size currently used, which are in agreement with those used by Circom. A sponge function hash::poseidon::bn254::sponge is provided for variable-length inputs in accordance with a suggested application in §3 of the paper .

Summary of changes

Addition of Poseidon hash under noir_stdlib/src/hash with corresponding declaration in hash.nr

Dependency additions / changes

Test additions / changes

Two test cases under nargo/tests/test_data, separated into permutation tests and a sponge test. The former test agrees with Circom's tests and the latter may be verified in Arkworks using appropriate incantations of absorb and squeeze_field_elements. These cases are also listed in the config.toml.

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.
  • This PR requires documentation updates when merged.

Additional context

Copy link
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this submission, I have added my comments below.

noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon/bn254/consts.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon/bn254.nr Show resolved Hide resolved
noir_stdlib/src/hash/poseidon/bn254/consts.nr Outdated Show resolved Hide resolved
@guipublic guipublic changed the title Implement Poseidon hash feat(stdlib): Implement Poseidon hash Feb 8, 2023
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some quick style suggestions

noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon/bn254/perm.nr Outdated Show resolved Hide resolved
Copy link
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I just have a few comments.

noir_stdlib/src/hash/poseidon.nr Outdated Show resolved Hide resolved
noir_stdlib/src/hash/poseidon/bn254.nr Outdated Show resolved Hide resolved
crates/nargo/tests/test_data/config.toml Outdated Show resolved Hide resolved
@ax0
Copy link
Contributor Author

ax0 commented Feb 8, 2023

Thanks for the comments. I've incorporated your changes.

A couple of things worth noting:

  • The poseidonperm_x5_254 test's proof is successfully generated but verification fails (cf. CI).
  • The poseidonsponge_x5_254 test no longer terminates with the unoptimised sponge function in there, at least not within 30 minutes. Curiously enough, if I revert f329379, it does terminate within 2-3 minutes.

guipublic
guipublic previously approved these changes Feb 9, 2023
Copy link
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fine for me.
The verification seems to be failing, which could be due to an existing issue.

@jfecher jfecher added stdlib Standard library shipped with Noir tooling compiler labels Mar 28, 2023
@kevaundray kevaundray requested review from jfecher and guipublic April 2, 2023 12:27
@jfecher jfecher added this pull request to the merge queue Apr 3, 2023
Merged via the queue into noir-lang:master with commit 779ab66 Apr 3, 2023
@kevaundray
Copy link
Contributor

@ax0 Thank you for the PR and for your patience in getting it merged. It's very much appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler stdlib Standard library shipped with Noir tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Poseidon hash function
4 participants