Add a function to create a decoder that checks the size of the input bytes #944
+173
−0
Conversation
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
🦋 Changeset detectedLatest commit: 7635876 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BundleMonFiles updated (7)
Unchanged files (123)
Total files change +1.03KB +0.29% Final result: ✅ View report in BundleMon website ➡️ |
mcintyre94
force-pushed
the
assert-uses-entire
branch
2 times, most recently
from
4bab6b3 to
9529670
Compare
Contributor
|
Documentation Preview: https://kit-docs-by9igyds3-anza-tech.vercel.app |
steveluscher
approved these changes
Oct 6, 2025
| * decoder.decode(new Uint8Array([0, 0, 0, 0]), 1); // throws | ||
| * ``` | ||
| */ | ||
| export function createDecoderThatUsesExactByteArray<T>(decoder: Decoder<T>): Decoder<T> { |
Collaborator
There was a problem hiding this comment.
ConsumesEntire?
¯\_(°ペ)_/¯
Member
Author
There was a problem hiding this comment.
The reason I switched from Entire to Exact was because I started writing tests for things like "this decoder is going to read 4 bytes, and you give it 3 bytes, this gives you a useful error". So I was checking too long and too short, which Exact felt like a better fit for.
But I think this was the wrong approach because eg. our number codecs already error when the input bytes are too short, and I think it probably makes sense to leave that as the responsibility for the codec and just cover the case where the input bytes are too long here - which we don't want codecs to check by default.
I'll change this to just check for a byte array being too long, which I think your other suggestions fit better with too.
Comment on lines
38
to
40
| bytesLength: bytes.length, | ||
| offsetAfterDecoding: newOffset, | ||
| offsetBeforeDecoding: offset, |
Collaborator
There was a problem hiding this comment.
This makes people do math. Just expectedBytes/actualBytes or just numExcessBytes?
Member
Author
There was a problem hiding this comment.
Yep agreed. Again this was because I wanted to cover the case where you provide too few bytes, and figured the offset before/after is the most helpful information there. But I can remove that case and provide a more helpful error that just considers the bytes being too long.
packages/errors/src/messages.ts
Outdated
| [SOLANA_ERROR__CODECS__UNION_VARIANT_OUT_OF_RANGE]: | ||
| 'Union variant out of range. Expected an index between $minRange and $maxRange, got $variant.', | ||
| [SOLANA_ERROR__CODECS__BYTES_LENGTH_DECODER_MISMATCH]: | ||
| 'Byte array of length `$bytesLength` was not decoded to the last byte by the provided decoder. End offset was `$offsetAfterDecoding`.', |
Collaborator
There was a problem hiding this comment.
Maybe something with a suggestion of what to do, more like:
This decoder expected a byte array of exactly
$expectedLengthbytes, but$numExcessBytesunexpected excess bytes remained after decoding. Are you sure that you have chosen the correct decoder for this data?
steveluscher
reviewed
Oct 6, 2025
lorisleiva
reviewed
Oct 7, 2025
| * ``` | ||
| */ | ||
| export function createDecoderThatUsesExactByteArray<T>(decoder: Decoder<T>): Decoder<T> { | ||
| return transformDecoder(decoder, (value, bytes, offset, newOffset) => { |
Member
There was a problem hiding this comment.
Since transformDecoder is just:
return createDecoder({
...decoder,
read: (bytes, offset) => {
// Some mapping.
},
});Could we not avoid the extra argument to transformDecoder and simply use createDecoder for this helper?
Member
Author
There was a problem hiding this comment.
That makes sense to me and should make this PR simpler, will refactor. Thanks for the pointer!
mcintyre94
force-pushed
the
assert-uses-entire
branch
from
9529670 to
7635876
Compare
Member
Author
|
Thanks both for the review! I've made a bunch of changes:
|
Contributor
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
This was referenced Oct 8, 2025
Merged
Contributor
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
When an app has access to many codecs, for example when using generated clients, it is easy to write code that accidentally uses the wrong type of
Decoderfor a given byte array. This is difficult to debug as you will often be able to use the wrong decoder without any errors, but will be getting invalid data.Summary of Changes
This PR adds a new helper function
createDecoderThatUsesExactByteArraythat can wrap any decoder. The transformed decoder throws if the new offset after decoding a byte array, is not at the end of the input bytes.This means that if you pass a byte array that is longer or shorter than the wrapped decoder expects, the decode will fail.
As noted in the issue this is not a strong guarantee that you're using the correct decoder, many decoders are the same size or variable size. But it should catch a lot of difficult to debug cases in practice, such as decoding account data using the wrong decoder.
Note that to enable this, the signature of
transformDecoderis modified to additionally provide thenewOffsetto the callback. We use both the original offset and the new offset in the error context.Fixes #755