fix: randomize UUIDv7 rand_a on millisecond-resolution clocks#217
fix: randomize UUIDv7 rand_a on millisecond-resolution clocks#217mimifuwacc wants to merge 1 commit into
Conversation
|
This fix is not correct. In the Although on the other hand this implementation is not using the optional counter, just the timestamp and either 62 or 74 bits of random data, depending from timestamp precision available. So all of the timestamp comparison logic, which applies when one is using an infixed counter between the timestamp and the random data per RFC 9562 § 6.2, is inapplicable. On the gripping hand, this implementation is not doing the RFC 9562 § 6.2 monotonicity check properly. The whole UUID, timestamp plus random data, must monotonically increment. |
|
I think it's ok to fill rand_a with random values, and monotonic increment is just an option: https://datatracker.ietf.org/doc/html/rfc9562#section-5.7-2
|
Summary
UUIDv7 fills the 12-bit
rand_afield with a sub-millisecond fraction of the timestamp ((nano % 1ms) >> 8). On platforms whose wall clock has no sub-millisecond resolution this fraction is always zero, sorand_acarries no entropy and is effectively a constant.This adds a fallback: when the platform is not guaranteed to have a sub-millisecond clock,
rand_ais filled with random bits instead, as permitted by RFC 9562. Ordering within a millisecond is still guaranteed by the existinglastV7time + 1counter, so monotonicity is unaffected.Details
getV7Timenow branches on a compile-time constanthasSubMilliClock:true→ existing behavior (sub-millisecond timestamp fraction).false→ random 12-bitrand_avia the package'srander(soSetRand/ the rand pool are still honored).hasSubMilliClockis defined per-platform via build constraints:version7_wasm.go(js || wasip1) →falseversion7_other.go(!js && !wasip1) →truejs || wasip1rather than justjs: on wasm targets the clock resolution is host-dependent and sub-millisecond precision is not guaranteed, so narrowing tojswould miss WASI hosts (e.g. Node's WASI) that only expose millisecond precision. The only trade-off of includingwasip1is that on a wasip1 host that does have sub-ms resolution we use random bits instead of the fractional-timestamp ordering — but intra-millisecond ordering is still preserved by the counter, so this is acceptable.Tests
No new test is included. The new branch is selected by a compile-time constant that is only
falseon wasm targets (js,wasip1), and CI runsgo test ./...on native (amd64) only, where the constant istrue. A test would therefore exercise the existing path, not the new one, unless CI is extended to run under a wasm runtime. Behavior was instead verified manually:native(amd64)go test ./...js/wasmrand_abranch)wasip1/wasmgo test -run Test ./...pass (random branch; theFuzz*seed-corpus read fails only due to the WASI filesystem sandbox, unrelated to this change)Happy to add a wasm CI job (or refactor the
rand_acomputation into a unit-testable helper) if preferred.