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

Remove duplicate orchard crates, change dependencies to zcash/librustzcash #4767

Closed
teor2345 opened this issue Jul 11, 2022 · 4 comments · Fixed by #4926
Closed

Remove duplicate orchard crates, change dependencies to zcash/librustzcash #4767

teor2345 opened this issue Jul 11, 2022 · 4 comments · Fixed by #4926
Assignees
Labels
A-dependencies Area: Dependency file updates C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jul 11, 2022

Motivation

We're currently compiling 3 different versions of the orchard crates into Zebra:

   Compiling halo2_gadgets v0.1.0                                                              
   Compiling halo2_gadgets v0.2.0                                                              
   Compiling orchard v0.1.0                                                                    
   Compiling orchard v0.2.0                                                                    
   Compiling zcash_primitives v0.6.0                                                                                                                                                           
   Compiling zcash_primitives v0.7.0                                                           
   Compiling zcash_primitives v0.7.0 (https://github.com/ZcashFoundation/librustzcash.git?rev=4567a37ceccbd506a58aaaded39ba14c952c1510#4567a37c)

Since we're having some trouble with slow syncs, we want to be using the latest version of halo2 in all our code.

Tasks

Change dependencies:

Duplicate dependencies:

  • Use cargo tree to find how these duplicates are happening
  • Remove duplicates by fixing dependencies in Cargo.toml
  • Remove any exceptions for these crates in deny.toml
@teor2345 teor2345 added C-bug Category: This is a bug A-dependencies Area: Dependency file updates S-needs-triage Status: A bug report needs triage P-High 🔥 I-integration-fail Continuous integration fails, including build and test failures labels Jul 11, 2022
@teor2345
Copy link
Contributor Author

@conradoplg I think you were the last person to update these dependencies, did we deliberately allow duplicates?

@conradoplg
Copy link
Collaborator

This was blocked by zcashd dependencies being updated, but 5.1.0 was released 3 days ago, we can now update zcash_script and remove these

zebra/deny.toml

Lines 34 to 38 in c4f89ed

# wait until zcash updates its halo2, orchard, etc. dependencies
# (which is likely to happen in the release after 5.0.0)
{ name = "halo2_gadgets", version = "=0.1.0" },
{ name = "halo2_proofs", version = "=0.1.0" },
{ name = "orchard", version = "=0.1.0" },

The last one is caused by our zcash_proofs fork IIRC

@teor2345
Copy link
Contributor Author

This was blocked by zcashd dependencies being updated, but 5.1.0 was released 3 days ago, we can now update zcash_script and remove these

zebra/deny.toml

Lines 34 to 38 in c4f89ed

# wait until zcash updates its halo2, orchard, etc. dependencies
# (which is likely to happen in the release after 5.0.0)
{ name = "halo2_gadgets", version = "=0.1.0" },
{ name = "halo2_proofs", version = "=0.1.0" },
{ name = "orchard", version = "=0.1.0" },

The last one is caused by our zcash_proofs fork IIRC

To make all our dependencies to use the zcash_proofs fork, we need to add the same git dependency to all our crates, and add it as a crates.io patch.

@teor2345
Copy link
Contributor Author

teor2345 commented Aug 5, 2022

Our zcash_proofs fork got merged into librustzcash, so we can just use the zcash/* dependencies now:
zcash/librustzcash#459

@teor2345 teor2345 changed the title Remove duplicate orchard crates Remove duplicate orchard crates, change dependencies to zcash/librustzcash Aug 5, 2022
@ftm1000 ftm1000 removed the S-needs-triage Status: A bug report needs triage label Aug 19, 2022
@mergify mergify bot closed this as completed in #4926 Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants