-
Notifications
You must be signed in to change notification settings - Fork 439
fix hacky registration of MiMC #1502
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
Conversation
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.
Looks good - I would only change the the order of import to avoid import cycles in the future.
Seems like having |
I'll check the import cycle. Maybe indeed having the |
@Tabaie - so I tried a few options and imo this looks the best. Imo having the constructor which returns the direct type Another option would have to move But - before I'm satisfied with the PR fully, I'd still want to make the hash constants more type safe than they are right now. Imo using strings could cause issues and better to use the approach from Go std lib and gnark-crypto where we have the hash functions registered by constants. I'll do it tomorrow. |
No description provided.