-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat: add shared lockbox spec #465
Open
agusduha
wants to merge
3
commits into
ethereum-optimism:main
Choose a base branch
from
defi-wonderland:sc-feat/add-shared-lockbox
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+478
−8
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
# OptimismPortal | ||
|
||
<!-- 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) | ||
- [Integrating `SharedLockbox`](#integrating-sharedlockbox) | ||
- [`depositTransaction`](#deposittransaction) | ||
- [`finalizeWithdrawalTransactionExternalProof`](#finalizewithdrawaltransactionexternalproof) | ||
- [Invariants](#invariants) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
## Overview | ||
|
||
The `OptimismPortal` contract is upgraded to integrate the `SharedLockbox` and start using the shared ETH liquidity. | ||
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. "using the shared ETH liquidity" is a bit ambiguous imo, as each |
||
This liquidity consists of every ETH balance migrated from each `OptimismPortal` | ||
when joining the op-governed dependency set. | ||
|
||
It is possible to upgrade to this version without being part of the op-governed dependency set. In this case, | ||
the corresponding chain would need to deploy and manage its own `SharedLockbox` and `SuperchainConfig`. | ||
|
||
### Integrating `SharedLockbox` | ||
|
||
The integration with the `SharedLockbox` involves locking ETH when executing deposit transactions and unlocking ETH | ||
when finalizing withdrawal transactions, without altering other aspects of the current `OptimismPortal` implementation. | ||
|
||
To implement this solution, the following changes are needed: | ||
|
||
#### `depositTransaction` | ||
|
||
Calls `lockETH` on the `SharedLockbox` with the `msg.value`. | ||
|
||
- The function MUST call `lockETH` with `msg.value` on the `SharedLockbox` if: | ||
- The token is `ETHER`. | ||
- `msg.value` is greater than zero. | ||
|
||
```mermaid | ||
sequenceDiagram | ||
participant User | ||
participant OptimismPortal | ||
participant SharedLockbox | ||
|
||
User->>OptimismPortal: depositTransaction(...) | ||
OptimismPortal->>SharedLockbox: lockETH() | ||
OptimismPortal->>OptimismPortal: emit TransactionDeposited() | ||
``` | ||
|
||
#### `finalizeWithdrawalTransactionExternalProof` | ||
|
||
Calls `unlockETH` on the `SharedLockbox` with the `tx.value`. | ||
|
||
- The function MUST call `unlockETH` on the `SharedLockbox` if: | ||
- The token is `ETHER`. | ||
- `tx.value` is greater than zero. | ||
- The ETH is received by the `OptimismPortal` and then sent with the withdrawal transaction | ||
|
||
```mermaid | ||
sequenceDiagram | ||
participant User | ||
participant OptimismPortal | ||
participant SharedLockbox | ||
|
||
User->>OptimismPortal: finalizeWithdrawalTransactionExternalProof(...) | ||
OptimismPortal->>SharedLockbox: unlockETH(uint256 value) | ||
SharedLockbox->>OptimismPortal: donateETH() | ||
OptimismPortal->>OptimismPortal: emit WithdrawalFinalized() | ||
``` | ||
|
||
### Invariants | ||
|
||
- It MUST lock the ETH amount on the `SharedLockbox` when on a deposit transaction with value greater than zero | ||
|
||
- It MUST unlock the ETH amount being withdrawn from the `SharedLockbox` if it is greater than zero | ||
|
||
- It MUST NOT hold any ETH balance from any deposit transaction. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
# Shared Lockbox - Upgrade and migration process | ||
|
||
<!-- 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) | ||
- [Add the chain to the op-governed dependency set in `SuperchainConfig`](#add-the-chain-to-the-op-governed-dependency-set-in-superchainconfig) | ||
- [Migrate ETH liquidity from `OptimismPortal` to `SharedLockbox`](#migrate-eth-liquidity-from-optimismportal-to-sharedlockbox) | ||
- [`LiquidityMigrator`](#liquiditymigrator) | ||
- [`OptimismPortal` code upgrade](#optimismportal-code-upgrade) | ||
- [Batch transaction process](#batch-transaction-process) | ||
- [Diagram](#diagram) | ||
- [Future Considerations / Additional Notes](#future-considerations--additional-notes) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
## Overview | ||
|
||
Based on the assumption that a chain joining the op-governed dependency set is an irreversible process, | ||
it is assumed that joining the Shared Lockbox is equivalent to it. | ||
|
||
The upgrade process consists of three main points: | ||
|
||
- Add the chain to the op-governed dependency set in `SuperchainConfig` | ||
- Move ETH liquidity from `OptimismPortal` to `SharedLockbox` | ||
- Upgrade the code of `OptimismPortal` to include the `SharedLockbox` integration | ||
|
||
This process also requires that: | ||
|
||
- `SharedLockbox` is deployed | ||
- `SuperchainConfig` is upgraded to manage the dependency set | ||
- `SystemConfig` is upgraded to the interop contract version | ||
|
||
### Add the chain to the op-governed dependency set in `SuperchainConfig` | ||
|
||
The `SuperchainConfig` contract will be responsible for storing and managing the dependency set. | ||
Its `addChain` function will be used to add the chain to the dependency set and call the `SystemConfig` of each chain | ||
to keep them in sync. | ||
It will also allowlist the corresponding `OptimismPortal`, enabling it to lock and unlock ETH from the `SharedLockbox`. | ||
Once `addChain` is called, the system will be ready to process deposits and withdrawals. | ||
|
||
### Migrate ETH liquidity from `OptimismPortal` to `SharedLockbox` | ||
|
||
The ETH will be transferred from the `OptimismPortal` to the `SharedLockbox` using the `LiquidityMigrator` contract. | ||
This contract functions similarly to upgrades using the `StorageSetter`, being updated immediately before to the real implementation. | ||
Its sole purpose is to transfer the ETH balance. | ||
This approach eliminates the need for adding code to move the liquidity to the lockbox that won't be used again. | ||
|
||
#### `LiquidityMigrator` | ||
|
||
This contract is meant to be used as an intermediate step for the liquidity migration. | ||
Its unique purpose is to transfer the whole ETH balance from `OptimismPortal` to `SharedLockbox`. | ||
This approach avoids adding extra code to the `initialize` function, which could be prone to errors in future updates. | ||
|
||
**Interface and properties** | ||
|
||
**`migrateETH`** | ||
|
||
Transfers the entire ETH balance from the `OptimismPortal` to the `SharedLockbox`. | ||
|
||
```solidity | ||
function migrateETH() external; | ||
``` | ||
|
||
**Invariants** | ||
|
||
- It MUST migrate the whole `OptimismPortal` ETH balance to the `SharedLockbox` | ||
|
||
- It MUST emit `ETHMigrated` when migrating the balance | ||
|
||
### `OptimismPortal` code upgrade | ||
|
||
The `OptimismPortal` will start locking and unlocking ETH through the `SharedLockbox`. | ||
It will continue to handle deposits and withdrawals but won't directly hold the ETH liquidity. | ||
To set this up, the upgrade function will be called via `ProxyAdmin` to implement the new code, | ||
which includes the necessary `SharedLockbox` integration. | ||
|
||
## Batch transaction process | ||
|
||
The approach consists of handling the entire migration process in a single batched transaction. | ||
This transaction will include: | ||
|
||
1. Call `addChain` in the `SuperchainConfig` | ||
- Sending chain ID + system config address | ||
2. Call `upgradeAndCall` in the `ProxyAdmin` for the `OptimismPortal` | ||
- Update provisionally to the `LiquidityMigrator` to transfer the whole ETH balance to the `SharedLockbox` in this call. | ||
3. Call `upgrade` in the `ProxyAdmin` for the `OptimismPortal` | ||
- The `SharedLockbox` address is set as immutable in the new implementation | ||
|
||
The L1 ProxyAdmin owner (L1PAO) will execute this transaction. As the entity responsible for updating contracts, | ||
it has the authority to perform the second and third steps. | ||
For the first step, the L1PAO has to be set as authorized for adding a chain to the op-governed dependency set | ||
on the `SuperchainConfig` when initializing. | ||
This process can be set as a [superchain-ops](https://github.com/ethereum-optimism/superchain-ops) task. | ||
|
||
### Diagram | ||
|
||
```mermaid | ||
sequenceDiagram | ||
participant L1PAO as L1 ProxyAdmin Owner | ||
participant ProxyAdmin as ProxyAdmin | ||
participant SuperchainConfig | ||
participant OptimismPortalProxy as OptimismPortal | ||
participant LiquidityMigrator | ||
participant SharedLockbox | ||
|
||
Note over L1PAO: Start batch | ||
|
||
%% Step 1: Add chain to SuperchainConfig | ||
L1PAO->>SuperchainConfig: addChain(chainId, SystemConfig address) | ||
SuperchainConfig->>SharedLockbox: authorizePortal(OptimismPortal address) | ||
|
||
%% Step 2: Upgrade OptimismPortal to intermediate implementation that transfers ETH | ||
L1PAO->>ProxyAdmin: upgradeAndCall() | ||
ProxyAdmin->>OptimismPortalProxy: Upgrade to LiquidityMigrator | ||
OptimismPortalProxy->>LiquidityMigrator: Call migrateETH() | ||
OptimismPortalProxy->>SharedLockbox: Transfer entire ETH balance | ||
|
||
%% Step 3: Upgrade OptimismPortal to final implementation | ||
L1PAO->>ProxyAdmin: upgrade() | ||
ProxyAdmin->>OptimismPortalProxy: Upgrade to new OptimismPortal implementation | ||
|
||
Note over L1PAO: End batch | ||
``` | ||
|
||
## Future Considerations / Additional Notes | ||
|
||
- Before calling `addChain`, it MUST be ensured that the `chainId` and `systemConfig` match |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We should be clear that its possible to upgrade to this hardfork and not be part of a cluster. The requirement would be that a chain has its own lockbox