Remove endo from SRS

[mimoo]

the SRS struct has two endo fields endo_r and endo_q:

pub struct SRS<G: CommitmentCurve> {
    /// The vector of group elements for committing to polynomials in coefficient form
    #[serde_as(as = "Vec<o1_utils::serialization::SerdeAs>")]
    pub g: Vec<G>,
    /// A group element used for blinding commitments
    #[serde_as(as = "o1_utils::serialization::SerdeAs")]
    pub h: G,

    // TODO: the following field should be separated, as they are optimization values
    /// Commitments to Lagrange bases, per domain size
    #[serde(skip)]
    pub lagrange_bases: HashMap<usize, Vec<G>>,
    /// Coefficient for the curve endomorphism
    #[serde(skip)]
    pub endo_r: G::ScalarField,
    /// Coefficient for the curve endomorphism
    #[serde(skip)]
    pub endo_q: G::BaseField,
}

We should be able to remove these fields, as they are part of the KimchiCurve trait now: https://github.com/o1-labs/proof-systems/blob/master/kimchi/src/curve.rs#L25

[vivanraaj]

the "SRS.endo_r" and "SRS.endo_q" are used in https://github.com/o1-labs/proof-systems/blob/master/poly-commitment/src/evaluation_proof.rs#L244 and https://github.com/o1-labs/proof-systems/blob/master/poly-commitment/src/evaluation_proof.rs#L274.

does the whole system design allows the kimchi trait to be imported into the files mentioned above to provide the "endo_r" and "endo_q" variables?

[mimoo]

No because poly-commitment is a dependency of kimchi, and so you'd get a dependency cycle if you would do that.

I guess there's two solutions:

1. Moving the KimchiCurve trait to a crate that can be imported by both would fix that (for example mina-curves).
2. Pass these endo values as argument in the functions that need them

I liked solution 2 initially, as I was thinking that a poly-commitment library should only be aware of one curve, and not two, but I guess our poly-commitment might be aware of that. So maybe solution 1 is better here. cc @mrmr1993 who probably has a better opinion

[github-actions[bot]]

Stale issue message

[duguorong009]

@mimoo I have one question about the issue. In fact, the endo_q and endo_r are initialized with endos::<G>() in here. Also, this endos::<G>() is also used for KimchiCurve implementation.

Is there any need to explicitly remove the endo_q and endo_r from the SRS struct? My point is that endos::<G>() is already used everywhere.

[mimoo]

I thought I had answered this but apparently not, the point is that it doesn't really seem related to the SRS which should be a "commitment"-focused data structure (https://o1-labs.github.io/proof-systems/specs/urs.html). Semantically it doesn't seem to make sense to keep it there.

[duguorong009]

@mimoo Please review the PR #895 I created for the solution.