-
Notifications
You must be signed in to change notification settings - Fork 100
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
Update for single step upgrades #486
base: 12-20-fixes_to_opcm_spec
Are you sure you want to change the base?
Update for single step upgrades #486
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. This stack of pull requests is managed by Graphite. Learn more about stacking. |
1. the first upgrade is to an `InitializerResetter` which resets the `initialized` value. | ||
1. the implementation is updated to the final address and `upgrade()` is called on that address. | ||
3. For each address: | ||
1. If it is receiving new state variables (only the SystemConfig for Isthmus), a call is made to: |
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 don't think its great to leak the implementation details like only sys config for isthmus into the spec. How I would propose structuring this is having a section "Network Upgrades" and then having a subheading for each network upgrade and define the specifics for that upgrade in there
Thus for Isthmus, the System config will receive the following new function. | ||
|
||
```solidity | ||
function upgradeIsthmus(IsthmusConfig _isthmusConfig) external; |
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.
What is the definition of this config?
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.
specs/specs/experimental/op-contracts-manager.md
Lines 214 to 220 in bb119d0
```solidity | |
struct IsthmusConfig { | |
uint32 public operatorFeeScalar; | |
uint64 public operatorFeeConstant; | |
} | |
function upgrade(ISystemConfig[] _systemConfigs, IProxyAdmin[] _proxyAdmins, IsthmusConfig[] _isthmusConfigs) public; |
This approach requires that all contracts have an `upgrade()` function which sets the `initialized` | ||
value to `true`. The `upgrade` function body should be empty unless it is used to set a new state | ||
variable added to that contract since the last upgrade. | ||
Thus for Isthmus, the System config will receive the following new 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.
We would replace this function in the next release and put upgradeKarst(KarstConfig _config)
?
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.
hypothetically if there was another change
Changes: