Refactor curve and pairing traits

[Pratyush]

Description

closes:

• Improve ec traits. #294
• Change GroupAffine/GroupProjective to maintain invariants #268
• Simplify Curve API #41
• Clarify guarantees in traits around point validity #27
• Split up affine and projective files #97

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (master)
• Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the GitHub PR explorer

[Pratyush]

This is blocked on merging #446

[mmagician]

Suggested change

	pub(crate) type EllCoeff<P> = (
	Fp2<<P as Bls12Parameters>::Fp2Config>,
	Fp2<<P as Bls12Parameters>::Fp2Config>,
	Fp2<<P as Bls12Parameters>::Fp2Config>,
	);
	type F2<P> = Fp2<<P as Bls12Parameters>::Fp2Config>;
	pub(crate) type EllCoeff<P> = (F2<P>, F2<P>, F2<P>);

[mmagician]

maybe we can introduce another local type here?

[weikengchen]

I think this is a small change that we may just skip---we now have to be selective.

The main issue here is probably the naming of this local type. F2 could be confusing, and maybe the easiest way is just to not create this local type since it is a one-time use.

[mmagician]

I think I understand the reason for naming it multi_miller_loop (base & parallel configuration squashed into one?), but it could be confusing - it almost sounds like multiple complete runs of Miller loop are computed. Why not stick to the old miller_loop?

[weikengchen]

I think it is because the implementation here does not strictly follow what a Miller loop should be.

We often say Miller loop as f_{T, P)(Q) or its variants. In this case, we are actually doing the following, rather than a common Miller loop.
f_{T, P_1}(Q_1) \cdot f_{T, P_2}(Q_2) \cdot ...

The difference will be evident when one does a large number of Miller loops, likely in the case of batch verification of zero-knowledge proofs in order to improve the TPS, because of the parallelization support here.

[Pratyush]

we could also call this batch_miller_loop and the corresponding pairing method batch_pairing. Does that sound better?

[weikengchen]

Batching usually refers to checking a bunch, likely involving a randomizer. In this Miller loop we just add them together.

[mmagician]

Are you wrapping TargetField in another struct just to be able to avoid Rust's orphan rule, or is there an underlying API design decision?
It seems somewhat awkward that we're mixing a very high-level API names (MillerLoopOutput) with names of algebraic structures in the miller loop method signature. For consistency, I'd suggest either:

    fn multi_miller_loop(
        a: impl IntoIterator<Item = impl Into<Self::MillerLoopInputA>>,
        b: impl IntoIterator<Item = impl Into<Self::MillerLoopInputB>>,
    ) -> MillerLoopOutput<Self>;

or something along the lines of:

    fn multi_miller_loop(
        a: ...G1Prepared...,
        b: ...G2Prepared...,
    ) -> FqkEqClass;

of which I'd go for the latter, though maybe FqkEqClass is a bit too explicit? I don't think any of our cuves implement the Weil pairing though, so that should always be the output of the miller loop.

Same for PairingOutput, maybe Gt instead?

[weikengchen]

I think FqkEqClass is unlikely going to be used in another place, and for one-time use, I prefer to dismiss it.

The output is an element in Fqk, we just need to do a final exp after that. Note that we do not always use it just for equality check. Pairing could also be used, without a equality check in the middle, for group commitments.

For this reason, I would not recommend FqkEqClass.

[weikengchen]

By the way, I think dedicating a struct for this is good MillerLoopOutput as it prevents developers from forgetting to do final exp.

For this purpose, I recommend we drop the derivation of PartialEq, Eq, PartialOrd, Ord.

[Pratyush]

I'd be fine with MillerLoopInput1, but that it seems a bit less informative than G1Prepared.

The reason to have MillerLoopOutput be not just Self::TargetField is to make it explicit that it's not meant to be used by itself.

[mmagician]

this pattern of impl<> From<> for G1Prepared<> repeats in each model - could we maybe have a macro to generate this impls?

[weikengchen]

Good points. And it seems that only BN, BLS, BW also implements is_zero, but not MNT46. Any reason why?

[Pratyush]

It depends entirely on what the corresponding pairing implementations require; the MNT46 impls don't seem to require that method.

[Pratyush]

I think this PR is good to merge. I have some more things I'd like to add, but let's do that in a followup PR.

[weikengchen]

I think we can merge. I probably will fix the downstream in a few days.