feat: Set up repo with ACL and upgradeability (SC-550) #1
Conversation
hexonaut
left a comment
hexonaut
left a comment
There was a problem hiding this comment.
We should stick to using OZ AccessControl as it's a solved problem we don't need to keep re-rolling code to do roles.
In this case we may want multiple addresses for relayer and freezer which is easily made with OZ AccessControl.
hexonaut
left a comment
hexonaut
left a comment
There was a problem hiding this comment.
I don't really like having both the AccessControl and the wards pattern combined. Thinking some more this upgradable contract thing may be more trouble than it's worth. Replacing this contract with a newly deployed controller is easy enough, so maybe let's do that instead of dealing with upgradability on the contract.
We will need to do things like setup the rate limits and roles again, but upgradability has extra cognitive load as well. Let's drop the upgradability of the contract to simplify things.
src/L1Controller.sol
Outdated
| /*** Freezer Functions ***/ | ||
| /**********************************************************************************************/ | ||
|
|
||
| function setActive(bool active_) external onlyRole("FREEZER") { |
There was a problem hiding this comment.
The pattern is to hash the string as a constant: https://github.com/marsfoundation/spark-gov-relay/blob/master/src/Executor.sol#L22
|
Coverage after merging sc-550-set-up-repo into master will be
Coverage Report
|
|||||||||||||||||||
hexonaut
left a comment
hexonaut
left a comment
There was a problem hiding this comment.
As I mentioned some logic like freezer only being able to freeze, but not unfreeze should be made, but just to get the scaffolding done this can be merged as-is.
No description provided.