feat: impl tree view serialize API - #138
Conversation
Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>
Summary of ChangesHello @GrapeBaBa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the SSZ (Simple Serialize) library by introducing a robust serialization API for its tree view data structures. The changes enable developers to easily convert in-memory tree views into their binary SSZ representation and determine their exact serialized size. This is a foundational improvement for data persistence, network transmission, and overall interoperability within systems utilizing SSZ, ensuring that complex data structures can be efficiently handled and validated. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a serialize API for tree views, which is a significant and useful feature. The implementation is comprehensive, covering various SSZ types and including a good set of tests. However, I've identified several areas for improvement, primarily concerning redundant operations, adherence to the style guide regarding return value assertions, and potential integer truncation issues. Addressing these points will enhance the code's efficiency, correctness, and robustness.
| try self.commit(); | ||
| const size = self.serializedSize(); | ||
| const output = try self.base_view.allocator.alloc(u8, size); | ||
| errdefer self.base_view.allocator.free(output); | ||
| _ = try self.serializeIntoBytes(output); | ||
| return output; |
There was a problem hiding this comment.
This serialize function has a couple of issues:
- Redundant
commit()call: The call totry self.commit()on line 85 is redundant becauseserializeIntoBytes(called on line 89) also callscommit(). This results in unnecessary work. - Ignored return value: The number of bytes written by
serializeIntoBytesis ignored. It's important to verify that the number of bytes written matches the allocated size to prevent potential buffer issues. An assertion should be added, as per the style guide's recommendation to assert return values (lines 51-56).
const size = self.serializedSize();
const output = try self.base_view.allocator.alloc(u8, size);
errdefer self.base_view.allocator.free(output);
const written = try self.serializeIntoBytes(output);
std.debug.assert(written == size);
return output;
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
| try self.commit(); | ||
| const size = try self.serializedSize(); | ||
| const output = try self.base_view.allocator.alloc(u8, size); | ||
| errdefer self.base_view.allocator.free(output); | ||
| _ = try self.serializeIntoBytes(output); | ||
| return output; |
There was a problem hiding this comment.
This serialize function is inefficient due to redundant calls to self.commit() and has a potential correctness issue.
- Redundant
commit()calls:serializecallsself.commit(), thenself.serializedSize()which also callscommit(), and finallyself.serializeIntoBytes()which callscommit()again. This results in threecommit()calls for a single serialization, which is inefficient. - Ignored return value: The number of bytes written by
serializeIntoBytesis ignored. It's important to verify that the number of bytes written matches the allocated size to prevent potential buffer issues. An assertion should be added, as per the style guide's recommendation to assert return values (lines 51-56).
const size = try self.serializedSize();
const output = try self.base_view.allocator.alloc(u8, size);
errdefer self.base_view.allocator.free(output);
const written = try self.serializeIntoBytes(output);
std.debug.assert(written == size);
return output;
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
| try self.commit(); | ||
| const size = try self.serializedSize(); | ||
| const output = try self.base_view.allocator.alloc(u8, size); | ||
| errdefer self.base_view.allocator.free(output); | ||
| _ = try self.serializeIntoBytes(output); | ||
| return output; |
There was a problem hiding this comment.
This serialize function is inefficient due to redundant calls to self.commit() and has a potential correctness issue.
- Redundant
commit()calls:serializecallsself.commit(), thenself.serializedSize()which also callscommit(), and finallyself.serializeIntoBytes()which callscommit()again. This results in threecommit()calls for a single serialization, which is inefficient. - Ignored return value: The number of bytes written by
serializeIntoBytesis ignored. It's important to verify that the number of bytes written matches the allocated size to prevent potential buffer issues. An assertion should be added, as per the style guide's recommendation to assert return values (lines 51-56).
const size = try self.serializedSize();
const output = try self.base_view.allocator.alloc(u8, size);
errdefer self.base_view.allocator.free(output);
const written = try self.serializeIntoBytes(output);
std.debug.assert(written == size);
return output;
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
| try self.commit(); | ||
| const size = try self.serializedSize(); | ||
| const output = try self.base_view.allocator.alloc(u8, size); | ||
| errdefer self.base_view.allocator.free(output); | ||
| _ = try self.serializeIntoBytes(output); | ||
| return output; |
There was a problem hiding this comment.
This serialize function is inefficient due to redundant calls to self.commit() and has a potential correctness issue.
- Redundant
commit()calls:serializecallsself.commit(), thenself.serializedSize()which also callscommit(), and finallyself.serializeIntoBytes()which callscommit()again. This results in threecommit()calls for a single serialization, which is inefficient. - Ignored return value: The number of bytes written by
serializeIntoBytesis ignored. It's important to verify that the number of bytes written matches the allocated size to prevent potential buffer issues. An assertion should be added, as per the style guide's recommendation to assert return values (lines 51-56).
const size = try self.serializedSize();
const output = try self.base_view.allocator.alloc(u8, size);
errdefer self.base_view.allocator.free(output);
const written = try self.serializeIntoBytes(output);
std.debug.assert(written == size);
return output;
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
| try self.commit(); | ||
| const size = try self.serializedSize(); | ||
| const output = try self.base_view.allocator.alloc(u8, size); | ||
| errdefer self.base_view.allocator.free(output); | ||
| _ = try self.serializeIntoBytes(output); | ||
| return output; |
There was a problem hiding this comment.
This serialize function is inefficient due to redundant calls to self.commit() and has a potential correctness issue.
- Redundant
commit()calls:serializecallsself.commit(), thenself.serializedSize()which also callscommit(), and finallyself.serializeIntoBytes()which callscommit()again. This results in threecommit()calls for a single serialization, which is inefficient. - Ignored return value: The number of bytes written by
serializeIntoBytesis ignored. It's important to verify that the number of bytes written matches the allocated size to prevent potential buffer issues. An assertion should be added, as per the style guide's recommendation to assert return values (lines 51-56).
const size = try self.serializedSize();
const output = try self.base_view.allocator.alloc(u8, size);
errdefer self.base_view.allocator.free(output);
const written = try self.serializeIntoBytes(output);
std.debug.assert(written == size);
return output;
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
|
|
||
| const value_serialized = try allocator.alloc(u8, TestContainer.serializedSize(&value)); | ||
| defer allocator.free(value_serialized); | ||
| _ = TestContainer.serializeIntoBytes(&value, value_serialized); |
There was a problem hiding this comment.
The return value of TestContainer.serializeIntoBytes is ignored. According to the style guide (lines 51-56), return values should be asserted to ensure correctness. You should assert that the number of bytes written is equal to the size of the value_serialized slice.
try std.testing.expectEqual(value_serialized.len, TestContainer.serializeIntoBytes(&value, value_serialized));
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
|
|
||
| const value_serialized = try allocator.alloc(u8, ListU8Type.serializedSize(&value)); | ||
| defer allocator.free(value_serialized); | ||
| _ = ListU8Type.serializeIntoBytes(&value, value_serialized); |
There was a problem hiding this comment.
The return value of ListU8Type.serializeIntoBytes is ignored. According to the style guide (lines 51-56), return values should be asserted to ensure correctness. You should assert that the number of bytes written is equal to the size of the value_serialized slice.
try std.testing.expectEqual(value_serialized.len, ListU8Type.serializeIntoBytes(&value, value_serialized));
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
|
|
||
| const value_serialized = try allocator.alloc(u8, ListU64Type.serializedSize(&value)); | ||
| defer allocator.free(value_serialized); | ||
| _ = ListU64Type.serializeIntoBytes(&value, value_serialized); |
There was a problem hiding this comment.
The return value of ListU64Type.serializeIntoBytes is ignored. According to the style guide (lines 51-56), return values should be asserted to ensure correctness. You should assert that the number of bytes written is equal to the size of the value_serialized slice.
try std.testing.expectEqual(value_serialized.len, ListU64Type.serializeIntoBytes(&value, value_serialized));
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
|
|
||
| const value_serialized = try allocator.alloc(u8, ListRootsType.serializedSize(&value)); | ||
| defer allocator.free(value_serialized); | ||
| _ = ListRootsType.serializeIntoBytes(&value, value_serialized); |
There was a problem hiding this comment.
The return value of ListRootsType.serializeIntoBytes is ignored. According to the style guide (lines 51-56), return values should be asserted to ensure correctness. You should assert that the number of bytes written is equal to the size of the value_serialized slice.
try std.testing.expectEqual(value_serialized.len, ListRootsType.serializeIntoBytes(&value, value_serialized));
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
|
|
||
| const value_serialized = try allocator.alloc(u8, ListContainerType.serializedSize(&value)); | ||
| defer allocator.free(value_serialized); | ||
| _ = ListContainerType.serializeIntoBytes(&value, value_serialized); |
There was a problem hiding this comment.
The return value of ListContainerType.serializeIntoBytes is ignored. According to the style guide (lines 51-56), return values should be asserted to ensure correctness. You should assert that the number of bytes written is equal to the size of the value_serialized slice.
try std.testing.expectEqual(value_serialized.len, ListContainerType.serializeIntoBytes(&value, value_serialized));
References
- Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The purpose of a function is to increase the probability that a program is correct. Assertions within a function are part of how functions serve this purpose. The assertion density of the code must average a minimum of two assertions per function. (link)
Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>
There was a problem hiding this comment.
Pull request overview
This PR implements the tree view serialize API for SSZ types, enabling direct serialization from TreeViews without converting back to value types. The implementation adds serialize, serializeIntoBytes, and serializedSize methods across all TreeView types and underlying SSZ type trees.
Key changes:
- Added tree-level serialization methods to core SSZ types (uint, bool, bitlist/bitvector, byte list/vector, list/vector/container variants)
- Implemented TreeView serialize methods that commit pending changes before delegating to tree serialization
- Added comprehensive test coverage with roundtrip tests, root validation, and size assertions
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/int/ssz/tree_view/list_composite.zig | Added serialization tests for ByteVector32 and Container lists, including push operations |
| test/int/ssz/tree_view/list_basic.zig | Added serialization tests for uint8/uint64 lists with push and sliceTo operations |
| test/int/ssz/tree_view/container.zig | Added serialization tests for containers with basic fields and nested lists |
| test/int/ssz/tree_view/array_composite.zig | Added serialization tests for ByteVector32 and Container vectors |
| test/int/ssz/tree_view/array_basic.zig | Added serialization tests for uint8/uint64 vectors |
| src/ssz/type/vector.zig | Implemented tree serializeIntoBytes/serializedSize for fixed and variable vectors |
| src/ssz/type/uint.zig | Implemented tree serializeIntoBytes/serializedSize for uint types |
| src/ssz/type/list.zig | Implemented tree serializeIntoBytes/serializedSize for fixed and variable lists |
| src/ssz/type/container.zig | Refactored and implemented tree serializeIntoBytes/serializedSize for containers |
| src/ssz/type/byte_vector.zig | Implemented tree serializeIntoBytes/serializedSize for byte vectors |
| src/ssz/type/byte_list.zig | Implemented tree serializeIntoBytes/serializedSize for byte lists |
| src/ssz/type/bool.zig | Implemented tree serializeIntoBytes/serializedSize for bool type |
| src/ssz/type/bit_vector.zig | Implemented tree serializeIntoBytes/serializedSize for bit vectors |
| src/ssz/type/bit_list.zig | Implemented tree serializeIntoBytes/serializedSize for bit lists with padding |
| src/ssz/tree_view/list_composite.zig | Added serialize/serializeIntoBytes/serializedSize methods to ListCompositeTreeView |
| src/ssz/tree_view/list_basic.zig | Added serialize/serializeIntoBytes/serializedSize methods to ListBasicTreeView |
| src/ssz/tree_view/container.zig | Added serialize/serializeIntoBytes/serializedSize methods to ContainerTreeView |
| src/ssz/tree_view/array_composite.zig | Added serialize/serializeIntoBytes/serializedSize methods to ArrayCompositeTreeView |
| src/ssz/tree_view/array_basic.zig | Added serialize/serializeIntoBytes/serializedSize methods to ArrayBasicTreeView |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The variable Uint64 is declared but never used in this test. It should be removed to clean up the code.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
| const Uint64 = ssz.UintType(64); | ||
| const ListU64 = ssz.FixedListType(Uint64, 128); | ||
| const TestContainer = ssz.VariableContainerType(struct { | ||
| a: ssz.FixedListType(ssz.UintType(64), 128), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = ListU64; |
There was a problem hiding this comment.
The variable ListU64 is declared but never used in this test. It should be removed to clean up the code.
| const Uint64 = ssz.UintType(64); | |
| const ListU64 = ssz.FixedListType(Uint64, 128); | |
| const TestContainer = ssz.VariableContainerType(struct { | |
| a: ssz.FixedListType(ssz.UintType(64), 128), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = ListU64; | |
| const TestContainer = ssz.VariableContainerType(struct { | |
| a: ssz.FixedListType(ssz.UintType(64), 128), | |
| b: ssz.UintType(64), | |
| }); |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The variable Uint64 is declared but never used in this test. It should be removed to clean up the code.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
| const Node = @import("persistent_merkle_tree").Node; | ||
| const Gindex = @import("persistent_merkle_tree").Gindex; | ||
| const isBasicType = @import("../type/type_kind.zig").isBasicType; | ||
| const isFixedType = @import("../type/type_kind.zig").isFixedType; |
There was a problem hiding this comment.
The import isFixedType is declared but never used in this file. It should be removed to clean up the code.
| const isFixedType = @import("../type/type_kind.zig").isFixedType; |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The variable Uint64 is declared but never used in this test. It should be removed to clean up the code.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The variable Uint64 is declared but never used in this test. It should be removed to clean up the code.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
twoeths
left a comment
twoeths
left a comment
There was a problem hiding this comment.
looks good to me, I dropped one minor comment regarding not to support serialize() apis for now
Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const Uint64 = ssz.UintType(64); | ||
| const ListU64 = ssz.FixedListType(Uint64, 128); | ||
| const TestContainer = ssz.VariableContainerType(struct { | ||
| a: ssz.FixedListType(ssz.UintType(64), 128), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = ListU64; |
There was a problem hiding this comment.
The line _ = Uint64; declares and immediately discards the Uint64 variable. This appears to be dead code that serves no purpose. Consider removing this unused variable declaration.
| const Uint64 = ssz.UintType(64); | |
| const ListU64 = ssz.FixedListType(Uint64, 128); | |
| const TestContainer = ssz.VariableContainerType(struct { | |
| a: ssz.FixedListType(ssz.UintType(64), 128), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = ListU64; | |
| const TestContainer = ssz.VariableContainerType(struct { | |
| a: ssz.FixedListType(ssz.UintType(64), 128), | |
| b: ssz.UintType(64), | |
| }); |
| @@ -155,24 +155,20 @@ pub fn FixedContainerType(comptime ST: type) type { | |||
| return try Node.fillWithContents(pool, &nodes, chunk_depth); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Missing documentation comment for the public serializeIntoBytes function. Consider adding a doc comment that explains the function's purpose, parameters, return value, and any error conditions, consistent with the documentation style in other tree view files.
| /// Serialize the container represented by `node` into SSZ bytes. | |
| /// | |
| /// The serialized bytes are written into `out`, which must be large | |
| /// enough to hold the fixed-size serialization of this container | |
| /// (typically `fixed_size`). | |
| /// | |
| /// Returns the number of bytes written into `out`. | |
| /// | |
| /// Errors: | |
| /// * Propagates any error from `node.getNodesAtDepth`. | |
| /// * Propagates any error returned by the `serializeIntoBytes` | |
| /// implementations of the container's field types. |
| } | ||
| return try Node.fillWithContents(pool, &nodes, chunk_depth); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation comment for the public serializeIntoBytes function. Consider adding a doc comment that explains the function's purpose, parameters, return value, and any error conditions.
| /// Serializes the vector represented by `node` into the provided byte buffer. | |
| /// | |
| /// This function reads the backing nodes for the vector from `pool`, then writes | |
| /// the SSZ-serialized bytes into `out`. For basic element types, the elements are | |
| /// read directly from the leaf chunks; for composite element types, serialization | |
| /// is delegated to `Element.tree.serializeIntoBytes` for each element. | |
| /// | |
| /// Parameters: | |
| /// - node: Root node identifier of the vector in the persistent Merkle tree. | |
| /// - pool: Pointer to the node pool used to resolve `node` and its children. | |
| /// - out: Destination buffer; must be at least `fixed_size` bytes long. | |
| /// | |
| /// Returns: | |
| /// The total number of bytes written to `out`, which is always `fixed_size` on success. | |
| /// | |
| /// Errors: | |
| /// Propagates any error returned by `node.getNodesAtDepth` or, for composite | |
| /// element types, by `Element.tree.serializeIntoBytes`. |
| } | ||
| return fixed_size; | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation comment for the public serializedSize function. Consider adding a doc comment that explains the function's purpose, parameters, and return value.
| /// Returns the size in bytes of the SSZ-serialized form of this vector. | |
| /// The result is constant for this fixed-size type and does not depend on | |
| /// the specific `node` in the tree or the contents of the `pool`. | |
| /// | |
| /// Parameters: | |
| /// - `node`: Tree node representing the vector (unused, as the size is fixed). | |
| /// - `pool`: Node pool associated with the tree (unused for size computation). | |
| /// | |
| /// Returns: | |
| /// - The fixed serialized size, in bytes, of this vector type. |
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The line _ = Uint64; declares and immediately discards the Uint64 variable. This appears to be dead code that serves no purpose. Consider removing this unused variable declaration.
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| a: Uint64, | |
| b: Uint64, | |
| }); |
| return i; | ||
| return offset; | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation comment for the public serializedSize function. Consider adding a doc comment that explains the function's purpose, parameters, and return value, consistent with the documentation style in other tree view files.
| /// Returns the fixed number of bytes required to serialize this container. | |
| /// | |
| /// This implementation ignores the `node` and `pool` parameters because | |
| /// fixed-size containers always have the same serialized length, independent | |
| /// of their contents. | |
| /// | |
| /// Parameters: | |
| /// node - Root node of the container in the backing tree (unused). | |
| /// pool - Tree node pool used for allocation and lookup (unused). | |
| /// | |
| /// Returns: The constant SSZ serialized size, in bytes, for this container type. |
|
|
||
| var total_size: usize = 0; | ||
| for (0..len) |i| { | ||
| total_size += Element.tree.serializedSize(nodes[i], pool); |
There was a problem hiding this comment.
The serializedSize function for composite lists should return an error union type like serializedSize in other composite types (list_composite, variable containers). However, it's being called without error handling in line 338. This inconsistency could lead to compilation issues or incorrect behavior when the function signature requires error handling.
| total_size += Element.tree.serializedSize(nodes[i], pool); | |
| total_size += try Element.tree.serializedSize(nodes[i], pool); |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The line _ = Uint64; declares and immediately discards the Uint64 variable. This appears to be dead code that serves no purpose. Consider removing this unused variable declaration.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The line _ = Uint64; declares and immediately discards the Uint64 variable. This appears to be dead code that serves no purpose. Consider removing this unused variable declaration.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
| const Uint64 = ssz.UintType(64); | ||
| const TestContainer = ssz.FixedContainerType(struct { | ||
| a: ssz.UintType(64), | ||
| b: ssz.UintType(64), | ||
| }); | ||
| _ = Uint64; |
There was a problem hiding this comment.
The line _ = Uint64; declares and immediately discards the Uint64 variable. This appears to be dead code that serves no purpose. Consider removing this unused variable declaration.
| const Uint64 = ssz.UintType(64); | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); | |
| _ = Uint64; | |
| const TestContainer = ssz.FixedContainerType(struct { | |
| a: ssz.UintType(64), | |
| b: ssz.UintType(64), | |
| }); |
Motivation
Implement the tree view serialize API
Description
serialize,serializeIntoBytes, andserializedSizemethods to the TreeViews(array basic/composite, list basic/composite, container), committing pending changes and serializing from the backing merkle tree.serializeIntoBytes/serializedSizefor core SSZ types (uint, bool, bitlist/bitvector, byte list/vector, list/vector/container variants) so TreeViews can delegate to the underlying type’s tree serialization.