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

Replace primitives_types with uint #2350

Merged
merged 1 commit into from Jun 18, 2021

Conversation

conradoplg
Copy link
Contributor

@conradoplg conradoplg commented Jun 18, 2021

Motivation

We need to use the recent zcash_primitives in librustzcash for ZIP-244/221 support. However, they have updated their dependencies to use bitvec 0.22, while we use 0.17. Due to a package conflict issue with funty, we must then update all dependencies that use bitvec.

One of those is primitive-types, however it hasn't been updated to use bitvec 0.22, and was due to being replaced in #829 anyway. That issue proposes replacing it with bigint, however that package has a different API and it will take some time to adapt to it.

Until we do that, we can use uint instead (which primitives-types also uses and therefore does not require any big changes in our code). This will unblock further work on the librustzcash update and we can eventually use bigint instead per #829.

Specifications

N/A

Designs

N/A

Solution

Use uint to define a U256 type which is essentially the same as the one defined in primitive-types.

Note that this does not prevent the packages conflicts by itself, we keep using bitvec 0.17 here. That will be solved in a separated PR because the bitvec upgrade is non-trivial and requires some code changes.

Review

Anyone can review, it's a simple change.

This blocks further work on the librustzcash update which is necessary to merge ZIP-244 PRs.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

In #829 we'll change it again to use bigint in order to have the same dependency as librustzcash.

@zfnd-bot zfnd-bot bot added this to In progress in 🦓 Jun 18, 2021
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@conradoplg conradoplg merged commit c9e93a7 into main Jun 18, 2021
@conradoplg conradoplg deleted the replace-primitive-types-with-uint branch June 18, 2021 18:35
🦓 automation moved this from In progress to Done Jun 18, 2021
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

👍

@dconnolly dconnolly added A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks labels Jun 18, 2021
@dconnolly dconnolly added this to the 2021 Sprint 12 milestone Jun 18, 2021
@teor2345 teor2345 mentioned this pull request Nov 4, 2021
1 task
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 A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks
Projects
No open projects
🦓
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants