chore: move utils/root_cache.zig -> cache/RootCache.zig by spiral-ladder · Pull Request #170 · ChainSafe/lodestar-z

spiral-ladder · 2026-01-15T11:14:32Z

part of #169

Also make it a top-level struct.

gemini-code-assist · 2026-01-15T11:14:49Z

Summary of Changes

Hello @spiral-ladder, I'm Gemini Code Assist¹! 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 focuses on improving the project's internal structure by relocating the RootCache component to a more semantically appropriate directory and enhancing its definition as a top-level struct. This change contributes to better code organization and maintainability, streamlining future development and adherence to architectural patterns.

Highlights

File Relocation: The RootCache struct and its associated implementation have been moved from src/state_transition/utils/root_cache.zig to src/state_transition/cache/RootCache.zig.
Struct Definition Update: The RootCache struct has been refactored to be a top-level struct within its new file, utilizing the @This() keyword for clarity and direct access.
Import Path Adjustments: All existing import statements referencing RootCache in files like process_attestation_altair.zig and upgrade_state_to_altair.zig have been updated to reflect its new location.

🧠 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors RootCache by moving it to a new file and making it a top-level struct. The changes are a good step towards better code organization. However, there are a few violations of the repository's style guide that should be addressed. Specifically, the new file name does not follow the snake_case convention, the init and deinit functions use heap allocation which is discouraged after startup, and the functions are missing required assertions. My review includes suggestions to align the code with the style guide.

gemini-code-assist · 2026-01-15T11:16:29Z

+pub fn init(allocator: Allocator, cached_state: *const CachedBeaconStateAllForks) !*RootCache {
+    const instance = try allocator.create(RootCache);
+    errdefer allocator.destroy(instance);
+    const state = cached_state.state;
+    instance.* = RootCache{
+        .allocator = allocator,
+        .current_justified_checkpoint = state.currentJustifiedCheckpoint().*,
+        .previous_justified_checkpoint = state.previousJustifiedCheckpoint().*,
+        .state = state,
+        .block_root_epoch_cache = std.AutoHashMap(Epoch, Root).init(allocator),
+        .block_root_slot_cache = std.AutoHashMap(Slot, Root).init(allocator),
+    };
+
+    return instance;
+}
+
+pub fn getBlockRoot(self: *RootCache, epoch: Epoch) !Root {
+    if (self.block_root_epoch_cache.get(epoch)) |root| {
+        return root;
+    } else {
+        const root = try getBlockRootFn(self.state, epoch);
+        try self.block_root_epoch_cache.put(epoch, root);
+        return root;
+    }
+}
+
+pub fn getBlockRootAtSlot(self: *RootCache, slot: Slot) !Root {
+    if (self.block_root_slot_cache.get(slot)) |root| {
+        return root;
+    } else {
+        const root = try getBlockRootAtSlotFn(self.state, slot);
+        try self.block_root_slot_cache.put(slot, root);
+        return root;
+    }
+}
+
+pub fn deinit(self: *RootCache) void {
+    self.block_root_epoch_cache.deinit();
+    self.block_root_slot_cache.deinit();
+    self.allocator.destroy(self);
+}


The current implementation of init allocates RootCache on the heap, and deinit frees it. This pattern of dynamic memory allocation during runtime is contrary to the style guide, which states, 'No memory may be dynamically allocated (or freed and reallocated) after initialization.' (lines 93-94).

To adhere to the guide and also follow the preference for in-place initialization of large structs (lines 314-321), RootCache should be allocated on the stack by the caller and initialized in-place.

This requires changing init to accept a pointer to a RootCache instance and deinit to no longer deallocate the instance itself.

pub fn init(self: *RootCache, allocator: Allocator, cached_state: *const CachedBeaconStateAllForks) void { const state = cached_state.state; self.* = RootCache{ .allocator = allocator, .current_justified_checkpoint = state.currentJustifiedCheckpoint().*, .previous_justified_checkpoint = state.previousJustifiedCheckpoint().*, .state = state, .block_root_epoch_cache = std.AutoHashMap(Epoch, Root).init(allocator), .block_root_slot_cache = std.AutoHashMap(Slot, Root).init(allocator), }; } pub fn getBlockRoot(self: *RootCache, epoch: Epoch) !Root { if (self.block_root_epoch_cache.get(epoch)) |root| { return root; } else { const root = try getBlockRootFn(self.state, epoch); try self.block_root_epoch_cache.put(epoch, root); return root; } } pub fn getBlockRootAtSlot(self: *RootCache, slot: Slot) !Root { if (self.block_root_slot_cache.get(slot)) |root| { return root; } else { const root = try getBlockRootAtSlotFn(self.state, slot); try self.block_root_slot_cache.put(slot, root); return root; } } pub fn deinit(self: *RootCache) void { self.block_root_epoch_cache.deinit(); self.block_root_slot_cache.deinit(); }

