-
Notifications
You must be signed in to change notification settings - Fork 242
revokeAll function #459
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: next
Are you sure you want to change the base?
revokeAll function #459
Changes from 4 commits
8c1a871
cd06917
c8c0d18
0ed9484
b2be01e
a9421cb
324bde0
9294f59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { | |
| string private constant ERROR_AUTH_INIT_KERNEL = "ACL_AUTH_INIT_KERNEL"; | ||
| string private constant ERROR_AUTH_NO_MANAGER = "ACL_AUTH_NO_MANAGER"; | ||
| string private constant ERROR_EXISTENT_MANAGER = "ACL_EXISTENT_MANAGER"; | ||
| string private constant ERROR_ROLE_ERA_INCREMENT = "ACL_ROLE_ERA_INCREMENT"; | ||
|
|
||
| // Whether someone has a permission | ||
| mapping (bytes32 => bytes32) internal permissions; // permissions hash => params hash | ||
|
|
@@ -54,6 +55,9 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { | |
| // Who is the manager of a permission | ||
| mapping (bytes32 => address) internal permissionManager; | ||
|
|
||
| // Eras for the different roles | ||
| mapping (bytes32 => uint256) internal roleEras; | ||
|
|
||
| event SetPermission(address indexed entity, address indexed app, bytes32 indexed role, bool allowed); | ||
| event SetPermissionParams(address indexed entity, address indexed app, bytes32 indexed role, bytes32 paramsHash); | ||
| event ChangePermissionManager(address indexed app, bytes32 indexed role, address indexed manager); | ||
|
|
@@ -145,6 +149,21 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { | |
| _setPermission(_entity, _app, _role, NO_PERMISSION); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Revokes all permissions for a specified role if allowed. This requires `msg.sender` to be the the permission manager | ||
| * @notice Revoke from all the ability to perform actions requiring `_role` on `_app` | ||
| * @param _app Address of the app in which the role will be revoked | ||
| * @param _role Identifier for the group of actions in app being revoked | ||
| */ | ||
| function revokeAll(address _app, bytes32 _role) | ||
| external | ||
| onlyPermissionManager(_app, _role) | ||
| { | ||
| uint256 newRoleEra = roleEras[roleHash(_app, _role)] + 1; | ||
| require(newRoleEra >= roleEras[roleHash(_app, _role)], ERROR_ROLE_ERA_INCREMENT); | ||
| roleEras[roleHash(_app, _role)] = newRoleEra; | ||
|
||
| } | ||
|
|
||
| /** | ||
| * @notice Set `_newManager` as the manager of `_role` in `_app` | ||
| * @param _newManager Address for the new manager | ||
|
|
@@ -470,7 +489,13 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { | |
| return keccak256(abi.encodePacked("ROLE", _where, _what)); | ||
| } | ||
|
|
||
| function permissionHash(address _who, address _where, bytes32 _what) internal pure returns (bytes32) { | ||
| return keccak256(abi.encodePacked("PERMISSION", _who, _where, _what)); | ||
| function permissionHash(address _who, address _where, bytes32 _what) internal view returns (bytes32) { | ||
| uint256 roleEra = roleEras[roleHash(_where, _what)]; | ||
|
|
||
| // Backward compatibility for DAOs with earlier versions of the ACL | ||
| if (roleEra == 0) | ||
|
||
| return keccak256(abi.encodePacked("PERMISSION", _who, _where, _what)); | ||
|
|
||
| return keccak256(abi.encodePacked("PERMISSION", roleEra, _who, _where, _what)); | ||
| } | ||
| } | ||
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 might as well use
SafeMathhere :).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.
Sure, although it seems really unlikely that this function gets called
2^256times for the same app and role, I guess you mean instead of the next line, right?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 added SafeMath and will remove the require on the next line :)