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

Use rand crate re-exports when available #64

Merged

Conversation

michalkucharczyk
Copy link
Contributor

Re-exports from rand crate shall be used. Otherwise trait bounds in Mnemonic::generate_in_with for rand::thread_rng object can get unsatisfied if crate deps get ouf of sync.

This commit is fixing following errors:

error[E0277]: the trait bound `ThreadRng: rand_core::RngCore` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::RngCore` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |::
    = help: the following other types implement trait `rand_core::RngCore`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:6
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |            ^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

error[E0277]: the trait bound `ThreadRng: rand_core::CryptoRng` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:27
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |                                 ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

@tcharding
Copy link
Member

Why is rand_core even used? rand v0.6 exposes RngCore and CryptoRng - why are we not just using them? @stevenroose do you remember?

@ggwpez
Copy link

ggwpez commented Feb 8, 2024

I also get this compile error. This would not happen if this crate had a Cargo.lock, or?

@tcharding
Copy link
Member

tcharding commented Feb 8, 2024

Can you share the build command that triggers this please?

@ggwpez
Copy link

ggwpez commented Feb 9, 2024

It happens when upstream dependencies pull in specific versions, but it can be reproduced here as follows.
Looking at the version requirements first, we have:

  • rand ">=0.4.0, <0.7.0"
  • rand_core ">=0.6.0, <0.9.0"

Now the following versions do fit these requirements:

  • rand 0.5.1
  • rand_core 0.8.5

But are incompatible with each other. Cargo does not complain about incompatible versions though, so when applying this change, it will fail to build:

 [dependencies]
-rand_core = { version = ">=0.4.0, <0.7.0", optional = true }
-crate_rand = { package = "rand", version = ">=0.6.0, <0.9.0", optional = true }
+rand_core = { version = "0.5.1", optional = true }
+crate_rand = { package = "rand", version = "0.8.5", optional = true }
 serde = { version = "1.0", default-features = false, features = [ "alloc" ], optional = true }

@michalkucharczyk is this how you encountered it as well?

@michalkucharczyk
Copy link
Contributor Author

I believe so, probably something similar. deps involved on my side were:

  • rand 0.5.1
  • rand 0.6.4
  • rand_core 0.8.5

@stevenroose
Copy link
Collaborator

Why is rand_core even used? rand v0.6 exposes RngCore and CryptoRng - why are we not just using them? @stevenroose do you remember?

I don't remember, I think in some old version of rand, there was no rand_core and now these traits live in rand_core? I think we should probably only need one of them, but not sure which, I'd say rand_core?

@stevenroose stevenroose mentioned this pull request Feb 9, 2024
@stevenroose
Copy link
Collaborator

Can you rebase on master to pass CI?

@tcharding
Copy link
Member

Ok, so to keep the loose dependency requirements we have to do this, that is probably better than tightening the dependency requirements.

src/lib.rs Outdated Show resolved Hide resolved
Re-exports from `rand` crate shall be used. Otherwise trait bounds in
`Mnemonic::generate_in_with` for `rand::thread_rng` object can get
unsatisfied if crate deps get ouf of sync.

This commit is fixing following errors:
```
error[E0277]: the trait bound `ThreadRng: rand_core::RngCore` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::RngCore` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |::
    = help: the following other types implement trait `rand_core::RngCore`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:6
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |            ^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

```

```
error[E0277]: the trait bound `ThreadRng: rand_core::CryptoRng` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:27
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |                                 ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

```

Co-authored-by: Tobin C. Harding <me@tobin.cc>
@michalkucharczyk
Copy link
Contributor Author

Can you rebase on master to pass CI?

Done (force push)

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