[codex] default devnet to single sequencer#1010
Open
panos-xyz wants to merge 11 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSwitches the devnet to a 2-node base topology with a rewritten HA cluster overlay, adds sequencer CLI-driven L1 registration and finalization waits, updates node generation and Makefile wiring, and adds config validation tests. ChangesDevnet 2-node refactor and single-sequencer wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/devnet-morph/devnet/__init__.py`:
- Around line 373-383: The wait_for_l1_finalized helper only logs a warning on
timeout, which allows configure_l1_sequencer() to keep going and start L2 with
stale state. Update wait_for_l1_finalized to fail fast when the finalized block
never reaches min_block by raising an error instead of returning after the
timeout, and make sure the caller in configure_l1_sequencer() does not proceed
past this check unless finality is confirmed.
In `@ops/docker/docker-compose-devnet.yml`:
- Around line 11-14: The devnet service port mappings are exposing internal RPC
and node interfaces on all host interfaces, and the single-port entry for the
auth RPC is also publishing a random host port. Update the affected port lists
in the docker-compose devnet services to bind only intended user-facing ports to
127.0.0.1, and move container-only ports such as auth RPC, debug/admin, metrics,
and node ports to expose instead of ports; use the service definitions in
docker-compose-devnet.yml as the place to tighten this across the listed blocks.
🪄 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: 8ace3f43-dea4-447d-9622-b33d103c1233
📒 Files selected for processing (56)
Makefileops/devnet-morph/devnet/__init__.pyops/devnet-morph/devnet/setup_nodes.pyops/docker-sequencer-test/Dockerfile.l2-geth-testops/docker-sequencer-test/Dockerfile.l2-node-testops/docker-sequencer-test/Dockerfile.tx-submitter-testops/docker-sequencer-test/README.mdops/docker-sequencer-test/check-whitelist-test.shops/docker-sequencer-test/docker-compose.ha-override.ymlops/docker-sequencer-test/docker-compose.override.ymlops/docker-sequencer-test/docker-compose.reorg-test.override.ymlops/docker-sequencer-test/entrypoint-l2.shops/docker-sequencer-test/run-ha-test.shops/docker-sequencer-test/run-perf-test.shops/docker-sequencer-test/run-test.shops/docker-sequencer-test/scripts/tx-generator.shops/docker/Dockerfile.indexerops/docker/Dockerfile.l1ops/docker/Dockerfile.l1-beaconops/docker/Dockerfile.l2-node-1ops/docker/Dockerfile.l2-node-4ops/docker/Dockerfile.token-price-oracleops/docker/Makefile.layer1ops/docker/consensus/config.ymlops/docker/docker-compose-4nodes.ymlops/docker/docker-compose-devnet.ymlops/docker/docker-compose-reth.ymlops/docker/entrypoint-l1.shops/docker/execution/genesis.jsonops/docker/execution/jwtsecretops/docker/execution/keystore/UTC--2024-02-05T07-24-26.460740423Z--ca062b0fd91172d89bcd4bb084ac4e21972cc467ops/docker/execution/passwordops/docker/go-rust-builder.Dockerfileops/docker/grafana/dashboards/dashboards.ymlops/docker/grafana/dashboards/json/morph-node.jsonops/docker/grafana/datasources/prometheus.ymlops/docker/layer1/docker-compose.ymlops/docker/layer1/scripts/clean.shops/docker/layer1/scripts/start.shops/docker/node0/eth_acc_key.jsonops/docker/node1/eth_acc_key.jsonops/docker/node1/priv_validator_key.jsonops/docker/node2/eth_acc_key.jsonops/docker/node2/node_key.jsonops/docker/node2/priv_validator_key.jsonops/docker/node3/eth_acc_key.jsonops/docker/node3/node_key.jsonops/docker/node3/priv_validator_key.jsonops/docker/node4/node_key.jsonops/docker/node5/node_key.jsonops/docker/nodekey2ops/docker/nodekey3ops/docker/prometheus/prometheus.ymlops/docker/static-nodes.jsonops/docker/tendermint-devnet-genesis.jsonops/docker/tendermint-setup.sh
💤 Files with no reviewable changes (47)
- ops/docker/execution/jwtsecret
- ops/docker/execution/password
- ops/docker-sequencer-test/Dockerfile.tx-submitter-test
- ops/docker-sequencer-test/docker-compose.reorg-test.override.yml
- ops/docker/nodekey2
- ops/docker/node3/node_key.json
- ops/docker/node3/priv_validator_key.json
- ops/docker/node4/node_key.json
- ops/docker/node2/node_key.json
- ops/docker/node5/node_key.json
- ops/docker/node2/priv_validator_key.json
- ops/docker/tendermint-setup.sh
- ops/docker/node3/eth_acc_key.json
- ops/docker/prometheus/prometheus.yml
- ops/docker/node1/eth_acc_key.json
- ops/docker-sequencer-test/entrypoint-l2.sh
- ops/docker-sequencer-test/README.md
- ops/docker/node0/eth_acc_key.json
- ops/docker/Dockerfile.l1
- ops/docker/consensus/config.yml
- ops/docker/node2/eth_acc_key.json
- ops/docker/grafana/datasources/prometheus.yml
- ops/docker/nodekey3
- ops/docker/grafana/dashboards/json/morph-node.json
- ops/docker-sequencer-test/docker-compose.override.yml
- ops/docker/layer1/docker-compose.yml
- ops/docker-sequencer-test/Dockerfile.l2-geth-test
- ops/docker/grafana/dashboards/dashboards.yml
- ops/docker/Dockerfile.token-price-oracle
- ops/docker/Dockerfile.l2-node-4
- ops/docker/execution/keystore/UTC--2024-02-05T07-24-26.460740423Z--ca062b0fd91172d89bcd4bb084ac4e21972cc467
- ops/docker-sequencer-test/docker-compose.ha-override.yml
- ops/docker-sequencer-test/Dockerfile.l2-node-test
- ops/docker-sequencer-test/check-whitelist-test.sh
- ops/docker/Dockerfile.indexer
- ops/docker/execution/genesis.json
- ops/docker/node1/priv_validator_key.json
- ops/docker/go-rust-builder.Dockerfile
- ops/docker/Dockerfile.l1-beacon
- ops/docker-sequencer-test/run-test.sh
- ops/docker/Dockerfile.l2-node-1
- ops/docker-sequencer-test/run-ha-test.sh
- ops/docker/docker-compose-reth.yml
- ops/docker-sequencer-test/scripts/tx-generator.sh
- ops/docker/docker-compose-4nodes.yml
- ops/docker/entrypoint-l1.sh
- ops/docker-sequencer-test/run-perf-test.sh
Add a layer1-derivation fullnode to the default devnet and wire an optional HA sequencer cluster through the existing devnet entrypoints. Cache Go dependency downloads in devnet Dockerfiles and restore project-label volume cleanup so future compose services are covered. Constraint: Preserve the deleted sequencer-test functionality through the main devnet path. Confidence: medium Scope-risk: moderate Not-tested: Full make devnet-up or HA runtime startup; Docker builds were not run.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/devnet-morph/devnet/setup_nodes.py`:
- Around line 83-86: The node setup in setup_nodes.py should fail fast when the
primary Tendermint key material is missing instead of continuing with mismatched
peer IDs. Update the logic around persistent_peers_value and the key-copy
fallback in the setup routine so that if the primary source directory or
validator key material for node0/node1/node2 is unavailable, the script raises
an error and stops rather than generating random keys or keeping hard-coded peer
IDs. Make the guard explicit in the code path that handles the source directory
fallback and the node0 validator key copy so the resulting configs always match
the persistent peer identities.
In `@ops/docker/docker-compose-cluster.yml`:
- Around line 9-14: The compose service definitions are publishing internal
RPC/auth/metrics/p2p ports on all host interfaces via the current ports
mappings. Update the affected service blocks in docker-compose-cluster.yml so
only intentional host access is bound to 127.0.0.1, and move internal-only
endpoints such as the auth/metrics/p2p ports from ports to expose. Apply the
same tightening to every repeated service section noted in the diff, using the
service port mappings in those blocks to keep the same internal connectivity
while preventing public exposure.
🪄 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: f7dffecd-ff71-4f29-bfd4-bdb7f90fdff0
📒 Files selected for processing (12)
Makefileops/devnet-morph/devnet/__init__.pyops/devnet-morph/devnet/setup_nodes.pyops/devnet-morph/tests/test_devnet_config.pyops/docker/Dockerfile.l2-gethops/docker/Dockerfile.l2-nodeops/docker/docker-compose-cluster.ymlops/docker/docker-compose-devnet.ymlops/docker/docker-compose-reth.ymlops/docker/node2/node_key.jsonops/docker/nodekey2ops/docker/static-nodes.json
✅ Files skipped from review due to trivial changes (3)
- ops/docker/nodekey2
- ops/docker/node2/node_key.json
- ops/docker/static-nodes.json
🚧 Files skipped from review as they are similar to previous changes (2)
- Makefile
- ops/devnet-morph/devnet/init.py
Comment on lines
83
to
+86
| persistent_peers_value = ( | ||
| "93e27ea2306e158a8146d5f44caaab97496797d2@node-0:26656," | ||
| "7f78b7d7a7e6bad4faf68d5731d437f4288d96d0@node-1:26656," | ||
| "06c699be2f9aeb9f7ec79f508a95ff80576deb12@node-2:26656," | ||
| "b1a131f40d5d3abefe0dd787513c936ef62ac2d6@node-3:26656," | ||
| "dae813274913aaf39e7cd3226a0aa8bce00644e1@sentry-node-0:26656" | ||
| "06c699be2f9aeb9f7ec79f508a95ff80576deb12@node-2:26656" |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast when primary Tendermint key material is missing.
persistent_peers_value hard-codes node-0/node-1/node-2 IDs, but Line 131 silently keeps generated random keys if a primary source directory is absent. That leaves every config pointing at peer IDs that do not exist; node0 can also lose its validator key via the Line 134 fallback path.
🛠️ Proposed guard
for node in node_dirs:
source_dir = os.path.join(docker_dir, node)
dest_dir = os.path.join(devnet_dir, node, "config")
+ primary_node = node in ("node0", "node1", "node2")
if not os.path.isdir(dest_dir):
print(f"Error: Missing destination directory for {node}. Exiting.")
sys.exit(1)
- if os.path.isdir(source_dir):
- shutil.copyfile(os.path.join(source_dir, "node_key.json"), os.path.join(dest_dir, "node_key.json"))
+ node_key = os.path.join(source_dir, "node_key.json")
+ if primary_node and not os.path.isfile(node_key):
+ print(f"Error: Missing static node key for {node} at {node_key}. Exiting.")
+ sys.exit(1)
+ if os.path.isfile(node_key):
+ shutil.copyfile(node_key, os.path.join(dest_dir, "node_key.json"))
- if node == "node0" and os.path.isdir(source_dir):
- shutil.copyfile(os.path.join(source_dir, "priv_validator_key.json"), os.path.join(dest_dir, "priv_validator_key.json"))
+ validator_key = os.path.join(source_dir, "priv_validator_key.json")
+ if node == "node0":
+ if not os.path.isfile(validator_key):
+ print(f"Error: Missing validator key for node0 at {validator_key}. Exiting.")
+ sys.exit(1)
+ shutil.copyfile(validator_key, os.path.join(dest_dir, "priv_validator_key.json"))Also applies to: 131-135
🤖 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/setup_nodes.py` around lines 83 - 86, The node setup
in setup_nodes.py should fail fast when the primary Tendermint key material is
missing instead of continuing with mismatched peer IDs. Update the logic around
persistent_peers_value and the key-copy fallback in the setup routine so that if
the primary source directory or validator key material for node0/node1/node2 is
unavailable, the script raises an error and stops rather than generating random
keys or keeping hard-coded peer IDs. Make the guard explicit in the code path
that handles the source directory fallback and the node0 validator key copy so
the resulting configs always match the persistent peer identities.
Comment on lines
9
to
+14
| ports: | ||
| - "9145:8545" | ||
| - "9146:8546" | ||
| - "8551" | ||
| - "6060" | ||
| - "30303" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep HA RPC, metrics, and consensus ports off public host interfaces.
These ports entries expose internal RPC/auth/metrics/p2p endpoints on all host interfaces; single-port entries also publish random host ports. Bind intentional host access to 127.0.0.1 and move internal-only ports to expose.
🛡️ Example tightening pattern
ports:
- - "9145:8545"
- - "9146:8546"
- - "8551"
- - "6060"
- - "30303"
+ - "127.0.0.1:9145:8545"
+ - "127.0.0.1:9146:8546"
+ expose:
+ - "8551"
+ - "6060"
+ - "30303" ports:
- - "26656"
- - "27657:26657"
- - "26658"
- - "26660"
- - "9501:9401"
+ - "127.0.0.1:27657:26657"
+ - "127.0.0.1:9501:9401"
+ expose:
+ - "26656"
+ - "26658"
+ - "26660"Also applies to: 34-39, 59-64, 86-91, 126-131, 166-171
🤖 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/docker-compose-cluster.yml` around lines 9 - 14, The compose
service definitions are publishing internal RPC/auth/metrics/p2p ports on all
host interfaces via the current ports mappings. Update the affected service
blocks in docker-compose-cluster.yml so only intentional host access is bound to
127.0.0.1, and move internal-only endpoints such as the auth/metrics/p2p ports
from ports to expose. Apply the same tightening to every repeated service
section noted in the diff, using the service port mappings in those blocks to
keep the same internal connectivity while preventing public exposure.
Make devnet-clean-build clear the full devnet compose set so callers do not need to remember the cluster or reth flags, and remove the redundant reth-specific clean-build target. Constraint: Avoid committing local generated env/deploy-config changes. Confidence: high Scope-risk: narrow Not-tested: Runtime docker cleanup against live containers was not run.
Drop the reth-specific devnet-clean wrapper now that devnet-clean-build always tears down the full devnet compose set, including reth and cluster overrides. Constraint: Keep cleanup entrypoints minimal after unifying compose cleanup. Confidence: high Scope-risk: narrow Not-tested: Runtime docker cleanup against live containers was not run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
make devnet-upandmake devnet-up-debugcccalways start centralized single-sequencer mode; the PBFT fallback flag was removed.node-0is the sequencer,node-1is a non-sequencer fullnode.docker-compose-4nodes.ymltodocker-compose-devnet.yml.L1SequencerviasetFirstSequencerbefore L2 startup.setFirstSequencer, defaulting toDEPLOYER_PRIVATE_KEYor the Hardhat dev key, instead of the block signer key.node-0, and inject the sequencer upgrade time into both nodes.make devnet-up-rethsupport for the new 2-node topology, so bothmorph-el-0andmorph-el-1can run morph-reth..devnetnode config when an old 6-node layout is detected..devnet/el0and.devnet/el1bind mounts; named volumes are kept only for L1 EL/CL/VC data.tx-submitterand gas oracle environment variables from the compose file.ops/docker, including old Dockerfiles, old L1 compose/entrypoint files, old monitoring configs, and unused node2-node5 keys.ops/docker-sequencer-testharness, which still depended on the deleteddocker-compose-4nodes.yml/ old PBFT+sentry topology.Why
The devnet target previously carried the PBFT-era topology even when running a centralized sequencer flow. That starts unnecessary L2 nodes and EL clients locally, and makes the block-tag/finalized behavior harder to reason about. The default devnet now matches the main local testing need: one sequencer plus one syncing non-sequencer node.
The
tx-submitterservice is intentionally still present. It is the local component that seals/commits/finalizes batches, so removing it would make the safe/finalized/block-tag path less representative and could hide the exact class of finalized-tag issues this devnet is meant to test. This PR only removes redundant/default env entries from that service.L1Sequencer.initialize(owner)uses the Hardhat deployer as the owner. The previous devnet script attemptedsetFirstSequencerwithBLOCK_SIGNER_PRIVATE_KEY, whose address is not the owner, so fullmake devnet-up-rethfailed withOwnable: caller is not the owner. The script now derives and checks the deployer address againstowner()before sending the transaction.Validation
python3 -m py_compile ops/devnet-morph/devnet/__init__.py ops/devnet-morph/devnet/setup_nodes.pygit diff --checkgit diff --cached --checkdocker compose --env-file .env -f docker-compose-devnet.yml config --quietfromops/dockerdocker compose --env-file .env -f docker-compose-devnet.yml -f docker-compose-reth.yml config --quietfromops/dockermake -n devnet-upmake -n devnet-up-rethconfigure_l1_sequencer: verifiedsetFirstSequenceris sent with the deployer key, notBLOCK_SIGNER_PRIVATE_KEY.setup_devnet_nodes()generation check: verified onlynode0andnode1are generated, onlynode0has validator key/state,node1has no validator key, and genesis contains one validator.ops/dockerfiles: confirmed the removed Dockerfiles, old monitoring configs, old L1 compose files, old node2-node5 key files, andops/docker-sequencer-testare not referenced by the current devnet path.I did not run a full
make devnet-upin this branch because it would stop/replace local devnet containers and regenerate local deployment artifacts.Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests