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

WIP: RevenueSharer predeploy #115

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
89ca8ad
add skeleton structure for revenue sharing spec
geoknee Apr 5, 2024
cbb3174
fill out symbols, names and definitions of some quantitites
geoknee Apr 8, 2024
6bef64c
explain simplification of e=0
geoknee Apr 8, 2024
511bccb
wip
geoknee Apr 8, 2024
36529c6
tweak
geoknee Apr 8, 2024
10663a5
update twin approach to deployment
geoknee Apr 8, 2024
846a7d0
fix hyperlink
geoknee Apr 9, 2024
1abccaa
change RevenueSharer predeploy address
geoknee Apr 9, 2024
bf699ff
remove -Proxy suffixes
geoknee Apr 9, 2024
03a74ce
Apply suggestions from code review
geoknee Apr 10, 2024
44406ff
remove statement about existing chains
geoknee Apr 9, 2024
a33d9f0
add note about upgrading feevault contracts
geoknee Apr 9, 2024
e66e20c
fix note on deployment
geoknee Apr 9, 2024
05c8c50
fix expressions
geoknee Apr 10, 2024
8aa907b
Revert "remove proposed qualifier for ecotone upgrade"
geoknee Apr 10, 2024
8fe7d9e
harmonize file structure
geoknee Apr 10, 2024
826e691
change OptimismCollective to Beneficiary
geoknee Apr 10, 2024
b135699
tweak section on fee vaults
geoknee Apr 11, 2024
91ac53d
Update instructions for deploying RevenueSharer to existing chains
geoknee Apr 11, 2024
d45794f
fix
geoknee Apr 11, 2024
60c3e7b
update link to RevenueSharer implementation
geoknee Apr 15, 2024
2248b74
rename variable
geoknee Apr 15, 2024
1c54fc3
don't specify immutable
geoknee Apr 16, 2024
91ab308
remove section about deploying to new chains
geoknee Apr 16, 2024
cad87eb
specify funds can go to L1 or L2
geoknee Apr 16, 2024
ba125ef
add definition of execute fn
geoknee Apr 16, 2024
4bfd920
add section on invariants
geoknee Apr 16, 2024
86313ad
lint
geoknee Apr 16, 2024
5f2ab2e
add definitions for BENEFICIARY AND REMAINDER_BENEFICIARY
geoknee Apr 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions specs/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- [Batch Submitter](./protocol/batcher.md)
- [Predeploys](./protocol/predeploys.md)
- [Preinstalls](./protocol/preinstalls.md)
- [Revenue Sharing](./protocol/revenue-sharing.md)
- [Superchain]()
- [Superchain Configuration](./protocol/superchain-configuration.md)
- [Superchain Upgrades](./protocol/superchain-upgrades.md)
Expand Down
2 changes: 1 addition & 1 deletion specs/protocol/exec-engine.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The three types of fees are collected in 3 distinct L2 fee-vault deployments for
fee payments are not registered as internal EVM calls, and thus distinguished better this way.

These are hardcoded addresses, pointing at pre-deployed proxy contracts.
The proxies are backed by vault contract deployments, based on `FeeVault`, to route vault funds to L1 securely.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we should be too specific here on the behavior of the FeeVault. This is free software, it can be configured however

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggested wording? Can we just replace "are" with "may"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps something like "based on FeeVault, to collect fees"

simple and not opinionated on where the fees go

The proxies are backed by vault contract deployments, based on `FeeVault`. Funds are moved on from these vaults according to [Revenue Sharing](./revenue-sharing.md).

| Vault Name | Predeploy |
| ------------------- | ------------------------------------------------------ |
Expand Down
12 changes: 12 additions & 0 deletions specs/protocol/predeploys.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [L1FeeVault](#l1feevault)
- [SchemaRegistry](#schemaregistry)
- [EAS](#eas)
- [RevenueSharer](#revenuesharer)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

Expand Down Expand Up @@ -69,6 +70,7 @@ or `Bedrock` or `Canyon`. Deprecated contracts should not be used.
| L1FeeVault | 0x420000000000000000000000000000000000001a | Bedrock | No | Yes |
| SchemaRegistry | 0x4200000000000000000000000000000000000020 | Bedrock | No | Yes |
| EAS | 0x4200000000000000000000000000000000000021 | Bedrock | No | Yes |
| RevenueSharer | 0x4200000000000000000000000000000000000024 | TBC | No | Yes |
blmalone marked this conversation as resolved.
Show resolved Hide resolved

## LegacyMessagePasser

Expand Down Expand Up @@ -344,3 +346,13 @@ protocol.
Address: `0x4200000000000000000000000000000000000021`

The `EAS` predeploy implements the `Ethereum Attestation Service` protocol.

## RevenueSharer
blmalone marked this conversation as resolved.
Show resolved Hide resolved
[Implementation](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L2/RevenueSharer.sol)
<!---
TODO: update above link when implementation is merged
-->

Address: `0x4200000000000000000000000000000000000024`
geoknee marked this conversation as resolved.
Show resolved Hide resolved

The `RevenueSharer` predepoly implements Revenue Sharing, disbursing funds from FeeVault contracts to the Optimism Collective.
80 changes: 80 additions & 0 deletions specs/protocol/revenue-sharing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Revenue Sharing
<!-- 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)
- [Definitions](#definitions)
- [`RevenueSharer` predeploy](#revenuesharer-predeploy)
- [Invariants](#invariants)
- [Deploying `RevenueSharer`](#deploying-revenuesharer)
- [Existing Chains](#existing-chains)
- [Execution](#execution)
- [Simplified L1 Data Fee Expenditure](#simplified-l1-data-fee-expenditure)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

# Overview

Revenue sharing is the process by which chains in the superchain contribute a portion of their revenue or profit to a beneficiary such as the Optimism collective. They may do this in return for support with the OP Stack and/or other benefits.


## Definitions
tynes marked this conversation as resolved.
Show resolved Hide resolved
| Term | Name | Definition |
| -------|--------------| ----------- |
| $d$ | L1 Data Fee Revenue | ETH transferred to the sequencer with L2 transactions to cover estimated L1 Data Fees(see below). Accumulates to `L1FeeVault`.
| $e$ | L1 Data Fee Expenditure| ETH spent by the batcher on L1 to make transaction data available
| $b$ | L2 Base Gas Revenue | Base fee portion of L2 Gas Fee (ETH transferred to the sequencer to cover execution of L2 transactions). Accumulates to `BaseFeeVault`.
| $q$ | L2 Priority Gas Revenue | Priority fee portion of L2 Gas Fee (ETH transferred to the sequencer to cover execution of L2 transactions). Accumulates to `SequencerFeeVault`.
| $g$ | L2 Gas Revenue | $b + q$
| $r$ | Sequencer Revenue | $d + b + q$
| $p$ | Sequencer Profit | $r - e$
| $s$ | Revenue share due to `BENEFICIARY`| $\max(0.15r,0.025p)$
| `BENEFICIARY` | e.g. Optimism Collective |
| `REMAINDER_BENEFICIARY` | e.g. Chain Operator |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add definitions for BENEFICIARY and REMAINDER_BENEFICIARY (or whatever terms we settle on).

The gas paying token address is network specific configuration, therefore it MUST be set in storage and not as an immutable. This ensures that the same contract bytecode can be used by multiple OP Stack chains.

I spotted this line on #111, so I guess we should follow that pattern here (REMAINDER_BENEFICIARY is chain specific).

## `RevenueSharer` predeploy
Revenue sharing is achieved through an L2 [predeploy](./predeploys.md) contract `RevenueSharer` with address `0x4200000000000000000000000000000000000024`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps giving a simple interface + description of execute could be useful


### Invariants
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vaults must be configured to withdraw to this predeploy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is relevant to your other comment.

The model I initially had in mind was to hardcode the RevenueShare(r) as the output of the FeeVaults. To my mind, this represents a greater level of enshrinement of revenue sharing, which I believe is our overarching goal. But it does have some downsides:

  1. It could be offputting or confusing to any chain operator who isn't sure about doing revenue sharing in the first place. As you say, from an open source perspective it's nice to include this set of users.
  2. It reduces overall configurability over how fees can flow around. Although we can ensure that the RevenueShare(r) can still be configured to send all funds to e.g. the chain operator or any other wallet, this approach does tie all three FeeVaults together (not possible to withdraw from one without withdrawing from the others).

We can make a different choice and make the the output of the FeeVaults freely configurable. This represents a somewhat lesser enshrinement of revenue sharing. The downside could be that:

  1. We risk not achieving our overall goal. Chains can easily bypass the RevenueShare(r) predeploy, leaving it dangling there doing nothing,
  2. There is more opportunity to deploy a "bad" configuration which could lead to incorrect behaviour. Of course this may be addressable through documentation and/or runtime checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From slack thread: would be nice to have a simple way to opt out of revenue sharing via a flag in the deploy configuration. When opting out, the existing/legacy behaviour can prevail.

* The `RevenueSharer` contract should not accumulate any ETH or Tokens itself.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a "Security Considerations" section at the bottom with responses to questions around the security, ie "what if a user sends ether or a token to the contract directly?" Will this mess with its accounting?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geoknee let me know if you want input on this section 👍🏻

* It should share revenue in the propotions `s, r-s` when the `execute()` function is called, to `BENEFICIARY` and `REMAINDER_BENEFICIARY` respectively.
* It should emit an event when `execute()` is called.

### Deploying `RevenueSharer`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on RevenueShare vs RevenueSharer? Seems a bit easier to say and gets the same point across

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have super strong opinion. I think having the -er follows existing predeploys such as L2ToL1MessagePasser , L2CrossDomainMessenger and so on. And we do already have some hard-to-say names such as OptimismMintableERC721Factory.

Other options are RevShare and RevSharer. Base are using something they call a FeeDisburser.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a preference to just call it RevenueShare to make it easier to say. the "revenue share predeploy" vs the "revenue sharer predeploy"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slack poll so far indicating a preference for RevenueSharer. I'm going to to continue using this name for now, it should be easy to replace down the line if necessary.


The `RevenueSharer` contract is now included in the L2 genesis and is not scheduled to be part of a hardfork.

#### Existing Chains
For chains without the `RevenueSharer` in their genesis, the following transactions may be executed to deploy it
:
1. The `RevenueSharer` should be deployed manually to an arbitrary address `REVENUE_SHARER_IMPLEMENTATION_ADDRESS`
2. For EACH of the three FeeVault contracts ({name} = BASE, L1, SEQUENCER):

a. A new version should be deployed with the `RECIPIENT` variable set to `0x4200000000000000000000000000000000000024` in the constructor, and other constructor parameters set to existing values. This contract has an arbitrary address `NEW_{name}_FEE_VAULT_ADDRESS`.
b. The ProxyAdminOwner should call `ProxyAdmin.upgrade({name}_FEE_VAULT, NEW_{name}_FEE_VAULT_ADDRESS)`

### Execution
Revenue sharing is executed periodically, and can be configured to send funds to L1 or to L2.
The `RevenueSharer` is responsible for computing $s$ and sending it to a predetermined address controlled by the Optimism Collective. At the end of execution, `SequencerFeeVault`, `L1FeeVault` and `BaseFeeVault` and `RevenueSharer` should be completely depleted of ETH. This allows the contract to be stateless.

```mermaid
flowchart TD
SequencerFeeVault
BaseFeeVault
L1FeeVault
BeneficiaryWallet
RemainderWallet
RevenueSharer
SequencerFeeVault-->|q|RevenueSharer
L1FeeVault-->|d|RevenueSharer
BaseFeeVault-->|b|RevenueSharer
RevenueSharer-->|s|BeneficiaryWallet
RevenueSharer-->|r-s|RemainderWallet
```

The execution of Revenue Sharing is achieved by calling `RevenueSharer.execute()`.

## Simplified L1 Data Fee Expenditure
As a part of a gradual rollout of protocol enshrined revenue sharing, the `RevenueSharer` uses a fixed value of $e=0$. The sequencer makes a profit on data availability of $d-e$. Note that a negative profit, i.e. a loss, is possible. By assuming $e=0$, the simplification implies that data availability revenue is all profit. This will be addressed in a future protocol upgrade.

3 changes: 2 additions & 1 deletion specs/protocol/superchain-upgrades.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ but the matching L1-origin information may not be present at the time of activat
[Governance Proposal](https://gov.optimism.io/t/final-upgrade-proposal-2-canyon-network-upgrade/7088).
- `v5.0.0`: 2024 Feb 22nd - Delta network upgrade (Span Batches).
[Governance Proposal](https://gov.optimism.io/t/final-upgrade-proposal-3-delta-network-upgrade/7310).
- `v6.0.0`: 2024 Mar 14th - Ecotone network upgrade (4844 Blob Batches + Cancun).
- `v6.0.0`: 2024 Mar 14th - _Proposed_ Ecotone network upgrade (4844 Blob Batches + Cancun).
Still under review by governance.
[Governance Proposal](https://gov.optimism.io/t/upgrade-proposal-5-ecotone-network-upgrade/7669).

## Post-Bedrock Network upgrades
Expand Down
1 change: 1 addition & 0 deletions specs/root.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ that maintains 1:1 compatibility with Ethereum.
- [Bridges](protocol/bridges.md)
- [Predeploys](protocol/predeploys.md)
- [Preinstalls](protocol/preinstalls.md)
- [Revenue Sharing](protocol/revenue-sharing.md)
- [Glossary](glossary.md)

### Experimental
Expand Down