fix: generate L2 genesis allocs before op-node genesis l2#389
Conversation
The generateL2Genesis function in start-deploy.sh was calling
'op-node genesis l2' without the required --l2-allocs flag,
causing genesis generation to fail.
This fix adds a forge script step that runs L2Genesis.s.sol to
generate the L2 genesis state dump (state-dump-{l2ChainID}.json),
then passes the resulting file via --l2-allocs to op-node genesis l2.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue in the L2 genesis generation process, which previously failed due to a missing required flag. By introducing an explicit step to generate the necessary L2 genesis allocations using a Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug in L2 genesis generation by adding the required --l2-allocs flag. The changes introduce a new step to generate the necessary allocation file using a forge script before calling op-node genesis l2. The implementation looks solid. I've added a few suggestions to improve the robustness and maintainability of the shell script.
|
|
||
| # Generate L2 genesis allocs using forge script | ||
| echo "Generating L2 genesis allocs via forge script..." | ||
| cd $projectRoot/packages/tokamak/contracts-bedrock |
There was a problem hiding this comment.
It's good practice to add error handling for cd commands, especially when the script is not run with set -e. If this directory change fails, subsequent commands would execute in an incorrect directory, potentially leading to confusing errors.
| cd $projectRoot/packages/tokamak/contracts-bedrock | |
| cd "$projectRoot/packages/tokamak/contracts-bedrock" || { echo "❌ Error: could not change directory to contracts-bedrock"; return 1; } |
| forge script scripts/L2Genesis.s.sol:L2Genesis \ | ||
| --rpc-url $L1_RPC_URL; then | ||
| echo "❌ Error: Failed to generate L2 genesis allocs" | ||
| cd $currentPWD |
There was a problem hiding this comment.
This function has multiple exit points where you restore the original working directory with cd $currentPWD. To reduce repetition and ensure the directory is always restored correctly, you could use a trap at the beginning of the function:
function generateL2Genesis() {
trap 'cd "$currentPWD"' RETURN
# ... function body ...
}This will execute cd "$currentPWD" whenever the function returns, regardless of the exit point. You could then remove all the individual cd $currentPWD calls before each return statement, simplifying the error handling logic and making the function more robust.
| fi | ||
|
|
||
| # Determine state dump path: uses l2ChainID from deploy config | ||
| l2ChainID=$(jq -r '.l2ChainID' $DEPLOY_CONFIG_PATH) |
There was a problem hiding this comment.
It's a good practice to validate the output of the jq command. If jq fails (e.g., it's not installed, the JSON is invalid, or the key is not found), $l2ChainID could be empty. This would lead to a confusing file-not-found error on line 413. Checking if $l2ChainID has a value before using it would make the script more robust.
| l2ChainID=$(jq -r '.l2ChainID' $DEPLOY_CONFIG_PATH) | |
| l2ChainID=$(jq -r '.l2ChainID' $DEPLOY_CONFIG_PATH) | |
| if [ -z "$l2ChainID" ]; then | |
| echo "❌ Error: Failed to get l2ChainID from $DEPLOY_CONFIG_PATH" | |
| cd $currentPWD | |
| return 1 | |
| fi |
1 participant
Summary
generateL2Genesisinstart-deploy.shwas callingop-node genesis l2without the required--l2-allocsflag, causing genesis generation to fail with a missing flag errorforge script scripts/L2Genesis.s.sol:L2Genesisstep that generates the L2 genesis state dump (state-dump-{l2ChainID}.json) using deployed L1 contract addresses--l2-allocstoop-node genesis l2Root Cause
The current version of
op-node genesis l2requires--l2-allocs(path to L2 genesis state dump), but thegenerateL2Genesisfunction did not generate this file or pass it to the command.Changes
forge script scripts/L2Genesis.s.sol:L2GenesiswithCONTRACT_ADDRESSES_PATHpointing to the deploy result JSON before callingop-node genesis l2l2ChainIDfrom the deploy config to locate the generated state dump atstate-dump-{l2ChainID}.json--l2-allocsflag to theop-node genesis l2invocationTest plan
start-deploy.sh generate -e .env -c deploy-config.jsonagainst Sepolia testnetbuild/genesis.jsonandbuild/rollup.jsonare generated successfully🤖 Generated with Claude Code