ethereum-optimism / optimism

Optimism is Ethereum, scaled.
https://optimism.io
MIT License
5.66k stars 3.29k forks source link

Standard L2 Genesis: Isthmus Tracking #12302

Open tynes opened 1 month ago

tynes commented 1 month ago

Overview

The Standard L2 Genesis aims to solve contract versioning and upgrades for predeploys. The main idea is that the network specific chain configuration should be moved to a single location rather than being in contract storage of individual contracts and is pulled out of being embedded in the genesis state and instead is deposited into the L2 as the first transactions that the system processes. Details can be found in the design doc: https://github.com/ethereum-optimism/design-docs/pull/97 as well as the specs

Prototype implementation

Tickets

tynes commented 1 month ago

Update as of Oct 16th, 2024

The code is nearly complete in https://github.com/ethereum-optimism/optimism/pull/12057, there is one open decision around the role that can modify the fee vault config. To keep it 1:1 today, there needs to be a new role that is added to the SystemConfig that can modify the fee vault config. A standard chain would set this value to be the L1 ProxyAdmin owner. It is possible to source this value from the SuperchainConfig if we wanted to, then the SystemConfig would need to know about the SuperchainConfig which it does not today, I think its fine to just have this role directly in the SystemConfig. After we make this decision, we just need additional unit tests, get the various CI checks passing and then rebase on develop which should contain the holocene changes for the SystemConfig.

mds1 commented 1 month ago

A standard chain would set this value to be the L1 ProxyAdmin owner.

Hmm are you sure this is the right configuration for this new role? This means the security council (SC) would need to be involved with fee vault changes, but since fee vault config is peripheral to the core protocol safety and security, I don't think the SC should need to be involved here.

In either case, assuming fee vault config can vary per-chain, then defining the role in the SystemConfig feels like the right spot to me. If we want it to be the same role holder for all chains in the superchain, then SuperchainConfig is the right spot

cc @maurelian

maurelian commented 4 weeks ago

To close out the discussion of which role can set the FeeVault config, we agreed that it will be set in the SystemConfig, and modifiable via initialize() (ie. an upgrade).

maurelian commented 2 weeks ago

Status here:

I wanted to move the initializer slot the right way, using the unstructured storage layout of OZ's v5 Initializable contract. Therefore I invested some time with #12356 to rebase it and get it working. Unfortunately, what seems like a simple version bump is causing a few surprising failures in the e2e tests, which I've already spent too much time on.

Therefore at this point I'll need to return to focusing on just #12057. That work will require at a minimum:

  1. clean up to deleted some dead code/comments and adding natspec where required
  2. reviewing and cleaning up the current storage layout diff
  3. I also need to understand this comment better.
tynes commented 2 weeks ago

Regarding 3)

It could make sense to do this, we could also do batcher as address rather than bytes32 to improve the ux. Also could be good to rename owner to chainOperator or chainGovernor

There is a Roles struct passed into SystemConfig.initialize. I recommend renaming owner to chainGovernor and adding batcher as an address then cast it to bytes32 on chain before setting it as batcherHash. If we want to use a new version of batcher hash, (first byte is version byte) then we will need an upgrade, which is fine because we will need an upgrade in the client software anyways.