rand update to v0.8

[ralpha]

The package rand has been updated to v0.8.X.
As this crate used rand v0.7.X it is not compatible with the current version (1.0.1) of this crate.

And will show error like this:

error[E0277]: the trait bound `OsRng: rand_core::CryptoRng` is not satisfied
   --> lib.rs:20:46
    |
315 |     let keypair: Keypair = Keypair::generate(&mut csprng);
    |                                              ^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `OsRng`
    | 
   ::: ed25519-dalek-1.0.1/src/keypair.rs:129:12
    |
129 |         R: CryptoRng + RngCore,
    |            --------- required by this bound in `Keypair::generate`

error[E0277]: the trait bound `OsRng: rand_core::RngCore` is not satisfied
   --> lib.rs:20:46
    |
315 |     let keypair: Keypair = Keypair::generate(&mut csprng);
    |                                              ^^^^^^^^^^^ the trait `rand_core::RngCore` is not implemented for `OsRng`
    | 
   ::: ed25519-dalek-1.0.1/src/keypair.rs:129:24
    |
129 |         R: CryptoRng + RngCore,
    |                        ------- required by this bound in `Keypair::generate`

On the code that looks like this (similar to example code in documentation).

use rand::rngs::OsRng;

let mut csprng = OsRng {};
let keypair: Keypair = Keypair::generate(&mut csprng);

So for the next version of this crate the rand version should be updated.
Current workaround is to downgrade to rand version 0.7.X.

[robjtede]

There's 2 (arguably 3) PRs open with this "fixed". The real problem is that a 0.x dependency is in the public API of a 1.x crate. This is becoming more of an issue in the ecosystem as crates hit 1.0 and it should be looked at how to avoid it in this crate.

[dvc94ch]

One option would be to just add a generate_with_osrng function that takes no parameters to avoid exposing rand in the public api and deprecate the existing generate. However this is somewhat ugly. If a major version bump is acceptable, I'd suggest just removing the parameter from generate.

[dvc94ch]

On the other hand the rand thing is already causing breakage, so just removing the parameter and calling it a "bug fix" would probably be acceptable in this case.

[fanatid]

One option would be to just add a generate_with_osrng function that takes no parameters to avoid exposing rand in the public api and deprecate the existing generate. However this is somewhat ugly. If a major version bump is acceptable, I'd suggest just removing the parameter from generate.

Good thing about current generate that you can pass custom rng which can be seedable (for tests purposes).

[dvc94ch]

For test purposes I like using [i; 32] as the secret key. I guess if your test requires more than 256 secret keys that doesn't work.

[Stargateur]

reexport rand ?

[robjtede]

reexport rand ?

it's not that simple, unfortunately

[Stargateur]

I don't see how, that why reexport exist.

Thus that not a big problem one can use https://stackoverflow.com/a/58739076/7076153

[robjtede]

As I said in an earlier comment, if you have a crate in the public API and bump it to a semver incompatible version, that is always a breaking change. This is the case with re-exports because there is the possibility of obtaining the same types directly.

The solutions available are:

• remove rand from public API until it is stabilized
• use feature flags to control which version of rand is used to avoid needing a major version bump in this lib

[dvc94ch]

Reexporting also works. But I prefer removing it from the public api.

[gilescope]

I can see lots of PRs for reexporting rand but none yet for removing rand from the public api. Has anyone got a PR for this tucked away somewhere?

[dvc94ch]

Yes, see closed PRs. The main issue preventing a resolution of this issue is maintainers simply not caring

[cryptoquick]

I tried the Zcash ed25519-zebra library, but it doesn't work quite the same, and they don't have a similar library for curve25519.

Really inconvenient that the maintainers dropped the ball here.

[robjtede]

You're free to fork projects, make changes, and publish them on crates.io (license permitting).

This repo has recent activity so maybe talk about which solution is preferable rather than complain about the maintainers.

[dvc94ch]

Well I opened a PR, not really sure what else you want me to do. It is obvious from discussion around this issue that the maintainers don't care about this particular one. I stated it as a fact, you down voting my comment doesn't change that.

[robjtede]

