From 62be15f293b109a7840d20f618cae1b8d447497a Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 30 Sep 2024 21:40:06 -0700 Subject: [PATCH 01/20] feat: standard l2 genesis Add design doc --- protocol/standard-l2-genesis.md | 128 ++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 protocol/standard-l2-genesis.md diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md new file mode 100644 index 00000000..7110d8b1 --- /dev/null +++ b/protocol/standard-l2-genesis.md @@ -0,0 +1,128 @@ +# Purpose + + + + + +# Summary + + + +The L2 predeploys are refactored in a way such that the network specific configuration +is all sourced from a single location where it is ultimately set from L1 deposit transactions. +Any initializable logic in the L2 predeploys is also removed, to make the deposit transaction +based upgrade scheme simple to reason about. + +This will accelerate the ability to ship secure software, as we will be able to get chains +on the same versions of the software and know which versions work very well together. + +# Problem Statement + Context + + + +There is currently no good way to do releases of L2 predeploys. Historically, chains +have launched with an arbitrary commit for their L2 genesis, making the block history integrity +checks that are part of the superchain registry very difficult. We tell chains that are +trying to launch to use governance approved L1 contracts, the same should apply for L2. + +Given all of the work for making releases and upgrades nice for the L1 contracts and the client software, +it is at a waste if we cannot also have good releases of the L2 predeploys. + +Right now, OP Mainnet is running contracts at various versions of the software. It is actually very difficult +to reproduce the exact OP Mainnet set of contracts being used. It would require cherry picking bytecode from many +different commits. We run no tests against this particular combination of contracts. We believe it is safe +given our development practices, but every time that we do want to do a release it results in a lot of time +being spent trying to make sure that we are upgrading to a compatible set of contracts. + +# Proposed Solution + + + +A WIP implementation can be found [here](https://github.com/ethereum-optimism/optimism/pull/12057). +The specs can be found [here](https://github.com/ethereum-optimism/specs/tree/17ef36cdc3bb9893b206a93464122d56730d30fb/specs/protocol/holocene). + +Similar to the L1 MCP project, we move all of the network specific configuration out of the individual contracts +themselves and instead place all of it in a single place. The contracts will make a `CALL` rather than using +`sload` to read the values. These values will be sourced from L1 via deposit transactions that come from +the `SystemConfig.initialize` call. We need to make sure that the max deposit gas limit is at least able to +fullfill these deposit transactions. + +The general flow is as follows: + +```mermaid +graph LR + subgraph L1 + SystemConfig -- "setConfig(uint8,bytes)" --> OptimismPortal + end + subgraph L2 + L1Block + BaseFeeVault -- "baseFeeVaultConfig()(address,uint256,uint8)" --> L1Block + SequencerFeeVault -- "sequencerFeeVaultConfig()(address,uint256,uint8)" --> L1Block + L1FeeVault -- "l1FeeVaultConfig()(address,uint256,uint8)" --> L1Block + L2CrossDomainMessenger -- "l1CrossDomainMessenger()(address)" --> L1Block + L2StandardBridge -- "l1StandardBridge()(address)" --> L1Block + L2ERC721Bridge -- "l1ERC721Bridge()(address)" --> L1Block + OptimismMintableERC721Factory -- "remoteChainId()(uint256)" --> L1Block + end + OptimismPortal -- "setConfig(uint8,bytes)" --> L1Block +``` + +This is taken from the [specs](https://github.com/ethereum-optimism/specs/blob/17ef36cdc3bb9893b206a93464122d56730d30fb/specs/protocol/holocene/predeploys.md) and misses the `L2ProxyAdmin`. The `L2ProxyAdmin` must also be deterministic +and is explored in the following issue: https://github.com/ethereum-optimism/specs/issues/388. There is general +consensus on using the `DEPOSITOR_ACCOUNT` as the owner. + +When we do a contract release, we commit to that bytecode as part of consensus. That is the bytecode used +with deposit transactions doing upgrades to the network. In the L2 genesis creation script, we could have +a library for each release of the predeploys. The genesis script would take the bytecode from the library if +configured for a specific hardfork at genesis, otherwise it would use the compiled source code. This gives +us a lot of flexibility and simplicity when it comes to being able to recreate an L2 genesis deterministically. + +The block history integrity check becomes as simple as observing a 32 byte state root matches in the genesis +block matches the expected value. + +## Resource Usage + + + +The additional deposit gas is the only additional resource usage and its covered in the risks section +at the bottom of this document. + +# Alternatives Considered + + + +There is a long history of alternatives here. + +Another option would be to embed these config values directly into the client software's config and have the client +software create these deposit txs rather than the smart contracts. This is less flexible but comes with the tradeoff +of additional required rollup config and doesn't solve the problem for existing chains. Existing chains would need a way +to source this config, it would likely need to be hardcoded in the binary and that isn't super scalable. + +# Risks & Uncertainties + + + +There is a concern that the sequencer can include transactions before these values are set on L2. +If we define the `SystemConfig.startBlock` as the [first block to start derivation in](https://github.com/ethereum-optimism/optimism/blob/d05fb505809717282d5cee7264a09d26002a4ddd/op-node/cmd/genesis/cmd.go#L174C30-L174C40), +which is set on `SystemConfig.initialize` and also the deposit transactions that set these values are +sent in the same block, then we should have the guarantee that no user transactions are included before +the deposit transactions. + +There is a concern around the max deposit gas limit being too small so that the `SystemConfig` cannot +deposit all of these values, we should have logic that reverts if the deposit gas limit is too small +in the `ResourceConfig` sanity check function. Since the `ResourceConfig` can be modified during +`initialize`, its not that big of a deal and chain operators will see that they need to increase that +value. \ No newline at end of file From 75d831d81f923e7f8ccd85ded817d90284475c98 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 7 Oct 2024 14:14:08 -0600 Subject: [PATCH 02/20] more information --- protocol/standard-l2-genesis.md | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 7110d8b1..915413a5 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -90,6 +90,59 @@ us a lot of flexibility and simplicity when it comes to being able to recreate a The block history integrity check becomes as simple as observing a 32 byte state root matches in the genesis block matches the expected value. +### Rationale Behind Certain Changes + +#### SuperchainConfig "Upgrader" Role + +This new role is to allow for operational flexibility going into the future. If its not desired to add this role, +we could make the `guardian` able to faciliate the upgrade transactions. In practice, we may end up setting both +the `guardian` and the `upgrader` to the same account for the Superchain. + +Open to removing this from the spec and just using the `guardian` if preferred, but the possible operational +flexibility seems useful. + +The `upgrader` role can call the `OptimismPortal.upgrade(bytes memory data, uint32 gasLimit)` function +and it emits a deposit tx from the `DEPOSITOR_ACCOUNT` that calls the `L2ProxyAdmin`. Sourcing the auth +from the `SuperchainConfig` allows for simple management of this very important role, given that it impacts +stage 1 status. This is meant to simplify operations, ie remove the concept of the aliased L1 `ProxyAdmin` owner +being set as the L2 `ProxyAdmin`. + +The `data` and `gasLimit` are allowed to be specified since we don't fully know what sorts of calls we may have to do. +We may only need to do simple `upgradeTo` calls, but we may also need to do `upgradeToAndCall`. To support the +[liquidity migration](https://github.com/ethereum-optimism/design-docs/blob/4b62eb12eceb8e4867ac101134730102c0f5a989/protocol/superchainerc20/liquidity-migration.md), we need to backport storage slots into the `OptimismMintableERC20Factory` +contract. We may need to introduce multicall support into the `L2ProxyAdmin` as part of this. + +#### L2ProxyAdmin + +A new contract exists called the `L2ProxyAdmin`, it simply inherits from the `ProxyAdmin` and overrides the +`owner()(address)` function to return `DEPOSITOR_ACCOUNT`. + +Ideally we can remove the need for legacy proxy types since they don't exist on L2 eventually, but +that is considered a bonus when we do get around to it. + +#### SystemConfig + +The `SystemConfig` + +#### Initializable Predeploys Removed + +All predeploys are no longer initializable. This allows for upgrades issued by deposit transactions to be very smooth. +This impacts the following contracts: + +- `CrossDomainMessenger` +- `StandardBridge` +- `ERC721Bridge` + +#### CrossDomainMessenger + +Since the `CrossDomainMessenger` is no longer `initializable` we need to slightly modify the semantics around +the `xDomainMsgSender`. There is actually no need to set the value in storage during `initialize`, we could modify +the semantics such that if its `address(0)` in storage, then return the default value, otherwise return the +actual sender value. This should be safe since there is no way to be a sender from `address(0)`. + +Given this insight and the fact that there is reentrency check on `relayMessage`, it should be safe to use transient +storage without a call depth context. There is an [open PR](https://github.com/ethereum-optimism/optimism/pull/12356) to migrate to solc `0.8.25`. + ## Resource Usage The additional deposit gas is the only additional resource usage and its covered in the risks section at the bottom of this document. +This approach expands the ABI of the `L1Block` contract, meaning that automatically generated solidity dispatcher +will binary search over the possible function selectors, consuming a bit more gas. This is something that we could +avoid by writing a custom dispatcher in a fallback function or by grinding for names of functions. + # Alternatives Considered A WIP implementation can be found [here](https://github.com/ethereum-optimism/optimism/pull/12057). The specs can be found [here](https://github.com/ethereum-optimism/specs/tree/17ef36cdc3bb9893b206a93464122d56730d30fb/specs/protocol/holocene). -Similar to the L1 MCP project, we move all of the network specific configuration out of the individual contracts -themselves and instead place all of it in a single place. The contracts will make a `CALL` rather than using -`sload` to read the values. These values will be sourced from L1 via deposit transactions that come from -the `SystemConfig.initialize` call. We need to make sure that the max deposit gas limit is at least able to -fullfill these deposit transactions. +Similar to the L1 MCP project, we move all of the network specific configuration out of the +individual contracts themselves and instead place all of it in a single place. Rather than using +`sload` to read the values, the contracts will make a `CALL` to the L1Block contract. These values +will be sourced from L1 via deposit transactions that come from the `SystemConfig.initialize` call. +We need to make sure that the max deposit gas limit is at least able to fullfill these deposit +transactions. The general flow is as follows: @@ -87,7 +88,7 @@ a library for each release of the predeploys. The genesis script would take the configured for a specific hardfork at genesis, otherwise it would use the compiled source code. This gives us a lot of flexibility and simplicity when it comes to being able to recreate an L2 genesis deterministically. -The block history integrity check becomes as simple as observing a 32 byte state root matches in the genesis +The block history integrity check becomes as simple as observing that a 32 byte state root in the genesis block matches the expected value. ### Rationale Behind Certain Changes @@ -182,4 +183,4 @@ There is a concern around the max deposit gas limit being too small so that the deposit all of these values, we should have logic that reverts if the deposit gas limit is too small in the `ResourceConfig` sanity check function. Since the `ResourceConfig` can be modified during `initialize`, its not that big of a deal and chain operators will see that they need to increase that -value. \ No newline at end of file +value. From 77ac4ec2395ec86ac6d401499b9cd1bc294c1c73 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 12:39:10 -0400 Subject: [PATCH 05/20] Improve SuperchainConfig section --- protocol/standard-l2-genesis.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index f4ad0444..2601d861 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -95,19 +95,17 @@ block matches the expected value. #### SuperchainConfig "Upgrader" Role -This new role is to allow for operational flexibility going into the future. If its not desired to add this role, -we could make the `guardian` able to faciliate the upgrade transactions. In practice, we may end up setting both -the `guardian` and the `upgrader` to the same account for the Superchain. - -Open to removing this from the spec and just using the `guardian` if preferred, but the possible operational -flexibility seems useful. - The `upgrader` role can call the `OptimismPortal.upgrade(bytes memory data, uint32 gasLimit)` function and it emits a deposit tx from the `DEPOSITOR_ACCOUNT` that calls the `L2ProxyAdmin`. Sourcing the auth from the `SuperchainConfig` allows for simple management of this very important role, given that it impacts -stage 1 status. This is meant to simplify operations, ie remove the concept of the aliased L1 `ProxyAdmin` owner +stage 1 status. This is meant to simplify operations by removing the aliased L1 `ProxyAdmin` owner being set as the L2 `ProxyAdmin`. +Since the the L1 and L2 `ProxyAdmin` contracts are intended to have the same owner, +an additional improvement (which may be excluded to limit scope creep), would be to remove the +`Ownable` dependency on the L1 `ProxyAdmin` contract, and instead have it read the +`SuperchainConfig.upgrader()` role to authorize upgrades. + The `data` and `gasLimit` are allowed to be specified since we don't fully know what sorts of calls we may have to do. We may only need to do simple `upgradeTo` calls, but we may also need to do `upgradeToAndCall`. To support the [liquidity migration](https://github.com/ethereum-optimism/design-docs/blob/4b62eb12eceb8e4867ac101134730102c0f5a989/protocol/superchainerc20/liquidity-migration.md), we need to backport storage slots into the `OptimismMintableERC20Factory` From d4173d0a945a47932605c41c38c94787878e2773 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 12:39:18 -0400 Subject: [PATCH 06/20] Add FeeAdmin section --- protocol/standard-l2-genesis.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 2601d861..97439b90 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -111,6 +111,23 @@ We may only need to do simple `upgradeTo` calls, but we may also need to do `upg [liquidity migration](https://github.com/ethereum-optimism/design-docs/blob/4b62eb12eceb8e4867ac101134730102c0f5a989/protocol/superchainerc20/liquidity-migration.md), we need to backport storage slots into the `OptimismMintableERC20Factory` contract. We may need to introduce multicall support into the `L2ProxyAdmin` as part of this. +#### FeeAdmin role + +From time to time it may be necessary to modify the of the FeeVault predeploy contracts, but the +entity which should be authorized to make these modification must be able to vary from chain to +chain. + +Therefore a new `feeAdmin` role will be added to the `SystemConfig` contract. This role can call a +new `SystemConfig.setFeeConfig()` function which forwards config updates to `OptimismPortal.setConfig()` +with the appropriate `ConfigType`. + +This role will be set in `SystemConfig.initialize()`, meaning that it can only be updated by an upgrade. + +In order to be abundantly clear about auth: + +1. The `FeeAdmin` can update the `FeeConfig`. +2. The Upgrade Controller (aka [L1 ProxyAdmin Owner](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/stage-1.md#configuration-of-safes)) Safe cand update the `FeeAdmin`. + #### L2ProxyAdmin A new contract exists called the `L2ProxyAdmin`, it simply inherits from the `ProxyAdmin` and overrides the From 61d62ee9691b51c03f3d770cc95d350e8ca7470d Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 12:39:28 -0400 Subject: [PATCH 07/20] Add SystemConfig section --- protocol/standard-l2-genesis.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 97439b90..ef76cdb0 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -138,7 +138,27 @@ that is considered a bonus when we do get around to it. #### SystemConfig -The `SystemConfig` +The `SystemConfig`'s `initialize()` function will: + +- Accepts a new `Roles` struct, which will be composed of the existing `owner` address, and the new + `feeAdmin` role. +- Makes multiple calls to the OptimismPortal's `setConfig()` function to set the config values. + +> [!IMPORTANT] +> We should consider if there is a risk associated with 'resetting' these values. Similar to the OptimismPortal's +> `DEFAULT_L2_SENDER` [reinit issue](https://github.com/ethereum-optimism/optimism/pull/8864). +> I do not believe so as they are only modifiable in the `initializer` and so cannot be +> changed in normal operation. However the current design will require that all +> `SystemConfig` upgrades do not unintentionally modify the existing values. + +The `SystemConfig` will also get the following new external methods which are only callabled by the +`feeAdmin`. + +```solidity +function setBaseFeeVaultConfig(address _recipient, uint256 _min, Types.WithdrawalNetwork _network) external; +function setL1FeeVaultConfig(address _recipient, uint256 _min, Types.WithdrawalNetwork _network) external; +function setSequencerFeeVaultConfig(address _recipient, uint256 _min, Types.WithdrawalNetwork _network) external; +``` #### Initializable Predeploys Removed From 7d873ac26e4dd26e641bd9696ff2e918281b8730 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 14:11:28 -0400 Subject: [PATCH 08/20] Clean up Resource Usage section --- protocol/standard-l2-genesis.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index ef76cdb0..b8227ec3 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -188,8 +188,8 @@ The additional deposit gas is the only additional resource usage and its covered at the bottom of this document. This approach expands the ABI of the `L1Block` contract, meaning that automatically generated solidity dispatcher -will binary search over the possible function selectors, consuming a bit more gas. This is something that we could -avoid by writing a custom dispatcher in a fallback function or by grinding for names of functions. +will binary search over the possible function selectors, consuming a bit more gas. + # Alternatives Considered From 662797436269819a5621710b789a263304de9cdf Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 14:11:42 -0400 Subject: [PATCH 09/20] Release implications --- protocol/standard-l2-genesis.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index b8227ec3..61f3c0ad 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -190,6 +190,30 @@ at the bottom of this document. This approach expands the ABI of the `L1Block` contract, meaning that automatically generated solidity dispatcher will binary search over the possible function selectors, consuming a bit more gas. +## Implications for the Predeploy Releases Process + +### L2Genesis Generation + +When a new predeploy release is created, the bytcode from each predeploy should be placed into a +an new autogenerated library which resembles the following: + +```solidity +library HolocenePredeploys { + bytes constant L1Block = hex"..."; + ... +} +``` + +The `L2Genesis.s.sol` solidity script, will have additional functionality so that it can +optionally generate the L2 state from the current commit as it currently does using +`vm.getDeployedCode()`, or retrieve the code from the specified library. + +### Upgrade Process + +Note that this design modifies how L2 upgrade auth is managed (moving that management into a single +storage slot on L1), but does not change how upgrades to predeploy contracts are performed, ie. it +can still be done either via a `TransactionDeposited()` event, or a [network upgrade automation +transaction](https://github.com/ethereum-optimism/specs/blob/9f7226be064be0c87f90cbc6be6b0a4b4f58656a/specs/protocol/derivation.md#network-upgrade-automation-transactions). # Alternatives Considered From d2eba96111088903a8f98d37830cff7a46b62ebc Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 14:12:07 -0400 Subject: [PATCH 10/20] ResourceConfig concerns --- protocol/standard-l2-genesis.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 61f3c0ad..57bdb7c7 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -232,14 +232,24 @@ to source this config, it would likely need to be hardcoded in the binary and th +## Sequenced transactions on a fresh chain + There is a concern that the sequencer can include transactions before these values are set on L2. If we define the `SystemConfig.startBlock` as the [first block to start derivation in](https://github.com/ethereum-optimism/optimism/blob/d05fb505809717282d5cee7264a09d26002a4ddd/op-node/cmd/genesis/cmd.go#L174C30-L174C40), which is set on `SystemConfig.initialize` and also the deposit transactions that set these values are sent in the same block, then we should have the guarantee that no user transactions are included before the deposit transactions. +## Max Resource Limit on Deposit Transactions + There is a concern around the max deposit gas limit being too small so that the `SystemConfig` cannot deposit all of these values, we should have logic that reverts if the deposit gas limit is too small -in the `ResourceConfig` sanity check function. Since the `ResourceConfig` can be modified during +in the `setResourceConfig()` function's [sanity checks](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/packages/contracts-bedrock/src/L1/SystemConfig.sol#L538-L556). Since the `ResourceConfig` can be modified during `initialize`, its not that big of a deal and chain operators will see that they need to increase that value. + +There is a related concern about the `useGas()` +[function](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L156) +is implemented, as (unlike +[`_metered()`](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L128C1-L132C10) +it does not check that the max resource limit is not exceeded by the additional `prevBoughtGas`, or perhaps it should check that the SystemTxMaxGas is not exceeded (relevant code in [ResourceMetering.sol](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L43-L46), and [config.go](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/op-chain-ops/genesis/config.go#L109-L113)). This requires investigation. From 5d3d73e9f8aef7bd729413ff40db8c316bed2747 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 14:13:12 -0400 Subject: [PATCH 11/20] formatting fix --- protocol/standard-l2-genesis.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 57bdb7c7..064b8255 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -251,5 +251,5 @@ value. There is a related concern about the `useGas()` [function](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L156) is implemented, as (unlike -[`_metered()`](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L128C1-L132C10) +[`_metered()`](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L128C1-L132C10)) it does not check that the max resource limit is not exceeded by the additional `prevBoughtGas`, or perhaps it should check that the SystemTxMaxGas is not exceeded (relevant code in [ResourceMetering.sol](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L43-L46), and [config.go](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/op-chain-ops/genesis/config.go#L109-L113)). This requires investigation. From 4980faff965e6caa1fc35ea015654c17c5931d10 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 18 Oct 2024 14:28:52 -0400 Subject: [PATCH 12/20] Some cleanup --- protocol/standard-l2-genesis.md | 34 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 064b8255..1096204f 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -113,17 +113,14 @@ contract. We may need to introduce multicall support into the `L2ProxyAdmin` as #### FeeAdmin role -From time to time it may be necessary to modify the of the FeeVault predeploy contracts, but the -entity which should be authorized to make these modification must be able to vary from chain to -chain. - -Therefore a new `feeAdmin` role will be added to the `SystemConfig` contract. This role can call a -new `SystemConfig.setFeeConfig()` function which forwards config updates to `OptimismPortal.setConfig()` -with the appropriate `ConfigType`. +The entity which authorized to modify the various `FeeVault` configs must be able to vary from chain +to chain. Therefore a new `feeAdmin` role will be added to the `SystemConfig` contract. This role +can call a new `SystemConfig.setFeeConfig()` function which forwards config updates to +`OptimismPortal.setConfig()` with the appropriate `ConfigType`. This role will be set in `SystemConfig.initialize()`, meaning that it can only be updated by an upgrade. -In order to be abundantly clear about auth: +In summary: 1. The `FeeAdmin` can update the `FeeConfig`. 2. The Upgrade Controller (aka [L1 ProxyAdmin Owner](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/stage-1.md#configuration-of-safes)) Safe cand update the `FeeAdmin`. @@ -138,9 +135,9 @@ that is considered a bonus when we do get around to it. #### SystemConfig -The `SystemConfig`'s `initialize()` function will: +The `SystemConfig`'s `initialize()` function will be updated to: -- Accepts a new `Roles` struct, which will be composed of the existing `owner` address, and the new +- Accept a new `Roles` struct, composed of the existing `owner` address, and the new `feeAdmin` role. - Makes multiple calls to the OptimismPortal's `setConfig()` function to set the config values. @@ -151,8 +148,8 @@ The `SystemConfig`'s `initialize()` function will: > changed in normal operation. However the current design will require that all > `SystemConfig` upgrades do not unintentionally modify the existing values. -The `SystemConfig` will also get the following new external methods which are only callabled by the -`feeAdmin`. +The `SystemConfig` will also get the following new external methods which are only callable by the +`feeAdmin`: ```solidity function setBaseFeeVaultConfig(address _recipient, uint256 _min, Types.WithdrawalNetwork _network) external; @@ -211,8 +208,9 @@ optionally generate the L2 state from the current commit as it currently does us ### Upgrade Process Note that this design modifies how L2 upgrade auth is managed (moving that management into a single -storage slot on L1), but does not change how upgrades to predeploy contracts are performed, ie. it -can still be done either via a `TransactionDeposited()` event, or a [network upgrade automation +storage slot on L1), but does not change how upgrades to predeploy contracts are performed. +Predeploy upgrades can still be done either via a `TransactionDeposited()` event, or a [network +upgrade automation transaction](https://github.com/ethereum-optimism/specs/blob/9f7226be064be0c87f90cbc6be6b0a4b4f58656a/specs/protocol/derivation.md#network-upgrade-automation-transactions). # Alternatives Considered @@ -248,8 +246,8 @@ in the `setResourceConfig()` function's [sanity checks](https://github.com/ether `initialize`, its not that big of a deal and chain operators will see that they need to increase that value. -There is a related concern about the `useGas()` +A related concern is that the `useGas()` [function](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L156) -is implemented, as (unlike -[`_metered()`](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L128C1-L132C10)) -it does not check that the max resource limit is not exceeded by the additional `prevBoughtGas`, or perhaps it should check that the SystemTxMaxGas is not exceeded (relevant code in [ResourceMetering.sol](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L43-L46), and [config.go](https://github.com/ethereum-optimism/optimism/blob/feat/holocene-contracts/op-chain-ops/genesis/config.go#L109-L113)). This requires investigation. +which is called in the `upgrade()` and `setConfig()` functions does not check that the max resource limit is not exceeded by +the additional `prevBoughtGas`. This is in contrast with +[`_metered()`](https://github.com/ethereum-optimism/optimism/blob/f99424ded3917ddc0c4ef14355d61e50a38d4d0d/packages/contracts-bedrock/src/L1/ResourceMetering.sol#L128C1-L132C10) which does check `prevBoughtGas`. This requires investigation. From dff9a8453199caeec89be19e91bae0206a5c71e2 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 21 Oct 2024 16:53:19 -0400 Subject: [PATCH 13/20] fix nits --- protocol/standard-l2-genesis.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 1096204f..3223cab7 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -113,7 +113,8 @@ contract. We may need to introduce multicall support into the `L2ProxyAdmin` as #### FeeAdmin role -The entity which authorized to modify the various `FeeVault` configs must be able to vary from chain +The entity which is authorized to modify the various `FeeVault` configs must be able to vary from chain + to chain. Therefore a new `feeAdmin` role will be added to the `SystemConfig` contract. This role can call a new `SystemConfig.setFeeConfig()` function which forwards config updates to `OptimismPortal.setConfig()` with the appropriate `ConfigType`. @@ -201,7 +202,7 @@ library HolocenePredeploys { } ``` -The `L2Genesis.s.sol` solidity script, will have additional functionality so that it can +The `L2Genesis.s.sol` solidity script will have additional functionality so that it can optionally generate the L2 state from the current commit as it currently does using `vm.getDeployedCode()`, or retrieve the code from the specified library. From 567e27e2415102de1ad19a03d5959385997d3f32 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 21 Oct 2024 16:58:26 -0400 Subject: [PATCH 14/20] add note about continuity of feeadmin role --- protocol/standard-l2-genesis.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 3223cab7..718b8da9 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -121,6 +121,11 @@ can call a new `SystemConfig.setFeeConfig()` function which forwards config upda This role will be set in `SystemConfig.initialize()`, meaning that it can only be updated by an upgrade. +> [!NOTE] +> We need to guarantee 100% backwards compatibility in the roles during the upgrade, so for example +> the same 2/2 multisig that owns the L2 ProxyAdmin on base should be the FeeAdmin in base's +> SystemConfig. + In summary: 1. The `FeeAdmin` can update the `FeeConfig`. From d08f1af4904203a57db1aa08684f3640301be94f Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 21 Oct 2024 17:10:00 -0400 Subject: [PATCH 15/20] Removed multicall mention. --- protocol/standard-l2-genesis.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 718b8da9..982c8cad 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -109,7 +109,7 @@ an additional improvement (which may be excluded to limit scope creep), would be The `data` and `gasLimit` are allowed to be specified since we don't fully know what sorts of calls we may have to do. We may only need to do simple `upgradeTo` calls, but we may also need to do `upgradeToAndCall`. To support the [liquidity migration](https://github.com/ethereum-optimism/design-docs/blob/4b62eb12eceb8e4867ac101134730102c0f5a989/protocol/superchainerc20/liquidity-migration.md), we need to backport storage slots into the `OptimismMintableERC20Factory` -contract. We may need to introduce multicall support into the `L2ProxyAdmin` as part of this. +contract. #### FeeAdmin role From 9b8cb76f9ac05a8863bcaecaef9881926499fd4f Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 22 Oct 2024 15:01:38 -0400 Subject: [PATCH 16/20] Add note about preserving auth model --- protocol/standard-l2-genesis.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 982c8cad..6506e67e 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -104,7 +104,9 @@ being set as the L2 `ProxyAdmin`. Since the the L1 and L2 `ProxyAdmin` contracts are intended to have the same owner, an additional improvement (which may be excluded to limit scope creep), would be to remove the `Ownable` dependency on the L1 `ProxyAdmin` contract, and instead have it read the -`SuperchainConfig.upgrader()` role to authorize upgrades. +`SuperchainConfig.upgrader()` role to authorize upgrades. Regardless, in order to +preserve the existing auth model we MUST ensure that the `upgrader` is the same account as the +current L1 ProxyAdmin owner. The `data` and `gasLimit` are allowed to be specified since we don't fully know what sorts of calls we may have to do. We may only need to do simple `upgradeTo` calls, but we may also need to do `upgradeToAndCall`. To support the From 7cc8562dedeb9dc5fc40790b7929210084715939 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 22 Oct 2024 15:01:52 -0400 Subject: [PATCH 17/20] Add release process todo --- protocol/standard-l2-genesis.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 6506e67e..4c7a8f1a 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -197,6 +197,8 @@ will binary search over the possible function selectors, consuming a bit more ga ## Implications for the Predeploy Releases Process +- TODO: get clarity about how to package up L2 contracts releases. How will alternative clients consume the L2 Genesis? + ### L2Genesis Generation When a new predeploy release is created, the bytcode from each predeploy should be placed into a From 0cf08e8f7b0d1ec508564d5506d238aca3d3f7f1 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 22 Oct 2024 15:06:10 -0400 Subject: [PATCH 18/20] Indicate continued pref for upgrade automation tx --- protocol/standard-l2-genesis.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/protocol/standard-l2-genesis.md b/protocol/standard-l2-genesis.md index 4c7a8f1a..0d829c87 100644 --- a/protocol/standard-l2-genesis.md +++ b/protocol/standard-l2-genesis.md @@ -223,6 +223,9 @@ Predeploy upgrades can still be done either via a `TransactionDeposited()` event upgrade automation transaction](https://github.com/ethereum-optimism/specs/blob/9f7226be064be0c87f90cbc6be6b0a4b4f58656a/specs/protocol/derivation.md#network-upgrade-automation-transactions). +Routine hardforks should continue to prefer using network upgrade automation transactions, with +deposit transaction upgrades being reserved for incident response or other extenuating circumstances. + # Alternatives Considered