Fix array decoder empty check - #1884
Merged
Merged
Conversation
Replace the O(n) buffer copy in getArrayDecoder's emptiness check with an O(1) offset comparison
🦋 Changeset detectedLatest commit: 61288f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Member
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
LGTM ✅ (submitting as a comment — I don't have approve permissions on this repo)
Replaces bytes.slice(offset).length === 0 with offset >= bytes.length in getArrayDecoder's prefixed-size emptiness check, turning an O(remaining bytes) allocation+copy into an O(1) comparison. Since getMapDecoder and getSetDecoder delegate to getArrayDecoder, they pick up the fix for free. Fixes the quadratic decoding behaviour from #1877.
I verified the equivalence claim:
- For
offsetin[0, bytes.length),slice(offset)is non-empty andoffset >= bytes.lengthis false — same result. - For
offset >= bytes.length,slice(offset)is empty and the comparison is true — same result. - The only divergence is a negative
offsetagainst an empty buffer (old code returned[[], offset], new code falls through to the prefix read and throws), which the framework rules out:decode()starts at 0 and reads only ever advance the offset, as the PR description lays out. Arguably the new behaviour (throwing on an out-of-range offset) is more correct there anyway.
The check still only applies when typeof size === 'object' (prefixed sizes), so 'remainder' and fixed-number sizes are untouched.
Changeset is present with a patch bump for @solana/codecs-data-structures, which matches the perf-only, behaviour-preserving nature of the change.
Notes for other reviewers: nothing needs extra attention — it's a one-line hot-path change with a clear equivalence argument. If you want extra confidence, the existing array/map/set decoder tests covering empty-bytes decoding (decode(new Uint8Array([])) returning []) exercise the changed branch directly.
BundleMonUnchanged files (150)
Total files change -10B 0% Final result: ✅ View report in BundleMon website ➡️ |
mcintyre94
approved these changes
Aug 4, 2026
mcintyre94
left a comment
mcintyre94
left a comment
Member
There was a problem hiding this comment.
Awesome, thank you for this!
Contributor
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
This was referenced Aug 4, 2026
Merged
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
3 participants
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.
Problem
"getArrayDecoder" emptiness check copies the whole remaining buffer, making large-account decoding quadratic
Summary of Changes
For the non-negative offsets the codec framework guarantees (decode() starts at 0, composite reads only ever advance the offset, and offsetCodec normalises and asserts its offsets into range before the inner codec sees them), bytes.slice(offset).length === 0 is true precisely when offset >= bytes.length.
Fixes #1877