zkcrypto / group

Elliptic curve group traits and utilities.
Other
91 stars 34 forks source link

Add traits and APIs for dealing with curve coordinates #30

Open str4d opened 2 years ago

str4d commented 2 years ago

The original motivation for removing the Base associated type from the old CurveProjective and CurveAffine traits in https://github.com/zkcrypto/group/pull/7/commits/15bc62823c192af69d63fa5cc3f2439acd01ece5 was that dealing in coordinates directly is almost always incorrect for cryptographic protocols, so we didn't expose them (instead leaving it to the concrete curve impls to handle such needs themselves). And if we don't expose the coordinates anywhere, then there is no reason to expose the base field (the associated type was completely unused at the time).

However, in proving system contexts it is necessary to have access to the coordinates in order to re-implement EC arithmetic inside a circuit. My original plan at the time of the above refactor was to move such curve implementations concretely into the curve crates themselves, but that leads to some awkward dependency tree management (we'd need to split crates like bellman and halo2_proofs up in a way that enabled this). This is why the pasta_curves crate currently exposes the base field and coordinates APIs via a CurveExt trait, but we explicitly want to make that trait obsolete by upstreaming its functionality here (https://github.com/zcash/pasta_curves/issues/41), so we need to develop a solution for it.

str4d commented 2 years ago

We have a few things we need to expose:

str4d commented 2 years ago

Here's the current trait heirarchy:

graph RL
    subgraph external
        standard[Clone, Copy, Debug, Eq, Sized, Send, Sync]
        additive[Add, Sub, AddAssign, SubAssign]
        multiplicative[Mul, MulAssign]
        Neg
        Sum
    end

    subgraph ops
        GroupOps --> additive
        GroupOpsOwned -->|ref| GroupOps
        ScalarMul --> multiplicative
        ScalarMulOwned -->|ref| ScalarMul
    end

    subgraph group
        Group --> standard
        Group --> Sum
        Group --> Neg
        Group --> GroupOps
        Group --> GroupOpsOwned
        Group --> ScalarMul
        Group --> ScalarMulOwned

        GroupEncoding

        WnafGroup --> Group
    end

    subgraph curve
        Curve --> Group
        Curve -->|affine| GroupOps
        Curve -->|affine| GroupOpsOwned
    end

    subgraph prime
        PrimeGroup --> Group
        PrimeGroup --> GroupEncoding

        PrimeCurve --> Curve
        PrimeCurve --> PrimeGroup

        PrimeCurveAffine --> standard
        PrimeCurveAffine --> GroupEncoding
        PrimeCurveAffine --> Neg
        PrimeCurveAffine -->|scalar| multiplicative

        PrimeCurve -.- PrimeCurveAffine
    end

    subgraph cofactor
        CofactorGroup --> Group
        CofactorGroup --> GroupEncoding
        CofactorGroup -->|subgroup| GroupOps
        CofactorGroup -->|subgroup| GroupOpsOwned

        CofactorCurve --> Curve
        CofactorCurve --> CofactorGroup

        CofactorCurveAffine --> standard
        CofactorCurveAffine --> GroupEncoding
        CofactorCurveAffine --> Neg
        CofactorCurveAffine -->|scalar| multiplicative

        CofactorCurve -.- CofactorCurveAffine
    end

If we add the coordinate access APIs to affine representations, then we'd need to add them to both PrimeCurveAffine and CofactorCurveAffine. That's a bit annoying, as these traits are already somewhat duplicative and this would continue to grow them. At the time I implemented the refactor to the above trait graph, I believe I couldn't get the Rust type system to play ball with a CurveAffine trait, but it would be worth attempting this again. It would alter the graph to:

graph RL
    subgraph external
        standard[Clone, Copy, Debug, Eq, Sized, Send, Sync]
        additive[Add, Sub, AddAssign, SubAssign]
        multiplicative[Mul, MulAssign]
        Neg
        Sum
    end

    subgraph ops
        GroupOps --> additive
        GroupOpsOwned -->|ref| GroupOps
        ScalarMul --> multiplicative
        ScalarMulOwned -->|ref| ScalarMul
    end

    subgraph group
        Group --> standard
        Group --> Sum
        Group --> Neg
        Group --> GroupOps
        Group --> GroupOpsOwned
        Group --> ScalarMul
        Group --> ScalarMulOwned

        GroupEncoding

        WnafGroup --> Group
    end

    subgraph curve
        Curve --> Group
        Curve -->|affine| GroupOps
        Curve -->|affine| GroupOpsOwned

        CurveAffine --> standard
        CurveAffine --> GroupEncoding
        CurveAffine --> Neg
        CurveAffine -->|scalar| multiplicative

        Curve -.- CurveAffine
    end

    subgraph prime
        PrimeGroup --> Group
        PrimeGroup --> GroupEncoding

        PrimeCurve --> Curve
        PrimeCurve --> PrimeGroup

        PrimeCurveAffine --> CurveAffine

        PrimeCurve -.- PrimeCurveAffine
    end

    subgraph cofactor
        CofactorGroup --> Group
        CofactorGroup --> GroupEncoding
        CofactorGroup -->|subgroup| GroupOps
        CofactorGroup -->|subgroup| GroupOpsOwned

        CofactorCurve --> Curve
        CofactorCurve --> CofactorGroup

        CofactorCurveAffine --> CurveAffine

        CofactorCurve -.- CofactorCurveAffine
    end

We might be able to avoid the extra bidirectional associated types on CofactorCurve and CofactorCurveAffine by leveraging the ones on Curve and CurveAffine, but I seem to recall trying that in the original refactor and running into issues with viral where bounds. We can give it another go, or we can just come up with more creative associated type names.

Alternatively, we could define a CurveCoordinates trait to encapsulate the new functionality, and express it like so:

graph RL
    subgraph external
        standard[Clone, Copy, Debug, Eq, Sized, Send, Sync]
        additive[Add, Sub, AddAssign, SubAssign]
        multiplicative[Mul, MulAssign]
        Neg
        Sum
    end

    subgraph ops
        GroupOps --> additive
        GroupOpsOwned -->|ref| GroupOps
        ScalarMul --> multiplicative
        ScalarMulOwned -->|ref| ScalarMul
    end

    subgraph group
        Group --> standard
        Group --> Sum
        Group --> Neg
        Group --> GroupOps
        Group --> GroupOpsOwned
        Group --> ScalarMul
        Group --> ScalarMulOwned

        GroupEncoding

        WnafGroup --> Group
    end

    subgraph curve
        Curve --> Group
        Curve -->|affine| GroupOps
        Curve -->|affine| GroupOpsOwned

        CurveCoordinates
    end

    subgraph prime
        PrimeGroup --> Group
        PrimeGroup --> GroupEncoding

        PrimeCurve --> Curve
        PrimeCurve --> PrimeGroup

        PrimeCurveAffine --> standard
        PrimeCurveAffine --> GroupEncoding
        PrimeCurveAffine --> Neg
        PrimeCurveAffine -->|scalar| multiplicative
        PrimeCurveAffine --> CurveCoordinates

        PrimeCurve -.- PrimeCurveAffine
    end

    subgraph cofactor
        CofactorGroup --> Group
        CofactorGroup --> GroupEncoding
        CofactorGroup -->|subgroup| GroupOps
        CofactorGroup -->|subgroup| GroupOpsOwned

        CofactorCurve --> Curve
        CofactorCurve --> CofactorGroup

        CofactorCurveAffine --> standard
        CofactorCurveAffine --> GroupEncoding
        CofactorCurveAffine --> Neg
        CofactorCurveAffine -->|scalar| multiplicative
        CofactorCurveAffine --> CurveCoordinates

        CofactorCurve -.- CofactorCurveAffine
    end
tarcieri commented 2 years ago

Some related issues:

tarcieri commented 2 years ago

As a general comment, supporting exotic point encodings and making the existing implementations generic and reusable have been the main reasons for having some form of coordinate access in the https://github.com/RustCrypto/elliptic-curves crates.

So far our solution has been to impl these encodings in the respective crates where they can access private field members and FieldElement types. This has resulted in something of a maintenance burden and makes it look like we offer first-class support for some experimental and not-fully-standardized constructions like https://datatracker.ietf.org/doc/html/draft-jivsov-ecc-compact-05

str4d commented 1 year ago

I was able to implement a CurveAffine trait in #48, which means we can look more at the graph above corresponding to that approach. I'm still not sure though if a CurveAffine trait on its own is sufficient here, or if we should still have a standalone CurveCoordinates or similar.

We definitely want to introduce some concept of a curve model into whatever types or traits we build (https://github.com/zkcrypto/group/pull/29#issuecomment-1240549988). Doing so would help with https://github.com/zcash/pasta_curves/issues/41 as it would be a place we could easily expose the curve constants.

tarcieri commented 1 year ago

In the case of https://github.com/dalek-cryptography/curve25519-dalek/pull/553 we have a request for obtaining extended twisted Edwards coordinates (i.e. X/Y/Z/T). That may be esoteric enough a trait-based abstraction won't help there.

The main other use cases for this we have for @RustCrypto are all accessing affine coordinates, almost all of which are implementations of various wire formats for encoding those coordinates e.g. compact encoding, Elligator (Squared)

str4d commented 1 year ago

I'm not particularly interested in generically exposing the projective coordinates. We do have a use case for it in pasta_curves, but that's to implement hash-to-curve, which would be better suited by a separate trait or helper for implementing that specifically.

ycscaly commented 1 year ago

I've been asked by @tarcieri to move the discussion from elliptic-curves/#937 to here.

I am writing a Schnorr protocol and need access to the projective coordinates. Well, actually, I don't care about the coordinates per-se but some encoding of the projective point that I can use for commitment/decommitment, and to use on transcript.

Now, my only option is to switch to the affine coordinates. However, this is extremely costly in computation (e.g., in k256 it costs around 10 microseconds.) When writing a proof system that should handle O(1000) provers and O(1000) statements, this becomes a big deal. Actually, my proof system is aggregatable, and the base operations are orders of magnitude cheaper: scalar ops in k256 are in nanoseconds, whereas .to_affine() is in micro, which caused a much surprising bottleneck to the surface during commitment/transcript phases... imagine debugging and reaching that conclusion lol.

So, I need a way to uniquely encode projective points directly; and be able to transition in and out from those values and into a ProjectivePoint structure (I guess here, I need an Into<Vec<u8>> or actually a to_repr() method for Group, and a way to instantiate group elements using that same encoding.

Tony also asked me whether I care about other affine points mapping to the same projective point. I don't. Malicious provers won't be able to use that to their advantage, as they would be committing to the same value that the verifier would be using.

Thoughts on this?

Thanks.