Generate even and odd safe integers in Crypto random APIs - #6941
Conversation
|
There was a problem hiding this comment.
Important
This PR adds a focused regression test for the safe-integer domain bug in Crypto, but the implementation fix promised by its title is not present. The reproduction itself is correct, yet a few concrete items need to be addressed before merge.
Reviewed changes
packages/effect/test/CryptoNextIntDomain.test.ts— adds deterministic, failing reproduction tests forCrypto.make(...).nextIntUnsafeshowing that adjacent 53-bit fractions currently skip the adjacent even safe integer.
⚠️ Implementation fix is missing
The PR body already notes that the fix should follow, but the title "Generate even and odd safe integers in Crypto random APIs" reads like the fix is included. Before merge, packages/effect/src/Crypto.ts:232-233 needs an implementation that samples uniformly across the full [Number.MIN_SAFE_INTEGER, Number.MAX_SAFE_INTEGER] range rather than relying on Math.floor(nextDoubleUnsafe() * (2 ** 54 - 1)), which rounds the width to 2 ** 54 and produces only odd offsets.
I verified the reproduction locally: make(1n).nextIntUnsafe() returns -9007199254740989 (MIN_SAFE_INTEGER + 2) instead of the expected -9007199254740990 (MIN_SAFE_INTEGER + 1).
Existing Crypto tests still pass and pnpm check succeeds, so the change has no other regressions.
Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
packages/effect/test/CryptoNextIntDomain.test.ts:21(RIGHT) — line 21 (RIGHT) is not inside a diff hunk
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
| const make = (fraction: bigint) => Crypto.make({ | ||
| randomBytes: () => Uint8Array.of( | ||
| Number((fraction >> 48n) & 0x1fn), | ||
| Number((fraction >> 40n) & 0xffn), | ||
| Number((fraction >> 32n) & 0xffn), | ||
| Number((fraction >> 24n) & 0xffn), | ||
| Number((fraction >> 16n) & 0xffn), | ||
| Number((fraction >> 8n) & 0xffn), | ||
| Number(fraction & 0xffn) | ||
| ), | ||
| digest: (_algorithm, data) => Effect.succeed(data) | ||
| }) | ||
|
|
||
| it("maps adjacent 53-bit fractions to adjacent safe integers", () => { |
There was a problem hiding this comment.
This block needs dprint fmt; pnpm lint currently fails with Found 1 not formatted file on this file.
| const make = (fraction: bigint) => Crypto.make({ | |
| randomBytes: () => Uint8Array.of( | |
| Number((fraction >> 48n) & 0x1fn), | |
| Number((fraction >> 40n) & 0xffn), | |
| Number((fraction >> 32n) & 0xffn), | |
| Number((fraction >> 24n) & 0xffn), | |
| Number((fraction >> 16n) & 0xffn), | |
| Number((fraction >> 8n) & 0xffn), | |
| Number(fraction & 0xffn) | |
| ), | |
| digest: (_algorithm, data) => Effect.succeed(data) | |
| }) | |
| it("maps adjacent 53-bit fractions to adjacent safe integers", () => { | |
| const make = (fraction: bigint) => | |
| Crypto.make({ | |
| randomBytes: () => | |
| Uint8Array.of( | |
| Number((fraction >> 48n) & 0x1fn), | |
| Number((fraction >> 40n) & 0xffn), | |
| Number((fraction >> 32n) & 0xffn), | |
| Number((fraction >> 24n) & 0xffn), | |
| Number((fraction >> 16n) & 0xffn), | |
| Number((fraction >> 8n) & 0xffn), | |
| Number(fraction & 0xffn) | |
| ), | |
| digest: (_algorithm, data) => Effect.succeed(data) | |
| }) |

Summary
nextIntUnsafe and randomInt cannot produce any even safe integer, including zero, despite promising the full inclusive safe-integer range.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Safe-integer random generation reaches only odd values
Module:
CryptoAudit ID:
core-a-f-crypto-safe-integer-domainSeverity / confidence: medium / high
What happens
nextIntUnsafe and randomInt cannot produce any even safe integer, including zero, despite promising the full inclusive safe-integer range.
Why it happens
The mathematical width 2^54 - 1 rounds to 2^54 as a number. Multiplying a 53-bit fraction k / 2^53 therefore produces 2k, and adding odd Number.MIN_SAFE_INTEGER makes every output odd.
Expected behavior
nextIntUnsafe and randomInt generate an integer over the inclusive range from Number.MIN_SAFE_INTEGER through Number.MAX_SAFE_INTEGER.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/Crypto.ts:225-233packages/effect/src/Crypto.ts:78-82packages/effect/src/Crypto.ts:113-117View problematic code at
packages/effect/src/Crypto.ts:225-233View exact lines on GitHub
View problematic code at
packages/effect/src/Crypto.ts:78-82View exact lines on GitHub
View problematic code at
packages/effect/src/Crypto.ts:113-117View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/CryptoNextIntDomain.test.tsObserved failure: Adjacent 53-bit entropy values skipped the adjacent even safe integer.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/CryptoNextIntDomain.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715fcore-a-f-crypto-safe-integer-domain