Skip to content

types: read the wall clock through one target-split helper - #382

Closed
EnRaiha wants to merge 3 commits into
NodeDB-Lab:mainfrom
EnRaiha:fix/clock-target-split
Closed

EnRaiha wants to merge 3 commits into
NodeDB-Lab:mainfrom
EnRaiha:fix/clock-target-split

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

types: read the wall clock through one target-split helper

Why

wasm32-unknown-unknown has no std clock: SystemTime::now() panics with
"time not implemented on this platform". That target is live, not theoretical —
nodedb-array's HLC is what NodeDB-Lite's array::create_put_slice_roundtrip
drives — so every read on a path Lite compiles needs a clock that answers there.

Eighteen production reads existed across the five shared crates, each doing its
own SystemTime::now(). Nothing in the repo compiled for the target either, so
none of them was covered by anything.

What changed

  • clock::since_epoch() owns the split: js_sys::Date::now() on
    wasm32-unknown-unknown, std everywhere else, wasm32-wasip1 included.
  • Nineteen production reads route through it, each keeping its own
    conversion, saturation and pre-epoch fallback, so native behaviour is
    unchanged. No shared crate reads SystemTime::now() directly any more; the
    one that remains is inside id_gen's test module.
  • The browser arm is now compile-checked. Three generations of getrandom
    reach nodedb-types transitively and each refuses the target until its browser
    backend is selected — 0.2 (via aes-gcm) needs js, 0.3 needs wasm_js plus
    the matching cfg, and the direct 0.4 dependency needs wasm_js. With those in
    place cargo check -p nodedb-types --target wasm32-unknown-unknown compiles.
  • js-sys is a workspace dependency, referenced as { workspace = true }
    like every other shared dependency in this repo.

The pre-epoch guard

Date.now() returns an f64, and as u64 saturates a negative value to
zero rather than wrapping. Converting before guarding would report Some(0) — a
plausible-looking zero timestamp — where the std arm reports None, and callers
map that absence to an error. The guard lives in duration_from_epoch_millis,
declared for every target so the host can test it, and two tests pin the contract
either side of the epoch.

Exclusions, stated

  • No CI job and no schedule. The wasm CI for Lite belongs in the Lite repo;
    NodeDB runs no scheduled CI.
  • No WASI changes. Lite targets wasm32-unknown-unknown and never runs the
    WAL, mem, crdt or client crates under WASI.
  • No unrelated fixes. The 32-bit msgpack_scan overflow, the codec element
    count and the vector_primary lint are separate PRs, each with its own
    reproduction.

Steps to test

# native: the helper answers, and the clock does not go backwards
cargo test -p nodedb-types --lib clock

# the browser arm compiles (this is what was previously unverifiable)
RUSTFLAGS='--cfg getrandom_wasm_js' \
  cargo check -p nodedb-types --target wasm32-unknown-unknown

Red arm for the guard — delete the millis < 0.0 branch and the test fails with
left: Some(0ns), right: None.

Not proven here

The browser arm is compile-verified, not runtime-verified: nothing in this
repo executes wasm32-unknown-unknown. Runtime coverage belongs in Lite, whose
nodedb-lite-wasm/tests/array.rs already reaches the array HLC that panicked.

Closes #364

`wasm32-unknown-unknown` has no std clock: `SystemTime::now()` panics with
"time not implemented on this platform". That target is reachable — the array
HLC in `nodedb-array` is what NodeDB-Lite's `array::create_put_slice_roundtrip`
drives — so every read on a path Lite compiles needs a clock that answers there.

`clock::since_epoch()` owns the split: `js_sys::Date::now()` on
`wasm32-unknown-unknown`, std everywhere else, `wasm32-wasip1` included. All
nineteen production reads across the five shared crates route through it, each
keeping its own conversion, saturation and pre-epoch fallback, so native
behaviour is unchanged. No shared crate reads `SystemTime::now()` directly any
more; the one that remains is inside `id_gen`'s test module.

The wasm arm returns `None` for a pre-epoch clock rather than `Some(0)`:
`Date.now()` yields a negative `f64` there, and `as u64` saturates it to zero,
so converting before guarding would report a plausible zero timestamp where the
std arm reports the absence callers map to an error.

`js-sys` is declared in the workspace dependencies like every other shared
dependency and referenced from `nodedb-types`, scoped to the one target that
reaches it.
Nothing in this repo could compile for the target the clock fix exists for, so
the `js_sys` arm had no verification anywhere. Three generations of `getrandom`
reach this crate transitively and each refuses the target until its browser
backend is selected:

  * 0.2 (via `aes-gcm`) needs `js`
  * 0.3 needs `wasm_js` plus the matching cfg
  * 0.4 (a direct dependency) needs `wasm_js`

The workspace already aliases 0.2 and 0.3 with those features for the WAL crate;
naming both in this crate's wasm target dependencies is what puts them in its
resolution. 0.4 gains `wasm_js` at the workspace root.

With those in place `cargo check -p nodedb-types --target wasm32-unknown-unknown`
needs `--cfg getrandom_wasm_js` in RUSTFLAGS and then compiles, which is the only
way to type-check the browser clock arm. The cfg is scoped to that invocation,
not the workspace, so native builds are untouched.
The wasm arm answers None for a clock before the Unix epoch, and no test covered
it: the arm only compiles on a target nothing here runs, so the guard could only
be reviewed by eye — which is how it was missing in the first place.

The guard now lives in duration_from_epoch_millis, declared for every target so
the host can call it, and two tests pin the contract either side of the epoch.
Removing the guard fails them with the exact defect:

    assertion `left == right` failed
      left: Some(0ns)
     right: None

f64 as u64 saturates a negative reading to zero, so without the guard the browser
arm answers Some(0) where the std arm answers None, and a caller mapping absence
to an error would store a zero timestamp instead of reporting the fault.
Copilot AI lite review requested due to automatic review settings September 27, 2026 11:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@EnRaiha

EnRaiha commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing: the target split exists to serve wasm32, which this repository does not support. NodeDB is a server; wasm work belongs to NodeDB Lite. The part with standalone value — one clock helper over the 19 native call sites — returns as its own PR, without the wasm arm.

@EnRaiha EnRaiha closed this Sep 28, 2026
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.

wasm: 18 unguarded wall-clock reads in the shared crates \u2014 one of them reached a consumer and panicked

2 participants