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

Separate getrandom features for wasm32-unknown-unknown target #2218

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ninegua
Copy link

@ninegua ninegua commented Jan 11, 2025

Instead of just having one wasm32_unknown_unknown_js feature that imposes getrandom/js upon ring's users, two additional features are added to provide more flexibility.

  • wasm32_unknown_unknown_custom_or_js implements SecureRandom for SystemRandom, but does not make a choice between getrandom/js or getrandom/custom. It instead delegates the choice to the library's user.
  • wasm32_unknown_unknown_js enables both wasm32_unknown_unknown_custom_or_js and getrandom/js.
  • wasm32_unknown_unknown_custom enables both wasm32_unknown_unknown_custom_or_js and getrandom/custom.

This change is only relevant for the wasm32-unknown-unknown target.

…nknown_js, and wasm32_unknown_unknown_custom_or_js.
@briansmith
Copy link
Owner

I have researched this further.

With the WASI 0.2 component model, there's not much reason for wasm32-unknown-unknown to exist any longer. In any environment that currently uses wasm32-unknown-unknown, a minimal WASI 0.2 environment that implements just the wasi:random can be provided. It is easy to make a wasi-random implementation that delegates to a browser's getRandomValues() API for use in a web/extension environment, or to make a custom wasi-random implementation for some other wasm32 environment.

Consequently, I think we shouldn't bother trying to do more than we're currently doing.

@ninegua
Copy link
Author

ninegua commented Jan 16, 2025

Thanks for researching into this! However, I have to point out that WASI is not the only game in town, and there are plenty use cases where randomness is supplied through other means, or no randomness is required at all (e.g. when ring is only used for verification purpose).

Anyway, the problem with cargo features is that once a feature is brought in, there is no way to avoid it. And getrandom/js gets in the way because it imposes a very particular setup upon users, even when their use case has nothing to do with it. So I believe it is always better to make getrandom/js optional. WDYT?

PS: I work on Internet Computer that has deployed close to a million wasm programs/services onto the internet. Not an insignificant use case, I'd say. And these wasm programs do not run with WASI. Besides, if they need randomness, it has to be supplied asynchronously, so a synchronous interface (like the one in WASI) does not cut it. I hope I've made a valid case here. Thank you for the consideration!

@briansmith
Copy link
Owner

If you look at the design of ring::rand you can see that we have jumped through a lot of hoops to avoid "custom"/pluggable randomness implementations in favor of always using the system RNG. The advantage of wasm32-wasi is that we know what the system RNG API is for wasm32-wasi. The problem with wasm32-unknown-unknown is that there is no API for the system RNG API. This is why I'm eager for people to move away from wasm32-unknown-unknown. It also comes up in other areas like time APIs that don't apply to ring.

@briansmith
Copy link
Owner

Besides, if they need randomness, it has to be supplied asynchronously, so a synchronous interface (like the one in WASI) does not cut it.

ring also expects a synchronous randomness API with its current design. I'm not sure how we'd support an async API.

@ninegua
Copy link
Author

ninegua commented Jan 17, 2025

Not sure if I understood all the nuances of how getrandom works in wasm32-unknown-unknown, but isn't the custom feature delegating the randomness provision to the user, so that libraries using getrandom does not have to worry about them?

The documentation of getrandom also says the following about the js feature:

This feature should only be enabled for binary, test, or benchmark crates. Library crates should generally not enable this feature, leaving such a decision to users of their library. Also, libraries should not introduce their own js features just to enable getrandom’s js feature.

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.

2 participants