WIP: opcmv2 upgrade op chain#752
Conversation
0xOneTony
left a comment
There was a problem hiding this comment.
solidity side and tests looks good! one minor question, not needed to be applied
| require(address(_value.systemConfig) != address(0), "UpgradeOPCMInput: cannot set zero address"); | ||
| require(_value.disputeGameConfigs.length > 0, "UpgradeOPCMInput: cannot set empty dispute game configs array"); | ||
|
|
||
| if (_sel == this.upgradeInput.selector) _upgradeInput = abi.encode(_value); |
There was a problem hiding this comment.
Is it worth saying UpgradeOPCMV2Input instead?
There was a problem hiding this comment.
The thing is that it can also be used to store input for OPCM v1.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| "enabled": true, | ||
| "initBond": "0x0", | ||
| "gameType": 0, | ||
| "gameArgs": "0x" |
There was a problem hiding this comment.
Bug: Testdata config uses incompatible hex format for JSON fields
The config.json file uses hex string format ("0x0" for initBond and "0x" for gameArgs) which is incompatible with Go's standard JSON unmarshaling. The *big.Int type expects a JSON number or decimal string (not hex), and []byte expects base64-encoded data or null. If this config is loaded via json.Unmarshal, parsing will fail or produce incorrect zero values because the hex strings won't be properly converted.
| } | ||
|
|
||
| func UpgradeV2(host *script.Host, input UpgradeOPChainV2Input) error { | ||
| return opcm.RunScriptVoid(host, input, "UpgradeOPChainV2.s.sol", "UpgradeOPChain") |
There was a problem hiding this comment.
Bug: Referenced V2 script file does not exist
The UpgradeV2 function references "UpgradeOPChainV2.s.sol", UpgradeSuperchainV2 references "UpgradeSuperchainV2.s.sol", and DeployOPChainV2 references "DeployOPChainV2.s.sol", but these Solidity script files do not exist in the packages/contracts-bedrock directory. Calling these functions will fail at runtime when the script runner cannot locate the files.
Additional Locations (2)
| } | ||
| if deployer.IsDevFeatureEnabled(tt.devFeature, deployer.OpcmV2DevFlag) { | ||
| cfg.DevFeatureBitmap = deployer.OpcmV2DevFlag | ||
| } |
There was a problem hiding this comment.
Bug: DevFeatureBitmap overwrite loses combined feature flags in test
Line 210 sets cfg.DevFeatureBitmap to tt.devFeature, but lines 225-227 unconditionally overwrite it with just deployer.OpcmV2DevFlag when that flag is detected. This is currently harmless because the opcm-v2 test case only has OpcmV2DevFlag. However, if a combined test case is added (e.g., combining OpcmV2DevFlag with CannonKonaDevFlag), the overwrite would discard the additional flags, causing the test to use incorrect feature configuration. The conditional block appears redundant and potentially harmful.
* WIP: opcmv2 upgrade op chain (#752) * feat: upgrade op chain wip * feat: add deployv2 wip * fix: deployerv2 wip * refactor: remove additional V2 version of scripts * refactor: adds OPCM v2 support for UpgradeOPChain.s.sol * refactor: adds OPCM v2 support for UpgradeSuperchainConfig.s.sol * fix: uses correct json key for upgradeInput * fix: uses correct feature flag for OPCM v2 in devfeatures.go * chore: add json tag to ExtraInstruction --------- Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com> * feat: adds intermediary struct for op-deployer upgrade (#759) * feat: adds intermediary struct for op-deployer upgrade * chore: moved upgrade scripts to v6_0_0 * chore: remove v2 go scripts * refactor: make v2_0_0 upgrade path to support both OPCM v1 and v2 * feat: add support for both OPCM v1 and v2 on embedded upgrade * refactor: make v6_0_0 fall back to v2_0_0 * refactor: add extra instructions for upgrade superchain input * test: fix op-deployer tests * chore: remove deploy OP chain test step * test: use systemconfig proxy address * fix: remove the unneeded proxyadmin in OPChainConfig in op-deployer * refactor: revert v2_0_0 changes in favor of a new v6_0_0 that supports OPCMv2 * feat: add v6_0_0 upgrade * fix: check for 0 len OPChainConfigs * chore: pre-pr ready * Merge pull request #764 from defi-wonderland/fix/opcm2-upgradeopchain-comments fix: opcm2 upgradeopchain comments * refactor: remove enshrined v7_0_0 upgrade in op-deployer (#766) * test: use correct game type and pass correct extra instruction to UpgradeOPChain (#768) * test: add missing extra instructions for OPCM v2 input * fix: add correct game type for cannon kona * feat: initial add-game-type-v2 command * test: add tests for ShouldAllowV1 flag * refactor: remove shouldAllowV1 flag * chore: remove unused private key flag from the add game type v2 command * chore: remove skipping upgrade test * chore: name changes and comments * chore: update usage description add-game-type-v2 * test: update add-game-type-opcm-v2 tests --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com>
…-optimism#18593) * WIP: opcmv2 upgrade op chain (#752) * feat: upgrade op chain wip * feat: add deployv2 wip * fix: deployerv2 wip * refactor: remove additional V2 version of scripts * refactor: adds OPCM v2 support for UpgradeOPChain.s.sol * refactor: adds OPCM v2 support for UpgradeSuperchainConfig.s.sol * fix: uses correct json key for upgradeInput * fix: uses correct feature flag for OPCM v2 in devfeatures.go * chore: add json tag to ExtraInstruction --------- Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com> * feat: adds intermediary struct for op-deployer upgrade (#759) * feat: adds intermediary struct for op-deployer upgrade * chore: moved upgrade scripts to v6_0_0 * chore: remove v2 go scripts * refactor: make v2_0_0 upgrade path to support both OPCM v1 and v2 * feat: add support for both OPCM v1 and v2 on embedded upgrade * refactor: make v6_0_0 fall back to v2_0_0 * refactor: add extra instructions for upgrade superchain input * test: fix op-deployer tests * chore: remove deploy OP chain test step * test: use systemconfig proxy address * fix: remove the unneeded proxyadmin in OPChainConfig in op-deployer * refactor: revert v2_0_0 changes in favor of a new v6_0_0 that supports OPCMv2 * feat: add v6_0_0 upgrade * fix: check for 0 len OPChainConfigs * chore: pre-pr ready * Merge pull request #764 from defi-wonderland/fix/opcm2-upgradeopchain-comments fix: opcm2 upgradeopchain comments * refactor: remove enshrined v7_0_0 upgrade in op-deployer (#766) * test: use correct game type and pass correct extra instruction to UpgradeOPChain (#768) * test: add missing extra instructions for OPCM v2 input * fix: add correct game type for cannon kona * fix: opcm2 opd test and sc checks (#777) * chore: set test timeout to 30s (#778) --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: 0xOneTony <112496816+0xOneTony@users.noreply.github.com>
…timism#18660) * feat: initial add-game-type-v2 command (#770) * WIP: opcmv2 upgrade op chain (#752) * feat: upgrade op chain wip * feat: add deployv2 wip * fix: deployerv2 wip * refactor: remove additional V2 version of scripts * refactor: adds OPCM v2 support for UpgradeOPChain.s.sol * refactor: adds OPCM v2 support for UpgradeSuperchainConfig.s.sol * fix: uses correct json key for upgradeInput * fix: uses correct feature flag for OPCM v2 in devfeatures.go * chore: add json tag to ExtraInstruction --------- Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com> * feat: adds intermediary struct for op-deployer upgrade (#759) * feat: adds intermediary struct for op-deployer upgrade * chore: moved upgrade scripts to v6_0_0 * chore: remove v2 go scripts * refactor: make v2_0_0 upgrade path to support both OPCM v1 and v2 * feat: add support for both OPCM v1 and v2 on embedded upgrade * refactor: make v6_0_0 fall back to v2_0_0 * refactor: add extra instructions for upgrade superchain input * test: fix op-deployer tests * chore: remove deploy OP chain test step * test: use systemconfig proxy address * fix: remove the unneeded proxyadmin in OPChainConfig in op-deployer * refactor: revert v2_0_0 changes in favor of a new v6_0_0 that supports OPCMv2 * feat: add v6_0_0 upgrade * fix: check for 0 len OPChainConfigs * chore: pre-pr ready * Merge pull request #764 from defi-wonderland/fix/opcm2-upgradeopchain-comments fix: opcm2 upgradeopchain comments * refactor: remove enshrined v7_0_0 upgrade in op-deployer (#766) * test: use correct game type and pass correct extra instruction to UpgradeOPChain (#768) * test: add missing extra instructions for OPCM v2 input * fix: add correct game type for cannon kona * feat: initial add-game-type-v2 command * test: add tests for ShouldAllowV1 flag * refactor: remove shouldAllowV1 flag * chore: remove unused private key flag from the add game type v2 command * chore: remove skipping upgrade test * chore: name changes and comments * chore: update usage description add-game-type-v2 * test: update add-game-type-opcm-v2 tests --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> * chore: opcm2 addgametypev2 comments (#780) * chore: move ABI types to lib file * chore: move hardcoded values to constants file * refactor: make abi_types use MustType * test: deploys OPCM v2 for use during CLI testing (#784) * test: deploys OPCM v2 for use during CLI testing * chore: reference a github issue for TODO * test: add assertions on the output of add game type command * test: update cli tests (#792) * chore: Address review comments (#793) * refactor: remove re-declared flags from add-game-type v2 * chore: remove irrelevant TODO * refactor: rename "add-game-type-opcm-v2" command to "add-game-type-v2" * test: add cache dir check for add-game-type-v2 * test: removes cache dir files check on e2e test (#794) --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com>
Note
Introduce OPCM v2 feature flag and end-to-end support to upgrade superchain/chains and deploy new chains via OPCM v2, with new embedded runners and dual-version scripts/tests.
OpcmV2DevFlagand gate OPCM v2 paths viaIsDevFeatureEnabled.embedded.DeployOPChainV2withFullConfigV2andChainContractsencoding helpers.embedded.UpgradeSuperchainV2andUpgradeSuperchainV2Inputencoder.embedded.UpgradeOPChainV2,UpgraderV2, and encoding/types for dispute games and extra instructions.upgrade/v5_0_0) with ABI encoders, artifacts locator, tests, and testdata.opcm-v2: verify deployed code, run superchain upgrade v2, deploy a new chain via v2, and perform v2 chain upgrade; increase timeout to 180s.mustEncodeGameArgsfor dispute game args; minor CLI test TODO for v5.0.0.UpgradeOPChain.s.solandUpgradeSuperchainConfig.s.solto support both OPCM v1 and v2 via opaqueupgradeInput, version detection, and dual dummy callers; remove v1-only assumptions.UpgradeOPChain.t.sol,UpgradeSuperchainConfig.t.sol) with v1/v2 mocks and validation.Written by Cursor Bugbot for commit 61ffba6. This will update automatically on new commits. Configure here.