Skip to content

[codex] support reth execution client in devnet#947

Merged
panos-xyz merged 10 commits intomainfrom
codex/devnet-reth-client
May 9, 2026
Merged

[codex] support reth execution client in devnet#947
panos-xyz merged 10 commits intomainfrom
codex/devnet-reth-client

Conversation

@panos-xyz
Copy link
Copy Markdown
Contributor

@panos-xyz panos-xyz commented May 7, 2026

Summary

  • Add EXECUTION_CLIENT=geth|reth devnet selection and make devnet-up-reth / make devnet-clean-build-reth helpers.
  • Add a reth compose override that reuses the existing execution-client service names (morph-el-*, validator-el) for geth/reth parity.
  • Default reth devnet startup to ghcr.io/morph-l2/morph-reth:latest, while preserving local source builds via MORPH_RETH_BUILD_FROM_SOURCE=true.
  • Keep reth startup args aligned with the current supported RPC modules and use --nat=none plus --disable-discovery.
  • Rename execution-client references from geth-specific names to *-el where the same wiring is shared by both clients.

Notes

The default reth image is multi-arch and uses /usr/local/bin/morph-reth as its entrypoint. Local source builds still use the builder target and /app/morph-reth.

This PR intentionally does not fix the tx-submitter cold-start panic observed when the submitter queries block 1 while the L2 latest block is still 0. That behavior was reproduced in both geth and reth cold starts and should be handled as a separate tx-submitter boundary fix.

Validation

  • make -n devnet-up-reth
  • make -n devnet-up-reth MORPH_RETH_BUILD_FROM_SOURCE=true
  • make -n devnet-clean-build-reth
  • docker compose -f ops/docker/docker-compose-4nodes.yml config
  • docker compose -f ops/docker/docker-compose-4nodes.yml -f ops/docker/docker-compose-reth.yml config
  • docker manifest inspect ghcr.io/morph-l2/morph-reth:latest

Summary by CodeRabbit

  • New Features

    • Added support for selecting execution client (Geth or Reth) in devnet via new --execution-client option
    • New devnet utilities: devnet-logs, devnet-up-reth, and devnet-clean-build-reth targets
  • Chores

    • Reorganized devnet service architecture with updated naming conventions
    • Updated Docker configurations and build contexts across deployment scenarios

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors the devnet infrastructure to support dynamic execution client selection (geth/reth) and standardizes execution layer service naming from morph-geth-* to morph-el-* across docker compose configurations, build orchestration, and test scripts.

Changes

Execution Layer Refactor & Execution Client Selection

