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

Update bip0039 for Arti support in Zebra #1377

Closed
wants to merge 1 commit into from

Conversation

emersonian
Copy link

Upgrading bip0039 to v0.12 in zcash_primitives resolves Zebra's ability to build with Arti (Tor) support.

Patch versions will need to be released of the following crates to simplify integration with Zebra:

  • zcash_primitives: v0.13.1
  • zcash_client_backend: v0.10.1 (depending upon zcash_primitives v0.13.1)
  • zcash_proofs: v0.13.1 (depending upon zcash_primitives v0.13.1)
  • zcash_script: v0.1.16 (depending upon zcash_primitives v0.13.1)

Demonstration of a working build

See ZcashFoundation/zebra#8328

Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NACK for the same reason as when this was proposed in #1261.

We attempted to upgrade from bip0039 0.10 in #808, but had to downgrade in #812 because the change in bip0039 0.11 to use generics resulted in me spending several hours fighting and losing to the C++ template system in zcashd.

While zcashd depends on bip0039 0.10 via zcash_primitives::zip339, we will not be upgrading this dependency. If someone else wants to fight the C++ template system to enable this, let me know!

@str4d
Copy link
Contributor

str4d commented Apr 29, 2024

Also, as a side-note:

Patch versions will need to be released

As documented in the Cargo.toml that you edited, bip0039 is exposed in our API, and thus changing its left-most non-negative version integer is a SemVer-breaking change in Rust.

@nuttycom
Copy link
Contributor

nuttycom commented May 7, 2024

@str4d zip339 is simply a reexport of the bip0039 crate, and nothing in librustzcash depends upon it. Also, given the extraction of zcash_keys, I don't think that it's appropriate for this to live in zcash_primitives any longer anyway. I recommend that we simply remove the zip339 module entirely, which then decouples the use of bip0039 in zcashd from any potential use in zebra.

@nuttycom
Copy link
Contributor

nuttycom commented May 7, 2024

@emersonian instead of this, please just revert #424 instead. This will require a major version bump to zcash_primitives and downstream crates, so make sure to note the zip339 module as removed in the CHANGELOG so that we remember that a major version bump is required.

@daira
Copy link
Contributor

daira commented May 14, 2024

Superceded by #1391.

@daira daira closed this May 14, 2024
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

Successfully merging this pull request may close these issues.

None yet

4 participants