feat: update L2 genesis specs #597
Conversation
agusduha
requested review from
protolambda,
tynes,
ajsutton,
sebastianst and
mslipper
as code owners
|
|
||
| ```solidity | ||
| function setBaseFeeVaultConfig(address,uint256,WithdrawalNetwork) | ||
| function setFeeVaultConfig(ConfigType,address,uint256,WithdrawalNetwork) |
There was a problem hiding this comment.
This MUST revert if the config type doesn't correspond to a fee vault config type
There was a problem hiding this comment.
Good catch! We are reverting in the implementation but it should be here too
| | ----------------------------------- | -------------------------------------------------------------------------- | -------------------------------------------------- | | ||
| | `BASE_FEE_VAULT_CONFIG` | `bytes32(uint256(keccak256("opstack.basefeevaultconfig")) - 1)` | The Fee Vault Config for the `BaseFeeVault` | | ||
| | `L1_FEE_VAULT_CONFIG` | `bytes32(uint256(keccak256("opstack.l1feevaultconfig")) - 1)` | The Fee Vault Config for the `L1FeeVault` | | ||
| | `SEQUENCER_FEE_VAULT_CONFIG` | `bytes32(uint256(keccak256("opstack.sequencerfeevaultconfig")) - 1)` | The Fee Vault Config for the `SequencerFeeVault` | |
There was a problem hiding this comment.
When we land the operator fee in ethereum-optimism/optimism#12166, we will need to rebase on top of that and then add that to the specs, both here and in the mermaid diagram
| @@ -290,9 +321,9 @@ via a deposit transaction from the `DEPOSITOR_ACCOUNT`. | |||
|
|
|||
| ##### `setIsthmus` | |||
There was a problem hiding this comment.
This is fine to leave for now but for clarity, we will need to modify this, unclear which hardfork this will land in exactly right now
| | `GAS_LIMIT` | `uint8(2)` | `abi.encode(uint64 _gasLimit)` | Modifies the L2 gas limit | | ||
| | `UNSAFE_BLOCK_SIGNER` | `uint8(3)` | `abi.encode(address)` | Modifies the account that is authorized to progress the unsafe chain | | ||
| | `EIP_1559_PARAMS` | `uint8(4)` | `uint256(uint64(uint32(_denominator))) << 32 \| uint64(uint32(_elasticity))` | Modifies the EIP-1559 denominator and elasticity | | ||
| | `FEE_VAULT_ADMIN` | `uint8(5)` | `abi.encode(address)` | Modifies the fee vault admin | |
There was a problem hiding this comment.
This doesn't seem right, why is fee vault admin added here? These are generally values that go into L1 attributes tx and we dont want to put the fee vault admin in there. The ConfigUpdate events influence L2 consensus
|
|
||
| #### `upgrade` | ||
|
|
||
| The `upgrade` function MUST only be callable by the `UPGRADER` role as defined | ||
| in the [`SuperchainConfig`](#superchainconfig). | ||
|
|
||
| ```solidity | ||
| function upgrade(bytes memory _data) external | ||
| function upgrade(uint32 _gasLimit, bytes memory _data) external |
There was a problem hiding this comment.
How will the _gasLimit value be set? Will it be hardcoded in the SystemConfig?
There was a problem hiding this comment.
Also, is upgrade() the right name for this? If "to is Predeploys.L1Block", them I think something like updateConfig() might be more appropriate.
| - `version` is `uint256(0)` | ||
| - `opaqueData` is the tightly packed transaction data where `mint` is `0`, `value` is `0`, the `gasLimit` | ||
| is `200_000`, `isCreation` is `false` and the `data` is the data passed into `upgrade`. | ||
| is `200_000`, `isCreation` is `false` and the `data` is the data passed into `upgrade`. |
There was a problem hiding this comment.
the gasLimit presumably needs to be updated if it becomes an input.
I'm also curious what the current thinking is around the correct behaviour of useGas():
CleanShot 2025-02-19 at 15 56 14@2x
Note that I don't agree with the recommended fix, as it will enables anyone to DoS a system deposit.
There was a problem hiding this comment.
We believe that we shouldnt add the check in useGas since this can cause further DoS issues by front-running the chain-operator. To compensate for this we agree with adding a test to make sure we dont exceed the buffer gas limit like mentioned here ethereum-optimism/design-docs#97 (comment)
* fix: review comments * fix: linting
* feat: update with operator fee vault * fix: vault config definition * feat: add upgrader and fee vault admin info * fix: add more details * fix: add operator fee vault to config type * fix: remove info
Description
Update L2 genesis specs to match the implementation being developed