-
Notifications
You must be signed in to change notification settings - Fork 89
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
Implementation of CryptographicSponge
for Merlin
#136
Conversation
1159d89
to
864b9fd
Compare
864b9fd
to
746e62e
Compare
I had to remove the redundant imports since |
@autquis ah yes see also this PR: arkworks-rs/algebra#790 to tackle the same sort of issues in algebra Edit: I actually wonder why this works, given that similar changes in poly commit failed no-std |
@mmagician I didn't understand what I should fix in arkworks-rs/algebra#790? As it is merged already. Why it works: I guess because it is not completely
|
What I meant is that the algebra PR addressed similar issues. |
A gentle ping :) |
Also @mmaker since I'm tagging you in a few places now, maybe you could also take a peek at this? |
CI failure is due to this change probably rust-lang/rust#121752 |
@autquis Yes sorry, I had to update the branch, I'll fix it |
Took a quick peek! The PR looks okay, but there isn't much to say. I'd personally encourage your to add a few lines of documentation so that people are well-aware of the limitations of this trait, especially concerning the statement serialization (the user is on their own) and the bit vs bytes part. That said, I'm happy to see that people are starting to need something that works both over bytes and algebraic sponges :) |
Using Merlin, we have the possibility of having a sponge that is not generic on a field.
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pending
section inCHANGELOG.md
Files changed
in the Github PR explorer