Well I opened a PR

And... closed it?... apparently. That's much less helpful to maintainers and other reviewers than leaving it open.

FWIW, I like your closed PR's approach if it were combined with changing the feature flag setup to reference the rand version, too. Eg. rand08. So that there would be no need for subsequent breaking changes in a situation where rand 0.9/1.0 is released.

[robjtede]

@isislovecruft Do you have any views on this particular issue? / Are you looking for additional maintainers?

[dvc94ch]

GitHub let's you track your open PR's. I use it to keep track of my work. If I'd keep every unmerged PR open it would be less useful for me.

[Stargateur]

Stay focus on the issue. Anyway yes the maintainer of this crates should give the lead cause no update on this issue and no PR merge since 1 year.

[gilescope]

Sorry for missing that closed PR! I know we've all got piles of work to do and a lot going on outside of coding life - life is never easy!

Interestingly when quickcheck removed rand from their public api that lead to a lot of upgrade pain for people. Most are still not upgraded. Maybe that's less of a problem here? Will ripping out rand from the public api cripple the usability of the crate? If not it seems a worthy goal, but I note that the dependency is already an optional dependency at this stage so maybe just upgrading rand is ok?

I don't mind either way but a decision on this point would be helpful.

[dvc94ch]

so there is a from_bytes that errors as it takes a slice. my preference would be to take a [u8; 32] and not error in the api (or alternatively provide a From<[u8; 32]> implementation). a generate method not exposing rand is useful, but I guess it's not essential. after a lot of messing about with boxed secrets etc, I've come to the conclusion that zeroizing bytes doesn't work reliably anyway, so I'd propose also deriving Clone and Copy for the SecretKey, because it's annoying and gives a false sense of there is only one in memory copy (in addition it's probably in multiple os buffers and cpu caches), when rust move semantics can cause multiple copies and there isn't much that can be done about it. So to recap my suggestion would be:

• make it easy to construct a secret key: add From<[u8; 32]> and Into<[u8; 32]>
• derive Clone and Copy
• optionally remove the &mut rng thing from the generate method

[cryptoquick]

@dvc94ch I'd recommend creating a new issue to discuss those changes (and linking it here if you like), but this one should stay focused on updating rand.

[cryptoquick]

@gilescope I looked into what you mentioned, I believe you mean this issue? BurntSushi/quickcheck#241

Not sure much came of the discussion. I've also read, not depending on the re-exported rand crate is shooting oneself in the foot? I'm not sure why, though... If a project needs rand, I would imagine it would be best to add the dependency within that project, and make sure the versions match across dependencies within the lockfile. That approach assumes some measure of coordination in other open source communities to prevent multiple versions of rand being included in compiled binaries... Which can be like herding cats in order to come up with the solutions that work best in each library that also depends on the same minor version.

[dvc94ch]

There are related. If you say generate should be disabled then make it convenient

[mimoo]

wondering about this as well o.o

[vincev]

A work around I am using for the rand issue:

let mut bytes = [0u8; 32];
rand::thread_rng().fill_bytes(&mut bytes);
let secret = ed25519_dalek::SecretKey::from_bytes(&bytes).expect("Invalid length");
let public: ed25519_dalek::PublicKey = (&secret).into();
let keypair = ed25519_dalek::Keypair { secret, public };

[ryankurte]

in case it's useful for anyone else... i threw together a quick compatibility layer for this somewhat familiar problem 😅

[rozbb]

Thanks all for all the solutions and comments. We are on the case and releasing soon! This has been resolved in #223

[vbrandl]

Are there plans to release this change? The last release on crates.io is from 2020. Should I still use this crate or is it abandoned?

[mrghosti3]

@vbrandl v2.0 has been merged into main branch (#278) recently, but not yet published.

[pinkforest]

2.0.0-pre.0 does have it published
https://crates.io/crates/ed25519-dalek/versions

[mrghosti3]

@pinkforest
Any time estimation for when v2 is fully released?

[pinkforest]

Full release isn't far away considering curve is already 4.0.0-rc.0 which had the major work - just tidying up things really

If people would be able to test them that would be nice