Dependency mess should be fixed

CPerezz commented 1 month ago

Currently as I worked on #142 I realized how complex the dependency graph of this workspace is. And it's not that the workspace is crazily complex. But rather that we have:

Lots of forks across different users and repos:

# These are some patches at top-level workspace
ark-r1cs-std = { git = "https://github.com/winderica/r1cs-std", branch="cherry-pick" }
ark-bn254 = { git = "https://github.com/arnaucube/ark-curves-cherry-picked", branch="cherry-pick"}
ark-grumpkin = { git = "https://github.com/arnaucube/ark-curves-cherry-picked", branch="cherry-pick"}

# These are some deps from `folding-schemes`
arkworks_backend = { git = "https://github.com/dmpierre/arkworks_backend", branch="feat/sonobe-integration" }
ark-circom = { git = "https://github.com/arnaucube/circom-compat", default-features = false }

Some dependencies that also fall under other users:

arkworks_backend = { git = "https://github.com/dmpierre/arkworks_backend", branch="feat/sonobe-integration" }

While I don't have any issues with repos being under the users, it makes it a nightmare to actually update dependencies across the entire workspace. As always some deps have again all the arkworks stack in a older version and it's the never-ending story.

We rely on non-existing versions of crates to make all this juggling to work

If you notice, in all the Cargo.toml files we depend on ark-grumpkin, we depend on it with version 0.4.0. But if you actually look at https://crates.io/crates/ark-grumpkin/versions, you'll realize that only ark-grumpkin v0.5.0-alpha.0 exists published. Therefore, we only use an "imaginary version" such that we can then patch it at top-level workspace Cargo.toml with a custom version that @arnaucube owns.

A lot of the arkworks cherry-picks that we currently patch have already been released.

For most of the arkworks backend, 0.5.0-alpha.0 is already out. This means most, to not say all of the things we have patched should disappear or minimize as much as possible.

Action items:

[ ] Update all the private-owned repos on which this crate relies on to v0.5.0-alpha.0.
[ ] Update current crate arkworks-dependencies to v0.5.0-alpha.0.
[ ] Try to eliminate as many patches as possible and justify well the ones that stay.
[ ] Instead of patching some crates, and owning the forks privately, why not adding them to the workspace with a subtree or something similar? On this way we can much more easily control all the updates.

pmikolajczyk41 commented 3 weeks ago

I noticed that the Git dependencies are currently pinned to a branch name. For better stability and reproducibility, would it be possible to switch to using a specific commit rev instead of a branch? This way, the dependency won't unintentionally change if new commits are pushed to the branch.

arnaucube commented 3 weeks ago

Agree. As commented in chat, the curves patches exist because when we started using the Grumpkin curve implementation there was no v0.5.0 of arkworks and Grumpkin was not included in v0.4.0, so we needed a way to use the last arkworks version while on a stable interface (ie. v0.4.0) but including also the Grumpkin curve, hence the patched fork.

We can do a 1-week coordinated effort this September (once current various implementations that are being done are completed to avoid more gitconflicts) where we coordinately upgrade all the dependencies and also Sonobe to arkworks v0.5.0 (to avoid different repos using different versions), cleaning part of the dependencies patches on the way, and we can use the same effort to try to minimize the other dependencies as you mention. Once we do that we can also change to specify the specific commit rev instead of branch in the cases that we still import a custom fork (ideally we would reduce the number of custom fork imports). Apart from the coordinated effort across multiple repos, it would be also across current branches being implemented to minimize git conflicts.

privacy-scaling-explorations / sonobe