Skip to content
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

Add ElGamal cipher #161

Merged
merged 10 commits into from
May 7, 2024
Merged

Add ElGamal cipher #161

merged 10 commits into from
May 7, 2024

Conversation

xevisalle
Copy link
Member

@xevisalle xevisalle commented Apr 29, 2024

Resolves: #162

@xevisalle xevisalle linked an issue Apr 29, 2024 that may be closed by this pull request
Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how these aes::encrypt and elgamal::decrypt now come together :)

One comment on the naming side: I prefer to name zk_encrypt -> encrypt_gadget since we are using that convention across our stack.

And I was wondering why there is only a zk_encrypt? Are we not decrypting in the circuit?

tests/encryption.rs Outdated Show resolved Hide resolved
miloszm
miloszm previously requested changes May 1, 2024
Cargo.toml Show resolved Hide resolved
@xevisalle
Copy link
Member Author

I like how these aes::encrypt and elgamal::decrypt now come together :)

One comment on the naming side: I prefer to name zk_encrypt -> encrypt_gadget since we are using that convention across our stack.

And I was wondering why there is only a zk_encrypt? Are we not decrypting in the circuit?

@marta-belles do you think it makes sense to have a decryption method in-circuit? I didn't implement it cause we don't need it, but I guess it doesn't hurt to have it just in case.

@xevisalle xevisalle requested a review from miloszm May 2, 2024 15:25
@marta-belles
Copy link
Contributor

I like how these aes::encrypt and elgamal::decrypt now come together :)
One comment on the naming side: I prefer to name zk_encrypt -> encrypt_gadget since we are using that convention across our stack.
And I was wondering why there is only a zk_encrypt? Are we not decrypting in the circuit?

@marta-belles do you think it makes sense to have a decryption method in-circuit? I didn't implement it cause we don't need it, but I guess it doesn't hurt to have it just in case.

I can't think of a case that we will prove decryption in-circuit, but there is no harm in having it there for completeness:)

src/encryption/elgamal.rs Show resolved Hide resolved
src/encryption/elgamal.rs Show resolved Hide resolved
@xevisalle xevisalle dismissed miloszm’s stale review May 3, 2024 14:34

Dismissed as it does not apply.

@xevisalle xevisalle requested a review from moCello May 3, 2024 14:34
@xevisalle xevisalle merged commit b6c51a1 into master May 7, 2024
5 checks passed
@xevisalle xevisalle deleted the elgamal branch June 6, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ElGamal cipher
4 participants