-
Notifications
You must be signed in to change notification settings - Fork 657
feat(random/unstable): allow generating seeded random bytes and 53-bit-entropy floats in [0, 1) (add getRandomValuesSeeded
and nextFloat64
)
#6626
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6626 +/- ##
==========================================
- Coverage 94.74% 94.71% -0.04%
==========================================
Files 583 567 -16
Lines 46478 46770 +292
Branches 6523 6578 +55
==========================================
+ Hits 44036 44297 +261
- Misses 2399 2431 +32
+ Partials 43 42 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sorry for the delay in review. Added functions seem fine to me. Some feedbacks about the organization of code/API structures.
I think these 2 function should be exported from its own files (like
I feel the name like |
* `crypto.getRandomValues`, i.e. taking a `Uint8Array` and mutating it by | ||
* filling it with random bytes, returning the mutated `Uint8Array` instance. | ||
* | ||
* @experimental **UNSTABLE**: New API, yet to be vetted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@experimental
doc tag might be unnecessary for now as the entire package is still unstable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, then let's keep them.
random/_pcg32_test.ts
Outdated
assertEquals(actual, expected); | ||
}); | ||
Deno.test("getRandomValues() writes bytes", () => { | ||
const pgc = new Pcg32(0n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is pgc
acronym intentional?
Yeah maybe. It's modeled after Now I think about it, maybe the non-u8 signatures should be supported, via sniffing the platform endianness and filling bytes accordingly. That would make it fully viable as a stub for |
318e4e4
to
fdc2985
Compare
fdc2985
to
792ad33
Compare
46230cf
to
05d8bda
Compare
getRandomValuesSeeded
and nextFloat64
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating! LGTM
Towards #6602
Currently, this doesn't change the existing
randomSeeded
, other than a tiny documentation change to indicate that it only provides 32 bits of entropy.Instead, it adds:
getRandomBytesSeeded
, which returns aRandomValueGenerator
function, fulfilling the same contract ascrypto.getRandomBytes
.nextFloat64
, which takes aRandomValueGenerator
function and uses it to get a float in[0, 1)
with 53 bits of entropy.Used together, the two functions can be used to get identical f64 sequences to the rust rand crate, given the same u64 seed.
getRandomBytesSeeded
can also be used by reading little-endian from appropriately sizedUint8Array
s to get identical sequences of various integer types (u8, i32, etc.) to the rust crate.There's still an open question as to whether the 32-bit-entropy
randomSeeded
should be superseded by the 53-bit-entropy version, and if so what the upgrade path looks like for that.