Remove all calls to pedersen from TS #435
Comments
|
It makes sense to start doing the task once #340 is merged |
ludamad
pushed a commit
that referenced
this issue
Jul 14, 2023
--------- Co-authored-by: zac-williamson <[email protected]>
|
I checked and the direct calls to pedersen are mostly used in the TS implementations of merkle trees. Given that we will eventually replace them with C++ version I think it doesn't make sense to remove the pedersen calls there. Other than that, there are only a few places out of trees where we do the direct pedersen calls (most of them has already been replaced with specific WASM version). I will pick this up once again once the |
4 tasks
PhilWindle
pushed a commit
that referenced
this issue
Oct 11, 2023
Fixes #435 This PR doesn't really address the original issue that much. I have reasons for this: 1. Most of the direct pedersen calls are used by the merkle tress. Given that the merkle trees will be fully replaced by an application in rust or C++ it doesn't make sense to invest time in that code. 2. The remaining calls are performed in tests where we check whether some hash corresponds to the one computed in Noir. I feel like it doesn't really make sense to create specific bindings for tests. But I don't really have a strong opinion on this. I marked the pedersen functions as deprecated with a message to not use it in production code. I think that's the best way to go about this at this point. # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [ ] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [ ] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [ ] Every change is related to the PR description. - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist).
codygunton
pushed a commit
that referenced
this issue
Jan 23, 2024
--------- Co-authored-by: zac-williamson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There currently exist numerous places throughout the codebase where hashing is performed and there is no consistent way of doing it. Some places just call a pedersen method directly either passing in a generator index or not. Other places we call a specific WASM function for the required hash (e.g. computeFunctionLeaf). We should adopt the latter model and have a specific, appropriately named WASM function for every hash that is needed by TS. This will keep the codebase more maintainable and ensures that all decisions around generators etc are localised to the circuits.
The text was updated successfully, but these errors were encountered: