Replace rand_core's std feature with alloc
#8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `std` feature from `rand_core` activates the `alloc` feature and the `getrandom` feature, which ends up including `getrandom` as a dependency. For WebAssembly targets, it's only possible to obtain entropy if there's an external function that provides that. Because of this, `getrandom` only works if there are Javascript APIs enabled. If there aren't, it fails to build. The source of entropy isn't needed inside the library, therefore, the `getrandom` feature does not need to be enabled. There is a use case in a documentation test, which uses `OsRng`. However, it's still not necessary to manually enable the `getrandom` feature in `dev-dependencies`, because the `curve25519-dalek` dependency's `default` feature enables `rand_core`'s `std` dependency.
JayWhite2357
pushed a commit
to spaceandtimelabs/sxt-proof-of-sql
that referenced
this pull request
Aug 28, 2024
# Rationale for this change To build this crate for `wasm32-unknown-unknown` to run on runtimes with a limited custom API available (e.g., Linera contracts), `wasm-bindgen` should not be a dependency. That's because `wasm-bindgen` generates bindings for the Javascript APIs available in browsers (and similar runtimes). For more bare-bones runtimes, these APIs aren't available. # What changes are included in this PR? Disabling the `default` features of `chrono`, `rand` and `rand-core`. Unfortunately, this doesn't complete the work to support `wasm32-unknown-unknown` without JS, because a fix is also needed in `merlin` (zkcrypto/merlin#8). # Are these changes tested? Tested with `cargo test`.
Closed
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The
stdfeature fromrand_coreactivates theallocfeature and thegetrandomfeature, which ends up includinggetrandomas a dependency.For WebAssembly targets, it's only possible to obtain entropy if there's an external function that provides that. Because of this,
getrandomonly works if there are Javascript APIs enabled. If there aren't, it fails to build.Solution
The source of entropy isn't needed inside the library, therefore, the
getrandomfeature does not need to be enabled. Therefore it's enough to replace enabling thestdfeature with enabling just theallocfeature.There is a use case in a documentation test, which uses
OsRng. However, it's still not necessary to manually enable thegetrandomfeature indev-dependencies, because thecurve25519-dalekdependency'sdefaultfeature enablesrand_core'sstddependency.Release Notes
I believe this is a backward compatible change. I think there could be a situation where some project has a dependency on
rand_corewithout its default features and onmerlin, and unknowingly usesgetrandom, which with this PR would fail to compile. However, the solution would be to fix therand_coredependency to enable the required feature.