feat: improve config and use toml by randygrok · Pull Request #2047 · evstack/ev-node

randygrok · 2025-03-06T16:53:38Z

Overview

coderabbitai · 2025-03-06T16:53:50Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

codecov · 2025-03-06T17:00:43Z

Codecov Report

Attention: Patch coverage is 64.55026% with 134 lines in your changes missing coverage. Please review.

Project coverage is 23.87%. Comparing base (2262207) to head (b3006cc).
Report is 3 commits behind head on feature/exec_api.

Files with missing lines	Patch %	Lines
node/full.go	16.66%	58 Missing and 2 partials ⚠️
block/manager.go	15.38%	21 Missing and 1 partial ⚠️
cmd/rollkit/commands/run_node.go	54.05%	14 Missing and 3 partials ⚠️
block/sync_service.go	0.00%	10 Missing ⚠️
config/config.go	93.33%	6 Missing and 3 partials ⚠️
cmd/rollkit/commands/toml.go	90.00%	3 Missing and 3 partials ⚠️
config/toml.go	78.94%	3 Missing and 1 partial ⚠️
node/light.go	0.00%	2 Missing ⚠️
node/node.go	0.00%	1 Missing and 1 partial ⚠️
cmd/rollkit/commands/intercept.go	50.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@                 Coverage Diff                  @@
##           feature/exec_api    #2047      +/-   ##
====================================================
- Coverage             24.11%   23.87%   -0.25%     
====================================================
  Files                    52       50       -2     
  Lines                  8231     8433     +202     
====================================================
+ Hits                   1985     2013      +28     
- Misses                 6089     6278     +189     
+ Partials                157      142      -15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… parameters

This refactoring improves Rollkit’s configuration management with the following changes: • Removes the GetViperConfig() method in favor of direct mapping • Updates mapstructure tags to reflect the configuration structure • Adds new P2P flags and adjusts instrumentation settings • Simplifies the node configuration integration test The main changes include removing the dependency on Viper for configuration and providing a cleaner structure for handling Rollkit’s configurations.

…kit-config-toml

tac0turtle

left some minor comments, overall nice job

config/config.go

config/defaults.go

config/instrumentation.go

node/light.go

cmd/rollkit/docs/rollkit_start.md

tac0turtle · 2025-03-11T19:53:33Z

cmd/rollkit/docs/rollkit_start.md

-      --consensus.create_empty_blocks                   set this to false to only produce blocks when there are txs or when the AppHash changes (default true)
-      --consensus.create_empty_blocks_interval string   the possible interval between empty blocks (default "0s")


how did this feature work @Manav-Aggarwal or were these flags used?

cmd/rollkit/commands/run_node.go

block/sync_service.go

config/config.go

cmd/rollkit/commands/toml.go

julienrbrt · 2025-03-11T21:40:14Z

cmd/rollkit/commands/toml.go

+		v.SetConfigType(rollconf.ConfigExtension)
+		v.AddConfigPath(currentDir)
+
+		// Create a map with the configuration structure


Adding the fields manually is viper is a bit weird.
Normally, AddConfigPath, BindPFlag, BindEnv, ReadInConfig should be sufficient.
The keys would be the toml key path, matching what this is doing.
Every new field addition would need to be added there otherwise.

config/config.go

julienrbrt · 2025-03-11T21:48:32Z

config/config.go

+
+	// Add search paths in order of precedence
+	// Current directory
+	v.AddConfigPath(".")


isn't there a home flag on the cobra command that could be used here?
Feels a bit weird

yeah, though rollkit config was somehow depending on the file in the root. Let me think about this tomorrow again.

julienrbrt · 2025-03-11T21:49:59Z

config/config.go

+	// FlagEntrypoint is a flag for specifying the entrypoint
+	FlagEntrypoint = "entrypoint"
+	// FlagChainConfigDir is a flag for specifying the chain config directory
+	FlagChainConfigDir = "chain.config_dir"


What's the difference between this and FlagRootDir?

It is related to the way the rollkit intercepts the chain command. For some reason is different than the home, but I did not dig. I can create issue just to verify that it can be merged.

@Manav-Aggarwal can you help with this? lets close this out here. Dont want to create issues for simple things and we end up merging techdebt

Seems like it's coming from intercept command rollkitConfig toml config, where chain.config_dir specified which chain config we want to use for intercepted command which essentially mean adding --home flag to the command.
Whereas FlagRootDir specified the directory where rollkit.toml is and is mostly used to check if it's not a rollkit itself, so there is no recursion in intercepting.

