Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-export Field/Point types used in curve cycle implementations #271

Open
huitseeker opened this issue Nov 23, 2023 · 9 comments
Open

Re-export Field/Point types used in curve cycle implementations #271

huitseeker opened this issue Nov 23, 2023 · 9 comments

Comments

@huitseeker
Copy link
Contributor

#267 made the provider modules, notably the two field types and the two point types for each curve cycle, private, only exposing the Engine.

However, downstream users of nova may eventually need to use a distinct version of the libraries Nova uses for those providers, and should they not be able to exactly match that version, they would no longer have a handle on those field and point types, which would make them incapable of implementing traits on them, etc.

See https://www.lurklurk.org/effective-rust/re-export.html for a comprehensive description of this issue.

A good practice would be to re-export either the entire dependency crates (pasta_curves, halo2curves) or the four core types used in the provider module. The bn256_grumpkin cycle, for instance, forms a handy set of aliases (aimed at making the halo2curves types amenable to the nova curves macros) that could be re-exported:

pub mod bn256 {
pub use halo2curves::bn256::{Fq as Base, Fr as Scalar, G1Affine as Affine, G1 as Point};
}

@srinathsetty
Copy link
Collaborator

Hi @huitseeker,

I must be missing something. The public interface of Nova does not involve the Point type. Isn't it? The step circuit that upstream needs to pass to Nova is defined only over a prime field that must match an associate type (Scalar) in Engine trait. Isn't it?

@huitseeker
Copy link
Contributor Author

huitseeker commented Dec 1, 2023

The encapsulation of all group types (notably those used in public parameters), constraining their generation to either of:

  • using the provided Nova methods (not always computationally optimal), or
  • type-unsafe methods (e.g. byte-mimicry followed by deserialization),

is in my opinion suboptimal. But that can certainly be tabled to a distinct discussion, as I suspect issues like #270 will force us to tackle that.

We don't even need the point types to show the current problem:
https://gist.github.com/huitseeker/cc0567e62b53828d0581c83241a6b6ac

@srinathsetty
Copy link
Collaborator

Hi @huitseeker,

I suspect issues like #270 will force us to tackle that.

I'm not sure this requires exposing Point type. The way I was envisioning it would be implemented inside the Provider module (or whichever code provides provider trait implementations). It can be curve specific. For example, similar to from_label, there could be another trait method (or a method in an entirely new trait) that can be implemented for each curve supported. So generic code like mlkzg.rs can just call that method whenever the particular trait is implemented for a curve cycle.

We don't even need the point types to show the current problem:
https://gist.github.com/huitseeker/cc0567e62b53828d0581c83241a6b6ac

In this particular example, when including halo2curves 0.4.0, the code works. Isn't it? Isn't this the right behavior?

As another instance, unless Nova includes the same version of ff as neptune or bellpepper (or several other dependencies of Nova), nova will not compile (ff is a dependency for all of them). Isn't requiring the right halo2curves the same requirement as in those cases? Perhaps, I'm missing something. If so, please clarify.

@huitseeker
Copy link
Contributor Author

In this particular example, when including halo2curves 0.4.0, the code works. Isn't it?

It does.

Isn't this the right behavior?

It is, unless the crate I'm using (nova_using) requires importing a different version of halo2curves (e.g. "0.5.0") for other reasons. Or if said crate wants to upgrade to halo2curves v0.5.0 without waiting for Nova to have done so. It would be particularly useful to have access to the specific Scalar::from_bytes method that Nova expects, in order to feed it the type it requires.

As another instance, unless Nova includes the same version of ff as neptune or bellpepper (or several other dependencies of Nova), nova will not compile (ff is a dependency for all of them). Isn't requiring the right halo2curves the same requirement as in those cases?

It is the same problem, and indeed, upgrades of the ff crate tend to go very slow, as crates are typically blocked in upgrading their version of ff until the last of their (transitive-)dependencies does.

Re-exports manage that pain: note halo2curves re-exports their version of ff. The glacial pace of ff development means it's likely one could find crates that all use the same version of ff. This isn't the case today with halo2curves.

huitseeker pushed a commit to huitseeker/Nova that referenced this issue Jan 24, 2024
@srinathsetty
Copy link
Collaborator

It would be particularly useful to have access to the specific Scalar::from_bytes method that Nova expects, in order to feed it the type it requires.

I'm still confused about this comment. Where exactly does Nova expects from_bytes method?

@huitseeker
Copy link
Contributor Author

huitseeker commented Feb 28, 2024

@srinathsetty the turn of phrase is unfortunate, what I meant is

It would be particularly useful to have access1 to the specific Scalar::from_bytes method so that a dependent crate can feed Nova the specific types it requires.

What Nova expects is the type: Scalars (among others) of an opaque (from the point of view of a Nova dependent) version of the pasta/halo2curves crates. Absent a re-export, the dependent will happen to be able to replicate those types if and only if its versions of those types coincide with those Nova uses.

On the other hand, if Nova re-exports its dependencies, then the dependent, as long as it has access to bytes, can always deserialize data to the particular types Nova expects, using bytes-to-type conversion methods such as Scalar::from_bytes, through the re-export provided by Nova.

Footnotes

  1. through a re-export.

@srinathsetty
Copy link
Collaborator

Thanks for the details! IIUC, this only requires exporting Scalar types, not the point types used internally by Nova, right?

@huitseeker
Copy link
Contributor Author

IIUC, this only requires exporting Scalar types, not the point types used internally by Nova, right?

This only requires re-exporting types from halo2curves (and pasta_curves) that appear in Nova's public APIs.

@srinathsetty
Copy link
Collaborator

Addressed in #343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants