zkcrypto / jubjub

Implementation of the Jubjub elliptic curve group
Other
122 stars 48 forks source link

Point negation is not homomorphic? #64

Open davidrusu opened 3 months ago

davidrusu commented 3 months ago

I was running into some issues with broken homomorphic pederson commitments when I subtracted point values. I've reduced it down to this case:

Am I holding it wrong?

    #[test]
    fn test_negation_homomorphism() {
        use group::Group;

        let x = jubjub::Scalar::from(10);
        let g = jubjub::ExtendedPoint::generator();

        assert_eq!(-g * x, g * (-x));
    }

Output

assertion `left == right` failed
  left: ExtendedPoint { u: 0x197db5e33c8ad9e1083ac27de3533eaa9d36d097ff497c7a1aefbe6fd79e970a, v: 0x014240fbe01fc832a8db3b31dad62b3342b6577af812708bd4eb037e9ec18940, z: 0x21de9d247dfd4612f2180b944f760b04804a148e7f6d07b1ac2a703c14c08366, t1: 0x1ad0c3e44043bd97c832b7bb93f2528a818acb98ed64023e0d9be596ea354ee1, t2: 0x6c9dfbcc6cb85642265607a563bb90395c824e73d5986d4571fd80f25570b146 }
 right: ExtendedPoint { u: 0x2cbdca09b095734a2f37e363921cfe8bb4c579dc5120aab8bb068c9ad310f0cc, v: 0x03ceb419990ddd670da7d465709e0b15a47f39ea6e8a59d351a57ebf47a5d9d6, z: 0x455532f07662b915a30e7fe0e12d6afde9d45d31ddebcc1d5dd3af8255ad5329, t1: 0x69031d715b41e5931ce23214aee1d100ccdd5421cf77f57708e90c6a5ed5837b, t2: 0x45fc30f281451a660751e6a3972364cc190b56721923afb370fa323452516884 }
Nashtare commented 3 months ago

That is because here g is an element of the full group, not the prime subgroup, although the Group trait emphasizes that the generator() method should be returning an element of the prime subgroup which is quite confusing given the current implementation of Jubjub.

You can see for instance in the implementation of Group for SubgroupPoint (an actual representation of the prime subgroup) that we explicitely call ExtendedPoint::generator() and then clear the cofactor, to end up with a torsion free element.

Here, rather than clarifying the documentation, I'd suggest updating the implementation of the Group trait for ExtendedPoint, to perform what the trait is expecting, i.e. outputting a prime-order generator of the associated subgroup (of scalar field Fr).

davidrusu commented 3 months ago

Aha! got you.

Is there any reason to provide a Group implementation for ExtendedPoint if we don't change the scalar field? After it's fixed, won't it be essentially identical to the Group impl. for SubgroupPoint?

At that point we could simply drop Group for ExtendedPoint and guide folks to use SubgroupPoint.