-
Notifications
You must be signed in to change notification settings - Fork 22
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
OPCM L1 Upgrades #159
Merged
Merged
OPCM L1 Upgrades #159
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a107124
Add L1 upgrades
maurelian ad9c0ae
--wip--
maurelian b7dcfe2
wip
maurelian 34679b3
Update SimpleStorage example
maurelian f5f9ac8
Clarifications
maurelian e327bb7
Updates
maurelian e72ae37
updates
maurelian 15af9a9
updates
maurelian 2dcece7
updates
maurelian 276176c
updates
maurelian 6e7bc65
updates
maurelian 9a60a5f
Post design review edits
maurelian 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,329 @@ | ||
# Purpose | ||
|
||
Our current upgrade process is onerous and error prone, requiring a significant amount of effort on | ||
the part of both the developer and signers. This process is not scalable enough for the needs of | ||
regular superchain wide upgrades. | ||
|
||
This document outlines an improved and standardized approach to upgrading L1 contracts. | ||
|
||
## Goals | ||
|
||
We take the following as our design goals: | ||
|
||
- **Atomicity:** It should be possible to complete an upgrade of the entire superchain's L1 | ||
contracts within one block. | ||
- **Declarative design:** As much as possible, a developer should be able to simply indicate what | ||
bytecode should be deployed, and what storage values (if any) should be initialized or modified. | ||
- **Deterministic upgrade calldata:** The exact calldata to be submitted during the upgrade should | ||
predictable at the time of the governance proposal. | ||
- **Safety:** Above all the upgrade must be safe, particularly with respect to the things which can | ||
typically go wrong with an upgrade, ie. | ||
- All contracts should be upgraded to the correct implementation. | ||
- Storage layouts should not be incorrectly modified. | ||
- Contracts should not be 're-initializable' | ||
- **Standardization:** We avoid trying to account for all special cases, and instead require contracts to | ||
be implemented in a manner compatible with the solution outlined here. | ||
- **Simplicity:** We avoid adding conventions which will require frequent conversations starting with "ok here's how it works..." | ||
- **Auditability:** An upgrade path should be easy to understand, either by reviewing the source | ||
code as in the case of a forthcoming upgrade, or by reviewing the on-chain history along with | ||
verified source code. | ||
|
||
## Non-goals | ||
|
||
This document is not concerned with: | ||
|
||
1. upgrading predeploys or other L2 contracts. | ||
1. modifications to the ownership system, ie, it does not attempt to modify Safes, modules or | ||
guards. | ||
|
||
More over, it does not present a plan for managing contracts which are not proxied, and not | ||
Multichain Prepared (MCP-L1) contracts (ie. FaultDisputeGame, AnchorStateRegistry). This | ||
work will either include, or else depend on, all L1 contracts becoming either: | ||
|
||
1. Proxied and MCP ready | ||
2. Superchain-shared contracts (proxied or not) | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Summary | ||
|
||
<!-- Most (if not all) documents should have a summary. | ||
While the length will likely be proportional to the length of the full document, | ||
the summary should be as succinct as possible. --> | ||
|
||
TODO | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Problem Statement + Context | ||
|
||
Our upgrades have the following issues: | ||
|
||
1. They are divorced from the implementation, meaning the work of writing the upgrade script occurs | ||
in `superchain-ops` apart from the work of writing the updated code in the `optimism` monorepo. | ||
2. The act of writing the upgrade script is onerous, requiring: | ||
1. A bespoke script with custom assertions. | ||
2. A bespoke Validations.md document | ||
3. The work required to perform the upgrade is difficult for signers, as they must work through the | ||
Validations.md document. This work has taken up to an hour in the past. | ||
4. The effort described above will scale linearly with the number of OP Chains. | ||
|
||
# Proposed Solution | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Each new release will have its own OP Contracts Manager which can perform the following distinct | ||
actions: | ||
|
||
1. Deploy new proxies for a new OP Chain (this work is already covered by [link to Blaine's doc]()) | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
1. Upgrade all existing OP Chains. | ||
1. Deploy any newly added superchain-shared contracts. | ||
1. Upgrade existing superchain-shared contracts. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Note that the above functionality also makes for a good set of milestones, each of which can and | ||
should be implemented incrementally. | ||
|
||
## Declarative storage management | ||
|
||
Taking inspiration from Chugsplash, we wish to separate the concern of what bytecode to run, and | ||
what to write to storage. However we also wish to avoid using the current `StorageSetter` pattern, | ||
which simply provides logic to "write anything anywhere" in storage. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
In order to achieve this, each contract will have its `initializer()` function moved intoa | ||
release specific `__Populator` contract. | ||
|
||
By way of example, we take an initializable version of `SimpleStorage`: | ||
|
||
- the current version of the contract has only a `uint256 foo` variable. | ||
- the new version of the contract introduces a `bytes32 bar` variable. | ||
|
||
The status quo implementation of the new Simple Storage would look like this: | ||
|
||
```solidity | ||
contract SimpleStorage is Initializable { | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
uint public foo; | ||
bytes32 public bar; // new variable added for the current release | ||
|
||
constructor() { | ||
_disableInitializers(); | ||
} | ||
|
||
/// @notice initialize is called whether for a fresh system or when upgrading an existing system. | ||
function initialize(uint _foo, bytes32 _bar) public initializer { | ||
// chains being upgraded need to read the current value of foo, then write it again. | ||
foo = _foo; | ||
bar = _bar; | ||
} | ||
} | ||
``` | ||
|
||
This document proposes that the above contract should be refactored accordingly: | ||
|
||
```solidity | ||
/// @notice The base contract with all runtime logic | ||
contract SimpleStorage { | ||
uint public foo; | ||
bytes32 public bar; // new variable added for the current release. | ||
} | ||
|
||
/// @notice an ephemeral contract which is temporarily set as the implementation during deployment | ||
/// or upgrade. | ||
contract SimpleStoragePopulator is SimpleStorage { | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// @notice Called to initialize a freshly deployed system. | ||
function initialize(uint _foo, bytes32 _bar) public { | ||
foo = _foo; | ||
bar = _bar; | ||
} | ||
|
||
/// @notice Called to upgrade a pre-existing system. | ||
/// This function is only necessary if new values are being added to the storage layout. | ||
function upgrade(bytes32 _bar) public { | ||
bar = _bar; | ||
} | ||
} | ||
``` | ||
|
||
With this architecture, the OPCM contract (which will be a non-proxied singleton per release) would | ||
then either: | ||
|
||
The new contract deployment flow used by the OPCM thus becomes: | ||
|
||
1. Deploy a new proxy | ||
2. If and only if the proxy has state variables: | ||
1. set its implementation to the `SimpleStoragePopulator` | ||
1. call `initialize()` | ||
3. Finally set its implementation to `SimpleStorage` | ||
|
||
And the contract upgrade flow used by the OPCM would be: | ||
|
||
1. If and only if the proxy has NEWLY ADDED state variables: | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
1. set its implementation to the `SimpleStoragePopulator` | ||
1. call `upgrade()` | ||
1. set its implementation to `SimpleStorage` | ||
|
||
This workflow has the benefit of removing the `initializer()` functions, which: | ||
|
||
- create a risk of reinit attacks | ||
- bloat the runtime bytecode size | ||
- are extremely verbose | ||
- require finesse to avoid modifying or resetting values which are not meant to be changed. | ||
|
||
## A single `SuperchainProxyAdmin` | ||
|
||
The existing L1 `ProxyAdmin` (of which there is currently one per OP Chain), will be replaced with a | ||
single L1 `SuperchainProxyAdmin`. The `SuperchainProxyAdmin`, in contrast with the L1 `ProxyAdmin`, | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
will have minimal storage, and will not internally track the type of Proxy which each contract is. | ||
|
||
This contract will be owned by the [Upgrade | ||
Controller](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/stage-1.md#L30), and | ||
will be a simple pass through contract with a minimal interface. | ||
|
||
```solidity | ||
/// @notice forwards calldata to the specified address. | ||
/// Reverts unless called by owner. | ||
function forward(address, bytes) external; | ||
|
||
/// @notice forwards calldata to the specified addresses. | ||
/// Reverts unless called by owner. | ||
function forward(address[], bytes[]) external; | ||
``` | ||
|
||
This architecture makes it possible to manage upgrade authorization for the whole superchain through | ||
a single storage value in a single contract (`SuperchainProxyAdmin.owner`), rather than having a separate contract per OP Chain. | ||
|
||
It also avoids the need for an `sload` to read the `proxyType` mapping for each contract. | ||
|
||
## Identifying OP Chains by the SystemConfig | ||
|
||
We will use the `SystemConfig` as the unique identifier for each OPChain, and get the contract | ||
addresses from it. The custom logic required for the two non-standard proxy types will be handled in | ||
the OPCM contract, and does not require tracking the proxy type (as currently done in the | ||
`ProxyAdmin`), since it is known already for each contract, ie. the only non-standard Proxies are | ||
the `L1StandardBridge` (`L1ChugsplashProxy`) and `L1CrossDomainMessenger` (`ResolvedDelegateProxy`). | ||
|
||
## Contract versioning | ||
|
||
As part of this work, we will cease to version each L1 contract, and instead move to a global L1 | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
contracts version. This version will be stored in the `SystemConfig` and OPCM. For backwards | ||
compatibility, the existing `version()` getters will return the value from `SystemConfig.version()`. | ||
|
||
The `DeployImplementations` script will write the version into the OPCM, which writes it into the | ||
`SystemConfig`. | ||
|
||
The `rc` tag will be included on the OPCM's initial version. It will be programmatically removed | ||
when `upgrade()` is called by the Upgrade Controller multisig, which signifies governance approval. | ||
|
||
## Proof system considerations | ||
|
||
### Adding the PermissionlessDisputeGame to a chain | ||
|
||
Given that not all chains will support the `PermissionlessDisputeGame` upon deployment, an | ||
`OPCM.addGameType()` method will be added which will handle this. | ||
|
||
WIP | ||
|
||
1. ideally we update op-program to remove the chain config, so that it is identical across chains | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
then it reads the config at run time. | ||
But we won't block isthmus on it, and will allow the `FaultDisputeGame` and `PermissionedDisputeGame` to be non-compliant. | ||
2. inputs for each hardfork: one prestatehash per chain. | ||
|
||
## Release process | ||
|
||
As part of the release process, the associated implementation contracts and a new OPCM, will be deployed. | ||
|
||
The Upgrade Controller (which MUST be a Safe), will `DELEGATECALL` the `OPCM.upgrade()` function, | ||
providing the address of the `SuperchainProxyAdmin` and a list of the `SystemConfig` contracts for the | ||
OP Chains to be upgraded. | ||
|
||
Importantly, | ||
|
||
Thus the high level logic for upgrading a contract should be roughly as follows: | ||
|
||
```js | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Some upgrades will | ||
struct NewChainConfig { | ||
address newValues; | ||
} | ||
|
||
Addresses immutable implementations; | ||
|
||
constructor(Addresses _implementations) { | ||
implementations = _implementations; | ||
} | ||
|
||
function upgrade(SuperchainProxyAdmin _admin, ISystemConfig[] _systemConfigs, NewChainConfig[] _newConfigs) public { | ||
for(uint i=0; i< systemConfigs.length; i++) { | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Read the `Addresses` struct from each `SystemConfig` in the `systemConfigs` mapping. | ||
// For each entry in the `Addresses` struct: | ||
// 1. Call the `SuperchainProxyAdmin` to update the implementation to the `Populator` contract | ||
// TODO: OR update the implementation to | ||
// 2. call | ||
// 2. Update the implementation to the final implementation contract. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
// Return ownership of the `SuperchainProxyAdmin` to the Upgrade Controller (`msg.sender`) | ||
// `SuperchainProxyAdmin`. The unique upgrade call encoding required for the non-standard proxies | ||
// should be handled here. | ||
} | ||
``` | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
**Notes:** | ||
|
||
- A new getter should be added to the `SystemConfig` to retrieve the `Addresses` in a single call. | ||
- The `AddressManager` for each chain must be used to upgrade the `L1CrossDomainMessenger`, therefore it should be added to the `Addresses` struct. | ||
|
||
## Development and Testing considerations | ||
|
||
This solution should be implemented such that it both enables and _requires_ developers to implement | ||
the upgrade path from the most recent release to the one curently being developed. This means it | ||
will be necessary to have a deployment of the previous release available on the current commit, in | ||
order to test the upgrade path. | ||
mds1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This can be achieved by storing an L1 Genesis representation of the latest release in the monorepo. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This genesis file can be generated by from a state dump of the currently deployed test system. The | ||
test suite should then be modified so that each test runs twice, one against a system freshly | ||
deployed by the OPCM, and another which was upgraded from the previous release. In CI these two | ||
major test groups can be run in parallel. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Resource Usage | ||
|
||
A rough per-chain estimate (accounting for lookups but not memory) based on the implementation described in [the upgrade process](#the-upgrade-process): | ||
|
||
- Cold `SLOAD` the `SystemConfig` address: 2100 | ||
- Cold `CALL` the `SystemConfig` address: 2600 | ||
- Cold `SLOAD` the `Addresses` struct (excluding `GasPayingToken`): 6 x 2100 | ||
- `CALL` the `SuperchainProxyAdmin`: 2600 | ||
- Do the following 6 times: | ||
- `CALL` the Proxy: 2600 | ||
- Cold `SSTORE` the temporary `Populator` implementation address over a non-zero slot: 5,000 | ||
- Cold `SSTORE` new values: 20,000 each assuming empty slots. | ||
- Warm `CALL` the Proxy again: 100 | ||
- Warm `SSTORE` the final implementation address over a non-zero slot: 100 | ||
|
||
Aggregating the above slightly: | ||
|
||
1. Per chain overhead: 2100 + 2600 + 6 x 2100 = 17,300 | ||
2. Per contract per chain: 2600 + 5000 + 100 + 100 = 7,800 | ||
3. Cost per new value in storage: 20,000 | ||
|
||
Since new values in storage are fairly rare, with the possible exception of the System Config, we'll assume just one new value per-chain, giving a cost of: 17,300 + 6\*7,800 + 20,000 = 84,100 | ||
|
||
This is a lower bound, but if we give a healthy margin of 3x,for | ||
about 250,000 per chain upgraded, we can fit 120 OP Chain upgrades | ||
inside an L1 block. | ||
|
||
We will likely need to handle that limitation at some point in the future, but can defer that for the time being. | ||
|
||
# Alternatives Considered | ||
|
||
1. Keeping the existing L1 Proxy Admin was rejected as being overly redundant and inefficient. | ||
2. | ||
3. Keeping initializers, and the StorageSetter was rejected as more over-prone, though possible simpler for testing purposes. | ||
maurelian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
4. | ||
|
||
# Risks & Uncertainties | ||
|
||
TODO | ||
|
||
--- | ||
|
||
6. We will track the correct addresses of the OPCMs in the registry. If you are upgrading by more than one version, you'll | ||
get the list from there. | ||
|
||
function upgrade() { | ||
require(systemConfig.releaseVersion() < ) | ||
} |
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.
@maurelian The
AnchorStateRegistry
is a possible exception to this where the calldata is based on recent chain state and that state cannot be known at the time of the governance proposal. While we can relax this chain state requirement for initializing the ASR, perhaps it would be good to callout this as design principle when writing smart contract initializers.