-
Notifications
You must be signed in to change notification settings - Fork 69
(Draft)update config for qa #804
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR updates the Qanet configuration across multiple files. The l1ChainID is changed from 900 to 560048 (Hoodi Chain ID), sequencer and staking addresses are replaced with new values, and the L2 genesis script sources a different environment file while disabling state root and withdraw root extraction. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
contracts/src/deploy-config/qanetl1.ts(2 hunks)ops/l2-genesis/deploy-config/qanet-deploy-config.json(2 hunks)ops/l2-genesis/qanet-l2genesis.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ops/l2-genesis/qanet-l2genesis.sh
[warning] 4-4: In POSIX sh, 'source' in place of '.' is undefined.
(SC3046)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
ops/l2-genesis/deploy-config/qanet-deploy-config.json (1)
36-41: LGTM! Addresses are consistent with the TypeScript config.The updated staking addresses match the sequencer addresses in
qanetl1.ts, maintaining consistency across configurations.contracts/src/deploy-config/qanetl1.ts (2)
7-8: LGTM! Chain ID update is well-documented.The comment clarifies that 560048 is the Hoodi chain ID, and the change is consistent with the JSON deployment config.
41-46: LGTM! Sequencer addresses are consistent with the JSON config.The updated addresses match the staking addresses in
qanet-deploy-config.json, maintaining consistency across both configuration files.ops/l2-genesis/qanet-l2genesis.sh (2)
4-4: LGTM! Environment file change aligns with QA configuration.Switching to
.env.qais appropriate for the Qanet deployment and aligns with the PR's objective to update QA configurations.
31-32: The commented-out extractions are safe for the current configuration but represent incomplete or undocumented design.The
qanetl1.tsdeploy config intentionally omitsrollupGenesisStateRootandwithdrawRootfields (unlikesepolia.ts,testnetl1.ts, andholesky.ts), so the commented sed replacements would have no target anyway. The script currently only populatesbatchHeader, which aligns with the actual config structure.However, the commented code suggests these extractions were deliberately removed without cleanup or documentation. Clarify:
- Is the minimal
qanetl1.tsconfig intentional for local/dev environments?- Should these fields be added to maintain consistency with other configs?
- If intentional, remove the commented code to avoid future confusion.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "l1ChainID": 900, | |||
| "l1ChainID": 560048, | |||
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.
🧩 Analysis chain
Verify all dependent systems are updated for the chain ID change.
Changing the L1 chain ID from 900 to 560048 (Hoodi Chain) is a breaking change that affects L1 network connectivity, cross-chain operations, and any external integrations. Ensure all dependent systems, SDKs, and documentation are updated accordingly.
Run the following script to check for hardcoded references to the old chain ID:
🏁 Script executed:
#!/bin/bash
# Search for hardcoded references to the old chain ID (900)
rg -nP '\b900\b' -g '!*.json' -g '!.git/**' --type-not binary -C3Length of output: 30
I'll correct the script and conduct a more comprehensive search for references to the old chain ID 900.
🏁 Script executed:
#!/bin/bash
# Search for hardcoded references to chain ID 900 in source files (not json or git)
echo "=== Searching for hardcoded '900' references in source files ==="
rg -nP '\b900\b' -g '!*.json' -g '!.git/**' -C3
echo ""
echo "=== Searching for chain ID references in config/constant files ==="
rg -nP '900|chainId|chain.*id' --type=ts --type=js --type=py -C2
echo ""
echo "=== Checking for qanet or chain ID in source files ==="
rg -nP 'qanet|hoodi|560048' -C2Length of output: 306850
Action required — update all hardcoded L1 chain-ID references (900 → 560048).
Changing L1 from 900 to 560048 is a breaking, network-level change. Search found hardcoded 900 in deployment, runtime, test and contract code. Update these references (or make the value configurable), coordinate a rollout with node operators/validators, and run full integration tests.
High‑priority locations to update now:
- git/tx-submitter/types/blob_params.go (DevnetChainConfig / ChainConfigMap).
- git/ops/docker/layer1/docker-compose.yml (geth --networkid=900).
- git/ops/docker/docker-compose-4nodes.yml and git/node/ops-morph/docker-compose*.yml (MORPH_NODE_L1_CHAIN_ID=900, TX_SUBMITTER_CHAIN_ID=900).
- git/contracts/hardhat.config.ts (l1/qanetl1 chainId: 900).
- git/contracts/src/deploy-config/l1.ts, testnetl1.ts (l1ChainID: 900).
- git/ops/l2-genesis/morph-chain-ops/genesis/layer_two_test.go (L1ChainID: 900).
Recommended next steps:
- Run a repo-wide search for '\b900\b' and review each hit; update deployment scripts, env vars, genesis files, tests, and docs.
- Coordinate an ops/validator cutover plan (node configs, JWT/engine auth, re-initialize/geth network IDs if needed).
- Update CI/Hardhat tasks and re-run unit + integration tests before merging.
🤖 Prompt for AI Agents
In ops/l2-genesis/deploy-config/qanet-deploy-config.json around line 2 the L1
chain ID is hardcoded as 900 and must be changed to 560048; update the value to
560048 and ensure the same replacement (or make it configurable via an
env/central config) across all hardcoded occurrences listed in the review
(git/tx-submitter/types/blob_params.go DevnetChainConfig/ChainConfigMap,
docker-compose files with geth --networkid and
MORPH_NODE_L1_CHAIN_ID/TX_SUBMITTER_CHAIN_ID, contracts hardhat.config.ts
l1/qanetl1 chainId, contracts/src/deploy-config/*.ts l1ChainID, and
layer_two_test.go L1ChainID), run a repo-wide search for '\b900\b' to find and
update any additional hits, update CI/Hardhat tasks and deployment scripts
accordingly, and coordinate a rollout with node operators/validators and full
integration tests before merging.
| set -eu | ||
|
|
||
| source ../../contracts/.env | ||
| source ../../contracts/.env.qa |
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.
Fix shell portability issue.
The script uses #!/bin/sh but the source command is bash-specific, not POSIX-compliant. Either change the shebang to #!/bin/bash or use the POSIX-compliant . (dot) command.
Apply this diff to fix the portability issue:
-source ../../contracts/.env.qa
+. ../../contracts/.env.qaOr alternatively, update the shebang:
-#!/bin/sh
+#!/bin/bash📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| source ../../contracts/.env.qa | |
| . ../../contracts/.env.qa |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 4-4: In POSIX sh, 'source' in place of '.' is undefined.
(SC3046)
🤖 Prompt for AI Agents
In ops/l2-genesis/qanet-l2genesis.sh around line 4, the script uses the
bash-only `source` command while the shebang is `#!/bin/sh`; replace `source
../../contracts/.env.qa` with the POSIX-compliant `. ../../contracts/.env.qa` to
keep the `#!/bin/sh` shebang, or alternatively change the shebang to
`#!/bin/bash` if you intend to rely on bash features—apply one of these two
fixes so the script is portable and consistent.
Summary by CodeRabbit