feat: add override_genesis_time flag in node cmd#188
Conversation
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for overriding the genesis time via CLI flag and configuration, while also fixing test configuration issues that prevented configs and node tests from running properly.
- Introduces
override_genesis_timeparameter to genesis configuration loading - Adds CLI flag support for overriding genesis time in node command
- Fixes test configuration in build.zig to include missing configs tests
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkgs/configs/src/lib.zig | Adds override_genesis_time parameter to genesisConfigFromYAML function and improves test cleanup |
| pkgs/cli/src/node.zig | Updates loadGenesisConfig to accept override_genesis_time parameter and fixes memory leak in tests |
| pkgs/cli/src/main.zig | Adds override_genesis_time CLI flag and enables recursive test discovery |
| build.zig | Adds missing configs_tests configuration to test suite |
Comments suppressed due to low confidence (1)
pkgs/cli/src/node.zig:1
- There's a typo in line 251:
gstd.testing.allocator.free(nodes)should bestd.testing.allocator.free(nodes). The 'g' prefix appears to be a typo.
const std = @import("std");
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| const genesis_config = try genesisConfigFromYAML(yaml); | ||
| std.debug.print("genesis config = {any}\n", .{genesis_config}); | ||
| const genesis_config = try genesisConfigFromYAML(yaml, null); |
There was a problem hiding this comment.
Remove the debug print statement that was commented out in the original code. The line std.debug.print("genesis config = {any}\n", .{genesis_config}); should be removed entirely to keep the test clean.
| defer start_options.deinit(allocator); | ||
|
|
||
| try node.loadGenesisConfig(allocator, custom_genesis, &start_options); | ||
| try node.loadGenesisConfig(allocator, custom_genesis, leancmd.override_genesis_time, &start_options); |
There was a problem hiding this comment.
make this a boolean flag and if true pass time.now here to laodgenesisconfig else null
There was a problem hiding this comment.
That will lead to different genesis time for different nodes, I think it doesn't make sense.
|
@GrapeBaBa now we can probably add a CI job |
This pull request introduces support for overriding the genesis time via CLI and configuration. At the same time fixs config and node tests not run bug.