ethereum-optimism / design-docs

MIT License
26 stars 20 forks source link

Add Operator Fee design #81

Closed puma314 closed 1 month ago

puma314 commented 2 months ago

A design doc for additional fee scalars to add to the fee formula that allows for more flexibility for chains leverage alt-DA, ZK proving, or custom gas tokens.

tynes commented 2 months ago

I am generally on board with this change, although we have scoped holocene already that doesn't mean that this cannot be included if it is code frozen by mid/late October

tynes commented 1 month ago

Some open considerations:

Roles. We need a section on the chain operator UX for modifying this value and who can do it. It should be really explicit about the SystemConfig being the place that this is modified. Who can do it? There are 2 options - the chain operator (the SystemConfig.owner() or we start moving towards an RBAC system in the system config. We generally want to move towards a more RBAC based system. It could be useful for this now and adding a new role specific to who can modify this value. Operationally this can be useful, if you want to have a less cold key be able to modify this value more regularly

Standardness. We have a definition of standard config here. If we do add a new role to be able to modify this value, then we would want to also define the standard config value. I would recommend making it the L1 Proxy Admin for simplicity

tynes commented 1 month ago

Fee Vault: While this went into the spec originally and then removed and was not mentioned in the design doc, I think that its a good idea to introduce a new fee vault for this change. Each fee vault captures revenue from a different resource, it will ultimately be more forwards compatible if the operator fees went into an OperatorFeeVault

tynes commented 1 month ago

Upgrading a live chain: The consideration here is for the ConfigUpdate event. Right now, the derivation pipeline will error on unknown events. Which is why we previously modified the existing events. We can do this again - first fork, then upgrade the L1 contracts that now emit the new serialization.

edit: context in https://github.com/ethereum-optimism/design-docs/pull/81#issuecomment-2387320192

yuwen01 commented 1 month ago

Upgrading a live chain: The consideration here is for the ConfigUpdate event. Right now, the derivation pipeline will error on unknown events. Which is why we previously modified the existing events. We can do this again - first fork, then upgrade the L1 contracts that now emit the new serialization.

I think we can update the existing GAS_CONFIG UpdateType (as of holocene, renamed to FEE_SCALARS). Right now, the GAS_CONFIG packs the blobbasefeeScalar and the basefeeScalar into a single uint256. There's still room for the other operatorFeeScalar and operatorFeeConstant in the higher bits of this uint256. This behavior is specified here.

yuwen01 commented 1 month ago

Some open considerations:

Roles. We need a section on the chain operator UX for modifying this value and who can do it. It should be really explicit about the SystemConfig being the place that this is modified. Who can do it? There are 2 options - the chain operator (the SystemConfig.owner() or we start moving towards an RBAC system in the system config. We generally want to move towards a more RBAC based system. It could be useful for this now and adding a new role specific to who can modify this value. Operationally this can be useful, if you want to have a less cold key be able to modify this value more regularly

Standardness. We have a definition of standard config here. If we do add a new role to be able to modify this value, then we would want to also define the standard config value. I would recommend making it the L1 Proxy Admin for simplicity

For making a new role, maybe we could make a new role called FeeManager or something that manages both the basefeeScalar, blobbasefeeScalar, and also the operator fee scalars? And we'd have setGasConfig only modifiable by the FeeManager? I mostly just feel like the entity who's managing the l1 fee scalars should also manage the operator fee scalars -- so I think the best alternative to this new role would be the System Config Owner.

Also, when you refer to "Standardness", you're referring to the "Requirements", right? I'm a bit confused if I need to set standard values for the operator fee scalars, since the basefeeScalar and blobbasefeeScalar don't have requirements.

tynes commented 1 month ago

Upgrading a live chain: The consideration here is for the ConfigUpdate event. Right now, the derivation pipeline will error on unknown events. Which is why we previously modified the existing events. We can do this again - first fork, then upgrade the L1 contracts that now emit the new serialization.

I think we can update the existing GAS_CONFIG UpdateType (as of holocene, renamed to FEE_SCALARS). Right now, the GAS_CONFIG packs the blobbasefeeScalar and the basefeeScalar into a single uint256. There's still room for the other operatorFeeScalar and operatorFeeConstant in the higher bits of this uint256. This behavior is specified here.

I would prefer if we add a new enum for ConfigUpdate specific for these values rather than modifying the existing one. The specific reason why this is preferred is because it will reduce need for us to update tooling that calls out to the system config. If they are all part of the same ConfigUpdate, then we will need a high level ABI change to the SystemConfig to a function that accepts 4 fee config fee params rather than 2 fee params. This also allows us to skip needing to pass in the values into SystemConfig.initialize and it becomes an opt in change - the only way to set them would be to explicitly call the new function on the SystemConfig that accepts these values and then emits the new enum in the ConfigUpdate event

tynes commented 1 month ago

Some open considerations: Roles. We need a section on the chain operator UX for modifying this value and who can do it. It should be really explicit about the SystemConfig being the place that this is modified. Who can do it? There are 2 options - the chain operator (the SystemConfig.owner() or we start moving towards an RBAC system in the system config. We generally want to move towards a more RBAC based system. It could be useful for this now and adding a new role specific to who can modify this value. Operationally this can be useful, if you want to have a less cold key be able to modify this value more regularly Standardness. We have a definition of standard config here. If we do add a new role to be able to modify this value, then we would want to also define the standard config value. I would recommend making it the L1 Proxy Admin for simplicity

For making a new role, maybe we could make a new role called FeeManager or something that manages both the basefeeScalar, blobbasefeeScalar, and also the operator fee scalars? And we'd have setGasConfig only modifiable by the FeeManager? I mostly just feel like the entity who's managing the l1 fee scalars should also manage the operator fee scalars -- so I think the best alternative to this new role would be the System Config Owner.

Also, when you refer to "Standardness", you're referring to the "Requirements", right? I'm a bit confused if I need to set standard values for the operator fee scalars, since the basefeeScalar and blobbasefeeScalar don't have requirements.

Given https://github.com/ethereum-optimism/design-docs/pull/81#issuecomment-2387320192, I think we should make the new role be called Operator Fee Manager and wrap function setOperatorFees(uint32,uint64) with onlyOperatorFeeManager

We would need to add a new section to the requirements that you linked with a new section that includes the operator fee

yuwen01 commented 1 month ago

I understand your point about separating these new parameters logically, and added the new role and updatetype as suggested. I've also started thinking about standard values for the new scalars, and will update you when those are finalized.

Regarding the new OperatorFeeManager role -- I was thinking that this role should only be responsible for tuning the operator fee scalars, and not be involved with fee vault collection at all. The job of directing fees to different vaults is already managed entirely by the chain governor.

For setting standard values for the operatorFeeScalar, I think setting a bound based on marginal gains might make the most sense, similar to this (perhaps outdated) parameter. Just wanted to check that something like that would be appropriate here.

edit: added standard values for the new scalars

tynes commented 1 month ago

Regarding the new OperatorFeeManager role -- I was thinking that this role should only be responsible for tuning the operator fee scalars, and not be involved with fee vault collection at all. The job of directing fees to different vaults is already managed entirely by the chain governor.

Agreed with this 100%, there is a FeeVault recipient that should be configurable by the chain operator