-
Notifications
You must be signed in to change notification settings - Fork 50
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
Create PermissionSplitter #300
Conversation
Signed-off-by: nicholaspai <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
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.
Left a few comments related to documentation and a small nit. LGTM
// Public function! | ||
// Note: these have two underscores in front to prevent any collisions with the target contract. |
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.
Do we want to be following the natspec for this?
It may be beneficial to include these doc comments (/** .. */
or ///
) for better IDE support / readability
// Public function! | ||
// Note: these have two underscores in front to prevent any collisions with the target contract. |
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.
Similar to above
|
||
function _isAllowedToCall(address caller, bytes calldata callData) internal view virtual returns (bool) { | ||
bytes4 selector; | ||
if (callData.length < 4) { |
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.
Nit: would we want to store this in a constant? The 4 seems like a magic number
This contract is a proxy that owns another contract and can split permissions by role based on what method is being called. All methods are always callable by the owner of the proxy.