-
Notifications
You must be signed in to change notification settings - Fork 46
add design doc for opcm improvements #303
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
base: main
Are you sure you want to change the base?
Conversation
2. **Validation Consolidation**: After the above, add `validate()` calls to OPCM's `deploy()` and `upgrade()` methods | ||
3. **Redundant Assertion Removal**: Remove duplicate validation logic from deploy scripts and other contracts, if they do not exist in the StandardValidator, they would be added to it. | ||
4. **Dispute Game Validation Enhancement**: Improve StandardValidator's dispute game validation logic | ||
5. **Remove proxyAdmin from OpChainConfig**: Remove the `proxyAdmin` field from the `OpChainConfig` struct used as input for `OPCM.upgrade()` |
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.
Something to look into is if we could/should either:
- hardcode a default
prestateHash
, or - pass in a single
prestateHash
toupgrade()/
updatePrestate()` rather than including it in the array.
We should ask Proofs about this.
|
||
### 2. Validation Integration in OPCM Methods | ||
|
||
Add `validate()` calls to OPCM's core methods: |
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 need to ensure that this won't brick functions due to allowed config overrides. ie. if a chain has a different L1PAO, there will be a non-empty error string.
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.
Should there be support for overrides passed into upgrade, deploy functions?
|
||
### Resource Usage | ||
|
||
- Running the StandardValidator's `validate()` function after upgrade or deploy functions might reduce the amount of these calls can be done within a block, though not by a significant amount. |
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 consider what the cost of running validate()
is as it could limit how many upgrades we can do within a single transaction.
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.
yeah i will do this after the new assertions are added
add design doc for opcm improvements