config/toml.go

…kit-config-toml

…ollkit into feat/rollkit-config-toml

tac0turtle · 2025-03-13T09:52:57Z

cmd/rollkit/docs/rollkit_completion_fish.md

 ### Options inherited from parent commands

 ```
-      --home string        directory for config and data (default "HOME/.rollkit")


where did home go?

tac0turtle · 2025-03-13T09:54:16Z

config/config.go

+	RootDir    string      `mapstructure:"home"`
+	DBPath     string      `mapstructure:"db_path"`


why no toml flag here?

tac0turtle

looks good, we should add comments and try to use tomlv2 as right now the toml has no comments so the options of what something means is not set

## Overview

randygrok added 2 commits March 6, 2025 11:38

Simplication of config tests.

bdbdbae

simplify home flag

bb966ab

github-project-automation bot added this to Evolve Mar 6, 2025

randygrok changed the base branch from main to feature/exec_api March 6, 2025 16:53

github-actions bot assigned randygrok Mar 6, 2025

change rollkit name mapstructure

7ce3b5a

randygrok added 18 commits March 6, 2025 23:39

refactor: restructure Rollkit configuration with RollkitConfig

c2a58e9

refactor: simplify function signatures and remove unnecessary context…

148c858

… parameters

pending changes

3be1803

Merge remote-tracking branch 'origin/feature/exec_api' into feat/roll…

e636d47

…kit-config-toml

lint fixes

7574a99

docs gen

0b6ffcb

fix linter errors on dockerfile

f557198

clean some config tests

bb1734d

migrate toml config to node config

8c86f27

now it loads also Rollkit config

6451df4

merge file

c7a1f8f

add tests for init command

28fffef

merge now with flags

af06803

go mod tidy

25c2131

Fix test

642db18

update test with flag for CI

6c58110

fix lint

90e2da1

Manav-Aggarwal moved this to In Progress in Evolve Mar 10, 2025

randygrok added 3 commits March 10, 2025 18:11

Update config flags

13b3fa5

update dockerfile

382a1f0

fix test

b04a3ce

randygrok requested review from tac0turtle and tzdybal March 11, 2025 18:39

tac0turtle reviewed Mar 11, 2025

View reviewed changes

randygrok commented Mar 11, 2025

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

Rename NodeConfig to Config

391d80f

julienrbrt reviewed Mar 11, 2025

View reviewed changes

cosmetic changes

962ffa5

randygrok mentioned this pull request Mar 11, 2025

feature: bring back pprof_laddr #2060

Closed

randygrok added 13 commits March 12, 2025 11:10

use RunE in toml cmd

a6994b4

rename rollkit to node in config flags

553e417

docs gen

c0d2df1

fix lint

3b96358

include pprof server

972e1d0

Merge remote-tracking branch 'origin/feature/exec_api' into feat/roll…

32a1bd9

…kit-config-toml

use logger

4724c20

Merge branch 'feat/rollkit-config-toml' of github.com-randy:rollkit/r…

868f519

…ollkit into feat/rollkit-config-toml

add logging flags

fdcd3e6

new flags for logging

3282033

go mod tidy

4ba029c

fix lint

737c98c

remove unused functions

b3006cc

tac0turtle reviewed Mar 13, 2025

View reviewed changes

tac0turtle approved these changes Mar 13, 2025

View reviewed changes

tac0turtle merged commit 1ea4930 into feature/exec_api Mar 13, 2025
25 checks passed

tac0turtle deleted the feat/rollkit-config-toml branch March 13, 2025 09:58

github-project-automation bot moved this from In Progress to Done in Evolve Mar 13, 2025

randygrok mentioned this pull request Mar 13, 2025

Cleanup config package #1906

Closed

tac0turtle removed this from Evolve Apr 24, 2025

		--consensus.create_empty_blocks set this to false to only produce blocks when there are txs or when the AppHash changes (default true)
		--consensus.create_empty_blocks_interval string the possible interval between empty blocks (default "0s")

		RootDir string `mapstructure:"home"`
		DBPath string `mapstructure:"db_path"`

Conversation

randygrok commented Mar 6, 2025

Overview

Uh oh!

coderabbitai bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

codecov bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tac0turtle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tac0turtle Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yarikbratashchuk Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tac0turtle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coderabbitai bot commented Mar 6, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

codecov bot commented Mar 6, 2025 •

edited

Loading

tac0turtle Mar 12, 2025 •

edited

Loading

yarikbratashchuk Mar 13, 2025 •

edited

Loading