-
Notifications
You must be signed in to change notification settings - Fork 48
Add L2 Contract Upgrades design doc #351
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
Conversation
protocol/l2-contract-upgrades.md
Outdated
| 1. deploy new implementations | ||
| 2. deploy a new `L2ContractsManager` | ||
| 3. update the `ProxyAdmin` (This step will only be needed the first time this scheme is used) | ||
| 4. execute a call to `ProxyAdmin.upgradePredeploys()` | ||
| 1. This function will `DELEGATECALL` the new `L2ContractsManager`'s `upgrade()` function. | ||
| 2. For all predeploys being upgrade, `L2ContractsManager.upgrade()` will make a call to that predeploy's `upgradeTo()` function |
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.
There is currently 1M gas available to do all of this in a way that is guaranteed to be safe. This is because upgrade transactions must be included in the block but also must fit within the block gas limit. Typically the L2 block gas limit is set pretty high and there's lots of room for upgrade transactions, but it's only guaranteed to be at least SystemConfig.minimumGasLimit() and _resourceConfig.systemTxMaxGas is set to 1000000 (though not sure how guaranteed that is across chains).
The simplest fix would be to increase systemTxMaxGas to ensure L2 block gas limits are set to something reasonable and that we can keep upgrade transactions below. Otherwise there was a suggestion from Seb to automatically increase the block gas limit to ensure upgrade transactions fit which would also work.
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.
Ouch, there is almost no way that 1M is going to be sufficient, thanks for calling that out.
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.
I will work with Seb to properly address this. It's not expected to be a blocker.
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.
Yes, we could increase the systemTxMaxGas, but then that change would also need to be propagated on L1 and I don't know which other places. And it would still not be future proof. Do we set it to 10M? What happens if we suddenly need 15M? Do we raise it again, including all the L1 work to do this?
Another solution that would always guarantee there's enough gas for the upgrade transactions, no matter how small the regular gasLimit, is to introduce the concept of additional upgrade gas (or maxing with the gas limit). I've implemented this for Isthmus already but it was then decided not to include: ethereum-optimism/optimism#14797 (The user tx omission part of this has now been included in Jovian). The actual changes necessary for upgrade gas are in file op-node/rollup/derive/attributes.go, it's only a few lines of code.
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 definitely do the 'additional gas per upgrade' approach here. I've refelcted this in the doc: 642d046
|
|
||
| ### New ConditionalDeployer | ||
|
|
||
| A new `ConditionalDeployer` contract is needed to ensure that step 2 "deploy new implementations" can preserve the following properties of the L1 OPCM system: |
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.
Is the intention to deploy the conditional deployer on every hard fork? I think it only needs to be done once, and if so, perhaps it's worth specifying it just like we do with ProxyAdmin. What do you think?
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.
good call out, addressed in this commit
sebastianst
left a comment
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.
I'm happy with the overall design! Thanks @maurelian! We can work out the details as we implement it.
| 3. regenerating the bundle | ||
| 4. verifying that it matches | ||
|
|
||
| The NUT bundle will be stored in the monorepo, will be generated by a `just` command, and tracked by git. |
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.
It's important that the NUTs for a particular fork are generated on-demand by a just command so that we can control when we want to generate new NUTs. We can work out the details as we implement this, but maybe something like just nuts karst would generate and possibly override an existing NUT JSON bundle at file op-code/upgrades/nuts/nuts-karst.json and the JSON would have a field that stores the commit hash, and when CI detects a new or changed NUT JSON bundle it confirms that it can be generated from the commit. Although we need to solve the chicken'n'egg problem because we don't know the commit hash of the merge commit that merges the PR that updates the NUTs...
Purpose
To enable deterministic, well tested, hard fork driven upgrades of L2 contracts, with both the implementation and upgrade path written in Solidity.
The customers for this work include:
Summary
The proposed design maintains the current method of injecting Network Upgrade Transactions (NUTs) at a specific fork block height, while improving on the development and testing process in order to better enable safe, well-tested, multi-client upgrades of L2 contracts.
This is achieved by putting the transaction data into a JSON file, and then executing it at the specified fork block.
The design aims to create a solution which parallels the OPCM, but for L2.