feat: Add EVM full node setup #2373
Conversation
WalkthroughThis update revises the EVM single node Docker Compose and documentation, renaming services, updating images, and restructuring commands. It introduces a new Docker Compose setup for a full node, enhances transaction sorting and execution logic, and improves test determinism. Additional logging and dependency updates are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SequencerNode
participant EVM_Layer
participant FullNode
User->>SequencerNode: Start Sequencer (per README)
SequencerNode->>EVM_Layer: Start EVM Layer via Docker Compose
User->>FullNode: Start Full Node (per new instructions)
FullNode->>EVM_Layer: Start EVM Layer via docker-compose-full-node.yml
FullNode->>SequencerNode: Copy genesis file, get P2P address
FullNode->>FullNode: Set environment, start node with correct flags
FullNode->>EVM_Layer: Connect to EVM Engine and ETH endpoints
Possibly related PRs
Suggested reviewers
Poem
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).
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2373 +/- ##
==========================================
+ Coverage 72.95% 73.08% +0.13%
==========================================
Files 67 67
Lines 6318 6319 +1
==========================================
+ Hits 4609 4618 +9
+ Misses 1314 1307 -7
+ Partials 395 394 -1
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:
|
Manav-Aggarwal
force-pushed
the
manav/add_evm_full_node_docker
branch
2 times, most recently
from
4f9c60d to
34ddd64
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds an EVM Full Node setup while updating various components to integrate the new rollkit-reth image and improve transaction processing and block synchronization.
- Updated go.mod to remove unnecessary dependencies and replace directives.
- Refactored test execution to add a built-in timestamp mechanism and store state roots during build and sync phases.
- Revised execution client logic for encoding transactions and enhanced docker-compose configurations for both single node and full node deployments.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| execution/evm/go.mod | Removed replace directive and unused dependencies. |
| execution/evm/execution_test.go | Added timestamp and state root tracking for improved test ordering. |
| execution/evm/execution.go | Modified transaction payload handling, added sorting, and updated forkchoice update handling. |
| execution/evm/docker/docker-compose.yml | Renamed service and updated image reference to rollkit-reth. |
| execution/evm/docker/docker-compose-full-node.yml | Added full node setup using rollkit-reth. |
| block/sync.go | Logged transaction count for better block synchronization tracking. |
| apps/evm/single/docker-compose.yml | Updated service naming and image version for EVM single node. |
| apps/evm/single/README.md | Adjusted instructions to reflect the new sequencer and full node setup. |
Comments suppressed due to low confidence (1)
execution/evm/execution.go:208
- It would be helpful to add additional documentation clarifying the rationale for using a zero hash for the parentBeaconBlockRoot, ensuring future maintainers understand this intentional fallback.
"parentBeaconBlockRoot": common.Hash{}.Hex(), // Use zero hash for rollkit
f5ca3a6 to
ffa67a5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
execution/evm/execution.go (1)
226-227: 🛠️ Refactor suggestionConsider replacing fixed sleep with robust synchronization.
Using a fixed 10ms delay for synchronization is brittle and may fail under different system loads or network conditions. Consider implementing a polling mechanism that checks payload readiness or using engine API events for proper synchronization.
Replace the fixed delay with a polling mechanism:
-// Small delay to allow payload building to complete -time.Sleep(10 * time.Millisecond) +// Poll for payload readiness with timeout +const maxRetries = 10 +const retryDelay = 5 * time.Millisecond +for i := 0; i < maxRetries; i++ { + // Check if payload is ready (this is a simplified example) + // You may need to implement a proper readiness check based on your engine API + if i > 0 { + time.Sleep(retryDelay) + } +}
🧹 Nitpick comments (7)
execution/evm/docker/docker-compose.yml (1)
38-40: Consider enabling default logging environment.Rather than leaving
RUST_LOGandRUST_BACKTRACEcommented out, consider exposing them via a.envfile or parameters for easier debugging.apps/evm/single/README.md (5)
10-13: Clarify<path_to>placeholder.Replace
<path_to>with a relative path (e.g.,./execution/evm/docker) or explicitly state the repo root to avoid confusion.
18-21: Fix markdown fenced code formatting.Add blank lines before and after fenced code blocks and specify the language (e.g., ```bash) to satisfy markdownlint rules.
39-39: Nitpick: duplicate placeholder note.The note on line 39 repeats guidance—consider consolidating it with the earlier
<path_to>clarification to reduce redundancy.
68-72: Consider quoting P2P_ID in export.Wrap the peer ID value in quotes if it contains special characters, or validate it matches the logged format.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
69-69: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
75-85: Nitpick: backslash continuation formatting.Remove the extra space before the backslash on line 76 and align the subsequent flags for consistency.
execution/evm/docker/docker-compose-full-node.yml (1)
29-33: Suggest adding comments for port offsets.Since external ports (9011, 8555, etc.) differ from internal ones, annotate them to clarify they're intentionally offset to avoid conflicts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
execution/evm/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
apps/evm/single/README.md(3 hunks)apps/evm/single/docker-compose.yml(3 hunks)block/sync.go(1 hunks)execution/evm/docker/docker-compose-full-node.yml(1 hunks)execution/evm/docker/docker-compose.yml(2 hunks)execution/evm/execution.go(6 hunks)execution/evm/execution_test.go(8 hunks)execution/evm/go.mod(0 hunks)
💤 Files with no reviewable changes (1)
- execution/evm/go.mod
🧰 Additional context used
🧬 Code Graph Analysis (1)
execution/evm/execution_test.go (1)
execution/evm/test_helpers.go (2)
GetRandomTransaction(125-149)SubmitTransaction(152-160)
🪛 markdownlint-cli2 (0.17.2)
apps/evm/single/README.md
56-56: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
58-58: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
60-60: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
69-69: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test / Run Integration Tests
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (22)
block/sync.go (1)
80-85: Approve addition of transaction count in debug logs.Including the
"txs"field improves observability of data processing without altering behaviour.execution/evm/docker/docker-compose.yml (4)
1-3: Approve project name update.Setting
name: "rollkit-reth"aligns the Compose project name with the renamed service.
21-24: Approve service rename and image update.Renaming
rethtorollkit-rethand updating toghcr.io/rollkit/rollkit-reth:v0.1.1follows repository conventions.
Ensure the specified image tag exists in the registry.
29-33: Approve clarified port mappings.The added comments for metrics, HTTP RPC, Engine API, and WebSocket RPC improve readability.
44-46: Approve updated entrypoint command.Switching to
rollkit-reth nodewith the necessary flags is consistent with the updated image.apps/evm/single/README.md (2)
51-54: Approve new full node section.Introducing the "Rollkit EVM Full Node" steps enriches the documentation and guides users through setup.
55-63: Verify path consistency for config directories.Ensure the source (
~/.evm-single/config/genesis.json) and destination (~/.evm-single-full/config/genesis.json) match the--homeflags to prevent misconfiguration.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
56-56: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
58-58: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
60-60: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
execution/evm/docker/docker-compose-full-node.yml (6)
1-3: Approve Docker Compose project naming.Defining
name: "rollkit-reth-full-node"is consistent with the service naming pattern.
4-20: Approve JWT initialization service.The
jwt-init-full-nodecontainer reliably generates and health-checks the JWT secret before the node starts.
21-28: Approverollkit-reth-full-nodeservice basics.Container naming, restart policy, image reference, and dependency on JWT init are correctly configured.
35-37: Approve volume mounts.Mounting chain data, JWT secret, and the
logs-full-nodevolume ensures proper persistence and access rights.
38-60: Approve full-node entrypoint configuration.The
rollkit-reth nodeinvocation and flags mirror the sequencer setup for parity.
62-65: Approve log volume declaration.Defining
logs-full-nodewith a local driver is suitable for persistent logging.apps/evm/single/docker-compose.yml (4)
4-7: Approverollkit-rethservice rename and image bump.Updating the service name and image version to
v0.1.1aligns with the other Compose files.
21-24: Approve updated entrypoint command.Switching to
rollkit-reth nodeand updating flags matches the execution Compose configuration.
52-54: Approverollkit-evm-singleimage version update.Bumped to
v0.1.6to reflect the latest release.
63-64: Approve environment variable updates.Pointing
EVM_ENGINE_URLandEVM_ETH_URLto therollkit-rethservice is correct.execution/evm/execution_test.go (3)
56-58: LGTM! Good test data organization.The addition of these slices to store payloads, timestamps, and state roots between the build and sync phases enables proper deterministic testing and verification.
135-135: Excellent improvements to test assertions!The change from
GreaterOrEqualto exact equality check for transaction count provides stronger verification. Combined with the state root matching between build and sync phases, this ensures true deterministic execution and replay capability.Also applies to: 200-205
270-270: Good test optimizations!
- Proper resource cleanup with
defer rpcClient.Close()prevents connection leaks.- Reducing transactions from 5000 to 100 per iteration improves test performance while maintaining adequate coverage.
Also applies to: 283-283
execution/evm/execution.go (2)
142-170: Well-implemented deterministic transaction ordering!The sorting logic properly handles nonce keys as integers with a string fallback, ensuring consistent transaction ordering across different execution contexts. This is crucial for deterministic state root generation.
189-196: Solid implementation of forkchoice state management!The mutex-protected access to current block hashes ensures thread safety, and the rollkit-compatible payload attributes structure with proper camelCase naming follows JSON conventions correctly. The use of zero hash for
parentBeaconBlockRootis appropriately documented.Also applies to: 200-217
| rollkit-reth: | ||
| condition: service_started | ||
| local-da: | ||
| condition: service_started |
There was a problem hiding this comment.
Raise: verify depends_on conditions.
In Compose v3.8, condition: service_started is not supported locally (only Swarm); use healthchecks with condition: service_healthy or custom wait logic.
🤖 Prompt for AI Agents
In apps/evm/single/docker-compose.yml around lines 55 to 58, the use of
'condition: service_started' under 'depends_on' is not supported in Compose v3.8
for local environments. Replace 'condition: service_started' with healthchecks
for the respective services and update the condition to 'service_healthy'.
Define appropriate healthcheck configurations for the services to ensure they
are fully ready before dependent services start.
c8af460 to
5319f1b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/evm/single/README.md (2)
64-71: Missing fenced-block language & blank linesThe log snippet is enclosed in triple back-ticks without a language identifier and without a blank line before/after, tripping markdown-lint rules (MD031/MD040). Add a blank line and annotate it as
text(orbash).-``` +```text ...--- `10-14`: **Indentation of nested list breaks Markdown rendering** Lines 12–14 are intended to be a sub-list of “1. Both EVM and DA layers …” but lack the required two-space indentation, so GitHub renders them as a *new* top-level list. Add two spaces in front of `1.`/`2.` or switch to bullet `-`. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c8af460c7b34eaac8f69fa37eaf4085c534fe9d9 and 5319f1bfe190e98eeed01cd60ae02b704b08893a. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `execution/evm/go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (8)</summary> * `apps/evm/single/README.md` (3 hunks) * `apps/evm/single/docker-compose.yml` (3 hunks) * `block/sync.go` (1 hunks) * `execution/evm/docker/docker-compose-full-node.yml` (1 hunks) * `execution/evm/docker/docker-compose.yml` (2 hunks) * `execution/evm/execution.go` (6 hunks) * `execution/evm/execution_test.go` (8 hunks) * `execution/evm/go.mod` (0 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * execution/evm/go.mod </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (6)</summary> * block/sync.go * execution/evm/docker/docker-compose.yml * execution/evm/execution_test.go * execution/evm/docker/docker-compose-full-node.yml * execution/evm/execution.go * apps/evm/single/docker-compose.yml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>apps/evm/single/README.md</summary> 56-56: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 58-58: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 60-60: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 62-62: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 64-64: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 64-64: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 69-69: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (6)</summary> * GitHub Check: test / Build All Rollkit Binaries * GitHub Check: test / Run Unit Tests * GitHub Check: test / Build Rollkit EVM Single Docker Image * GitHub Check: test / Build Rollkit Docker Image * GitHub Check: Analyze (go) * GitHub Check: Summary </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
tac0turtle
left a comment
tac0turtle
left a comment
There was a problem hiding this comment.
im working on fixing the docker system in lumen, once we have the docker system pushed lets update this and merge it
Manav-Aggarwal
force-pushed
the
manav/add_evm_full_node_docker
branch
from
5319f1b to
f65d54c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/evm/single/README.md (2)
76-78: Trailing whitespace breaks line-continuation & hard-coded home path remains
(Previously flagged, still unresolved)
- A space after the back-slash stops the line-continuation, causing
bash: \ : command not found.- Embedding
/Users/manavhurts portability.- ./evm-single start \ - --home /Users/manav/.evm-single-full-node \ + ./evm-single start \ + --home ~/.evm-single-full-node \
57-62: Wrong destination directory – full node fails to boot
cp ~/.evm-single/config/genesis.json ~/.evm-single-full/config/genesis.jsoncopies the file into a directory that doesn’t match the--home ~/.evm-single-full-nodeflag used later.
The full node will start with an emptygenesis.jsonand immediately exit.- cp ~/.evm-single/config/genesis.json ~/.evm-single-full/config/genesis.json + cp ~/.evm-single/config/genesis.json ~/.evm-single-full-node/config/genesis.json
🧹 Nitpick comments (2)
apps/evm/single/README.md (2)
12-14: RenameRethreference torollkit-rethfor consistencyThe Docker image and service were renamed in this PR, yet the README still instructs users to “run Reth …”.
This can confuse newcomers who will look for an image that no longer exists.- 1. For the EVM layer, Reth can be conveniently run using `docker compose` from <path_to>/execution/evm/docker. + 1. For the EVM layer, the `rollkit-reth` client can be run with `docker compose` from <path_to>/execution/evm/docker.
64-71: Code-block lint: missing blank lines & language identifierMarkdown-lint flags (MD031, MD040) are correct; add blank lines around the fenced block and specify
text(or omit) to silence the warnings and improve readability.-``` +```text</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5319f1bfe190e98eeed01cd60ae02b704b08893a and f65d54c5b3e1a6b7782dc8cf185487fd13f0147e. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `execution/evm/go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (8)</summary> * `apps/evm/single/README.md` (3 hunks) * `apps/evm/single/docker-compose.yml` (3 hunks) * `block/sync.go` (1 hunks) * `execution/evm/docker/docker-compose-full-node.yml` (1 hunks) * `execution/evm/docker/docker-compose.yml` (2 hunks) * `execution/evm/execution.go` (6 hunks) * `execution/evm/execution_test.go` (8 hunks) * `execution/evm/go.mod` (0 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * execution/evm/go.mod </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (6)</summary> * block/sync.go * execution/evm/docker/docker-compose.yml * execution/evm/execution_test.go * execution/evm/docker/docker-compose-full-node.yml * execution/evm/execution.go * apps/evm/single/docker-compose.yml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>apps/evm/single/README.md</summary> 56-56: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 58-58: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 60-60: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 62-62: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 64-64: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) --- 64-64: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 69-69: Fenced code blocks should be surrounded by blank lines null (MD031, blanks-around-fences) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (6)</summary> * GitHub Check: test / Run Unit Tests * GitHub Check: test / Build All Rollkit Binaries * GitHub Check: test / Build Rollkit EVM Single Docker Image * GitHub Check: test / Build Rollkit Docker Image * GitHub Check: Analyze (go) * GitHub Check: Summary </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Manav-Aggarwal
force-pushed
the
manav/add_evm_full_node_docker
branch
from
f65d54c to
49dbf07
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/evm/single/README.md (2)
61-62: Destination path is still wrong – the full node will not see the genesis file
(duplicate of a previous review)The
cpcommand writes into~/.evm-single-full/…, but the node is initialised with--home ~/.evm-single-full-node. This mismatch prevents the full node from findinggenesis.jsonand it fails to start.- cp ~/.evm-single/config/genesis.json ~/.evm-single-full/config/genesis.json + cp ~/.evm-single/config/genesis.json ~/.evm-single-full-node/config/genesis.json
75-78: Trailing back-slash whitespace + hard-coded home path break the start command
(duplicate of a previous review)
- There is a stray space after the line-continuation back-slash that makes
bashtreat “\ ” as a command.- An absolute
/Users/manav/...path is not portable – earlier you used~/.evm-single-full-node.- ./evm-single start \ - --home /Users/manav/.evm-single-full-node \ + ./evm-single start \ + --home ~/.evm-single-full-node \
🧹 Nitpick comments (1)
apps/evm/single/README.md (1)
55-63: Add blank lines around fenced code blocks to satisfy markdown-lint and improve readability
markdownlinterrors (MD031) are triggered because the fenced code blocks under items 2 & 3 aren’t surrounded by blank lines. Adding a single empty line before and after each```bash/```block resolves the warnings and keeps the list rendering predictable.-2. Initialize the full node: - ```bash +2. Initialize the full node: + + ```bash ./evm-single init --home ~/.evm-single-full-node - ``` + ``` + -3. Copy the genesis file from the sequencer node: - ```bash +3. Copy the genesis file from the sequencer node: + + ```bash cp ~/.evm-single/config/genesis.json ~/.evm-single-full/config/genesis.json - ``` + ``` +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
execution/evm/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
apps/evm/single/README.md(3 hunks)apps/evm/single/docker-compose.yml(3 hunks)block/sync.go(1 hunks)execution/evm/docker/docker-compose-full-node.yml(1 hunks)execution/evm/docker/docker-compose.yml(2 hunks)execution/evm/execution.go(6 hunks)execution/evm/execution_test.go(8 hunks)execution/evm/go.mod(0 hunks)
💤 Files with no reviewable changes (1)
- execution/evm/go.mod
🚧 Files skipped from review as they are similar to previous changes (6)
- block/sync.go
- execution/evm/docker/docker-compose.yml
- execution/evm/execution_test.go
- execution/evm/docker/docker-compose-full-node.yml
- apps/evm/single/docker-compose.yml
- execution/evm/execution.go
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
apps/evm/single/README.md
56-56: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
58-58: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
60-60: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
69-69: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: Analyze (go)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/evm/single/README.md (1)
75-78: Trailing\whitespace + hard-coded absolute path will break the start commandThe same problem was already flagged in a previous review but is still present:
- A space follows the line-continuation back-slash on line 76, so
bashinterprets it as\(a command) and aborts.- The
/Users/manav/...prefix makes the docs non-portable and also disagrees with the~/.evm-single-full-nodepath used earlier.- ./evm-single start \ - --home /Users/manav/.evm-single-full-node \ + ./evm-single start \ + --home ~/.evm-single-full-node \Please apply this fix (or an equivalent one) to avoid copy-paste failures.
🧹 Nitpick comments (1)
apps/evm/single/README.md (1)
56-71: Markdown lint violations around fenced code blocksSeveral fenced blocks (56-58, 60-62, 64-66, 69-71) lack:
- A blank line before/after the fence (MD031).
- A language hint for the log snippet on 64-66 (MD040).
These renderings trip linters and sometimes break list numbering. Quickly fixable:
@@ -2. Initialize the full node: - ```bash - ./evm-single init --home ~/.evm-single-full-node - ``` +2. Initialize the full node: + +```bash +./evm-single init --home ~/.evm-single-full-node +``` @@ -4. Identify the sequencer node's P2P address from its logs. It will look similar to: - ``` +4. Identify the sequencer node's P2P address from its logs. It will look similar to: + +```text @@ - ``` +```Apply similar spacing/language tweaks to the other blocks for clean Markdown.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/evm/single/README.md(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
apps/evm/single/README.md
56-56: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
58-58: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
60-60: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
69-69: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Run Integration Tests
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (1)
apps/evm/single/README.md (1)
53-53: Consistent Docker CLI syntaxEarlier instructions use
docker compose; here it switches to the legacydocker-compose. Pick one form (preferablydocker compose) to avoid confusion.[ suggest_optional_refactor ]
Manav-Aggarwal
force-pushed
the
manav/add_evm_full_node_docker
branch
from
568c6ad to
a539254
Compare
Manav-Aggarwal
force-pushed
the
manav/add_evm_full_node_docker
branch
from
6871e62 to
6617292
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/evm/single/README.md (1)
75-78: Trailing back-slash whitespace & hard-coded absolute$HOMEpath remainThe back-slash on line 76 is still followed by a space and will be interpreted by
bashas\, i.e. a literal back-slash + space →command not found.
Line 77 re-introduces/Users/manav/..., killing portability.- ./evm-single start \ - --home /Users/manav/.evm-single-full-node \ + ./evm-single start \ + --home ~/.evm-single-full-node \
🧹 Nitpick comments (3)
apps/evm/single/README.md (3)
55-63: Missing blank lines around fenced blocks triggers MD031Insert a blank line before and after each ```bash fenced block to silence markdownlint and improve readability.
2. Initialize the full node: - ```bash + + ```bash ./evm-single init --home ~/.evm-single-full-node ``` +
64-71: Code block lacks language spec & blank linesAdd
bashafter the back-ticks and surround the block with blank lines to avoid MD031 / MD040.- ``` + + ```bash ``` Create an environment variable with the P2P address: - ```bash + + ```bash export P2P_ID="12D3KooWJbD9TQoMSSSUyfhHMmgVY3LqCjxYFz8wQ92Qa6DAqtmh" ``` +
53-54:docker composevsdocker-composeinconsistencyEarlier you instruct users to run
docker compose(space), while here you usedocker-compose(dash). Stick to one form (the moderndocker composesub-command) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/evm/single/README.md(3 hunks)apps/evm/single/docker-compose.yml(3 hunks)execution/evm/docker/docker-compose-full-node.yml(1 hunks)execution/evm/docker/docker-compose.yml(2 hunks)execution/evm/execution_test.go(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- execution/evm/docker/docker-compose.yml
- execution/evm/docker/docker-compose-full-node.yml
- execution/evm/execution_test.go
- apps/evm/single/docker-compose.yml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
apps/evm/single/README.md
56-56: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
58-58: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
60-60: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
69-69: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test / Run E2E System Tests
- GitHub Check: test / Run EVM Execution Tests
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: Analyze (go)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/evm/single/README.md (7)
30-37: Remove any trailing whitespace after line-continuation\A stray space after the backslash will be interpreted as
\and makes the whole command fail withbash: \ : command not found.
Double-check lines 31-36 and strip trailing spaces, if any.
30-37: Path placeholder should be consistent across the docLines 32 and 33 still rely on
<path_to>whereas the note below (l. 39) clarifies that users should substitute the rollkit repo path. Consider replacing the placeholder with$ROLLKIT_ROOT(or similar) for consistency and copy-paste friendliness.
57-58: Use the samedocker composeinvocation everywhereEarlier sections use the space-separated
docker compose, whereas line 57 switches to the legacydocker-compose. Mixing the two can confuse newcomers and break shell history search.-1. ... using `docker-compose -f docker-compose-full-node.yml` +1. ... using `docker compose -f docker-compose-full-node.yml`
60-66: Add blank lines around fenced code blocks to satisfy markdownlint
MD031is triggered because the code fences are flush with surrounding text. Insert a blank line before```bashand after```:-2. Initialize the full node: -```bash +2. Initialize the full node: + +```bash ./evm-single init --home ~/.evm-single-full-node -``` +``` +Repeat for the
cpsnippet.
68-70: Specify a language for the log snippet
MD040warns about missing language identifiers. Mark the block as plain text:-``` +```text 1:55PM INF listening on address=/ip4/127.0.0.1/tcp/7676/p2p/12D3Koo...--- `79-89`: **Relative path to JWT token will break outside `apps/evm/single`** `../../../execution/evm/docker/jwttoken/jwt.hex` only works when the command is executed from `apps/evm/single`. Recommend using an absolute path or the same `$ROLLKIT_ROOT` placeholder used above: ```diff - --evm.jwt-secret $(cat ../../../execution/evm/docker/jwttoken/jwt.hex) \ + --evm.jwt-secret $(cat $ROLLKIT_ROOT/execution/evm/docker/jwttoken/jwt.hex) \
39-43: Tighten wording (“originally created”)The phrase “originally created” is redundant. A simpler alternative keeps the README concise:
-make sure to remove the originally created sequencer node directory +make sure to remove the sequencer node directoryRepeat the same change in the full-node section (l. 91).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/evm/single/README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/evm/single/README.md
[style] ~39-~39: This phrase is redundant. Consider writing “created”.
Context: ...t a fresh node, make sure to remove the originally created sequencer node directory using: ```bas...
(ORIGINALLY_CREATED)
[style] ~91-~91: This phrase is redundant. Consider writing “created”.
Context: ...t a fresh node, make sure to remove the originally created full node directory using: ```bash ...
(ORIGINALLY_CREATED)
🪛 markdownlint-cli2 (0.17.2)
apps/evm/single/README.md
60-60: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
64-64: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
66-66: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
68-68: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
68-68: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
73-73: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
95-95: Files should end with a single newline character
null
(MD047, single-trailing-newline)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Run Integration Tests
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: Analyze (go)
- GitHub Check: Summary
|
|
||
| err = c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", | ||
| engine.ForkchoiceStateV1{ | ||
| HeadBlockHash: prevBlockHash, |
| } | ||
| // } | ||
|
|
||
| // make sure that the timestamp is increasing |
Overview
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests