ethereum-optimism / ethereum-optimism/optimism

contracts-bedrock: harden OPCMMigrator and the MigrationValidator

Open
#22,771 0 comments 0 reactions 1 assignee Claimed by @0xiamflux View on GitHub
H-interop
Dominant language
Go
Stars
6.5k
Forks
4k
Avg merge
2d 15h
Merged PRs (30d)
145

Description

`migrate()` is the one-time entrypoint that moves a set of chains onto shared dispute contracts and a shared `ETHLockbox`. Goal is to improve and harden the migrate flow.

`OPContractsManagerMigrator` need to fix:

- Nothing prevents `migrate()` from being called twice. A second call deploys another set of shared proxies and re-migrates the portals onto them.
- `migrate()` does not check that `SuperchainConfig`, the portals or the SystemConfigs are on the OPCM's release; `upgrade()` does.
- `_makeSystemConfigInitArgs` exists twice with different signatures, in `OPContractsManagerV2` and in the migrator.
- `migrate()` grants `PERMIT_ALL_CONTRACTS_INSTRUCTION` to deploy three known proxies, while `deploy()` documents itself as the only place that grants it. Scope the permission to those three.
- Shared contracts are initialized directly with `chainSystemConfigs[0]`. Verify this is fully replaced by the lockbox binding from PR #22760.
- `startingAnchorRoot` is completely unvalidated, so zero | >= type(uint64).max | the `0x…dead` placeholder are all accepted.

`OPContractsManagerMigrationValidator`:

- The legacy-games-cleared checks are gated on a chain's derived `DisputeGameFactory` differing from the first chain's, which is never true after a successful migration. Take the old factory addresses as input and run the checks unconditionally.
- The clearing list checks six game types; the migrator zeroes seven. Add `ZK_DISPUTE_GAME` and derive both lists from one constant.
- The input has no anchor root, init bonds, respected game type or expected shared addresses, and nothing checks balances or stale pauses. Add these to the input and assert them: intended anchor root and respected game type, init bonds, shared contract addresses, old lockboxes and portals hold no ETH, no scoped pause active on a retired identifier.
- `_updateDelayedWETHLockbox` re-inits each chain's own DelayedWETH and `_migratePortal` re-inits each chain's old ASR, both to the shared lockbox, precisely so pre-existing games read the right pause state. But this cannot be asserted in the validator since there is no way to retrieve the legacy contracts.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.