Initial fixed array size implementation #119
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for fixed-size arrays in the ABI definitions, serialization logic, and encoding routines, and includes tests validating the new behavior.
- Introduces a
sizemodifier for fixed-length arrays and updates ABI synthesis and type formatting to emit[N]syntax. - Adjusts the encoder to omit the length prefix for fixed-size arrays.
- Updates chain ABI parsing to recognize and preserve
[N]in type names, plus tests for encoding/decoding fixed-size arrays.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/chain.ts | Added tests for fixed-size array handling in ABI and encoding |
| src/serializer/serializable.ts | Added size modifier and updated synthesizeABI/abiTypeString |
| src/serializer/encoder.ts | Modified encodeAny to skip length prefix for fixed-size arrays |
| src/chain/abi.ts | Extended ABI parser to detect and represent fixed-size arrays |
| Makefile | Updated Mocha options with --no-experimental-strip-types |
| .github/workflows/test.yml | Expanded CI matrix to include Node.js 20, 22, and 24 versions |
Comments suppressed due to low confidence (1)
test/chain.ts:867
- Add a test case to verify that providing an array with incorrect length for a fixed-size field throws a clear error, ensuring the validation path is covered.
})
| } | ||
| const len = value.length | ||
| ctx.encoder.writeVaruint32(len) | ||
| if (!type.size) { |
There was a problem hiding this comment.
Replace if (!type.size) with if (type.size === undefined) so that a fixed-size array of length 0 is not treated as a dynamic array and does not get a length prefix.
| if (!type.size) { | |
| if (type.size === undefined) { |
There was a problem hiding this comment.
I guess we could make this change, but it's probably fine to leave it as is cause I don't think that many devs are going to use an array of length 0 😅
There was a problem hiding this comment.
for what it's worth, arrays of size 0 are explicitely not supported in the abi, so this could be treated as an error: https://github.com/AntelopeIO/spring/wiki/ABI-1.3:-Fixed-sized-arrays
| if (!Array.isArray(value)) { | ||
| throw new Error(`Expected array for: ${type.typeName}`) | ||
| } | ||
| const len = value.length |
There was a problem hiding this comment.
Add a validation after computing len to ensure that when type.size is defined, len matches type.size, e.g., if (type.size !== undefined && len !== type.size) throw new Error(Expected fixed-size array of length ${type.size} for: ${type.typeName});.
| const len = value.length | |
| const len = value.length | |
| if (type.size !== undefined && len !== type.size) { | |
| throw new Error(`Expected fixed-size array of length ${type.size} for: ${type.typeName}`) | |
| } |
There was a problem hiding this comment.
I don't know if we want to throw an error in this situation..
Resolves #118