-
Notifications
You must be signed in to change notification settings - Fork 92
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
isthmus: operator fee #382
base: main
Are you sure you want to change the base?
Changes from 51 commits
f4ef24d
d2f40a0
7ddbce3
2e12898
1407fa9
50c22ec
78cb4cb
6a78e21
fb53c85
b56048a
7c90f2b
f4e27b3
1526415
7085d98
148230b
fbf8dfb
48a25af
98fdb84
ead7704
1ade65a
4673839
b945faf
2f73dc2
43c5ea9
2efe231
816cc6c
a8e094e
1314351
63d1760
76fa61a
3afa960
f45744f
2a64355
2f0187f
1625dab
bc293f2
ff1aa7f
317ac76
bbb9e05
4c3eaca
cfdc6fc
3db2793
e2af323
b88dcf8
dcf4bdd
6ad2938
3d9f967
3c99591
2bf6246
4e0f724
9ab37f9
c7efd31
6c57838
1aa27f4
219e72c
762dbf7
99c9f4d
18c8127
54ea6a4
6512664
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,22 +10,34 @@ | |
- [`SystemConfig`](#systemconfig) | ||
- [`ConfigUpdate`](#configupdate) | ||
- [Initialization](#initialization) | ||
- [Modifying Operator Fee Parameters](#modifying-operator-fee-parameters) | ||
- [Interface](#interface) | ||
- [Operator fee parameters](#operator-fee-parameters) | ||
- [`operatorFeeScalar`](#operatorfeescalar) | ||
- [`operatorFeeConstant`](#operatorfeeconstant) | ||
- [`setOperatorFeeScalars`](#setoperatorfeescalars) | ||
- [`setOperatorFeeManager`](#setoperatorfeemanager) | ||
- [Fee Vault Config](#fee-vault-config) | ||
- [`setBaseFeeVaultConfig`](#setbasefeevaultconfig) | ||
- [`setL1FeeVaultConfig`](#setl1feevaultconfig) | ||
- [`setSequencerFeeVaultConfig`](#setsequencerfeevaultconfig) | ||
- [`setOperatorFeeVaultConfig`](#setoperatorfeevaultconfig) | ||
- [`OptimismPortal`](#optimismportal) | ||
- [Interface](#interface-1) | ||
- [`setConfig`](#setconfig) | ||
- [`upgrade`](#upgrade) | ||
- [Consensus Parameters](#consensus-parameters) | ||
- [Operator Fee Scalar](#operator-fee-scalar) | ||
- [Operator Fee Constant](#operator-fee-constant) | ||
- [Service Roles](#service-roles) | ||
- [Operator Fee Manager](#operator-fee-manager) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
## Overview | ||
|
||
The `SystemConfig` and `OptimismPortal` are updated with a new flow for chain | ||
configurability. | ||
configurability. A new service role `OperatorFeeManager` is added to manage the operator fee collection. | ||
|
||
## Constants | ||
|
||
|
@@ -39,10 +51,11 @@ The `ConfigType` enum represents configuration that can be modified. | |
| `SET_BASE_FEE_VAULT_CONFIG` | `uint8(1)` | Sets the Fee Vault Config for the `BaseFeeVault` | | ||
| `SET_L1_FEE_VAULT_CONFIG` | `uint8(2)` | Sets the Fee Vault Config for the `L1FeeVault` | | ||
| `SET_SEQUENCER_FEE_VAULT_CONFIG` | `uint8(3)` | Sets the Fee Vault Config for the `SequencerFeeVault` | | ||
| `SET_L1_CROSS_DOMAIN_MESSENGER_ADDRESS` | `uint8(4)` | Sets the `L1CrossDomainMessenger` address | | ||
| `SET_L1_ERC_721_BRIDGE_ADDRESS` | `uint8(5)` | Sets the `L1ERC721Bridge` address | | ||
| `SET_L1_STANDARD_BRIDGE_ADDRESS` | `uint8(6)` | Sets the `L1StandardBridge` address | | ||
| `SET_REMOTE_CHAIN_ID` | `uint8(7)` | Sets the chain id of the base chain | | ||
| `SET_OPERATOR_FEE_VAULT_CONFIG` | `uint8(4)` | Sets the Fee Vault Config for the `OperatorFeeVault` | | ||
| `SET_L1_CROSS_DOMAIN_MESSENGER_ADDRESS` | `uint8(5)` | Sets the `L1CrossDomainMessenger` address | | ||
| `SET_L1_ERC_721_BRIDGE_ADDRESS` | `uint8(6)` | Sets the `L1ERC721Bridge` address | | ||
| `SET_L1_STANDARD_BRIDGE_ADDRESS` | `uint8(7)` | Sets the `L1StandardBridge` address | | ||
| `SET_REMOTE_CHAIN_ID` | `uint8(8)` | Sets the chain id of the base chain | | ||
|
||
## `SystemConfig` | ||
|
||
|
@@ -57,6 +70,8 @@ The following `ConfigUpdate` event is defined where the `CONFIG_VERSION` is `uin | |
| `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 | | ||
| `OPERATOR_FEE_PARAMS` | `uint8(5)` | `uint256(_operatorFeeScalar) << 64 \| _operatorFeeConstant` | Modifies the operator fee parameters | | ||
| `OPERATOR_FEE_MANAGER` | `uint8(6)` | `abi.encode(address)` | Modifies the operator fee manager | | ||
|
||
### Initialization | ||
|
||
|
@@ -67,19 +82,73 @@ The following actions should happen during the initialization of the `SystemConf | |
- `emit ConfigUpdate.GAS_LIMIT` | ||
- `emit ConfigUpdate.UNSAFE_BLOCK_SIGNER` | ||
- `emit ConfigUpdate.EIP_1559_PARAMS` | ||
- `emit ConfigUpdate.OPERATOR_FEE_PARAMS` | ||
- `emit ConfigUpdate.OPERATOR_FEE_MANAGER` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to have the manager be a |
||
- `setConfig(SET_GAS_PAYING_TOKEN)` | ||
- `setConfig(SET_BASE_FEE_VAULT_CONFIG)` | ||
- `setConfig(SET_L1_FEE_VAULT_CONFIG)` | ||
- `setConfig(SET_SEQUENCER_FEE_VAULT_CONFIG)` | ||
- `setConfig(SET_OPERATOR_FEE_VAULT_CONFIG)` | ||
- `setConfig(SET_L1_CROSS_DOMAIN_MESSENGER_ADDRESS)` | ||
- `setConfig(SET_L1_ERC_721_BRIDGE_ADDRESS)` | ||
- `setConfig(SET_L1_STANDARD_BRIDGE_ADDRESS)` | ||
- `setConfig(SET_REMOTE_CHAIN_ID)` | ||
|
||
These actions MAY only be triggered if there is a diff to the value. | ||
|
||
Since the `OperatorFeeVault` is new in Isthmus, the `setConfig(SET_OPERATOR_FEE_VAULT_CONFIG)` MUST be emitted. | ||
|
||
`ConfigUpdate.OPERATOR_FEE_PARAMS` and `ConfigUpdate.OPERATOR_FEE_MANAGER` MAY be emitted. If they are not emitted, | ||
the `operatorFeeScalar` and `operatorFeeConstant` are set to 0 by default, and the `OperatorFeeManager` | ||
is set to the chain governor by default. | ||
|
||
### Modifying Operator Fee Parameters | ||
|
||
A new `SystemConfig` `UpdateType` is introduced that enables the modification of | ||
the `operatorFeeScalar` and `operatorFeeConstant` by the [`OperatorFeeManager`](#operator-fee-manager). | ||
|
||
Another `UpdateType` is added to modify the [`OperatorFeeManager`]. | ||
|
||
### Interface | ||
|
||
#### Operator fee parameters | ||
|
||
##### `operatorFeeScalar` | ||
|
||
This function returns the currently configured operator fee scalar. | ||
|
||
```solidity | ||
function operatorFeeScalar()(uint32) | ||
``` | ||
|
||
##### `operatorFeeConstant` | ||
|
||
This function returns the currently configured operator fee constant. | ||
|
||
```solidity | ||
function operatorFeeConstant()(uint64) | ||
``` | ||
|
||
##### `setOperatorFeeScalars` | ||
|
||
This function sets the `operatorFeeScalar` and `operatorFeeConstant`. | ||
|
||
This function MUST only be callable by the [`OperatorFeeManager`](#operator-fee-manager). | ||
|
||
```solidity | ||
function setOperatorFeeScalar(uint32 _operatorFeeScalar, uint64 _operatorFeeConstant)() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. small nit: I think its fine to omit the trailing |
||
``` | ||
|
||
##### `setOperatorFeeManager` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to delete this setter and instead source the fee manager from the superchain config as mentioned in a previous comment |
||
|
||
This function sets the `operatorFeeManager`. | ||
|
||
This function MUST only be callable by the chain governor. | ||
|
||
```solidity | ||
function setOperatorFeeManager(address _operatorFeeManager)() | ||
``` | ||
|
||
#### Fee Vault Config | ||
|
||
For each `FeeVault`, there is a setter for its config. The arguments to the setter include | ||
|
@@ -106,6 +175,12 @@ function setL1FeeVaultConfig(address,uint256,WithdrawalNetwork) | |
function setSequencerFeeVaultConfig(address,uint256,WithdrawalNetwork) | ||
``` | ||
|
||
##### `setOperatorFeeVaultConfig` | ||
|
||
```solidity | ||
function setOperatorFeeVaultConfig(address,uint256,WithdrawalNetwork) | ||
``` | ||
|
||
## `OptimismPortal` | ||
|
||
The `OptimismPortal` is updated to emit a special system `TransactionDeposited` event. | ||
|
@@ -157,3 +232,28 @@ The following fields are included: | |
- `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`. | ||
|
||
## Consensus Parameters | ||
|
||
The operator fee scalar and constant are new consensus parameters, so we define standard values for them. | ||
|
||
### [Operator Fee Scalar](exec-engine.md#operator-fees) | ||
|
||
**Description:** Operator fee scalar -- used to calculate the operator fee<br/> | ||
**Administrator:** [Operator Fee Manager](#operator-fee-manager)<br/> | ||
**Requirement:** Between 0 and 0.5 * (baseFee + priorityFee) <br/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't noted anywhere, apologies about that, but its probably best to put these configurability requirements in this file so they are all in a single place Also we would like to say that the requirement for these values is 0 for now for the superchain, with op-succinct you can use them however you would like, then when we do add zk as part of the superchain's proof system we can define the ranges of values. We don't have bandwidth to really think deeply about what the standard values should be from a product perspective and while what you have now could make sense, I don't want to ratify something thru gov that ultimately isnt right |
||
|
||
### [Operator Fee Constant](exec-engine.md#operator-fees) | ||
|
||
**Description:** Operator fee constant -- used to calculate the operator fee<br/> | ||
**Administrator:** [Operator Fee Manager](#operator-fee-manager)<br/> | ||
**Requirement:** Between 0 and 600 Gwei <br/> | ||
|
||
## Service Roles | ||
|
||
### Operator Fee Manager | ||
|
||
**Description:** Account authorized to modify the operator fee scalar. <br/> | ||
**Administrator:** [System Config Owner](../configurability.md#system-config)<br/> | ||
**Requirement:** <br/> | ||
**Notes:** <br/> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Isthmus L2 Chain Derivation Changes | ||
|
||
<!-- START doctoc generated TOC please keep comment here to allow auto update --> | ||
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> | ||
**Table of Contents** | ||
|
||
- [Network upgrade automation transactions](#network-upgrade-automation-transactions) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
# Network upgrade automation transactions | ||
|
||
The Isthmus hardfork activation block contains the following transactions, in this order: | ||
|
||
- L1 Attributes Transaction | ||
- User deposits from L1 | ||
- Network Upgrade Transactions | ||
- L1Block deployment | ||
- Update L1Block Proxy ERC-1967 Implementation | ||
- L1Block Enable Isthmus | ||
- GasPriceOracle deployment | ||
- Update GasPriceOracle Proxy ERC-1967 Implementation | ||
- GasPriceOracle Enable Isthmus | ||
- OptimismMintableERC20Factory deployment | ||
- Update OptimismMintableERC20Factory Proxy ERC-1967 Implementation |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ | |
- [Rationale](#rationale) | ||
- [Forwards Compatibility Considerations](#forwards-compatibility-considerations) | ||
- [Client Implementation Considerations](#client-implementation-considerations) | ||
- [Fees](#fees) | ||
- [Operator Fee](#operator-fee) | ||
- [Configuring Parameters](#configuring-parameters) | ||
- [Fee Vaults](#fee-vaults) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
|
@@ -76,3 +80,45 @@ an outbound withdrawal for a long period of time, the node may not have access t | |
[`L2ToL1MessagePasser`][l2-to-l1-mp]. In this case, the client would be unable to keep consensus. However, most modern | ||
clients are able to at the very least reconstruct the account storage root at a given block on the fly if it does not | ||
directly store this information. | ||
|
||
[l2-to-l1-mp]: ../../protocol/predeploys.md#L2ToL1MessagePasser | ||
[output-root]: ../../glossary.md#l2-output-root | ||
|
||
## Fees | ||
|
||
New OP stack variants have different resource consumption patterns, and thus require a more flexible | ||
pricing model. To enable more customizable fee structures, Isthmus adds a new component to the fee | ||
calculation: the `operatorFee`, which is parameterized by two scalars: the `operatorFeeScalar` | ||
and the `operatorFeeConstant`. | ||
|
||
### Operator Fee | ||
|
||
The operator fee, is set as follows: | ||
|
||
`operatorFee = (gasUsed * operatorFeeScalar / 1e6) + operatorFeeConstant` | ||
|
||
Where: | ||
|
||
- `gasUsed` is amount of gas used by the transaction. | ||
- `operatorFeeScalar` is a `uint32` scalar set by the chain operator, scaled by `1e6`. | ||
- `operatorFeeConstant` is a `uint64` scalar set by the chain operator. | ||
|
||
#### Configuring Parameters | ||
|
||
`operatorFeeScalar` and `operatorFeeConstant` are loaded in a similar way to the `baseFeeScalar` and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw comment somewhere, maybe gone by now, that there would be restrictions on how large the operator fee scalar and fee constant should be set. If this feature becomes a standard chain features I think that this restriction is very important to protect user funds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the operator fee will probably not become part of the superchain config since it adds complexity. Its purpose is more intended for non-standard chains, like op-succinct chains, for example. My previous proposal wa the at the constant was between 0 and 600 Gwei and that the scalar was between 0 and 0.5x (basefee + priorityfee). |
||
`blobBaseFeeScalar` used in the [`L1Fee`](../../protocol/exec-engine.md#ecotone-l1-cost-fee-changes-eip-4844-da). | ||
calculation. In more detail, these paramters can be accessed in two interchangable ways. | ||
|
||
- read from the deposited L1 attributes (`operatorFeeScalar` and `operatorFeeConstant`) of the current L2 block | ||
- read from the L1 Block Info contract (`0x4200000000000000000000000000000000000015`) | ||
- using the respective solidity getter functions (`operatorFeeScalar`, `operatorFeeConstant`) | ||
- using direct storage-reads: | ||
- Operator fee scalar as big-endian `uint32` in slot `8` at offset `0`. | ||
- Operator fee constant as big-endian `uint64` in slot `8` at offset `4`. | ||
|
||
### Fee Vaults | ||
|
||
These collected fees are sent to a new vault for the `operatorFee`: the [`OperatorFeeVault`](predeploys.md#operatorfeevault). | ||
|
||
Like the existing vaults, this is a hardcoded address, pointing at a pre-deployed proxy contract. | ||
The proxy is backed by a vault contract deployment, based on `FeeVault`, to route vault funds to L1 securely. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# L1 Block Attributes | ||
|
||
<!-- START doctoc generated TOC please keep comment here to allow auto update --> | ||
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> | ||
**Table of Contents** | ||
|
||
- [Overview](#overview) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
## Overview | ||
|
||
The L1 block attributes transaction is updated to include the operator fee parameters. | ||
|
||
| Input arg | Type | Calldata bytes | Segment | | ||
| ----------------- | ------- | -------------- | ------- | | ||
| {0x098999be} | | 0-3 | n/a | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the call signature for the function. Those 4 bytes are the first four bytes of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bump on adding this to the spec to make it clear what this is |
||
| baseFeeScalar | uint32 | 4-7 | 1 | | ||
| blobBaseFeeScalar | uint32 | 8-11 | | | ||
| sequenceNumber | uint64 | 12-19 | | | ||
| l1BlockTimestamp | uint64 | 20-27 | | | ||
| l1BlockNumber | uint64 | 28-35 | | | ||
| basefee | uint256 | 36-67 | 2 | | ||
| blobBaseFee | uint256 | 68-99 | 3 | | ||
| l1BlockHash | bytes32 | 100-131 | 4 | | ||
| batcherHash | bytes32 | 132-163 | 5 | | ||
| operatorFeeScalar | uint32 | 164-167 | 6 | | ||
| operatorFeeConstant | uint64 | 168-175 | | | ||
|
||
In the first L2 block after the Isthmus activation block, the Isthmus L1 attributes are first used. | ||
|
||
The pre-Isthmus values are migrated over 1:1. | ||
Blocks after the Isthmus activation block contain all pre-Isthmus values 1:1, | ||
and also set the following new attributes: | ||
|
||
- The `operatorFeeScalar` is set to `0`. | ||
- The `operatorFeeConstant` is set to `0`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create a role in the
SuperchainConfig
called "operator fee manager" and source the value from there? https://github.com/ethereum-optimism/specs/blob/d6b979ab67bd98bc3a15bf4988df08a066788047/specs/protocol/isthmus/superchain-config.mdThis will make standard definition more simple, use the same superchain config that op gov recognizes, chain can also use their own superchain config if they are their own L2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up: trying to align ongoing work that is changing with this so that things are coherent, this is the design that we are landing on.
The
SystemConfig.initialize
function would accept aRoles
struct, it would be good to add theoperatorFeeManager
to thatRoles
struct and have no setter function meaning the operator can only change with another call toinitialize
. This helps keep the ABI slim and allows for each chain operator to have its own individual account that is used.It is still possible to have each chain have its own individual accounts in the world where its sourced from the
SuperchainConfig
, it just means that chains that aren't governed by Optimism would deploy their ownSuperchainConfig
. Apologies for the back and forth, was trying to be consistent with designs in the Standard L2 Genesis project and we were flip flopping internally on how we were going to do itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes more sense to me than the superchain solution, happy to proceed in this direction. Once the spec lands for the
Roles
struct then I'll update this spec accordingly -- I've been yet unable to find reference to the design choices you linked.