Repository navigation
fix(js)!: make BigInt conversions exact and add a shared LossyConversion error - #83
Draft
nazarhussain wants to merge 5 commits into
Draft
nazarhussain wants to merge 5 commits into
nazarhussain wants to merge 5 commits into
Conversation
napi fills the low-order words and the sign even when the BigInt needs more words than the buffer holds. Setting sign_bit before returning error.Overflow lets callers catch the error and read the truncated value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BigInt.toI64 and BigInt.toU64 now take no arguments and return error.InvalidArg when the value does not fit, instead of silently returning the low bits. The previous lossy behaviour moves to toI64LowBits and toU64LowBits, which keep the optional lossless flag. BigInt.toI128 now returns error.InvalidArg (was error.Overflow) for out-of-range values, and toI128LowBits adds BigInt.asIntN(128) truncation semantics. BREAKING CHANGE: BigInt.toI64(lossless) and BigInt.toU64(lossless) are now toI64() and toU64() and reject out-of-range values. Use toI64LowBits(lossless) / toU64LowBits(lossless) for truncation. BigInt.toI128 reports out-of-range values as error.InvalidArg. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onversion Exact conversions now share one error. BigInt previously reused InvalidArg, which collides with N-API's own invalid-argument status. BREAKING CHANGE: Number.toU32Exact and Number.toSafeInteger return error.LossyConversion instead of error.InvalidUnsignedInteger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nazarhussain
force-pushed
the
nh/bigint-exact-conversions
branch
from
October 19, 2026 10:31
0773d1c to
0dcc598
Compare
nazarhussain
added a commit
that referenced
this pull request
Nov 6, 2026
…90) Packages the fast-check fuzz targets from #41 as a Docker image for the [ChainSafe fuzz platform](https://github.com/ChainSafe/lodestar-ansible-development/tree/main/playbooks/fuzz), so they run continuously as one campaign (`zapi:round-trip`). > **Stacked on #41** (which is stacked on #83). Merge those first. ### Changes - `Dockerfile.fuzz` (+ `.dockerignore`): a build stage that downloads Zig 0.16.0 (checksum-verified) and builds `test_fuzz_numeric.node` with `ReleaseSafe` and `-Dcpu=baseline`. The runtime stage is `node:24-slim` with the addon, the tests and `node_modules`. - `tests/fuzz/platform/run-fuzz.ts`: the entrypoint, which implements the platform's container contract. It re-runs every `*.fuzz.test.ts` in a loop (100 000 cases per property by default, a fresh random seed each time) until SIGTERM. - A failing property is saved once to `$FUZZ_FAILURES/failure-<hash>.json`, de-duplicated by test name and shrunk counterexample. The file includes the seed and path for replay. Fuzzing then continues. - `$FUZZ_METRICS/status` is rewritten atomically after each iteration. It reports `execs_total`, `execs_per_second`, `crashes_total` and `last_find_timestamp_seconds`. There is no coverage or corpus field, because fast-check is not coverage-guided. - Counters persist in `$FUZZ_STATE/stats.json`. - A test file that fails to load, or a run with no tests, exits non-zero (an infrastructure failure). - `.github/workflows/fuzz-image.yml`: mirrors lodestar-z's `fuzz-image.yml`. A `build and test` job runs with read-only permissions and smoke-tests the image under the platform's restrictions (non-root uid, read-only rootfs, no capabilities, no network, `--init`). Publishing is **manual** (`workflow_dispatch`): a separate `publish` job, the only one with `packages: write`, pushes the exact tested image as `main` (run on `main`) or `branch-<name>` (any other branch), plus `sha-<short>`, and keeps the newest 20 versions. lodestar-z's AFL++-only steps (seed replay, `core_pattern`, minimize) are omitted; the platform reads none of them. - `tests/fuzz/README.md`: a section on continuous fuzzing. ### Verified locally (arm64 Docker, platform restrictions) - Runs continuously and stops on SIGTERM with exit 0. - The status file is written and updated. - Counters resume across restarts. - A planted failing property is saved once over about 20 failing iterations. - A hidden addon gives `FATAL a test suite failed to load` and exit 1. The amd64 build and the workflow itself have not run yet; this PR's CI is their first run. ### Follow-ups - After merging, publish by running the workflow on `main` from the Actions tab. Then set the GHCR package `zapi-fuzz` to **public**. The platform pulls without credentials. - Add the target to `group_vars/fuzzer/vars.yml` in lodestar-ansible-development: ```yaml - project: zapi target: round-trip image: ghcr.io/chainsafe/zapi-fuzz:main cpu_mhz: 1000 memory_mb: 1024 duration: 0 uid: 60011 gid: 60011 ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #41 so the library changes can be reviewed separately from the fuzz harness. The fuzz testing in #41 found these issues and depends on the new APIs, so #41 now targets this branch.
Why
The old
toI64(lossless)/toU64(lossless)truncated out-of-range BigInts silently unless the caller checked the optional flag. Passingnullthrew the signal away, so2n ** 64n + 5ncame back as5.assertI64/assertU64never caught overflow. They were documented to panic on failure, but they calledtoI64(null), which never errors for out-of-range values, so they returned truncated values. They now panic as documented.toU64doc was wrong. It said negative BigInts return an error. N-API actually succeeds, setslossless = false, and returns the wrapped bits.toI128, which already rejected out-of-range values on main.toI64LowBits(&lossless)makes deliberate truncation obvious in review.Changes
fix(js)!: BigInt conversions are exact by default.toI64()andtoU64()now returnerror.LossyConversionwhen the value does not fit, instead of silently returning the low bits.toI128()reports out-of-range values aserror.LossyConversion(waserror.Overflow).toI64LowBits(lossless)andtoU64LowBits(lossless)keep the previous N-API truncation behaviour.toI128LowBits()givesBigInt.asIntN(128, b)semantics.feat(js):BigInt.fromWords(sign_bit, words)builds BigInts wider than 64 bits, e.g.i128.fix(napi):getValueBigintWordssetssign_bitbefore returningerror.Overflow, so callers that catch the error can still read the truncated value with the correct sign.refactor(js)!: one error for lossy conversions.Number.toU32ExactandNumber.toSafeIntegernow returnerror.LossyConversion(waserror.InvalidUnsignedInteger), the same error as the exact BigInt conversions. A new name avoidsInvalidArg, which N-API itself returns for a bad argument.docs(js):Number.toI32,toU32andtoI64doc comments now describe N-API's truncation and clamping behaviour.error.LossyConversionAll exact integer conversions now fail with the same error when the JS value can't be represented exactly in the target Zig type:
error.LossyConversionwhenBigInt.toI64()i64rangeBigInt.toU64()u64rangeBigInt.toI128()[-2^127, 2^127)error.OverflowNumber.toU32Exact()maxInt(u32)error.InvalidUnsignedIntegerNumber.toSafeInteger()Number.MAX_SAFE_INTEGERerror.InvalidUnsignedIntegerThe
*LowBitsvariants never return it: they truncate by design and report precision loss through thelosslessflag.Why a new name instead of reusing an existing one:
InvalidArgis what N-API itself returns for a bad argument, so callers couldn't tell an N-API failure from a value that doesn't fit.InvalidUnsignedIntegerdoesn't describe the signed conversions (toI64,toI128).Overflowdoesn't describe fractional orNaNnumbers, andgetValueBigintWordsalready uses it to mean "buffer too small".losslessflag, so the error reads as "this conversion would not be lossless".Migration
Code that matched
error.OverflowfromtoI128, orerror.InvalidUnsignedIntegerfromNumber.toU32Exact/toSafeInteger, should matcherror.LossyConversion:On the JS side, the thrown error message changes from
InvalidUnsignedIntegertoLossyConversion.Testing
pnpm build,pnpm test:zig,pnpm test:js(147 tests) andpnpm lintpass.js_dslexample tests assert theLossyConversionerror name fortoU32Exact,toSafeIntegerand out-of-rangetoI128.🤖 Generated with Claude Code