Implement Elgamal encryption. #13
Conversation
Add benchamrking. Add support for range proofs. Some refactoring.
ParnianAlimi
requested review from
JesseALancaster,
fmiguelgarcia,
ajivanyan,
adamdossa and
a user
|
@ParnianAlimi , I see the encrypt function takes the witness as a parameter. For the application user, it would be more straightforward to provide only the "message" and the "public key", while the encrypt function would take care of generating the randomness. I think it would be better to have some "Encryptor" object initialized once by "RNG" which can encrypt multiple messages. The "Decryptor" object should be initialized by the secret key once, after which it will be able to decrypt multiple ciphertexts. |
Addresses CRYP-70.
ghost
mentioned this pull request
Add benchamrking. Add support for range proofs. Some refactoring.
| CipherText { x, y } | ||
| } | ||
|
|
||
| pub fn encrypt_value(&self, value: u32) -> Result<CipherText, AssetProofError> { |
There was a problem hiding this comment.
@ajivanyan I added this function to address your comment about simplifying the encrypt function to take a value instead of a commitment witness.
…rk/cryptography into CRYP-32/elgamal_encryption
| pub fn new(value: u32, blinding: Scalar) -> Result<CommitmentWitness, AssetProofError> { | ||
| // Since Elgamal decryption requires brute forcing over all possible values, | ||
| // we limit the values to 32-bit integers. | ||
| if value >= u32::max_value() { |
There was a problem hiding this comment.
Would this condition ever be true? value is u32 so it should not go over u32:max_value()
Having said that, it is probably good to have it in case we change value to be u64, etc later on.
There was a problem hiding this comment.
Yeah, exactly. From the conversions I've had with the core team, it looks like a token's value (or an account balance) can be up to 10^12 ~ 2^40, and we might limit the size of confidential tokens even further, so depending on what that value will be in the future this check will change, but I wanted to keep it here so the API would look the same (i.e. return an error if the inputted value isn't in the range) even if change the limit.
| impl ElgamalPublicKey { | ||
| pub fn encrypt(&self, witness: CommitmentWitness) -> CipherText { | ||
| let x = witness.blinding * self.pub_key; | ||
| let gens = PedersenGens::default(); |
There was a problem hiding this comment.
iirc, in Pedersen Commitment, h = g^s and s is known which causes problem for us. Since in the twisted ElGamal we are assuming that h is chosen orthogonally to g (hence the brute-forcing in the decryption). I am not sure how the bulletproof implementation of Pedersen works, but just by looking at its name, it looks like the original Pedersen implementation.
There was a problem hiding this comment.
I maybe misunderstanding the security property here. @ajivanyan does it matter if we know s for h=g^s ?
There was a problem hiding this comment.
Here's our setting, the bulletproof is proving that the plain text (value) is within a range by returning a commitment to it and a proof. @ajivanyan has picked the twisted Elgamal encryption so that this bulletproof's commitment is the same as the second part of the Elgamal encryption.
Here the Pedersen commitment is of the form s *g + blinding*h (s being the secret). Based on what I've learned so far Pedersen commitments can have many forms. Also see my comment on line 126 for a bit more context.
There was a problem hiding this comment.
Right, no argue with that :)
My concern was the relation of g and h. From line 126,
Y := blinding_factor * g + value * h since g and h are from the same group, we have h = g * s.
My question is if one knows s, would that affect the security of the protocols or not.
EDIT: since the formulas are in elliptic curve, changed the h and g relation for better readability.
| } | ||
|
|
||
| pub fn get_public_key(&self) -> ElgamalPublicKey { | ||
| let gens = PedersenGens::default(); |
There was a problem hiding this comment.
I am not well-versed in rust yet: I am seeing a couple of calls to PedersenGens::defaults() throughout the code, will they always return the same value? Asking this since G,g,h,q should be the same during the encrypt and decrypt phase (these are what I assume this generator returns).
There was a problem hiding this comment.
Yes, PedersenGens::defaults() returns the default bulletproofs' Pedersen generators: https://github.com/dalek-cryptography/bulletproofs/blob/main/src/generators.rs#L45. The first one is the Ristretto base and the other one is the SHA3 hash of the first one converted to a Ristretto point. So the default will always be the same. That's a very valid concern :)
* Implement Elgamal encryption. Add benchamrking. Add support for range proofs. Some refactoring. * Add the nightly flag to instructions. (#14) Addresses CRYP-70. * Update README for nightly build of wasm * add wasm-bindgen-test instructions * run existing tests in wasm mode as well * Address all review comments. * Implement Elgamal encryption. Add benchamrking. Add support for range proofs. Some refactoring. * Address all review comments. * Switch to zeroize. Tag unit tests with Co-authored-by: Arash Afshar <arash@polymath.network> Co-authored-by: Arash Afshar <63250695+aafshar-poly@users.noreply.github.com>
* Refactor the CIL CLI * CDDClaimData -> CddClaimData
Addresses CRYP-20, CRYP-31, CRYP-32, CRYP-33.
Add benchmarking.
Add support for range proofs and homomorphic operations.
Some refactoring.