References

All memory must be statically allocated at startup. No memory may be dynamically allocated (or freed and reallocated) after initialization. This avoids unpredictable behavior that can significantly affect performance, and avoids use-after-free. ^(link)

Construct larger structs in-place by passing an out pointer during initialization. This eliminates intermediate copy-move allocations, which can lead to undesirable stack growth. ^(link)

gemini-code-assist · 2026-01-15T11:16:29Z

@@ -0,0 +1,64 @@
+const RootCache = @This();


The filename RootCache.zig does not follow the snake_case convention specified in the style guide. Please rename it to root_cache.zig.

This change will also require updating the import paths in src/state_transition/block/process_attestation_altair.zig and src/state_transition/slot/upgrade_state_to_altair.zig.

References

Use snake_case for file names. We don't use Zig's CamelCase.zig style for "struct" files to keep the convention simple and consistent. ^(link)

gemini-code-assist · 2026-01-15T11:16:29Z

+pub fn getBlockRoot(self: *RootCache, epoch: Epoch) !Root {
+    if (self.block_root_epoch_cache.get(epoch)) |root| {
+        return root;
+    } else {
+        const root = try getBlockRootFn(self.state, epoch);
+        try self.block_root_epoch_cache.put(epoch, root);
+        return root;
+    }
+}


The functions in this file are missing assertions. The style guide mandates asserting all function arguments, preconditions, and invariants, with an average of at least two assertions per function (lines 51-55). Please add assertions to all functions in this file. For example, you should assert that self is not null.

pub fn getBlockRoot(self: *RootCache, epoch: Epoch) !Root { std.debug.assert(self != null); if (self.block_root_epoch_cache.get(epoch)) |root| { return root; } else { const root = try getBlockRootFn(self.state, epoch); try self.block_root_epoch_cache.put(epoch, root); return root; } }

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 assertion density of the code must average a minimum of two assertions per function. ^(link)

twoeths

may want to wait for the TreeView BeaconState because it returns Checkpoint TreeView for currentJustifiedCheckpoint() api https://github.com/ChainSafe/lodestar-z/pull/168/changes#diff-f3c8638ffff8c55ffd2c70324cc540954a42662eefe344195c145adb099b1f33R466

looks good to me other than that

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

spiral-ladder self-assigned this Jan 15, 2026

gemini-code-assist Bot reviewed Jan 15, 2026

View reviewed changes

spiral-ladder marked this pull request as ready for review January 15, 2026 12:39

spiral-ladder requested a review from a team as a code owner January 15, 2026 12:39

twoeths previously approved these changes Jan 16, 2026

View reviewed changes

utils/root_cache.zig -> cache/RootCache.zig

0a4f65c

spiral-ladder dismissed twoeths’s stale review via 0a4f65c January 19, 2026 14:59

spiral-ladder force-pushed the bing/utils-root-cache branch from 9fd5e20 to 0a4f65c Compare January 19, 2026 14:59

spiral-ladder requested a review from twoeths January 19, 2026 15:56

wemeetagain approved these changes Jan 19, 2026

View reviewed changes

wemeetagain changed the title ~~move utils/root_cache.zig -> cache/RootCache.zig~~ chore: move utils/root_cache.zig -> cache/RootCache.zig Jan 19, 2026

wemeetagain merged commit 86f430b into main Jan 19, 2026
8 checks passed

spiral-ladder mentioned this pull request Jan 22, 2026

re-organize utils directory #169

Open

9 tasks

twoeths added a commit that referenced this pull request Mar 2, 2026

move utils/root_cache.zig -> cache/RootCache.zig (#170)

c76b709

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: move utils/root_cache.zig -> cache/RootCache.zig#170

chore: move utils/root_cache.zig -> cache/RootCache.zig#170
wemeetagain merged 1 commit intomainfrom
bing/utils-root-cache

spiral-ladder commented Jan 15, 2026

Uh oh!

gemini-code-assist Bot commented Jan 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Uh oh!

twoeths left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

spiral-ladder commented Jan 15, 2026

Uh oh!

gemini-code-assist Bot commented Jan 15, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

twoeths left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants