Skip to content

Commit ce6370e

Browse files
twoethswemeetagainclaude
authored
refactor(ssz): drop BaseTreeView, use tuple for ContainerTreeView (#139)
## Motivation The previous `BaseTreeView` design had several structural problems: - Child TreeViews were returned as shallow copies (`{.base_view=...}`), risking double-free or dangling pointers - Extra per-type fields (e.g. `length`, cached child arrays) could not be stored in child views without hacks - All TreeView types were tightly coupled, making `ContainerNodeStruct` (and similar) hard to implement - Parent `commit()` could not propagate through modified child TreeViews because children were copies, not references Part of #78. ## Changes **Drop `BaseTreeView`** — each TreeView is now a self-contained struct. Reusable logic (e.g. `getChildNode`, `setChildNode`, `getLength`, `setLength`) is shared via standalone utilities, not inheritance. **`ContainerTreeView` now uses a comptime tuple** instead of a runtime Map. Each field slot holds either a reference to a child TreeView or a native basic-type value. This gives type-level access to the child type at a given index, enabling future `ContainerNodeStructTreeView`. **`ArrayBasicTreeView` / `ArrayCompositeTreeView`** now track: - A `Map` of dirty nodes (committed to cache only on `commit()`) - A `length` field updated lazily - For composite: a `Map` of child TreeView references (so parent `commit()` propagates correctly) --------- Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com> Co-authored-by: Cayman <caymannava@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 387e770 commit ce6370e

38 files changed

+1871
-1476
lines changed

bindings/napi/BeaconStateView.zig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ pub fn BeaconStateView_createMultiProof(env: napi.Env, cb: napi.CallbackInfo(1))
778778
// Get the root node from the state
779779
try cached_state.state.commit();
780780
const root_node = switch (cached_state.state.*) {
781-
inline else => |*state| state.base_view.data.root,
781+
inline else => |state| state.root,
782782
};
783783

784784
// Create proof input for compact multi-proof
@@ -880,7 +880,7 @@ pub fn BeaconStateView_serialize(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.
880880
pub fn BeaconStateView_serializedSize(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value {
881881
const cached_state = try env.unwrap(CachedBeaconState, cb.this());
882882
const size = switch (cached_state.state.*) {
883-
inline else => |*state| try state.serializedSize(),
883+
inline else => |state| try state.serializedSize(),
884884
};
885885
return try env.createInt64(@intCast(size));
886886
}
@@ -902,7 +902,7 @@ pub fn BeaconStateView_serializeToBytes(env: napi.Env, cb: napi.CallbackInfo(2))
902902

903903
const output_slice = output_info.data[offset..];
904904
const bytes_written = switch (cached_state.state.*) {
905-
inline else => |*state| try state.serializeIntoBytes(output_slice),
905+
inline else => |state| try state.serializeIntoBytes(output_slice),
906906
};
907907

908908
return try env.createInt64(@intCast(bytes_written));

src/fork_types/any_beacon_state.zig

Lines changed: 168 additions & 176 deletions
Large diffs are not rendered by default.

src/fork_types/beacon_state.zig

Lines changed: 50 additions & 61 deletions
Large diffs are not rendered by default.

src/ssz/root.zig

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,12 @@ pub const Hasher = hasher.Hasher;
4141
pub const HasherData = hasher.HasherData;
4242

4343
const tree_view = @import("tree_view/root.zig");
44-
pub const TreeViewData = tree_view.TreeViewData;
45-
pub const BaseTreeView = tree_view.BaseTreeView;
4644
pub const ContainerTreeView = tree_view.ContainerTreeView;
4745
pub const ArrayBasicTreeView = tree_view.ArrayBasicTreeView;
4846
pub const ArrayCompositeTreeView = tree_view.ArrayCompositeTreeView;
4947
pub const ListBasicTreeView = tree_view.ListBasicTreeView;
5048
pub const ListCompositeTreeView = tree_view.ListCompositeTreeView;
49+
pub const CloneOpts = @import("tree_view/utils/clone_opts.zig").CloneOpts;
5150

5251
test {
5352
testing.refAllDecls(@This());

src/ssz/tree_view/array_basic.zig

Lines changed: 67 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ const type_root = @import("../type/root.zig");
99
const itemsPerChunk = type_root.itemsPerChunk;
1010
const chunkDepth = type_root.chunkDepth;
1111

12-
const BaseTreeView = @import("root.zig").BaseTreeView;
1312
const BasicPackedChunks = @import("chunks.zig").BasicPackedChunks;
13+
const assertTreeViewType = @import("utils/assert.zig").assertTreeViewType;
14+
const CloneOpts = @import("utils/clone_opts.zig").CloneOpts;
1415

1516
/// A specialized tree view for SSZ vector types with basic element types.
1617
/// Elements are packed into chunks (multiple elements per leaf node).
@@ -24,8 +25,9 @@ pub fn ArrayBasicTreeView(comptime ST: type) type {
2425
}
2526
}
2627

27-
return struct {
28-
base_view: BaseTreeView,
28+
const TreeView = struct {
29+
allocator: Allocator,
30+
chunks: Chunks,
2931

3032
pub const SszType = ST;
3133
pub const Element = ST.Element.Type;
@@ -38,69 +40,96 @@ pub fn ArrayBasicTreeView(comptime ST: type) type {
3840
const items_per_chunk: usize = itemsPerChunk(ST.Element);
3941
const Chunks = BasicPackedChunks(ST, chunk_depth, items_per_chunk);
4042

41-
pub fn init(allocator: Allocator, pool: *Node.Pool, root: Node.Id) !Self {
42-
return Self{
43-
.base_view = try BaseTreeView.init(allocator, pool, root),
44-
};
43+
pub fn init(allocator: Allocator, pool: *Node.Pool, root: Node.Id) !*Self {
44+
const ptr = try allocator.create(Self);
45+
errdefer allocator.destroy(ptr);
46+
47+
try Chunks.init(&ptr.chunks, allocator, pool, root);
48+
ptr.allocator = allocator;
49+
return ptr;
4550
}
4651

47-
pub fn clone(self: *Self, opts: BaseTreeView.CloneOpts) !Self {
48-
return Self{ .base_view = try self.base_view.clone(opts) };
52+
pub fn clone(self: *Self, opts: CloneOpts) !*Self {
53+
const ptr = try self.allocator.create(Self);
54+
errdefer self.allocator.destroy(ptr);
55+
56+
try Chunks.clone(&self.chunks, opts, &ptr.chunks);
57+
ptr.allocator = self.allocator;
58+
return ptr;
4959
}
5060

5161
pub fn deinit(self: *Self) void {
52-
self.base_view.deinit();
62+
self.chunks.deinit();
63+
self.allocator.destroy(self);
5364
}
5465

5566
pub fn commit(self: *Self) !void {
56-
try self.base_view.commit();
67+
try self.chunks.commit();
5768
}
5869

5970
pub fn clearCache(self: *Self) void {
60-
self.base_view.clearCache();
71+
self.chunks.clearCache();
72+
}
73+
74+
pub fn hashTreeRootInto(self: *Self, out: *[32]u8) !void {
75+
try self.commit();
76+
out.* = self.chunks.state.root.getRoot(self.chunks.state.pool).*;
6177
}
6278

63-
/// Return the root hash of the tree.
64-
/// The returned array is owned by the internal pool and must not be modified.
6579
pub fn hashTreeRoot(self: *Self) !*const [32]u8 {
66-
return try self.base_view.hashTreeRoot();
80+
try self.commit();
81+
return self.chunks.state.root.getRoot(self.chunks.state.pool);
82+
}
83+
84+
pub fn fromValue(allocator: Allocator, pool: *Node.Pool, value: *const ST.Type) !*Self {
85+
const root = try ST.tree.fromValue(pool, value);
86+
errdefer pool.unref(root);
87+
return try Self.init(allocator, pool, root);
88+
}
89+
90+
pub fn toValue(self: *Self, _: Allocator, out: *ST.Type) !void {
91+
try self.commit();
92+
try ST.tree.toValue(self.chunks.state.root, self.chunks.state.pool, out);
93+
}
94+
95+
pub fn getRoot(self: *const Self) Node.Id {
96+
return self.chunks.state.root;
6797
}
6898

6999
pub fn get(self: *Self, index: usize) !Element {
70100
if (index >= length) return error.IndexOutOfBounds;
71-
return try Chunks.get(&self.base_view, index);
101+
return self.chunks.get(index);
72102
}
73103

74104
pub fn set(self: *Self, index: usize, value: Element) !void {
75105
if (index >= length) return error.IndexOutOfBounds;
76-
try Chunks.set(&self.base_view, index, value);
106+
try self.chunks.set(index, value);
77107
}
78108

109+
/// Caller is responsible for freeing the returned slice using the same allocator.
79110
pub fn getAll(self: *Self, allocator: Allocator) ![]Element {
80-
return try Chunks.getAll(&self.base_view, allocator, length);
111+
return try self.chunks.getAll(allocator, length);
81112
}
82113

83114
pub fn getAllInto(self: *Self, values: []Element) ![]Element {
84-
return try Chunks.getAllInto(&self.base_view, length, values);
115+
return try self.chunks.getAllInto(length, values);
85116
}
86117

87118
/// Serialize the tree view into a provided buffer.
88119
/// Returns the number of bytes written.
89120
pub fn serializeIntoBytes(self: *Self, out: []u8) !usize {
90121
try self.commit();
91-
return try ST.tree.serializeIntoBytes(self.base_view.data.root, self.base_view.pool, out);
122+
return try ST.tree.serializeIntoBytes(self.chunks.state.root, self.chunks.state.pool, out);
92123
}
93124

94125
/// Get the serialized size of this tree view.
95126
pub fn serializedSize(_: *Self) usize {
96127
return ST.fixed_size;
97128
}
98-
99-
pub fn toValue(self: *Self, _: Allocator, out: *ST.Type) !void {
100-
try self.commit();
101-
try ST.tree.toValue(self.base_view.data.root, self.base_view.pool, out);
102-
}
103129
};
130+
131+
assertTreeViewType(TreeView);
132+
return TreeView;
104133
}
105134

106135
const UintType = @import("../type/uint.zig").UintType;
@@ -135,12 +164,13 @@ test "TreeView vector element roundtrip" {
135164
var expected_root: [32]u8 = undefined;
136165
try VectorType.hashTreeRoot(&expected, &expected_root);
137166

138-
const actual_root = try view.hashTreeRoot();
167+
var actual_root: [32]u8 = undefined;
168+
try view.hashTreeRootInto(&actual_root);
139169

140-
try std.testing.expectEqualSlices(u8, &expected_root, actual_root);
170+
try std.testing.expectEqualSlices(u8, &expected_root, &actual_root);
141171

142172
var roundtrip: VectorType.Type = undefined;
143-
try VectorType.tree.toValue(view.base_view.data.root, &pool, &roundtrip);
173+
try VectorType.tree.toValue(view.getRoot(), &pool, &roundtrip);
144174
try std.testing.expectEqualSlices(u64, &expected, &roundtrip);
145175
}
146176

@@ -208,7 +238,6 @@ test "TreeView vector getAllAlloc repeat reflects updates" {
208238

209239
try view.set(3, 99);
210240

211-
try view.commit();
212241
const second = try view.getAll(allocator);
213242
defer allocator.free(second);
214243
values[3] = 99;
@@ -301,13 +330,13 @@ test "TreeView vector clone(true) does not transfer cache" {
301330
defer v.deinit();
302331

303332
_ = try v.get(0);
304-
try std.testing.expect(v.base_view.data.children_nodes.count() > 0);
333+
try std.testing.expect(v.chunks.state.children_nodes.count() > 0);
305334

306335
var cloned_no_cache = try v.clone(.{ .transfer_cache = false });
307336
defer cloned_no_cache.deinit();
308337

309-
try std.testing.expect(v.base_view.data.children_nodes.count() > 0);
310-
try std.testing.expectEqual(@as(usize, 0), cloned_no_cache.base_view.data.children_nodes.count());
338+
try std.testing.expect(v.chunks.state.children_nodes.count() > 0);
339+
try std.testing.expectEqual(@as(usize, 0), cloned_no_cache.chunks.state.children_nodes.count());
311340
}
312341

313342
test "TreeView vector clone(false) transfers cache and clears source" {
@@ -325,13 +354,13 @@ test "TreeView vector clone(false) transfers cache and clears source" {
325354
defer v.deinit();
326355

327356
_ = try v.get(0);
328-
try std.testing.expect(v.base_view.data.children_nodes.count() > 0);
357+
try std.testing.expect(v.chunks.state.children_nodes.count() > 0);
329358

330359
var cloned = try v.clone(.{});
331360
defer cloned.deinit();
332361

333-
try std.testing.expectEqual(@as(usize, 0), v.base_view.data.children_nodes.count());
334-
try std.testing.expect(cloned.base_view.data.children_nodes.count() > 0);
362+
try std.testing.expectEqual(@as(usize, 0), v.chunks.state.children_nodes.count());
363+
try std.testing.expect(cloned.chunks.state.children_nodes.count() > 0);
335364
}
336365

337366
// Tests ported from TypeScript ssz packages/ssz/test/unit/byType/vector/tree.test.ts
@@ -382,8 +411,9 @@ test "ArrayBasicTreeView - serialize (uint64 vector)" {
382411
const view_size = view.serializedSize();
383412
try std.testing.expectEqual(tc.expected_serialized.len, view_size);
384413

385-
const hash_root = try view.hashTreeRoot();
386-
try std.testing.expectEqualSlices(u8, &tc.expected_root, hash_root);
414+
var hash_root: [32]u8 = undefined;
415+
try view.hashTreeRootInto(&hash_root);
416+
try std.testing.expectEqualSlices(u8, &tc.expected_root, &hash_root);
387417
}
388418
}
389419

0 commit comments

Comments
 (0)