Repository navigation
chore: merge master into te/serialized_list - #557
Merged
wemeetagain merged 2 commits intoOct 16, 2026
Merged
wemeetagain merged 2 commits into
wemeetagain merged 2 commits into
Conversation
…lements (ChainSafe#556) **Motivation** `readOffsetsArrayComposite()` returned one offset per element, so both deserialize loops in `arrayComposite.ts` had to special case the last element (`i === length - 1 ? end : start + offsets[i + 1]`). The comments next to them ("Capture length before pushing end offset", "offests include the last element end") describe an n + 1 layout that the code no longer had. The fixed size branch also built an offsets array of `i * fixedSize` just so both element kinds could share one loop, while serialization already writes fixed size elements at `offset + i * fixedSize` directly. This also lets ChainSafe#555 drop the `withEndOffset` flag it adds to `readVariableOffsetsArrayComposite()`, since that is the layout `SerializedList` needs. **Description** - Fixed size elements are read at `start + i * fixedSize` in `value_deserializeFromBytesArrayComposite()`, `tree_deserializeFromBytesArrayComposite()` and `ProgressiveListCompositeType.tree_deserializeFromBytes()`, with no offsets array. The length checks (`size % fixedSize`, `assertValidArrayLength()`) and their errors are unchanged - `readVariableOffsetsArrayComposite()` now returns n + 1 offsets, with a trailing entry equal to `end - start`. An empty array returns `[0]` - `assertValidArrayLength()` still gets the element count (`offsets.length - 1`) - The variable size loops use `endEl = start + offsets[i + 1]` with no branch The offset readers are module private and the deserialize functions above are their only callers. ProgressiveList keeps its own variable size offset reader. Validation and error order are unchanged. The trailing write happens after all offset checks. Fixed size elements use plain number math, so they don't wrap at any size. Variable size offsets stay in a `Uint32Array`, except when the array is 2^32 bytes or larger. Then they go in a `Float64Array` so the trailing entry doesn't wrap, and the last element still ends at `end` as before. The SSZ spec caps the serialized size below 2^32, but the library accepted these inputs before this change. **Tests** - `pnpm test:unit`: 1814 passed - `pnpm test:spec-generic`: 5065 passed - `pnpm test:spec-static-minimal`: 13284 passed, `pnpm test:spec-static-mainnet`: 540 passed - `pnpm lint` clean **Benchmarks** Local, node 24 (V8 13.6), at 08422c6. Medians of 6 interleaved master/branch rounds per case, with the side that runs first alternating each round. `deserialize()` = value, `deserializeToView()` = tree. Cases up to 128 elements come from the small file (3 to 6 s per case), the rest from the large file (1.5 to 4 s per case). | case | value master | value branch | tree master | tree branch | | --- | --- | --- | --- | --- | | List[ByteList] 1 x 100B | 0.91 us | 0.90 us | 2.83 us | 2.73 us | | List[ByteList] 4 x 100B | 3.08 us | 2.99 us | 7.07 us | 6.84 us | | List[ByteList] 1000 x 1000B | 1.10 ms | 1.06 ms | 3.71 ms | 3.79 ms | | List[ByteList] 10000 x 100B | 9.14 ms | 8.51 ms | 24.66 ms | 20.82 ms | | List[ByteList] 100000 x 0B | 8.46 ms | 8.99 ms | 34.71 ms | 27.17 ms | | ProgressiveList[ProgressiveByteList] 10000 x 100B | 8.98 ms | 8.72 ms | - | - | | List[Container(fixed 40B)] 1 | 0.52 us | 0.48 us | 2.69 us | 2.54 us | | List[Container(fixed 40B)] 16 | 2.20 us | 2.11 us | 6.73 us | 6.23 us | | List[Container(fixed 40B)] 128 | 14.15 us | 13.01 us | 37.04 us | 35.58 us | | List[Container(fixed 40B)] 100000 | 23.81 ms | 26.08 ms | 50.51 ms | 47.99 ms | | ProgressiveList[Container(fixed 40B)] 16 | 2.50 us | 2.19 us | 5.85 us | 5.19 us | | ProgressiveList[Container(fixed 40B)] 100000 | 23.39 ms | 22.52 ms | 51.03 ms | 45.75 ms | | Vector[Bytes32, 4] | 0.63 us | 0.57 us | 0.98 us | 0.91 us | | Vector[Bytes32, 8192] | 453.48 us | 413.21 us | 1.15 ms | 960.27 us | The fixed size cases are a few percent faster than master because nothing is allocated for offsets anymore. This also removes the n = 16 regression the first commit had (`List[Container(fixed 40B)] 16` value: 2.16 us on master vs 2.89 us, from `Uint32Array(17)` crossing V8's 64 byte on heap limit). It's now 2.20 us on master vs 2.11 us. Variable size cases are unchanged within noise. On this machine the large cases vary by about 20% between rounds of the same code, so differences below that only show up in the small cases. <details><summary>Bench file (not committed). The small cases used the same file with counts 1/4 (ByteList), 1/16/128 (Container) and Vector[Bytes32, 4], without the ProgressiveList cases</summary> ```ts import {bench, describe} from "@chainsafe/benchmark"; import { ByteListType, ByteVectorType, CompositeType, ContainerType, ListCompositeType, ProgressiveByteListType, ProgressiveListCompositeType, UintNumberType, VectorCompositeType, } from "../../../src/index.ts"; const bytes32 = new ByteVectorType(32); const uint64 = new UintNumberType(8); const byteList = new ByteListType(2 ** 30); const fixedContainer = new ContainerType({root: bytes32, value: uint64}, {typeName: "Container(fixed 40B)"}); function filledBytes(len: number, seed: number): Uint8Array { const b = new Uint8Array(len); for (let i = 0; i < len; i++) b[i] = (seed + i) & 0xff; return b; } function byteLists(count: number, size: number): Uint8Array[] { return Array.from({length: count}, (_, i) => filledBytes(size, i)); } describe("arrayComposite deserialize", () => { const cases: {id: string; type: CompositeType<unknown, unknown, unknown>; value: unknown; tree: boolean}[] = []; // variable size elements, readVariableOffsetsArrayComposite() for (const [count, size] of [ [4, 100], [1_000, 1_000], [10_000, 100], [100_000, 0], ] as const) { cases.push({ id: `List[ByteList] ${count} x ${size}B`, type: new ListCompositeType(byteList, 2 ** 20), value: byteLists(count, size), tree: true, }); } cases.push({ id: "ProgressiveList[ProgressiveByteList] 10000 x 100B", type: new ProgressiveListCompositeType(new ProgressiveByteListType()), value: byteLists(10_000, 100), tree: false, }); // fixed size elements, start + i * fixedSize for (const count of [16, 100_000]) { cases.push({ id: `List[Container(fixed 40B)] ${count}`, type: new ListCompositeType(fixedContainer, 2 ** 40), value: Array.from({length: count}, (_, i) => ({root: filledBytes(32, i), value: i})), tree: true, }); } for (const count of [16, 100_000]) { cases.push({ id: `ProgressiveList[Container(fixed 40B)] ${count}`, type: new ProgressiveListCompositeType(fixedContainer), value: Array.from({length: count}, (_, i) => ({root: filledBytes(32, i), value: i})), tree: true, }); } cases.push({ id: "Vector[Bytes32, 8192]", type: new VectorCompositeType(bytes32, 8192), value: Array.from({length: 8192}, (_, i) => filledBytes(32, i)), tree: true, }); for (const {id, type, value, tree} of cases) { const bytes = type.serialize(value); bench({ id: `${id} - value`, fn: () => { type.deserialize(bytes); }, }); if (tree) { bench({ id: `${id} - tree`, fn: () => { type.deserializeToView(bytes); }, }); } } }); ``` </details> 🤖 Generated with AI assistance --------- Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
Resolve the conflict with ChainSafe#556 in readVariableOffsetsArrayComposite(): keep master's version, which always returns n + 1 offsets (Float64Array at 2^32 bytes and above), and keep it exported. Drop the withEndOffset argument in SerializedListBaseType and widen SerializedList.offsets to Uint32Array | Float64Array. 🤖 Generated with AI assistance
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
wemeetagain
approved these changes
Oct 16, 2026
wemeetagain
added a commit
that referenced
this pull request
Oct 16, 2026
PR #557 brought master's changes into this branch with a squash commit, so the PR comparison still includes the changes from #556. Record the actual merge with master to correct the comparison base. The resulting tree is identical to ca973a6; preserve the exported readVariableOffsetsArrayComposite helper required by SerializedListType.
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
2 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.
Motivation
#556 is merged and conflicts with this branch in
readVariableOffsetsArrayComposite().Description
Merges
masterintote/serialized_list:readVariableOffsetsArrayComposite()takes master's version and stays exported. It always returns n + 1 offsets now, sowithEndOffsetis gone. Offsets are aFloat64Arraywhen the list is 2^32 bytes or larger, so the trailing entry doesn't wrapSerializedListBaseType.value_deserializeFromBytes()drops thetrueargumentSerializedList.offsetsisUint32Array | Float64Arrayto match. Its doc comment no longer says that list types keep only n offsetsIf you'd rather keep
offsetsa plainUint32Array, the alternative is to reject serialized lists of 2^32 bytes or more inSerializedListBaseType. That would differ fromListCompositeType, which accepts them.Tests
pnpm test:unit: 2405 passedpnpm test:spec-generic: 5065 passedpnpm test:spec-static-minimal: 13284 passed,pnpm test:spec-static-mainnet: 540 passedpnpm lintclean🤖 Generated with AI assistance