Skip to content

refactor: improve start handler api - #41

Merged
tac0turtle merged 3 commits into
evstack:mainfrom
julienrbrt:julien/correct-starthandler-api
Apr 11, 2025
Merged

refactor: improve start handler api#41
tac0turtle merged 3 commits into
evstack:mainfrom
julienrbrt:julien/correct-starthandler-api

Conversation

@julienrbrt

@julienrbrt julienrbrt commented Apr 10, 2025

Copy link
Copy Markdown
Member

Overview

Simplifies StartHandler so it can be called directly in

server.AddCommandsWithStartCmdOptions(rootCmd, app.DefaultNodeHome, newApp, appExport, server.StartCmdOptions{
	StartCommandHandler: rollkitsrv.StartHandler(rootCmd),
        AddFlags: rollconf.AddFlags,
})

This requires a modification in LoadNodeConfig to only require viper (it should need the root command).

Summary by CodeRabbit

  • Chores
    • Updated various dependency versions to improve reliability and maintain compatibility.
  • Refactor
    • Streamlined internal code organization and simplified function signatures.
    • Modified command-line configuration options by removing outdated flags and renaming some options for clarity, ensuring a more intuitive user experience.

@coderabbitai

coderabbitai Bot commented Apr 10, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request updates dependency versions in the go.mod file, refactors code formatting and variable declarations in rpc/rpc.go, modifies function signatures in server/start.go by removing generic type parameters and introducing a type alias, and adjusts configuration options in server/utils.go by updating default settings and removing or renaming several flags.

Changes

File(s) Summary
go.mod Updated dependency versions for rollkit, rollkit/core, rollkit/da, sonic, sonic/loader, and base64x.
rpc/rpc.go Grouped variable declarations into a single block and applied minor comment/spacing adjustments, including updating the node identifier reference.
server/start.go Removed generic type parameters and introduced a StartCommandHandler alias. Updated function signatures (including replacing svrCtx with a direct logger in startNode) and added an import for filepath.
server/utils.go Changed default configuration assignment from DefaultNodeConfig to DefaultConfig; removed flags for chain config directory, sequencer options, and RPC port; renamed the flag from P2PSeeds to P2PPeers.

Sequence Diagram(s)

sequenceDiagram
    participant CMD as Cobra Command
    participant SH as StartHandler (StartCommandHandler)
    participant SA as startApp
    participant SIP as startInProcess
    participant SN as startNode
    participant LOG as Logger

    CMD->>SH: Invoke StartHandler
    SH->>SA: Call startApp to initialize application
    SA-->>SH: Return app instance, cleanup function, error
    SH->>SIP: Call startInProcess with app instance
    SIP->>SN: Call startNode with logger and app
    SN->>LOG: Log node startup details
    SN-->>SIP: Return node, RPC server, cleanup function, error
Loading

Poem

In a code garden I hop with delight,
Dependencies refreshed, everything’s light.
Variables grouped in a neat little row,
With functions refactored, on we go!
I nibble on changes, both simple and grand—
A rabbit’s ode to clean code in this land.
🐇✨

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 100b5ca and 997367e.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • go.mod (2 hunks)
  • rpc/rpc.go (3 hunks)
  • server/start.go (13 hunks)
  • server/utils.go (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/start.go (1)
rpc/rpc.go (2)
  • RPCServer (57-65)
  • NewRPCServer (140-142)
🔇 Additional comments (16)
rpc/rpc.go (3)

135-138: Improved variable declaration structure

The grouped variable declaration makes the interface implementations more readable and better organized.


409-409: Minor comment formatting improvement

The updated comment format for the subscriber variable is clearer and more consistent.


868-868: Updated reference from DefaultNodeID to NodeID

This change aligns with updates in the configuration source in server/utils.go where DefaultNodeConfig was replaced with DefaultConfig.

go.mod (2)

9-11: Updated Rollkit dependency versions

The PR updates the Rollkit dependency versions to more recent releases, which likely include bug fixes and improvements that support the refactoring of server/start.go and simplification of the StartHandler API.


22-26: Updated support library dependencies

The bytedance/sonic and cloudwego/base64x libraries have been updated to newer versions, which should provide performance improvements and bug fixes.

server/utils.go (2)

43-43: Updated default configuration source

Changed from DefaultNodeConfig to DefaultConfig, which aligns with the PR objective to simplify the LoadNodeConfig function to only require the Viper configuration library.


73-73: Renamed P2P seed nodes flag

Changed from FlagP2PSeeds to FlagP2PPeers to better reflect its purpose while maintaining the same functionality.

server/start.go (9)

57-59: Added type alias for improved readability

Creating the StartCommandHandler type alias improves code readability and makes the function signatures more concise. This aligns with the PR objective to simplify the API and make it more straightforward to use.


61-62: Simplified StartHandler function signature

Removed the generic type parameter T from the StartHandler function, returning the new type alias instead. This makes the API more approachable and aligns with the PR objective to simplify the StartHandler function.


85-85: Removed generic type parameter from startApp function

Simplifying the function signature makes the code more accessible and easier to maintain. This change is consistent with the removal of generic type parameters throughout the file.


108-110: Removed generic type parameter from startInProcess function

Continued simplification of the function signatures by removing unused generic type parameters, making the code more maintainable.


259-260: Improved dependency injection in startNode function

Changed the function to receive a logger directly rather than extracting it from the server context. This improves code clarity by making dependencies explicit and simplifies testing.


279-280: Updated configuration handling

Using the file path's directory to locate configuration is more robust than previous approaches. This change aligns with the simplification of the configuration loading process mentioned in the PR objectives.


324-334: Updated logger parameter usage

Consistently using the directly passed logger parameter instead of extracting it from context improves code maintainability and makes dependencies clearer.


384-384: Logger parameter passed directly to da client

Consistent with the pattern of passing the logger directly to components that need it rather than accessing it through context.


404-404: Direct logger usage in RPC server creation

Passing the logger directly to the RPC server constructor is consistent with the improved dependency injection approach throughout the file.

✨ Finishing Touches
  • 📝 Generate Docstrings

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 plan to trigger planning for file edits and PR creation.
  • @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.

@julienrbrt
julienrbrt marked this pull request as ready for review April 11, 2025 09:49
@tac0turtle
tac0turtle merged commit 3195217 into evstack:main Apr 11, 2025
@github-project-automation github-project-automation Bot moved this to Done in Evolve Apr 11, 2025
@julienrbrt
julienrbrt deleted the julien/correct-starthandler-api branch April 11, 2025 20:22
@tac0turtle tac0turtle removed this from Evolve Apr 24, 2025
This was referenced May 26, 2025
chatton pushed a commit that referenced this pull request Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants