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.

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