monorepo

[zhenfeizhang]

This PR includes the following halo2 dependencies

• ff 0.12.0
• group 0.12.1
• pairing 0.22.0
• curves 0.2.1
• pasta_curves 0.4.1
• poseidon 0.2.0
• halo proofs 0.2.0

It removes the gadgets repo. The idea is to have a separate monorepo that holds useful gadgets.

Eventually we want to have the following structure of.
The refactoring of pasta_curves -> EC_model will be done via a separate PR.

[jonathanpwang]

What is the purpose of the pairing crate? The current pairing implementations are all in curves right?

[zhenfeizhang]

What is the purpose of the pairing crate? The current pairing implementations are all in curves right?

It defines a uniform API for pairing trait. I think it is better to put it out than within bn254 crate, in case we introduce another curve, say bls12-381 later.

[han0110]

Is the reason to vendor ff and group to modify the trait bound for Goldilocks? Or we'd like to remove some trait bounds like ConditionallySelectable or ConstantTimeEq?

One small concern comes to my mind is when we vendor ff and group, we can no longer share components among different projects if they are using zcash/halo2 and halo2-ce/halo2 respectively (unless we can implement Field for each other? but I guess it incurs conflict so we could at most implement one way).

But if we can gain a lot by owning these traits, it should be fine since for component libraries (e.g. halo2wrong) we could still have some feature tag to conditionally require dependency zcash/halo2 or halo2-ce/halo2 for dependents to choose in case they want to use zcash/halo2.

[zhenfeizhang]

Is the reason to vendor ff and group to modify the trait bound for Goldilocks? Or we'd like to remove some trait bounds like ConditionallySelectable or ConstantTimeEq?

Yes I think we will need to touch FF for Goldilocks.
I don't have a strong opinion w.r.t. ConditionallySelectable and ConstantTimeEq

One small concern comes to my mind is when we vendor ff and group, we can no longer share components among different projects if they are using zcash/halo2 and halo2-ce/halo2 respectively (unless we can implement Field for each other? but I guess it incurs conflict so we could at most implement one way).

But if we can gain a lot by owning these traits, it should be fine since for component libraries (e.g. halo2wrong) we could still have some feature tag to conditionally require dependency zcash/halo2 or halo2-ce/halo2 for dependents to choose in case they want to use zcash/halo2.

Right. I agree with the concern, and it is perhaps not practical to implement each other's trait.
At the moment I don't think there is an ideal solution since we need to touch FF for FRI.

[CPerezz]

Hey @zhenfeizhang

As @han0110 I have the same concern. And I'm not really sure why you need to touch anything in ff to implement Godilocks.
If you check https://docs.rs/ff/0.12.1/ff/trait.Field.html and https://docs.rs/ff/0.12.1/ff/trait.PrimeField.html I don't really see what can't be implemented for the Goldilocks field.

The only thing I can imagine, is the fact that you need to ADD things to the traits. But not sure why either. I would appreciate if you can justify the changes to ff crate as that's a critical thing for inter-compatibility. Specially refering to:

At the moment I don't think there is an ideal solution since we need to touch FF for FRI.

[zhenfeizhang]

Hey @zhenfeizhang

As @han0110 I have the same concern. And I'm not really sure why you need to touch anything in ff to implement Godilocks. If you check https://docs.rs/ff/0.12.1/ff/trait.Field.html and https://docs.rs/ff/0.12.1/ff/trait.PrimeField.html I don't really see what can't be implemented for the Goldilocks field.

The only thing I can imagine, is the fact that you need to ADD things to the traits. But not sure why either. I would appreciate if you can justify the changes to ff crate as that's a critical thing for inter-compatibility. Specially refering to:

At the moment I don't think there is an ideal solution since we need to touch FF for FRI.

Understood. and also agree here.

The issue was from_repr and to_repr. Goldilocks doesn't require a Montgomery form. But I guess we can always use a same format for both the field itself and its representation form.

So I am fine to remove ff and group for now.

[CPerezz]

The last question I do have is why we need our pasta_curves version here.
Any changes to the arithmetic module that we want to perform? Or it's just that since we were already doing ff and group you added pasta_curves?

[jonathanpwang]

Since pasta_curves references ff and groups, it becomes very hard to manage if you don't include it without systemic patching everywhere.

I will note that I have found it useful to change ff::PrimeField to add further serialization options such as "field element to u64 digits" or "field elements to u128 digits" and also to expose from_raw in the trait - these are more for compatibility with BigInt. You can do it by going through bytes and so far there is not serious performance overhead. But I imagine for Goldilocks you would definitely want to have functions in the trait that just return the underlying u64 element, which currently can't be done using Repr: AsRef<[u8]>.

[zhenfeizhang]

The last question I do have is why we need our pasta_curves version here. Any changes to the arithmetic module that we want to perform? Or it's just that since we were already doing ff and group you added pasta_curves?

I guess we mainly want the EC models from pasta curves, which BN254 depends on.

• pasta -> ec_model + the actual curve
• ec_model as a separate crate
• the actual curve moves to curves with bn256

makes more sense to me.

[zhenfeizhang]

So c1b57ab implements a skeleton of Goldilocks -- it turns out that we don't need to touch FF so far. @han0110 @CPerezz

I will note that I have found it useful to change ff::PrimeField to add further serialization options such as "field element to u64 digits" or "field elements to u128 digits" and also to expose from_raw in the trait - these are more for compatibility with BigInt. You can do it by going through bytes and so far there is not serious performance overhead. But I imagine for Goldilocks you would definitely want to have functions in the trait that just return the underlying u64 element, which currently can't be done using Repr: AsRef<[u8]>.

Yeah. I also prefer to modify FF directly. But since we have temporarily put a pause on this direction, one (easy, but not clean) way to get around is to expose the u64 from Goldilocks...

[CPerezz]

Since pasta_curves references ff and groups, it becomes very hard to manage if you don't include it without systemic patching everywhere.

Yep I assumed that. But just wondered if there was another thing that I was missing.

But I imagine for Goldilocks you would definitely want to have functions in the trait that just return the underlying u64 element, which currently can't be done using Repr: AsRef<[u8]>.

Nothing prevents you to do something like:

impl AsRef<u64> 
impl AsRef<[u128]>

And so on for the things that you need that for. What I try to mean here is that we can't have all of the derefs we would like. But it's also non-sensical to make them. As then bls or bn would be forced to also implement these things. And that would not make any sense at all for them.
You could create also a trait in the downstream libs in de dep tree such as Halo2Curves or whatever and define there a new trait that is required in a supertrait used in halo.
And to this trait you add what you want. THis is what we did in the zkevm-circuits library. But can be done anywhere.

trait SuperField: Field + PrimeField + TraitNeeded1 + TraitNeeded2 + ....

custom CONST
custon fn() {}

WDYT?

[jonathanpwang]

This sounds good to me.

[str4d]

Just to clarify a misunderstanding:

The issue was from_repr and to_repr. Goldilocks doesn't require a Montgomery form. But I guess we can always use a same format for both the field itself and its representation form.

PrimeField::{from_repr, to_repr} are APIs for moving between the in-memory form (Self) and the encoded representation. That is, these could be equivalently named from_bytes and to_bytes. The reason for the PrimeField::Repr associated type is Rust language limitations; for type safety what we want the return type of PrimeField::to_repr to be is [u8; N], but Rust doesn't yet support a trait method returning a type containing an associated constant (like [u8; Self::REPR_LEN]), so we instead set type Repr = [u8; N] if N <= 32, or use a wrapper type otherwise.