Skip to content

feat(js): add exact u64 conversion - #80

Merged
wemeetagain merged 1 commit into
mainfrom
bing/to-u64-exact
Sep 25, 2026
Merged

wemeetagain merged 1 commit into
mainfrom
bing/to-u64-exact

Conversation

@spiral-ladder

@spiral-ladder spiral-ladder commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

@wemeetagain
wemeetagain merged commit 7626863 into main Sep 25, 2026
7 checks passed
@wemeetagain
wemeetagain deleted the bing/to-u64-exact branch September 25, 2026 13:24
wemeetagain pushed a commit that referenced this pull request Sep 25, 2026
🤖 I have created a release *beep* *boop*
---


##
[4.1.0](zapi-v4.0.0...zapi-v4.1.0)
(2026-09-25)


### Features

* **js:** add exact u64 conversion
([#80](#80))
([7626863](7626863))
* **js:** add js.spawn async task DSL
([#76](#76))
([e60cca6](e60cca6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@nazarhussain nazarhussain left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1. The name promises a range the function never returns in that range

toU64Exact maxes out at 2^53−1, everything from 2^53 to 2^64−1 throws. The doc comment says so, but the name sets the opposite expectation, and it sets it specifically because toU32Exact right above it is exact across its whole nominal range. Two parallel names, two different contracts.

So I suggest to keep the behaviour but change the name. toSafeInteger(), toU53Exact(), or just return u53 so the signature carries the cap instead of the prose.

2. It doesn't quite unblock the call site it came from

valueToU64 in lodestar-z#648 maps +Infinity -> maxInt(u64), that's how our chain configs spell "fork not scheduled". toU64Exact throws on Infinity, so the call site still needs a wrapper:

fn valueToU64(n: js.Number) !u64 {
    const d = try n.toF64();
    if (std.math.isPositiveInf(d)) return std.math.maxInt(u64);
    return n.toU64Exact() catch return error.InvalidChainConfigFieldValue;
}

Good addition, just worth saying the extraction is partial rather than a drop-in.

wemeetagain pushed a commit that referenced this pull request Sep 30, 2026
Follow-up to #80 / v4.1.0.

`toU64Exact` never returns a value above 2^53−1 — everything from 2^53
to 2^64−1 throws. The doc comment said so, but the name promised
otherwise, and it promised it *specifically* because `toU32Exact` sits
right above it and **is** exact across its entire nominal range. Two
parallel names, two different contracts.

### The 2^53 cap is correct and is kept

It isn't a shortcut — it's the only way to write this against an `f64`:

```
u53max as f64 = 9007199254740991       exact round-trip = true
u64max as f64 = 18446744073709552000   == 2^64, rounds_up = true
```

`@floatFromInt(std.math.maxInt(u64))` rounds **up** to exactly 2^64, so
a naive full-range `value > max` check would admit 2^64 itself and hand
it to `@intFromFloat`, which cannot represent it. So the behaviour
stays; only the name changes.

### What changed

- New private `toUnsignedExact(comptime T, comptime max_int)`, shared by
both public conversions — the ten-line body is no longer duplicated.
Each caller states its own bound, and a `comptime` assert makes a bound
above `maxInt(u53)` a compile error rather than one `@intFromFloat`
cannot represent. *(Updated from an earlier `@min`-derived bound after
review — see the thread on `src/js/number.zig`.)*
- `toU32Exact` — behaviour unchanged, now a one-liner over the helper.
- `toU64Exact` → `toSafeInteger() !u64`, with a doc comment that says
plainly that no exact `u64` conversion exists for a JS number and points
at `BigInt.toU64`.
- Adds DSL coverage for `toU32Exact`, which has had none since it landed
in #71, plus a case pinning `2**32` as the exact point where the two
conversions diverge.

### Breaking change

Renamed with no deprecation alias, and committed as `refactor!:` with a
`BREAKING CHANGE:` footer so release-please cuts a major. Keeping a
misleadingly-named alias for a whole major cycle would defeat the point
of the change.

**Migration is a rename at the call site** — behaviour is identical, the
same inputs are accepted and the same `error.InvalidUnsignedInteger` is
returned. lodestar-z is the only consumer, with 30 call sites:

| File | Sites |
|---|---|
| `bindings/napi/BeaconStateView.zig` | 29 |
| `bindings/napi/BeaconConfig.zig` | 1 |

`chainConfigU64` in `BeaconConfig.zig` already wraps the conversion to
map `+Infinity` to `maxInt(u64)` for disabled fork epochs; that wrapper
is unaffected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: bing <spiralladder@fastmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants