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

Add poseidon hash type #16

Merged
merged 3 commits into from
Apr 7, 2024
Merged

Conversation

danielost
Copy link
Contributor

Issue

The Poseidon hash type might be useful for those who work with ZKP, so this PR adds a new hash type that uses Poseidon under the hood

Changes

  • Add the poseidon hash type
  • Add tests

@mcdee
Copy link
Collaborator

mcdee commented Mar 27, 2024

Thank you for this PR. From a quick look at poseidon it appears to have a number of parameters that result in different outputs, as per https://www.poseidon-hash.info/#h.p_KNsDarG9gujB

If this implementation is using one particular selection of these parameters then this should be noted. Ideally, it should be possible to instantiate the function with different parameters (see the sha3 directory for multiple different hashes within the same "family").

Or am I misunderstanding the usage here?

@danielost
Copy link
Contributor Author

Thank you for this PR. From a quick look at poseidon it appears to have a number of parameters that result in different outputs, as per https://www.poseidon-hash.info/#h.p_KNsDarG9gujB

If this implementation is using one particular selection of these parameters then this should be noted. Ideally, it should be possible to instantiate the function with different parameters (see the sha3 directory for multiple different hashes within the same "family").

Or am I misunderstanding the usage here?

Thanks for the quick response. You're right, Poseidon has many parameters, but iden3 implementation provides us with only one parameter, which is frameSize. It can vary between 2 and 16, but 16 is considered to be a default value.

I don't think we should make it possible to instantiate the Poseidon hash func with different frame sizes, but it's reasonable to note that the value we use for frameSize is 16

@mcdee
Copy link
Collaborator

mcdee commented Apr 7, 2024

I'm not actually sure what "frame size" is here. Looking at what I think is the original Poseidon definition at https://eprint.iacr.org/2019/458.pdf there is no mention of this parameter.

I think that we're okay to merge this, but we may need to look at some sort of parameterization of this in future.

@mcdee mcdee merged commit 8170082 into wealdtech:master Apr 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants