search

dalek-cryptography / bulletproofs

A pure-Rust implementation of Bulletproofs using Ristretto.
MIT License
1.02k stars 218 forks source link

fix identity check with CompressedRistretto #267

Closed lovesh closed 5 years ago

lovesh commented 5 years ago

Signed-off-by: lovesh harchandani lovesh.bond@gmail.com

isislovecruft commented 5 years ago

Hi @lovesh, thanks for the PR! I'm not sure I see what this fixes.. could you elaborate more what the problem is?

lovesh commented 5 years ago

Hi @isislovecruft, the trait IsIdentity is not implemented for CompressedRistretto. Hence, when i run cargo +nightly check --all-features on develop branch i get this error 

error[E0599]: no method named `is_identity` found for type `curve25519_dalek::ristretto::CompressedRistretto` in the current scope
   --> src/r1cs/proof.rs:124:19
    |
124 |         self.A_I2.is_identity() && self.A_O2.is_identity() && self.S2.is_identity()
    |                   ^^^^^^^^^^^
    |
    = note: the method `is_identity` exists but the following trait bounds were not satisfied:
            `curve25519_dalek::ristretto::CompressedRistretto : curve25519_dalek::traits::IsIdentity`

Now i could either implement IsIdentity for CompressedRistretto or do the equality comparison with the identity element. I went with the latter as it was easier and less disruptive.

hdevalence commented 5 years ago

Hi @lovesh, try running cargo update? Thanks for giving some more context on the problem you ran into.

There was a minor version update to curve25519-dalek that added that trait impl. The bulletproofs Cargo.toml should have specified 1.0.3 instead of 1, but by mistake this wasn't done in the develop branch (because when the code was added, Cargo resolved the dependency to a compatible version). This problem was fixed in the main branch and the fix will get ported to the develop branch soon. In the meantime, running cargo update should push the dependency to a compatible version.

lovesh commented 5 years ago

Hi @hdevalence, thanks for the suggestion. It works now.