Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the formatting of the printSlot function to provide a more comprehensive and visually appealing chain status display. The changes replace a simple one-line log message with a detailed ASCII table format that shows additional blockchain state information.
- Enhanced chain status display with structured ASCII table formatting
- Added comprehensive blockchain state information including justified/finalized checkpoints and timeliness status
- Improved hex formatting for block roots and state information
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| self.logger.info( | ||
| \\+===============================================================+ | ||
| \\ CHAIN STATUS | ||
| \\+===============================================================+ | ||
| \\ Current Slot: {d} | Head Slot: {d} | Behind: {d} | ||
| \\+---------------------------------------------------------------+ | ||
| \\ Head Block Root: 0x{any} | ||
| \\ Parent Block Root: 0x{any} | ||
| \\ State Root: 0x{any} | ||
| \\ Timely: {s} | ||
| \\+---------------------------------------------------------------+ | ||
| \\ Latest Justified: Slot {d:>6} | Root: 0x{any} | ||
| \\ Latest Finalized: Slot {d:>6} | Root: 0x{any} | ||
| \\+===============================================================+ | ||
| , .{ | ||
| slot, | ||
| fcHead.blockRoot, | ||
| fcHead.slot, | ||
| blocksBehind, | ||
| std.fmt.fmtSliceHexLower(&fcHead.blockRoot), | ||
| std.fmt.fmtSliceHexLower(&fcHead.parentRoot), | ||
| std.fmt.fmtSliceHexLower(&fcHead.stateRoot), | ||
| if (isTimely) "YES" else "NO", | ||
| justified.slot, | ||
| std.fmt.fmtSliceHexLower(&justified.root), | ||
| finalized.slot, | ||
| std.fmt.fmtSliceHexLower(&finalized.root), | ||
| }); |
There was a problem hiding this comment.
The ASCII table formatting uses hardcoded spacing that may not align properly with different slot number lengths or terminal widths. Consider using dynamic width calculation or a more flexible formatting approach to ensure consistent alignment across different data ranges.
pkgs/node/src/chain.zig
Outdated
|
|
||
| // Calculate chain progress | ||
| const blocksBehind = if (slot > fcHead.slot) slot - fcHead.slot else 0; | ||
| const isTimely = fcHead.timeliness; |
There was a problem hiding this comment.
The variable name isTimely suggests a boolean, but fcHead.timeliness may not be a boolean type. Consider using a more descriptive name like timelinessStatus or ensure proper boolean conversion if needed.
pkgs/node/src/chain.zig
Outdated
| const finalized = self.forkChoice.fcStore.latest_finalized; | ||
|
|
||
| // Calculate chain progress | ||
| const blocksBehind = if (slot > fcHead.slot) slot - fcHead.slot else 0; |
There was a problem hiding this comment.
snake case variable is best practical follow the guide https://ziglang.org/documentation/0.14.1/#Style-Guide
There was a problem hiding this comment.
Thanks for the reference, I've updated it.
|
can you snapshot for how it prints? |
I've written a testcase for this, it's not a functional test, just an easy way to visualize the output. I can revert the commit if you think it's something we don't want to include in the codebase. |
Closes #103