-
Notifications
You must be signed in to change notification settings - Fork 245
chore: refactor evm-single entrypoint & reth volume persistence #2434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
""" WalkthroughThis change updates Docker Compose configurations for EVM-related services by modifying volume mounts from log directories to new data directories, updating service command-line arguments accordingly, and refactoring the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Entrypoint.sh
participant evm-single
User->>Entrypoint.sh: Provide command and optional flags
Entrypoint.sh->>Entrypoint.sh: Parse args, set default flags
Entrypoint.sh->>evm-single: Execute with combined (default + user) flags
evm-single-->>User: Runs with specified configuration
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @auricom, I'm Gemini Code Assist1! 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 significantly refactors the Docker entrypoint for the evm-single application, enhancing its configurability and flexibility within containerized deployments. The changes allow for dynamic argument generation based on environment variables while preserving the ability for users to provide custom command-line arguments, making the startup process more adaptable and comparable to other ecosystem tools.
Highlights
- Refactored Entrypoint Script: The
entrypoint.shscript forevm-singlehas been significantly refactored to provide a more flexible and robust startup mechanism. It now dynamically constructsinitandstartcommands based on environment variables, allowing for easier configuration within containerized environments. - User Argument Overrides: The updated entrypoint script now allows users to pass their own arguments directly to the
evm-singlebinary. These user-provided arguments will be appended to, or can override, the default arguments automatically generated from environment variables, providing greater control. - Conditional Node Initialization: The script now intelligently checks for the existence of
node_key.json(previouslysigner.json) before attempting to initialize theevm-singlenode, preventing unnecessary re-initialization. Theinitcommand's arguments are also dynamically built based on available environment variables. - Enhanced DA Configuration: Data Availability (DA) related parameters (
DA_ADDRESS,DA_AUTH_TOKEN,DA_NAMESPACE) are now seamlessly integrated into the dynamic argument building for theevm-single startcommand, streamlining DA configuration via environment variables. - Docker Image Update: The
docker-compose.ymlfile has been updated to reference themainbranch image forrollkit-evm-single, ensuring the use of the latest stable build.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 is currently in preview and 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 to provide feedback.
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.
Footnotes
-
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. ↩
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2434 +/- ##
=======================================
Coverage 72.71% 72.71%
=======================================
Files 67 67
Lines 6395 6395
=======================================
Hits 4650 4650
Misses 1347 1347
Partials 398 398
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request refactors the entrypoint script for the evm-single binary to propose arguments based on environment variables. It initializes the node if needed and allows launching in either sequencer or fullnode mode. The home directory is predefined but can be overridden with the --home argument. The entrypoint behavior is now comparable to celestia-appd while maintaining the automation from Manav's example. I've provided feedback on quoting the command when echoing it and using the defined variable in the exec command.
e1ce64e to
75689b2
Compare
75689b2 to
8381fe7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
execution/evm/docker/docker-compose.yml (1)
67-67: Add missing newline at end-of-file to satisfyyamllint.
yamllintis failing withno new line character at the end of file.reth: +apps/evm/single/docker-compose.yml (1)
55-56: Consider pinning therollkit-evm-singleimage by digest for reproducibility.Using the moving
:maintag makes builds non-deterministic and can break deployments when the tag is updated upstream.- image: ghcr.io/rollkit/rollkit-evm-single:main + # pin to a digest for reproducibility + image: ghcr.io/rollkit/rollkit-evm-single@sha256:<digest>(Substitute the latest digest.)
apps/evm/single/entrypoint.sh (2)
97-100: Fix shell-quoting issue flagged by ShellCheck (SC2145).Mixing a quoted string with
$@inside the same double-quoted expression results in the entire$@array being collapsed into one argument in the echo.
Use separate arguments instead:- echo "$START_COMMAND \"$@\"" + echo "$START_COMMAND" "$@"(The
execline is fine — word-splitting there is intended.)
72-74: Duplicate--rollkit.node.aggregatorflag.You append
--rollkit.node.aggregator=trueonce duringinit(line 38) and again duringstart(line 73).
While harmless, the duplication is noisy. Consider dropping it from one of the flag sets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/evm/single/docker-compose.yml(3 hunks)apps/evm/single/entrypoint.sh(1 hunks)execution/evm/docker/docker-compose-full-node.yml(2 hunks)execution/evm/docker/docker-compose.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
apps/evm/single/docker-compose.yml (2)
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors. They handle error checking internally using s.Require().NoError(err) for their operations, so no external error handling is needed when calling these methods.
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors, so no error handling is needed when calling these methods.
execution/evm/docker/docker-compose-full-node.yml (1)
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors. They handle error checking internally using s.Require().NoError(err) for their operations, so no external error handling is needed when calling these methods.
execution/evm/docker/docker-compose.yml (1)
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors. They handle error checking internally using s.Require().NoError(err) for their operations, so no external error handling is needed when calling these methods.
🪛 Shellcheck (0.10.0)
apps/evm/single/entrypoint.sh
[error] 99-99: Argument mixes string and array. Use * or separate argument.
(SC2145)
🪛 GitHub Check: lint / yamllint
execution/evm/docker/docker-compose.yml
[failure] 67-67:
67:8 [new-line-at-end-of-file] no new line character at the end of file
🪛 YAMLlint (1.37.1)
execution/evm/docker/docker-compose.yml
[error] 67-67: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test / Run Integration Tests
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: Summary
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
execution/evm/docker/docker-compose-full-node.yml (1)
37-45: Volume mount &--datadirflag are aligned — good call.The named volume
rethcorrectly backs/home/reth/eth-home, and the--datadirflag now points at the same path.
This should give the node durable state across container restarts without any additional tweaks. 👍execution/evm/docker/docker-compose.yml (1)
37-46: Volume ↔--datadiralignment looks solid.The service now persists its chain data in
/home/reth/eth-home, matching the--datadirflag.
No functional issues spotted here.apps/evm/single/docker-compose.yml (1)
17-24: Good switch to persistentrethvolume and--datadir.The mount and flag pair up correctly; this brings the single-node compose in line with the full-node setup.
tac0turtle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
evm-single entrypoint
Closes #2412
This changes the entrypoint behavior by proposing arguments for the
evm-singlebinary based on environment variables in the container runtime.Users can bypass these suggestions by adding their own arguments via command.
The script will initialize the node if needed.
The script allows launching in either sequencer or fullnode mode.
The home directory is predefined as
$HOME/.evm-singlebut users can override it with the--homeargument.The entrypoint behavior is now comparable to celestia-appd while maintaining the automation from Manav's example.
reth volume persistence
The reth docker compose definitions has been updated to persist data across restarts
Summary by CodeRabbit