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

Consider an alternative to random_32_bytes #522

Open
Kixunil opened this issue Nov 18, 2022 · 3 comments
Open

Consider an alternative to random_32_bytes #522

Kixunil opened this issue Nov 18, 2022 · 3 comments

Comments

@Kixunil
Copy link
Collaborator

Kixunil commented Nov 18, 2022

I don't like non-generic 32_bytes much. I was thinking of some alternative designs: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3123fb72993eabdeeb4a51eb2558be88

Originally posted by @Kixunil in #520 (comment)

@apoelstra
Copy link
Member

There are several places in this library where we need 32 uniformly random bytes, and zero where we need a different number of bytes or a different distribution. In fact if we ever use a different distribution this is nearly guaranteed to be a key-leaking catastrophe. If we generate too few bytes this is also a catastrophe. If we generate too many I guess that's fine, though depending how we implement things it could cause panics or something.

Secondly, the Standard distribution is documented as "usually" being uniform. We should use Uniform if we mean uniform.

Thirdly, I worry that using all these generics and stuff will make it hard to move away from rand in the future if we eventually find a simpler crate that can wrap getrandom and be more stable.

Basically, I think any move away from "always generate 32 bytes uniformly" will create an ongoing maintenance/review burden as we manually check that the new generic code is only ever used to uniformly generate 32 bytes.

@Kixunil
Copy link
Collaborator Author

Kixunil commented Nov 18, 2022

Secondly, the Standard distribution is documented as "usually" being uniform. We should use Uniform if we mean uniform.

Oh, interesting footgun, I totally missed that. Yeah, it'd have to be Uniform.

if we eventually find a simpler crate that can wrap getrandom and be more stable.

Unless that crate is rand_core this could be detrimental due to network effects.

Another approach is to only call T::random() inherent method from all code and have fill_bytes in its impl - just like Scalar does. So there should be exactly three calls to random_32_bytes/fill_bytes in the whole codebase.

@apoelstra
Copy link
Member

Another approach is to only call T::random() inherent method from all code and have fill_bytes in its impl - just like Scalar does.

Yeah, I wouldn't mind this.

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

No branches or pull requests

2 participants