docs(l2): add per-release upgrade test guide#6597
Conversation
…st.md. The procedure walks through running the previous release, draining the committer via the admin RPC, upgrading the contracts when they changed, and running the new release against the upgraded contracts. Only VERSION_FROM and VERSION_TO are placeholders; every other value is the default from crates/l2/Makefile.
🤖 Kimi Code ReviewReview of Security Issues:
Documentation Correctness:
Ethereum-Specific Accuracy:
Minor Improvements:
Overall Assessment: Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have everything I need for a thorough review. Review: PR #6597 —
|
Greptile SummaryThis PR adds
Confidence Score: 3/5Safe to merge once the two copy-paste blockers in Steps 3.2 and 3.3 are resolved; the rest of the guide is accurate and well-grounded in the actual codebase defaults. The "Build and deploy" compile step silently does nothing (the env var is build-time only), leaving the docs/developers/l2/upgrade-test.md — specifically the "Build and deploy" block in Step 3.2 and both
|
| Filename | Overview |
|---|---|
| docs/developers/l2/upgrade-test.md | New per-release upgrade test guide; well-structured overall but has two blockers: the "Build and deploy" compile step is a no-op against a pre-compiled binary, and two <SECURITY_COUNCIL_PK> placeholders in Step 3.3 break the copy-paste promise. |
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
docs/developers/l2/upgrade-test.md:276-291
**Misleading compile step — `COMPILE_CONTRACTS` is a build-time variable, not a runtime one**
`COMPILE_CONTRACTS` is read exclusively by `cmd/ethrex/build.rs` (the Cargo build script) at compile time; setting it when invoking an already-compiled binary has no effect. The command `COMPILE_CONTRACTS=true ./ethrex l2 deploy --help >/dev/null` silently does nothing useful: `--help` exits immediately and the env var is ignored by the running process.
As the next comment in the code block already notes, the bytecode is embedded in the binary at build time. To obtain standalone `.bytecode` files needed for `rex deploy`, a user must either (a) recompile the binary from the `$VERSION_TO` source tree with `COMPILE_CONTRACTS=true cargo build --release --features l2,l2-sql` and extract the files from `$OUT_DIR/contracts/solc_out/`, or (b) compile with `forge`/`solc` directly against the contract sources. Neither path is actually shown, leaving `rex deploy <NEW_IMPL_BYTECODE>` as an unactionable placeholder.
### Issue 2 of 4
docs/developers/l2/upgrade-test.md:309-313
**`<SECURITY_COUNCIL_PK>` left as a placeholder, breaking the copy-paste promise**
The introduction states "the commands below are ready to copy-paste; the only thing you need to substitute is the version tags." However, Step 3.3 uses `<SECURITY_COUNCIL_PK>` without resolving it to a concrete value. The Security Council key corresponds to the `--on-chain-proposer-owner` address (`0x4417092b70a3e5f10dc504d0947dd256b965fc62`), whose private key is the `BRIDGE_OWNER_PRIVATE_KEY` used everywhere else in this guide (`0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e`). Substituting the literal hex value is consistent with how every other key in this document is presented.
```suggestion
UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_OCP_IMPL_ADDR> 0x)
rex send "$ETHREX_TIMELOCK_ADDRESS" \
'emergencyExecute(address,uint256,bytes)' "$ETHREX_COMMITTER_ON_CHAIN_PROPOSER_ADDRESS" 0 "$UPGRADE_CALLDATA" \
--private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \
--rpc-url http://localhost:8545
```
### Issue 3 of 4
docs/developers/l2/upgrade-test.md:321-325
Same unresolved `<SECURITY_COUNCIL_PK>` placeholder in the Timelock-self-upgrade block. Should be the same concrete key as above.
```suggestion
UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_TIMELOCK_IMPL_ADDR> 0x)
rex send "$ETHREX_TIMELOCK_ADDRESS" \
'emergencyExecute(address,uint256,bytes)' "$ETHREX_TIMELOCK_ADDRESS" 0 "$UPGRADE_CALLDATA" \
--private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \
--rpc-url http://localhost:8545
```
### Issue 4 of 4
docs/developers/l2/upgrade-test.md:432-435
**Redundant `.env` copy in Step 5**
Step 4.1 already copies `cmd/.env` into the `$VERSION_TO` tree. Repeating the copy in Step 5 is a no-op and may confuse readers into thinking the file might have changed between steps.
```suggestion
cd "$WORK/ethrex-$VERSION_TO/crates/l2"
make test
```
Reviews (1): Last reviewed commit: "Remove the expected-duration sentence fr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ```bash | ||
| cd "$WORK/ethrex-$VERSION_TO" | ||
| # Compile the contracts into local bytecode files (writes to target/.../solc_out/). | ||
| COMPILE_CONTRACTS=true ./ethrex l2 deploy --help >/dev/null # touches the build script | ||
| # The compiled bytecode is embedded in the binary; to get standalone .bytecode files use forge/solc on | ||
| # the contract sources under crates/l2/contracts/src/l1/ — see the build script in cmd/ethrex/build_l2.rs. | ||
| ``` | ||
|
|
||
| For each upgradable contract listed in the migration guide: | ||
|
|
||
| ```bash | ||
| # 1. Deploy the new implementation. Use the deployer's private key; this is just a code deployment. | ||
| rex deploy <NEW_IMPL_BYTECODE> 0 0x385c546456b6a603a1cfcaa9ec9494ba4832da08dd6bcf4de9a71e4a01b74924 \ | ||
| --rpc-url http://localhost:8545 | ||
| # rex prints the new implementation address — note it. | ||
| ``` |
There was a problem hiding this comment.
COMPILE_CONTRACTS is a build-time variable, not a runtime one
COMPILE_CONTRACTS is read exclusively by cmd/ethrex/build.rs (the Cargo build script) at compile time; setting it when invoking an already-compiled binary has no effect. The command COMPILE_CONTRACTS=true ./ethrex l2 deploy --help >/dev/null silently does nothing useful: --help exits immediately and the env var is ignored by the running process.
As the next comment in the code block already notes, the bytecode is embedded in the binary at build time. To obtain standalone .bytecode files needed for rex deploy, a user must either (a) recompile the binary from the $VERSION_TO source tree with COMPILE_CONTRACTS=true cargo build --release --features l2,l2-sql and extract the files from $OUT_DIR/contracts/solc_out/, or (b) compile with forge/solc directly against the contract sources. Neither path is actually shown, leaving rex deploy <NEW_IMPL_BYTECODE> as an unactionable placeholder.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/l2/upgrade-test.md
Line: 276-291
Comment:
**Misleading compile step — `COMPILE_CONTRACTS` is a build-time variable, not a runtime one**
`COMPILE_CONTRACTS` is read exclusively by `cmd/ethrex/build.rs` (the Cargo build script) at compile time; setting it when invoking an already-compiled binary has no effect. The command `COMPILE_CONTRACTS=true ./ethrex l2 deploy --help >/dev/null` silently does nothing useful: `--help` exits immediately and the env var is ignored by the running process.
As the next comment in the code block already notes, the bytecode is embedded in the binary at build time. To obtain standalone `.bytecode` files needed for `rex deploy`, a user must either (a) recompile the binary from the `$VERSION_TO` source tree with `COMPILE_CONTRACTS=true cargo build --release --features l2,l2-sql` and extract the files from `$OUT_DIR/contracts/solc_out/`, or (b) compile with `forge`/`solc` directly against the contract sources. Neither path is actually shown, leaving `rex deploy <NEW_IMPL_BYTECODE>` as an unactionable placeholder.
How can I resolve this? If you propose a fix, please make it concise.| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_OCP_IMPL_ADDR> 0x) | ||
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | ||
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_COMMITTER_ON_CHAIN_PROPOSER_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | ||
| --private-key <SECURITY_COUNCIL_PK> \ | ||
| --rpc-url http://localhost:8545 |
There was a problem hiding this comment.
<SECURITY_COUNCIL_PK> left as a placeholder, breaking the copy-paste promise
The introduction states "the commands below are ready to copy-paste; the only thing you need to substitute is the version tags." However, Step 3.3 uses <SECURITY_COUNCIL_PK> without resolving it to a concrete value. The Security Council key corresponds to the --on-chain-proposer-owner address (0x4417092b70a3e5f10dc504d0947dd256b965fc62), whose private key is the BRIDGE_OWNER_PRIVATE_KEY used everywhere else in this guide (0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e). Substituting the literal hex value is consistent with how every other key in this document is presented.
| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_OCP_IMPL_ADDR> 0x) | |
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | |
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_COMMITTER_ON_CHAIN_PROPOSER_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | |
| --private-key <SECURITY_COUNCIL_PK> \ | |
| --rpc-url http://localhost:8545 | |
| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_OCP_IMPL_ADDR> 0x) | |
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | |
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_COMMITTER_ON_CHAIN_PROPOSER_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | |
| --private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \ | |
| --rpc-url http://localhost:8545 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/l2/upgrade-test.md
Line: 309-313
Comment:
**`<SECURITY_COUNCIL_PK>` left as a placeholder, breaking the copy-paste promise**
The introduction states "the commands below are ready to copy-paste; the only thing you need to substitute is the version tags." However, Step 3.3 uses `<SECURITY_COUNCIL_PK>` without resolving it to a concrete value. The Security Council key corresponds to the `--on-chain-proposer-owner` address (`0x4417092b70a3e5f10dc504d0947dd256b965fc62`), whose private key is the `BRIDGE_OWNER_PRIVATE_KEY` used everywhere else in this guide (`0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e`). Substituting the literal hex value is consistent with how every other key in this document is presented.
```suggestion
UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_OCP_IMPL_ADDR> 0x)
rex send "$ETHREX_TIMELOCK_ADDRESS" \
'emergencyExecute(address,uint256,bytes)' "$ETHREX_COMMITTER_ON_CHAIN_PROPOSER_ADDRESS" 0 "$UPGRADE_CALLDATA" \
--private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \
--rpc-url http://localhost:8545
```
How can I resolve this? If you propose a fix, please make it concise.| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_TIMELOCK_IMPL_ADDR> 0x) | ||
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | ||
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_TIMELOCK_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | ||
| --private-key <SECURITY_COUNCIL_PK> \ | ||
| --rpc-url http://localhost:8545 |
There was a problem hiding this comment.
<SECURITY_COUNCIL_PK> placeholder in the Timelock-self-upgrade block. Should be the same concrete key as above.
| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_TIMELOCK_IMPL_ADDR> 0x) | |
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | |
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_TIMELOCK_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | |
| --private-key <SECURITY_COUNCIL_PK> \ | |
| --rpc-url http://localhost:8545 | |
| UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_TIMELOCK_IMPL_ADDR> 0x) | |
| rex send "$ETHREX_TIMELOCK_ADDRESS" \ | |
| 'emergencyExecute(address,uint256,bytes)' "$ETHREX_TIMELOCK_ADDRESS" 0 "$UPGRADE_CALLDATA" \ | |
| --private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \ | |
| --rpc-url http://localhost:8545 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/l2/upgrade-test.md
Line: 321-325
Comment:
Same unresolved `<SECURITY_COUNCIL_PK>` placeholder in the Timelock-self-upgrade block. Should be the same concrete key as above.
```suggestion
UPGRADE_CALLDATA=$(rex encode 'upgradeToAndCall(address,bytes)' <NEW_TIMELOCK_IMPL_ADDR> 0x)
rex send "$ETHREX_TIMELOCK_ADDRESS" \
'emergencyExecute(address,uint256,bytes)' "$ETHREX_TIMELOCK_ADDRESS" 0 "$UPGRADE_CALLDATA" \
--private-key 0x941e103320615d394a55708be13e45994c7d93b932b064dbcb2b511fe3254e2e \
--rpc-url http://localhost:8545
```
How can I resolve this? If you propose a fix, please make it concise.| cd "$WORK/ethrex-$VERSION_TO" | ||
| cp "$WORK/ethrex-$VERSION_FROM/cmd/.env" cmd/.env # contract addresses from the v$VERSION_FROM deploy | ||
| cd crates/l2 | ||
| make test |
There was a problem hiding this comment.
.env copy in Step 5
Step 4.1 already copies cmd/.env into the $VERSION_TO tree. Repeating the copy in Step 5 is a no-op and may confuse readers into thinking the file might have changed between steps.
| cd "$WORK/ethrex-$VERSION_TO" | |
| cp "$WORK/ethrex-$VERSION_FROM/cmd/.env" cmd/.env # contract addresses from the v$VERSION_FROM deploy | |
| cd crates/l2 | |
| make test | |
| cd "$WORK/ethrex-$VERSION_TO/crates/l2" | |
| make test |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/l2/upgrade-test.md
Line: 432-435
Comment:
**Redundant `.env` copy in Step 5**
Step 4.1 already copies `cmd/.env` into the `$VERSION_TO` tree. Repeating the copy in Step 5 is a no-op and may confuse readers into thinking the file might have changed between steps.
```suggestion
cd "$WORK/ethrex-$VERSION_TO/crates/l2"
make test
```
How can I resolve this? If you propose a fix, please make it concise.
🤖 Codex Code Review
Aside from those documentation blockers, the PR is docs-only and I did not see runtime/EVM logic changes to review. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
There was a problem hiding this comment.
Pull request overview
Adds a new developer-facing L2 “upgrade acceptance test” runbook to standardize the per-release procedure for stopping a running stack, upgrading L1/L2 contracts, and validating the upgraded system via the existing integration test suite.
Changes:
- Introduces a step-by-step upgrade test guide covering workspace setup, safe shutdown, contract upgrade paths, restart, and validation.
- Documents how to pin the
$VERSION_FROML2 genesis to avoid chain divergence during upgrades. - Recommends using the standard L2 integration tests (
make test) as the final acceptance check.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Compile the contracts into local bytecode files (writes to target/.../solc_out/). | ||
| COMPILE_CONTRACTS=true ./ethrex l2 deploy --help >/dev/null # touches the build script | ||
| # The compiled bytecode is embedded in the binary; to get standalone .bytecode files use forge/solc on | ||
| # the contract sources under crates/l2/contracts/src/l1/ — see the build script in cmd/ethrex/build_l2.rs. |
| pkill -INT -f 'ethrex l2 --watcher' || true # sequencer | ||
| pkill -INT -f 'ethrex l2 prover' || true # prover | ||
| sleep 2 | ||
| sudo ss -tlnp | grep -E ':(1729|3900|5555)\b' || echo "L2 ports clean" |
| make test | ||
| ``` | ||
|
|
||
| `make test` calls `cargo test -p ethrex-test l2_integration_test --features l2 -- --nocapture`. It assumes the dev node and prover are already up on the default ports (`L2 RPC 1729`, `proof coordinator 3900`), which is exactly the state Step 4 leaves you in. |
| # Upgrade test | ||
|
|
||
| This is a per-release acceptance test. The goal is to verify that: | ||
|
|
||
| 1. A node running the **previous release** (`<VERSION_FROM>`) can be cleanly stopped. | ||
| 2. The L1 contracts can be upgraded following the per-release migration guide. | ||
| 3. The **new release** (`<VERSION_TO>`) sequencer and prover resume operation against the upgraded contracts without re-deploying from scratch. | ||
|
|
||
| The commands below are ready to copy-paste. The only thing you need to substitute is the version tags. Every other value (private keys, addresses, ports, fee parameters) is taken from the defaults in `crates/l2/Makefile`; feel free to override them, but the values below give you a working baseline. |
Motivation
No documented procedure for the per-release upgrade acceptance test.
Description
Adds
docs/developers/l2/upgrade-test.md. Only<VERSION_FROM>/<VERSION_TO>are placeholders; the rest of the commands use Makefile defaults.Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) — N/A, docs only.