Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: Poseidon2 stdlib impl #3551
feat: Poseidon2 stdlib impl #3551
Changes from 7 commits
aa536b5
89f4bc4
e5298b4
e80739d
751d74b
4b3ca57
4b68594
aaec935
45dc5d4
d933eef
171b0e8
dc5f3f9
26b1930
0e312e9
4126d04
41aa7c1
8f8b799
7266f35
f766e2b
db908d7
9dc8392
cc9edd9
6ddfcae
e15be66
222e532
eedcefb
c24436f
13807f1
7dbb45c
8b25996
bf75776
6f04eb0
ff22a5f
2419db5
aeaf1a1
f134a94
6b67f11
e9fd7da
2c66d1f
4b7d151
8bd4df0
af5e017
8aa065f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you not use input.slice in the native implementation as well to keep the two as close as possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input.slice is a function that is specific to byte_array, so we have to define another slice one for the native hash_buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain where the -1 comes from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we choose the rate to be t-1 (so 3 field elements) and our capacity to be 1. Rate and capacity should always add to t.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok, can you add a comment above please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed with Kesha, this is fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this is not done in native implementation either but we should cite where we got this 5 from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't it say that d=5 is the smallest element such that gcd(d, p-1) = 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh my bad was looking somewhere else in the native implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should you not create external/internal gates for the first linear layer as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, if i understand correctly, you create them at the first iteration of the foor loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first linear layer is just an external matrix mul, and the gates are created in
initial_external_matrix_multiplication
, not in the for loop.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could just create normal add gate here instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this got moved to a different line for some reason, but it was referring to the rounds which only use 3 of the wires and only do an add (gates 4 and 6)