Conversation
quietbits
left a comment
There was a problem hiding this comment.
I'm not super familiar with this repo, but the update looks OK to me.
There was a problem hiding this comment.
Pull request overview
This PR addresses silent truncation when encoding/constructing LargeInt-backed integers by introducing explicit range checks (instead of relying on BigInt.asUintN/asIntN, which truncate) and expanding unit coverage around overflow/underflow behavior.
Changes:
- Tighten
LargeIntread/write +isValidto respect signedness and enforce[MIN_VALUE, MAX_VALUE]. - Update bigint assembly logic to validate slice inputs (preventing per-slice truncation) and to fast-fail on single-value overflow/underflow.
- Add unit tests for overflow/underflow for
Hyper/UnsignedHyper, and add a new 128-bitLargeInttest suite.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/large-int.js |
Read/write paths updated to handle signed 64-bit reads correctly and to enforce range checks; isValid now validates range. |
src/bigint-encoder.js |
Adds single-value bounds validation and slice-fit assertions to prevent silent truncation during assembly. |
test/unit/bigint-encoder_test.js |
Adds tests for slice overflow/underflow and exact boundary acceptance. |
test/unit/hyper_test.js |
Adds overflow/underflow construction tests for signed 64-bit Hyper. |
test/unit/unsigned-hyper_test.js |
Adds overflow/underflow construction tests for unsigned 64-bit UnsignedHyper. |
test/unit/large-int-128_test.js |
New coverage for >64-bit LargeInt paths including construction, read/write, and isValid. |
CHANGELOG.md |
Documents the truncation fix in Unreleased notes (but see review comments re: accuracy/formatting). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
There are multiple consecutive blank lines added after this bullet, which introduces unnecessary whitespace in the Unreleased section. Consider removing the extra empty lines to keep the changelog formatting consistent.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: George <Shaptic@users.noreply.github.com>
What
This fixes silent truncation that occurred when encoding
LargeIntby checking that the bytes to be encoded are of the appropriate size. This is necessary as theBigInt.asUintNandBigInt.asintNmethods truncate the higher bits when they exceed the providedsizeparam given to the function.Validation
Tests were added to check functionality at the max and min bounds of the sized integer. Values that are beyond the bounds were tested to throw.