Layer / File(s) Summary
Execution Client Selection & Configuration
Makefile, ops/devnet-morph/devnet/__init__.py
Added EXECUTION_CLIENT variable (default geth) to Makefile with conditional DEVNET_COMPOSE_FILES and DEVNET_EXECUTION_DEPS computation. New --execution-client CLI argument and compose_file_args() function in Python devnet launcher.
Reth Compose Template
ops/docker/docker-compose-reth.yml
New file with reusable YAML anchors (x-reth-command, x-reth-service) defining shared reth configuration for morph-el-* and validator-el services.
Devnet Orchestration Targets
Makefile, ops/devnet-morph/devnet/__init__.py
Updated devnet-up and devnet-up-debugccc to depend on computed execution prerequisites and pass --execution-client argument. Added devnet-up-reth, devnet-logs, reth-image, and reth targets. Updated devnet-down and devnet-clean-build to use dynamic compose files.
Primary Compose Configuration
ops/docker/docker-compose-4nodes.yml
Renamed all L2 execution services from morph-geth-* to morph-el-*, updated volume names (sequencer_geth_datasequencer_el_data), rewired node/validator/sentry/submitter services to new execution layer RPC endpoints, added TX_SUBMITTER_SEAL_BATCH=true.
Secondary Compose & Network Configuration
node/ops-morph/docker-compose.yml, node/ops-morph/testnet/*, ops/docker-sequencer-test/docker-compose.override.yml, ops/docker/static-nodes.json, oracle/docker-compose.yml
Applied service naming updates and RPC wiring consistency across all environments. Updated static node enode identifiers from morph-geth-* to morph-el-*.
Test Scripts & Build Context
ops/docker-sequencer-test/run-test.sh, ops/docker-sequencer-test/scripts/tx-generator.sh, ops/docker-sequencer-test/Dockerfile.*
Updated service references to morph-el-*, changed Docker build context from BITGET_ROOT to POLYREPO_ROOT, changed L2 RPC defaults to morph-el-0:8545, updated test image naming to morph-el-test.
Documentation & Helper Functions
ops/devnet-morph/devnet/__init__.py
Updated docstrings for main entry point, devnet lifecycle functions, and utility methods to reflect execution client parameter and orchestration behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • morph-l2/morph#925: Modifies devnet-clean-build Makefile target with docker compose and dynamic file handling, closely related to execution client selection refactoring.

Suggested reviewers

  • twcctop
  • tomatoishealthy

Poem

🐰 Services dance with shiny new names,
morph-el emerges from morph-geth flames,
Reth joins the quest with YAML anchors bright,
Dynamic clients configured just right, 🎭
A devnet reborn, more flexible and light!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[codex] support reth execution client in devnet' directly and clearly describes the main change: adding support for reth as an execution client option in the devnet infrastructure.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/devnet-reth-client

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@panos-xyz panos-xyz marked this pull request as ready for review May 7, 2026 14:25
@panos-xyz panos-xyz requested a review from a team as a code owner May 7, 2026 14:25
@panos-xyz panos-xyz requested review from r3aker86 and removed request for a team May 7, 2026 14:25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
Makefile (1)

186-218: ⚡ Quick win

Add -reth wrappers for devnet-down and devnet-logs for symmetry.

devnet-clean-build-reth exists, but devnet-down/devnet-logs rely on EXECUTION_CLIENT being passed every invocation. After running make devnet-up-reth, a follow-up make devnet-down defaults to EXECUTION_CLIENT=geth and only references docker-compose-4nodes.yml. Containers will still stop by name (services are reused), but anything declared only in docker-compose-reth.yml (extra volumes/networks/anonymous mounts) won't be cleaned by down --volumes.

♻️ Proposed wrappers
 devnet-down:
 	cd ops/docker && docker compose $(DEVNET_COMPOSE_FILES) down
 .PHONY: devnet-down
 
+devnet-down-reth:
+	$(MAKE) devnet-down EXECUTION_CLIENT=reth
+.PHONY: devnet-down-reth
+
 devnet-clean-build: devnet-l1-clean
@@
 devnet-logs:
 	@(cd ops/docker && docker compose $(DEVNET_COMPOSE_FILES) logs -f)
 .PHONY: devnet-logs
+
+devnet-logs-reth:
+	$(MAKE) devnet-logs EXECUTION_CLIENT=reth
+.PHONY: devnet-logs-reth
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 186 - 218, Add symmetric wrapper targets
devnet-down-reth and devnet-logs-reth that invoke the existing devnet-down and
devnet-logs targets with EXECUTION_CLIENT=reth so the reth-specific compose
file/volumes/networks are honored; implement them similarly to
devnet-clean-build-reth (e.g., have devnet-down-reth run $(MAKE) devnet-down
EXECUTION_CLIENT=reth and devnet-logs-reth run $(MAKE) devnet-logs
EXECUTION_CLIENT=reth) and mark both .PHONY.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Makefile`:
- Around line 186-218: Add symmetric wrapper targets devnet-down-reth and
devnet-logs-reth that invoke the existing devnet-down and devnet-logs targets
with EXECUTION_CLIENT=reth so the reth-specific compose file/volumes/networks
are honored; implement them similarly to devnet-clean-build-reth (e.g., have
devnet-down-reth run $(MAKE) devnet-down EXECUTION_CLIENT=reth and
devnet-logs-reth run $(MAKE) devnet-logs EXECUTION_CLIENT=reth) and mark both
.PHONY.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bfa20dbb-5265-4d94-9c82-036def2ab39b

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb95dc and 74eed9e.

📒 Files selected for processing (12)
  • Makefile
  • node/ops-morph/docker-compose.yml
  • node/ops-morph/testnet/docker-compose.yml
  • node/ops-morph/testnet/static-nodes.json
  • ops/devnet-morph/devnet/__init__.py
  • ops/docker-sequencer-test/docker-compose.override.yml
  • ops/docker-sequencer-test/run-test.sh
  • ops/docker-sequencer-test/scripts/tx-generator.sh
  • ops/docker/docker-compose-4nodes.yml
  • ops/docker/docker-compose-reth.yml
  • ops/docker/static-nodes.json
  • oracle/docker-compose.yml

panos-xyz added 2 commits May 8, 2026 10:55
Replace the internal codename "bitget" with the neutral term "polyrepo"
in build context references, variable names, and container paths.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ops/devnet-morph/devnet/__init__.py (1)

272-283: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Passing GETH_DATA_DIR to reth is dead configuration.

The reth service definition in docker-compose-reth.yml hardcodes --datadir=/db and never references the GETH_DATA_DIR environment variable. When execution_client == 'reth', this variable is still injected unconditionally in the Python code, creating unused and misleading configuration. An operator reading the code might expect GETH_DATA_DIR to control reth's data directory location, when in fact it is ignored.

Consider either:

  • Conditionally supply GETH_DATA_DIR only when execution_client != 'reth'
  • Or ensure and document that the reth compose override intentionally hardcodes the path
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ops/devnet-morph/devnet/__init__.py` around lines 272 - 283, The env dict
passed to run_command (see the run_command call and compose_file_args usage and
the execution_client variable) incorrectly always injects GETH_DATA_DIR even
though the reth compose file hardcodes --datadir=/db; change the call to build
the environment dict first and only add the 'GETH_DATA_DIR' key when
execution_client != 'reth' (or when a helper like compose_file_args indicates a
non-reth client), so that GETH_DATA_DIR is not supplied for reth and no longer
produces misleading configuration.
🧹 Nitpick comments (1)
ops/docker-sequencer-test/Dockerfile.l2-node-test (1)

38-45: 💤 Low value

Final stage reuses the heavy builder image.

The final stage is FROM ghcr.io/morph-l2/go-ubuntu-builder:go-1.24-ubuntu — the same image used for compilation — which bundles the full Go toolchain. For a test image this is not a blocker, but it inflates the image size considerably vs. a slim base (e.g. ubuntu:22.04 or gcr.io/distroless/base). The binary-only artifacts (tendermint, morphnode) are copied in, so no Go toolchain is needed at runtime.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ops/docker-sequencer-test/Dockerfile.l2-node-test` around lines 38 - 45,
Final stage uses the heavy builder image
(ghcr.io/morph-l2/go-ubuntu-builder:go-1.24-ubuntu) which unnecessarily includes
the Go toolchain; change the final FROM to a slim runtime base (e.g.,
ubuntu:22.04 or gcr.io/distroless/base) and ensure any runtime packages like
ca-certificates are installed, then keep the existing COPY --from=builder lines
that copy the binary artifacts (COPY --from=builder
/polyrepo/morph/node/build/bin/tendermint /usr/local/bin/ and COPY
--from=builder /polyrepo/morph/node/build/bin/morphnode /usr/local/bin/) so only
the runtime image with required deps contains the binaries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ops/docker-sequencer-test/Dockerfile.l2-node-test`:
- Around line 25-26: The Dockerfile uses incorrect source paths with a
"./morph/" prefix that doesn't exist, causing COPY to fail; update every COPY
that references "./morph/..." (for example the entries that should copy go.work,
node/go.mod, bindings/go.mod) to use the correct root-level paths (remove the
"./morph/" prefix) so sources like ./go.work, ./node/go.mod, ./bindings/go.mod
are used, and keep the existing targets (e.g., /polyrepo/morph/,
/polyrepo/morph/node/) so WORKDIR and RUN go mod download -x remain valid.

---

Outside diff comments:
In `@ops/devnet-morph/devnet/__init__.py`:
- Around line 272-283: The env dict passed to run_command (see the run_command
call and compose_file_args usage and the execution_client variable) incorrectly
always injects GETH_DATA_DIR even though the reth compose file hardcodes
--datadir=/db; change the call to build the environment dict first and only add
the 'GETH_DATA_DIR' key when execution_client != 'reth' (or when a helper like
compose_file_args indicates a non-reth client), so that GETH_DATA_DIR is not
supplied for reth and no longer produces misleading configuration.

---

Nitpick comments:
In `@ops/docker-sequencer-test/Dockerfile.l2-node-test`:
- Around line 38-45: Final stage uses the heavy builder image
(ghcr.io/morph-l2/go-ubuntu-builder:go-1.24-ubuntu) which unnecessarily includes
the Go toolchain; change the final FROM to a slim runtime base (e.g.,
ubuntu:22.04 or gcr.io/distroless/base) and ensure any runtime packages like
ca-certificates are installed, then keep the existing COPY --from=builder lines
that copy the binary artifacts (COPY --from=builder
/polyrepo/morph/node/build/bin/tendermint /usr/local/bin/ and COPY
--from=builder /polyrepo/morph/node/build/bin/morphnode /usr/local/bin/) so only
the runtime image with required deps contains the binaries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8043b7fb-d5e8-4d13-b100-a5286467e21e

📥 Commits

Reviewing files that changed from the base of the PR and between 74eed9e and 1672b69.

📒 Files selected for processing (4)
  • ops/devnet-morph/devnet/__init__.py
  • ops/docker-sequencer-test/Dockerfile.l2-geth-test
  • ops/docker-sequencer-test/Dockerfile.l2-node-test
  • ops/docker-sequencer-test/run-test.sh
✅ Files skipped from review due to trivial changes (1)
  • ops/docker-sequencer-test/Dockerfile.l2-geth-test
🚧 Files skipped from review as they are similar to previous changes (1)
  • ops/docker-sequencer-test/run-test.sh

Comment thread ops/docker-sequencer-test/Dockerfile.l2-node-test
@panos-xyz
Copy link
Copy Markdown
Contributor Author

Re: CodeRabbit's Dockerfile.l2-node-test path warning — this is a false positive.

Dockerfile.l2-node-test is built with the polyrepo root (the parent directory of morph/) as the Docker build context, not the morph repo root. The build command in run-test.sh is:

cd "$POLYREPO_ROOT"   # e.g. /workspace (parent of morph/)
docker build -f morph/ops/docker-sequencer-test/Dockerfile.l2-node-test .

With this context, ./morph/go.workworkspace/morph/go.work ✓, ./go-ethereum/ → sibling go-ethereum repo ✓, ./tendermint/ → sibling tendermint repo ✓. The paths are correct as-is; CodeRabbit's verification ran inside the morph repo where ./morph/ does not exist, hence the false alarm.

@panos-xyz
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Cover the rest of the file with one-line docstrings to satisfy
CodeRabbit's docstring coverage threshold.
@panos-xyz
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…g Dockerfile.l2-geth

Move the morph-el-0 build: section from docker-compose-4nodes.yml into a
dedicated docker-compose-geth-build.yml, included only when
EXECUTION_CLIENT=geth. The reth overlay now sees no build: on morph-el-0,
eliminating the risk of docker compose up building geth code and tagging
it as the reth image when the reth image is absent.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Makefile (1)

146-147: ⚡ Quick win

Pin the default reth image instead of using :latest.

Lines 146-147 use a mutable image tag by default, so the same checkout can pull different binaries over time. This makes local failures and CI drift harder to reproduce. Use an immutable image digest or explicit version tag, and leave latest as an optional override.

♻️ Suggested change
-MORPH_RETH_IMAGE ?= ghcr.io/morph-l2/morph-reth:latest
+MORPH_RETH_IMAGE ?= ghcr.io/morph-l2/morph-reth@sha256:<published-digest>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 146 - 147, The default MORPH_RETH_IMAGE currently uses
the mutable ":latest" tag which causes non-reproducible pulls; change the
default value of MORPH_RETH_IMAGE to an immutable tag (e.g., a specific version
like "ghcr.io/morph-l2/morph-reth:vX.Y.Z" or a digest
"ghcr.io/morph-l2/morph-reth@sha256:...") while keeping MORPH_RETH_ENTRYPOINT
as-is and still allowing consumers to override MORPH_RETH_IMAGE externally;
update the Makefile assignment for MORPH_RETH_IMAGE to use that explicit
version/digest instead of ":latest".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 161-173: The Makefile currently validates EXECUTION_CLIENT at
parse time (the conditional block setting DEVNET_EXECUTION_DEPS /
DEVNET_COMPOSE_FILES and the $(error ...)) which can abort unrelated targets;
move this validation into the devnet-specific target logic or switch to a
separate variable name (e.g. DEVNET_EXECUTION_CLIENT) to avoid global parse-time
evaluation. Specifically, replace references to EXECUTION_CLIENT with
DEVNET_EXECUTION_CLIENT (or wrap the existing conditional inside the devnet
target recipe) and ensure MORPH_RETH_BUILD_FROM_SOURCE, DEVNET_EXECUTION_DEPS
and DEVNET_COMPOSE_FILES are set only when the devnet target is invoked; remove
or relocate the $(error ...) so it runs at target execution time rather than
during Makefile parsing.

---

Nitpick comments:
In `@Makefile`:
- Around line 146-147: The default MORPH_RETH_IMAGE currently uses the mutable
":latest" tag which causes non-reproducible pulls; change the default value of
MORPH_RETH_IMAGE to an immutable tag (e.g., a specific version like
"ghcr.io/morph-l2/morph-reth:vX.Y.Z" or a digest
"ghcr.io/morph-l2/morph-reth@sha256:...") while keeping MORPH_RETH_ENTRYPOINT
as-is and still allowing consumers to override MORPH_RETH_IMAGE externally;
update the Makefile assignment for MORPH_RETH_IMAGE to use that explicit
version/digest instead of ":latest".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2adfdd8-cbb2-48cb-9d53-0169baaaccdc

📥 Commits

Reviewing files that changed from the base of the PR and between 1672b69 and 6d8d745.

📒 Files selected for processing (4)
  • Makefile
  • ops/devnet-morph/devnet/__init__.py
  • ops/docker/docker-compose-4nodes.yml
  • ops/docker/docker-compose-geth-build.yml
✅ Files skipped from review due to trivial changes (1)
  • ops/docker/docker-compose-geth-build.yml
🚧 Files skipped from review as they are similar to previous changes (2)
  • ops/devnet-morph/devnet/init.py
  • ops/docker/docker-compose-4nodes.yml

Comment thread Makefile
Comment on lines +161 to +173
ifeq ($(EXECUTION_CLIENT),geth)
DEVNET_EXECUTION_DEPS := submodules
DEVNET_COMPOSE_FILES += -f docker-compose-geth-build.yml
else ifeq ($(EXECUTION_CLIENT),reth)
DEVNET_COMPOSE_FILES += -f docker-compose-reth.yml
ifeq ($(MORPH_RETH_BUILD_FROM_SOURCE),true)
DEVNET_EXECUTION_DEPS := reth
else
DEVNET_EXECUTION_DEPS := reth-image
endif
else
$(error unsupported EXECUTION_CLIENT "$(EXECUTION_CLIENT)", expected "geth" or "reth")
endif
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

cat >/tmp/exec-client-parse.mk <<'EOF'
EXECUTION_CLIENT ?= bad
ifeq ($(EXECUTION_CLIENT),geth)
else ifeq ($(EXECUTION_CLIENT),reth)
else
$(error unsupported EXECUTION_CLIENT "$(EXECUTION_CLIENT)")
endif

lint:
	@:
EOF

env EXECUTION_CLIENT=bad make -f /tmp/exec-client-parse.mk lint

Repository: morph-l2/morph

Length of output: 99


🏁 Script executed:

sed -n '161,173p' Makefile

Repository: morph-l2/morph

Length of output: 490


Avoid validating EXECUTION_CLIENT at parse time.

Line 172 is evaluated while Make parses the file, so an inherited EXECUTION_CLIENT=foo will break unrelated targets like make lint before target resolution. Please scope this check to the devnet targets, or rename the knob to something less collision-prone such as DEVNET_EXECUTION_CLIENT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 161 - 173, The Makefile currently validates
EXECUTION_CLIENT at parse time (the conditional block setting
DEVNET_EXECUTION_DEPS / DEVNET_COMPOSE_FILES and the $(error ...)) which can
abort unrelated targets; move this validation into the devnet-specific target
logic or switch to a separate variable name (e.g. DEVNET_EXECUTION_CLIENT) to
avoid global parse-time evaluation. Specifically, replace references to
EXECUTION_CLIENT with DEVNET_EXECUTION_CLIENT (or wrap the existing conditional
inside the devnet target recipe) and ensure MORPH_RETH_BUILD_FROM_SOURCE,
DEVNET_EXECUTION_DEPS and DEVNET_COMPOSE_FILES are set only when the devnet
target is invoked; remove or relocate the $(error ...) so it runs at target
execution time rather than during Makefile parsing.

Keep the base devnet compose file self-contained for geth while using the reth overlay to explicitly reset inherited geth build definitions.

Constraint: Do not include the devnet execution-client test file in this commit
Rejected: Keep a separate geth build compose file | changes direct base compose usage
Confidence: high
Scope-risk: narrow
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
Makefile (1)

161-172: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Parse-time EXECUTION_CLIENT validation still affects unrelated targets.

The $(error …) on line 171 fires while Make parses the file, so an inherited or stale EXECUTION_CLIENT=foo will abort non-devnet targets like make lint or make bindings. Either gate the validation behind the devnet recipes or rename to a devnet-scoped variable (e.g. DEVNET_EXECUTION_CLIENT).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 161 - 172, The parse-time error triggered by the
top-level EXECUTION_CLIENT conditional should be moved so it only runs for
devnet workflows: either rename EXECUTION_CLIENT to a devnet-scoped variable
(e.g. DEVNET_EXECUTION_CLIENT) and update all references (DEVNET_EXECUTION_DEPS,
DEVNET_COMPOSE_FILES, MORPH_RETH_BUILD_FROM_SOURCE checks) accordingly, or keep
EXECUTION_CLIENT but remove the top-level $(error …) and instead perform the
validation inside the devnet target recipe (or a devnet-specific include) so the
error is only raised when running devnet-related targets; update the conditional
branches (the geth/reth/else logic) to use the chosen variable placement so
non-devnet targets like lint or bindings are not affected during Makefile parse
time.
🧹 Nitpick comments (1)
Makefile (1)

186-218: ⚡ Quick win

Add reth-specific lifecycle targets for consistency.

After make devnet-up-reth, the devnet-down and devnet-logs targets default back to EXECUTION_CLIENT=geth since they don't propagate this variable (unlike devnet-up-reth and devnet-clean-build-reth). This causes services exclusive to the reth overlay—morph-el-0, morph-el-1, morph-el-2, morph-el-3, sentry-el-0, validator-el—to be orphaned on shutdown and omitted from log tails.

♻️ Suggested convenience targets
 devnet-down:
 	cd ops/docker && docker compose $(DEVNET_COMPOSE_FILES) down
 .PHONY: devnet-down
+
+devnet-down-reth:
+	$(MAKE) devnet-down EXECUTION_CLIENT=reth
+.PHONY: devnet-down-reth
@@
 devnet-logs:
 	@(cd ops/docker && docker compose $(DEVNET_COMPOSE_FILES) logs -f)
 .PHONY: devnet-logs
+
+devnet-logs-reth:
+	$(MAKE) devnet-logs EXECUTION_CLIENT=reth
+.PHONY: devnet-logs-reth
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 186 - 218, devnet-down and devnet-logs don't honor
EXECUTION_CLIENT=reth so reth services get orphaned; add reth-specific targets
(e.g., devnet-down-reth and devnet-logs-reth) that invoke the existing commands
with EXECUTION_CLIENT=reth (mirror how devnet-clean-build-reth calls
devnet-clean-build) or modify devnet-down and devnet-logs to accept and forward
the EXECUTION_CLIENT variable into the docker compose invocation; reference
targets: devnet-down, devnet-logs, devnet-clean-build-reth, and the new
devnet-down-reth/devnet-logs-reth to ensure shutdown and log-follow include the
reth overlay.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@Makefile`:
- Around line 161-172: The parse-time error triggered by the top-level
EXECUTION_CLIENT conditional should be moved so it only runs for devnet
workflows: either rename EXECUTION_CLIENT to a devnet-scoped variable (e.g.
DEVNET_EXECUTION_CLIENT) and update all references (DEVNET_EXECUTION_DEPS,
DEVNET_COMPOSE_FILES, MORPH_RETH_BUILD_FROM_SOURCE checks) accordingly, or keep
EXECUTION_CLIENT but remove the top-level $(error …) and instead perform the
validation inside the devnet target recipe (or a devnet-specific include) so the
error is only raised when running devnet-related targets; update the conditional
branches (the geth/reth/else logic) to use the chosen variable placement so
non-devnet targets like lint or bindings are not affected during Makefile parse
time.

---

Nitpick comments:
In `@Makefile`:
- Around line 186-218: devnet-down and devnet-logs don't honor
EXECUTION_CLIENT=reth so reth services get orphaned; add reth-specific targets
(e.g., devnet-down-reth and devnet-logs-reth) that invoke the existing commands
with EXECUTION_CLIENT=reth (mirror how devnet-clean-build-reth calls
devnet-clean-build) or modify devnet-down and devnet-logs to accept and forward
the EXECUTION_CLIENT variable into the docker compose invocation; reference
targets: devnet-down, devnet-logs, devnet-clean-build-reth, and the new
devnet-down-reth/devnet-logs-reth to ensure shutdown and log-follow include the
reth overlay.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14303773-835c-4e07-bf9a-ca2ce3646d7b

📥 Commits

Reviewing files that changed from the base of the PR and between 6d8d745 and 8a5fc0a.

📒 Files selected for processing (4)
  • Makefile
  • ops/devnet-morph/devnet/__init__.py
  • ops/docker/docker-compose-4nodes.yml
  • ops/docker/docker-compose-reth.yml
✅ Files skipped from review due to trivial changes (1)
  • ops/docker/docker-compose-reth.yml
🚧 Files skipped from review as they are similar to previous changes (2)
  • ops/devnet-morph/devnet/init.py
  • ops/docker/docker-compose-4nodes.yml

@panos-xyz panos-xyz merged commit 58c70f9 into main May 9, 2026
13 checks passed
@panos-xyz panos-xyz deleted the codex/devnet-reth-client branch May 9, 2026 03:06
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.

2 participants