Version 0.9.2 breaks semver

[thiolliere]

related paritytech/substrate#8208

semver breakage in 0.9.2

it seems 0.9.2 is breaking semver because of merlin major update (from 2.0 to 3.0)

It is because merlin is publicly used: SigningTranscript is implemented on merlin::Transcript

sidenote about rng_hack failing in std or with u64_backend and getrandom but no std.

schnorrkel with features "std" fails with:

error[E0425]: cannot find function `thread_rng` in crate `rand`
   --> src/lib.rs:228:13
    |
228 |     ::rand::thread_rng()
    |             ^^^^^^^^^^ not found in `rand`

for this we need to set the feature rand/std_rng.

And schnrrkel with features "u64_backend" and "getrandom" (and no default features) fails with:

error[E0425]: cannot find value `OsRng` in crate `rand_core`
   --> src/lib.rs:233:18
    |
233 |     ::rand_core::OsRng
    |                  ^^^^^ not found in `rand_core`

I think to fix it we should add specific features to set the rng:

rand_std_rng = ["rand/std_rng"]
getrandom_rng = ["rand_core/getrandom"]
std = ["rand_std_rng", ...]

[burdges]

I'll need to yank 0.9.2 and push a 0.10 then I guess. cc #63

We had better CI here once upon a time I thought..

We should fix rng_hack at the same time. I should've checked deeper for rand breakage. lol

[thiolliere]

actually what was weird is that doing cargo check in schnorrkel repo worked. Then I removed the dev dependencies and did cargo check again and then it fails with the thread_rng not found.

I didn't thought dev dependency was leaking features in cargo check :-/ 🤷

[burdges]

I should add your features and make this work in other words? Or should I disable anything from dev?

cargo +stable test --no-default-features --features "u64_backend getrandom"

I've no idea about std_rng, formally that meant something else, but maybe not anymore.

[thiolliere]

I should add your features and make this work in other words? Or should I disable anything from dev?

I don't understand what you mean, do you mean in order to fix, or in order to reproduce my issues ?

My issue is that when I create a new crate and depend only on schnorrkel with default features, then it won't compile because thread_rng is not found in some code inside schnorrkel.
Then I was surprised that doing cargo check inside the repo was actually working, but then I discovered that if I removed the dev dependency in schnorrkel repo and do cargo check then it fails with thread_rng not found (which is expected as it is what happens for a crate depending solely on schnorrkel.

So basically to reproduce in the repo you have to do:

diff --git a/Cargo.toml b/Cargo.toml
index fb81210..25b1acf 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -88,14 +88,14 @@ default-features = false
 features = ["zeroize_derive"]
 
 [dev-dependencies]
-rand = "0.8.3"
-rand_chacha = "0.3"
-# hex = "0.3.2"
-hex-literal = "0.2.1"
-sha2 = "0.9.3"
-sha3 = "0.9.1"
-bincode = "1.3.2"
-criterion = "0.3.4"
+# rand = "0.8.3"
+# rand_chacha = "0.3"
+# # hex = "0.3.2"
+# hex-literal = "0.2.1"
+# sha2 = "0.9.3"
+# sha3 = "0.9.1"
+# bincode = "1.3.2"
+# criterion = "0.3.4"
 
 [[bench]]
 name = "schnorr_benchmarks"

And then run the commands:

cargo check or for the second scenario cargo check --no-default-features --features "u64_backend getrandom".

I've no idea about std_rng, formally that meant something else, but maybe not anymore.

AFAICT rand/std_rng is just a feature to expose thread_rng and use OsRng under the hood.

At the end maybe we can do something like this:

[thiolliere@localhost schnorrkel]$ git diff
diff --git a/Cargo.toml b/Cargo.toml
index fb81210..d618a0a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -107,9 +107,14 @@ preaudit_deprecated = []
 nightly = ["curve25519-dalek/nightly", "rand/nightly"] # "zeroize/nightly"
 alloc = ["curve25519-dalek/alloc", "rand_core/alloc"]
 chacha = ["rand_chacha"]
-std = ["getrandom", "curve25519-dalek/std", "rand/std"] # "failure/std"
+std = ["getrandom", "curve25519-dalek/std", "rand/std", "rand_std_rng"] # "failure/std"
 wasm-bindgen = ["getrandom/wasm-bindgen"]
 asm = ["sha2/asm"]
 u64_backend = ["curve25519-dalek/u64_backend"]
 u32_backend = ["curve25519-dalek/u32_backend"]
 avx2_backend = ["curve25519-dalek/avx2_backend"]
+
+# rng from rand using thread_rng
+rand_std_rng = ["rand/std", "rand/std_rng"]
+# rng from rand_core using OsRng
+rand_core_rng = ["rand_core/getrandom"]
diff --git a/src/lib.rs b/src/lib.rs
index a992eec..d9ff197 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -223,17 +223,17 @@ extern crate alloc;
 
 use rand_core::{RngCore,CryptoRng};
 
-#[cfg(all(feature = "getrandom", feature = "rand"))] 
+#[cfg(feature = "rand_std_rng")]
 fn rand_hack() -> impl RngCore+CryptoRng {
     ::rand::thread_rng()
 }
 
-#[cfg(all(feature = "getrandom", not(feature = "rand")))] 
+#[cfg(all(feature = "rand_core_rng", not(feature = "rand_std_rng")))]
 fn rand_hack() -> impl RngCore+CryptoRng {
     ::rand_core::OsRng
 }
 
-#[cfg(not(feature = "getrandom"))]
+#[cfg(all(not(feature = "rand_core_rng"), not(feature = "rand_std_rng")))]
 fn rand_hack() -> impl RngCore+CryptoRng {
     const PRM : &'static str = "Attempted to use functionality that requires system randomness!!";
 

[burdges]

I'm shocked they name the exact same functionality std_rng in rang and getrandom in rand_core, but yes that's exactly what they do: https://github.com/rust-random/rand/pull/948/files

I'm not going to make matters worse by adding even more feature names. I'll remove the rand dependency for now, but leave a note about how to fix the minor performance regressions.

I've always felt the SeedableRng and thread_rng machinery belongs outside rand itself, probably rand_core, but whatever.

[burdges]

I've posted 0.10 now and yanked 0.9.2, thanks!