Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standard L2 Genesis: Isthmus Tracking #12302

Open
9 of 16 tasks
tynes opened this issue Oct 3, 2024 · 5 comments
Open
9 of 16 tasks

Standard L2 Genesis: Isthmus Tracking #12302

tynes opened this issue Oct 3, 2024 · 5 comments

Comments

@tynes
Copy link
Contributor

tynes commented Oct 3, 2024

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: ethereum-optimism/design-docs#97 as well as the specs

Prototype implementation

Tickets

@tynes
Copy link
Contributor Author

tynes commented Oct 15, 2024

Update as of Oct 16th, 2024

The code is nearly complete in #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
Copy link
Contributor

mds1 commented Oct 16, 2024

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
Copy link
Contributor

maurelian commented Oct 21, 2024

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
Copy link
Contributor

maurelian commented Oct 30, 2024

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
Copy link
Contributor Author

tynes commented Oct 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants