-
Notifications
You must be signed in to change notification settings - Fork 132
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
Simplify PolynomialCommitment
trait: remove the generic on S: CryptographicSponge
#145
Conversation
Instead, pass `&mut impl CryptographicSponge` as an argument to the functions that need it
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.
Much better, indeed!
I pushed this to make the tests pass. I can also open another PR to the master for this and then you can rebase this one on that; let me know. |
@autquis Thank you! |
It's a bit annoying with that stable/nightly discrepancy on imports |
clean up imports acc to nightly Revert "dont need to use full path" This reverts commit b272524abeaa9f7d6697539f22682bc081e3e8a5. more fmt
d93d25b
to
38ca957
Compare
@autquis I had to undo your changes, as no-std was failing. In the end I ended up following the same strategy as https://github.com/arkworks-rs/algebra/pull/790/files#diff-6035356f5adc30af6393d5492dbc1d9d8ec187272030fdd95709b1b30f1275fc |
I wonder, isn't it clenaer if we use |
That's worth exploring yes |
3e699e7
to
739345a
Compare
739345a
to
acdbaae
Compare
I added Also, I am doing a bit of cleaning up in the order of imports, as we are touching imports anyway. |
b3b04f2
to
ef199a1
Compare
Another ping. @weikengchen @Pratyush @z-tech Also, it seems the |
Gentle reminder @Pratyush |
Reminder for this @Pratyush I would appreciate it if you could have a look |
The CI build failure is puzzling; DefaultHashBuilder does implement Clone… https://docs.rs/hashbrown/latest/hashbrown/type.DefaultHashBuilder.html |
@Pratyush Thanks for checking! There were two issues:
Now CI is okay. |
a9ee2f4
to
3010903
Compare
3010903
to
2cc20f7
Compare
Just a reminder to add this to the merge queue :) @Pratyush |
Regarding the dummy doc, could you change it so that in the test modules we have |
Done! :) |
Description
Quality of life improvement for downstream users of the library.
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