search

privacy-scaling-explorations / halo2

https://privacy-scaling-explorations.github.io/halo2/
Other
201 stars 121 forks source link

reexport `halo2curves` in `halo2_middleware` so users don't need it as explicit dep #323

Closed Vindaar closed 4 months ago

Vindaar commented 4 months ago

This way users of halo2_middleware don't need to depend explicitly on halo2curves. Relevant context: https://github.com/mratsim/constantine/pull/377

Q: Given that all the other modules part of the workspace depend on halo2_middleware, should one update the Cargo.toml files of those to remove the explicit halo2curves dep? Seems like a sane idea (with my very limited rust experience) to remove the number of explicit dependencies with version numbers to avoid possible conflicts?

Q2: Also, it seems like all the Cargo.toml files still point to the original zcash repo. Oversight? Should they be updated (it seems to me like it'll remain a fork, no?)

~~Q3: Should the halo2curves version be updated to 0.6.1? ~~ Since learned that 0.6.0 allows for 0.6.1.

davidnevadoc commented 4 months ago

The dependency should also be updated in halo2_backend and halo2_frontend right?

Also, it seems like all the Cargo.toml files still point to the original zcash repo. Oversight? Should they be updated (it seems to me like it'll remain a fork, no?)

Good catch, I think they should be updated, yes.

Vindaar commented 4 months ago

The dependency should also be updated in halo2_backend and halo2_frontend right?

Can you clarify what you mean?

Also, it seems like all the Cargo.toml files still point to the original zcash repo. Oversight? Should they be updated (it seems to me like it'll remain a fork, no?)

Good catch, I think they should be updated, yes.

Done (and also updated the doc link).

ed255 commented 4 months ago

This way users of halo2_middleware don't need to depend explicitly on halo2curves. Relevant context: mratsim/constantine#377

Q: Given that all the other modules part of the workspace depend on halo2_middleware, should one update the Cargo.toml files of those to remove the explicit halo2curves dep? Seems like a sane idea (with my very limited rust experience) to remove the number of explicit dependencies with version numbers to avoid possible conflicts?

I think a better option would be to define the common dependencies in the workspace Cargo.toml following this https://doc.rust-lang.org/cargo/reference/workspaces.html#the-package-table

~~Q3: Should the halo2curves version be updated to 0.6.1? ~~ Since learned that 0.6.0 allows for 0.6.1.

Sounds good to me to update it to 0.6.1.