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

[Partial revert] Make sure we use the provided CSPRNG everywhere #342

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

gferon
Copy link
Collaborator

@gferon gferon commented Nov 3, 2024

I just merged #341 but after talking with @boxdot the correct change is to do the opposite: make sure we use whatever the library user provide as CSPRNG.

Note: the csprng: &mut R is supposed to be at the end of function parameters to mimick what libsignal does and make it consistent.

There are only two places where we still use rand::thread_rng: in tests and when generating an ID for the next websocket request.

@gferon gferon requested a review from rubdos November 3, 2024 17:59
@gferon gferon changed the title [Partial revert] Make sure we use the provided CSPRNG everywhere. [Partial revert] Make sure we use the provided CSPRNG everywhere Nov 3, 2024
@gferon gferon force-pushed the rng-template-param branch from 839a99a to 0fb2030 Compare November 3, 2024 20:36
@gferon gferon force-pushed the rng-template-param branch from 0fb2030 to 2d9088e Compare November 3, 2024 20:37
@rubdos
Copy link
Member

rubdos commented Nov 3, 2024

It makes sense both ways. By #331, we chose that we are an opinionated library. By being generic, we show we're flexible. Either way is fine by me.

@rubdos
Copy link
Member

rubdos commented Nov 5, 2024

@gferon conflicts here :-(

@gferon gferon merged commit 6401dc3 into main Nov 5, 2024
6 of 7 checks passed
@gferon gferon deleted the rng-template-param branch November 5, 2024 15:18
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