Implementation of `CryptographicSponge` for Merlin

arkworks-rs / crypto-primitives

Interfaces and implementations of cryptographic primitives, along with R1CS constraints for them

https://www.arkworks.rs

Apache License 2.0

165 stars 79 forks source link

Implementation of `CryptographicSponge` for Merlin #136

Open autquis opened 7 months ago

autquis commented 7 months ago

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.

[ ] Targeted PR against correct branch (main)
[ ] Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
[ ] Wrote unit tests
[ ] Updated relevant documentation in the code
[ ] Added a relevant changelog entry to the Pending section in CHANGELOG.md
[ ] Re-reviewed Files changed in the Github PR explorer

mmagician commented 6 months ago

@hdevalence

autquis commented 5 months ago

I had to remove the redundant imports since cargo +nightly check complained. Specifically, Vec is in the prelude and is redundant to import.

mmagician commented 5 months ago

@autquis ah yes see also this PR: https://github.com/arkworks-rs/algebra/pull/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

autquis commented 5 months ago

@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 no-default-features and r1cs is enabled. This is the command for no_std check:

cargo build --no-default-features --features=r1cs --target aarch64-unknown-none

mmagician commented 5 months ago

What I meant is that the algebra PR addressed similar issues.

autquis commented 4 months ago

A gentle ping :)

mmagician commented 2 months ago

Also @mmaker since I'm tagging you in a few places now, maybe you could also take a peek at this?

autquis commented 2 months ago

CI failure is due to this change probably rust-lang/rust#121752 @Cesar199999 @antonio95

Cesar199999 commented 2 months ago

@autquis Yes sorry, I had to update the branch, I'll fix it

mmaker commented 2 months ago

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 :)