feat: Add OPCM upgrades FMA #173
Conversation
Co-authored-by: blaine <[email protected]>
Co-authored-by: blaine <[email protected]>
Co-authored-by: blaine <[email protected]>
There was a problem hiding this comment.
Did my second pass of this pr. Not approving just yet as I've left some question that will help me better understand some sections.
Out of all the failure modes 'FM4: Failure to follow upgrade path' jumps out to me as the most likely to hit (mainly because humans are heavily involved). Making sure that the superchain-registry is properly updated is super important here. I think we may need to define a process around this to make sure it always contains the correct list of OPCM addresses.
security/fma-l1-upgrades.md
Outdated
| #### Action items: | ||
|
|
||
| - [ ] Expose the upgraded state of the contracts being upgraded. | ||
| - [ ] Add a check to the OPCM to verify the upgraded state of the contracts being upgraded. |
There was a problem hiding this comment.
What does this look like in practice? Can you give me an example to help me understand?
I see the spec contains a comment in the OPCM upgrade function:
// run safety assertions to validate the upgrade
Is this comment related to this action item?
There was a problem hiding this comment.
the current upgrade doesn't give us much that we can make assertions on, at least not without going into the nitty-gritty details (ie. checking all the getters, etc).
We are not yet able to check init'd status on-chain.
We could check that semvers are set as expected?
At the least we should create an internal _postCheck() function in the OPCM which prompts developers to define whatever checks are appropriate while creating an upgrade.
Co-authored-by: blaine <[email protected]>
security/fma-l1-upgrades.md
Outdated
| | Author | Maurelian | | ||
| | Created at | 2024-03-26 | | ||
| | Initial Reviewers | @blmalone | | ||
| | Need Approval From | [TBD] | |
There was a problem hiding this comment.
| | Need Approval From | [TBD] | | |
| | Need Approval From | @blmalone | |
FMA associated with OPCM Upgrades.