-
Notifications
You must be signed in to change notification settings - Fork 12.4k
Add AccessControlDefaultAdminRules #4009
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
Merged
frangio
merged 78 commits into
OpenZeppelin:master
from
ernestognw:lib-618-accesscontrol-admin-rules
Feb 24, 2023
Merged
Changes from 8 commits
Commits
Show all changes
78 commits
Select commit
Hold shift + click to select a range
3989054
Create `AccessControlAdminRules` and its corresponding interface
ernestognw da52678
Add tests
ernestognw f7738a6
Add changeset
ernestognw e2eb45d
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw 68adaf5
Correctly set unlocked crite
ernestognw 0aff3ff
Completed docs
ernestognw 4090a3e
Fix linter
ernestognw 21aacf9
Refactor tests so coverage is not reduced
ernestognw 2863047
Address PR review comments
ernestognw a9a3495
Update contracts/access/AccessControlAdminRules.sol
ernestognw 59f3b62
Add draft-ERC5313
ernestognw 2bfb028
Remove property-based test
ernestognw 7ae4aa7
Fix typo
ernestognw 2d630c3
Add IERC5313 docs
ernestognw 192c5bc
Change owner for admin
ernestognw 021ee37
Call overrideable functions
ernestognw a54dd2b
Update IERC5313 to final
ernestognw 85cff36
Reword private function
ernestognw 8baa13b
Remove property test again
ernestognw fb895d2
Rename again
ernestognw 3fbfbac
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw b70ae34
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw d120c1d
Update .changeset/silent-dancers-type.md
ernestognw 4322ed7
Update contracts/access/AccessControl.sol
ernestognw d52b371
Update docs/modules/ROOT/pages/access-control.adoc
ernestognw 54184af
Rename to defaultAdmin
ernestognw d1064f4
Update contracts/access/AccessControlAdminRules.sol
ernestognw d352694
Update contracts/access/AccessControlAdminRules.sol
ernestognw 53e8087
Rename delayedUntil
ernestognw 56b5df6
Lint
ernestognw 30306c3
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw 701d341
Small fixes
ernestognw 8ebb27b
More typos and minor corrections
ernestognw f7c6596
Allow defaultAdmin to start transfer again
ernestognw 5092d70
Add extra test
ernestognw 4642a12
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw 630df0d
Rename to AccessControlDefaultAdminRules
ernestognw 07d12a2
Removed IAccessControlDefaultAdminRules
ernestognw 41f4ba3
Add note in Extending Contracts section
ernestognw 6da27ad
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 4b39c30
Fix comments example
ernestognw da8227f
Add virtual to delay()
ernestognw 6139905
Replace _delay with getter
ernestognw f420162
Add IAccessControlDefaultAdmin back
ernestognw 8747bf4
Created interal functions for beginning and accepting transfers
ernestognw 444908c
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw 5743d7a
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 0b9ea0b
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 780e901
Fixed test error message
ernestognw 24ce7fa
Change delay to defaultAdminDelay
ernestognw a8bbf18
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw 2c4e15e
Change long variable name
ernestognw d171299
Run prettier
ernestognw 281209c
Remove IAccessControlDefaultAdminRules from docs
ernestognw 2c49a5d
Remove camel case from error messages
ernestognw cb98ad9
Update test/access/AccessControl.behavior.js
ernestognw 88168be
Update test/access/AccessControl.behavior.js
ernestognw 39d176a
Use beforeEach in begin transfer tests
ernestognw 9e52530
Update test/access/AccessControl.behavior.js
ernestognw 4721d63
Update test/access/AccessControl.behavior.js
ernestognw 0e532c7
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 70c5cae
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 667e3da
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw 5a31431
Remove unnecessary async
ernestognw 14994be
Change `is met` for `has passed`
ernestognw fd64f5d
Change note in docs
ernestognw 9e04336
Update contracts/access/AccessControlDefaultAdminRules.sol
ernestognw ac093f4
Simplify renouncing tests
ernestognw 3948246
Extend cancel tests
ernestognw 34ed74f
Rename delay test variables
ernestognw 880ac0b
Remove confusing comment
ernestognw 3337f61
Replace met with passed
ernestognw a43abe7
Simplified acceptance tests
ernestognw b4ee08a
Add IAccessControlEnumerable interface removed accidentally from docs
ernestognw b2b9e81
Merge branch 'master' into lib-618-accesscontrol-admin-rules
ernestognw d1289a2
Merge branch 'master' into lib-618-accesscontrol-admin-rules
frangio 7d37c81
fix flaky tests
frangio b164929
lint
frangio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'openzeppelin-solidity': minor | ||
| --- | ||
|
|
||
| `AccessControlAdminRules`: Add extension to `AccessControl` to add extra conditions to manage the `DEFAULT_ADMIN_ROLE` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,220 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // OpenZeppelin Contracts (last updated v4.8.0) (access/AccessControlAdminRules.sol) | ||
|
|
||
| pragma solidity ^0.8.0; | ||
|
|
||
| import "./AccessControl.sol"; | ||
| import "./IAccessControlAdminRules.sol"; | ||
| import "../utils/math/SafeCast.sol"; | ||
|
|
||
| /** | ||
| * @dev Extension of {AccessControl} that allows to specify special rules to manage | ||
| * the `DEFAULT_ADMIN_ROLE` 's owner, which is a sensitive role with special permissions | ||
| * over other valid roles that may potentially have rights. | ||
| * | ||
| * If a specific role doesn't have an `adminRole` assigned, the holder of the | ||
| * `DEFAULT_ADMIN_ROLE` will have the ability to manage it, as determined by the | ||
| * function {getRoleAdmin}'s default value (`address(0)`). | ||
| * | ||
| * This contract implements the following risk mitigations on top of the AccessControl implementation: | ||
| * | ||
| * * Only one account holds the `DEFAULT_ADMIN_ROLE` at every time after construction except when renounced. | ||
| * * Enforce a 2-step process to transfer the `DEFAULT_ADMIN_ROLE` to another account. | ||
| * - Even when it's been renounced. | ||
| * * Enforce a configurable delay between the two steps, with the ability to cancel in between. | ||
| * - Even after the timer has passed to avoid locking it forever. | ||
| * * The `DEFAULT_ADMIN_ROLE` 's admin can be only held by itself. | ||
| * | ||
| * Once you understand what the {constructor} parameters, you can use this reference implementation: | ||
| * | ||
| * ```solidity | ||
| * contract MyToken is AccessControlAdminRules { | ||
| * constructor() AccessControlAdminRules( | ||
| * 60 * 60 * 24, // 3 days | ||
| * _msgSender() // Explicit initial `DEFAULT_ADMIN_ROLE` holder | ||
| * ) {} | ||
| *} | ||
| * ``` | ||
| * | ||
| * NOTE: The `delay` is only configurable in the constructor to avoid issues related with handling | ||
| * delay management during the transfer is pending to be completed. | ||
| * | ||
| * _Available since v4.9._ | ||
| */ | ||
| abstract contract AccessControlAdminRules is IAccessControlAdminRules, AccessControl { | ||
| uint48 private immutable _delay; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| address private _currentAdmin; | ||
| uint48 private _delayedUntil; | ||
|
|
||
| address private _pendingAdmin; | ||
Amxx marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @dev Sets the initial values the internal delay and {owner}. | ||
| * | ||
| * The `delay` value is immutable. It can only be set at the constructor. | ||
| */ | ||
| constructor(uint48 delay, address initialAdmin) { | ||
Amxx marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| _delay = delay; | ||
| _grantRole(DEFAULT_ADMIN_ROLE, initialAdmin); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-owner} | ||
| */ | ||
| function owner() public view virtual returns (address) { | ||
| return _currentAdmin; | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-delayedUntil} | ||
| */ | ||
| function delayedUntil() public view virtual returns (uint48) { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return _delayedUntil; | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-pendingAdmin} | ||
| */ | ||
| function pendingAdmin() public view virtual returns (address) { | ||
| return _pendingAdmin; | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IERC165-supportsInterface}. | ||
| */ | ||
| function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { | ||
| return interfaceId == type(IAccessControlAdminRules).interfaceId || super.supportsInterface(interfaceId); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-beginAdminTransfer} | ||
| */ | ||
| function beginAdminTransfer(address newAdmin) public override onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| require(delayedUntil() == 0, "AccessControl: pending admin already set"); | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| _delayedUntil = SafeCast.toUint48(block.timestamp) + _delay; | ||
| _pendingAdmin = newAdmin; | ||
| emit AdminRoleChangeStarted(pendingAdmin(), delayedUntil()); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-acceptAdminTransfer} | ||
| */ | ||
| function acceptAdminTransfer() public { | ||
| address pendingAdminOwner = pendingAdmin(); | ||
| require( | ||
| _adminTransferIsUnlocked() && _msgSender() == pendingAdminOwner, | ||
| "AccessControl: delay must be met and caller must be pending admin" | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ); | ||
| _revokeRole(DEFAULT_ADMIN_ROLE, owner()); | ||
| _grantRole(DEFAULT_ADMIN_ROLE, pendingAdminOwner); | ||
| _resetAdminTransfer(); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IAccessControlAdminRules-cancelAdminTransfer} | ||
| */ | ||
| function cancelAdminTransfer() external override onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| _resetAdminTransfer(); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Revokes `role` from the calling account. | ||
| * | ||
| * For `DEFAULT_ADMIN_ROLE`, only allows renouncing in two steps, so it's required | ||
| * that the {delayedUntil} is met and the pending admin is the zero address. | ||
| * After its execution, it will not be possible to call `onlyRole(DEFAULT_ADMIN_ROLE)` | ||
| * functions. | ||
| * | ||
| * For other roles, see {AccessControl-renounceRole}. | ||
| * | ||
| * NOTE: Renouncing `DEFAULT_ADMIN_ROLE` will leave the contract without an owner, | ||
| * thereby disabling any functionality that is only available to the default admin, and the | ||
| * possibility of reassigning a non-administrated role. | ||
| */ | ||
| function renounceRole(bytes32 role, address account) public override(IAccessControl, AccessControl) { | ||
| if (role == DEFAULT_ADMIN_ROLE) { | ||
| require( | ||
| pendingAdmin() == address(0) && _adminTransferIsUnlocked(), | ||
| "AccessControl: admin can only renounce in two delayed steps" | ||
| ); | ||
| } | ||
| super.renounceRole(role, account); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {AccessControl-grantRole}. Reverts for `DEFAULT_ADMIN_ROLE`. | ||
| */ | ||
| function grantRole( | ||
| bytes32 role, | ||
| address account | ||
| ) public override(IAccessControl, AccessControl) onlyRole(getRoleAdmin(role)) { | ||
| require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't directly grant admin role"); | ||
| super.grantRole(role, account); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {AccessControl-revokeRole}. Reverts for `DEFAULT_ADMIN_ROLE`. | ||
| */ | ||
| function revokeRole( | ||
| bytes32 role, | ||
| address account | ||
| ) public override(IAccessControl, AccessControl) onlyRole(getRoleAdmin(role)) { | ||
| require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't directly revoke admin role"); | ||
| super.revokeRole(role, account); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {AccessControl-_setRoleAdmin}. Reverts for `DEFAULT_ADMIN_ROLE`. | ||
| */ | ||
| function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal override { | ||
| require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't override admin's admin"); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| super._setRoleAdmin(role, adminRole); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Grants `role` to `account`. | ||
| * | ||
| * For `DEFAULT_ADMIN_ROLE`, it only allows granting if there isn't already a role's owner | ||
| * or if the role has been previously renounced. | ||
| * | ||
| * For other roles, see {AccessControl-renounceRole}. | ||
| * | ||
| * NOTE: Exposing this function through another mechanism may make the | ||
| * `DEFAULT_ADMIN_ROLE` assignable again. Make sure to guarantee this is | ||
| * the expected behavior in your implementation. | ||
| */ | ||
| function _grantRole(bytes32 role, address account) internal override { | ||
| if (role == DEFAULT_ADMIN_ROLE) { | ||
| require(owner() == address(0), "AccessControl: admin already granted"); | ||
| _currentAdmin = account; | ||
| } | ||
| super._grantRole(role, account); | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {AccessControl-_revokeRole}. | ||
| */ | ||
| function _revokeRole(bytes32 role, address account) internal override { | ||
| if (role == DEFAULT_ADMIN_ROLE) { | ||
| delete _currentAdmin; | ||
| } | ||
| super._revokeRole(role, account); | ||
| } | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @dev Resets the pending admin and delayed until. | ||
| */ | ||
| function _resetAdminTransfer() private { | ||
| delete _pendingAdmin; | ||
| delete _delayedUntil; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Checks if a {delayedUntil} has been set and met. | ||
| */ | ||
| function _adminTransferIsUnlocked() private view returns (bool) { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| uint48 delayedUntilTimestamp = delayedUntil(); | ||
| return delayedUntilTimestamp > 0 && delayedUntilTimestamp < block.timestamp; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // OpenZeppelin Contracts v4.9.0 (access/IAccessControlAdminRules.sol) | ||
|
|
||
| pragma solidity ^0.8.0; | ||
|
|
||
| import "./IAccessControl.sol"; | ||
|
|
||
| /** | ||
| * @dev External interface of AccessControladminRules declared to support ERC165 detection. | ||
| * | ||
| * _Available since v4.9._ | ||
| */ | ||
| interface IAccessControlAdminRules is IAccessControl { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /** | ||
| * @dev Emitted when an `DEFAULT_ADMIN_ROLE` transfer is started, setting `newAdmin` | ||
| * as the next owner to be claimed after `delayedUntil` is met. | ||
| */ | ||
| event AdminRoleChangeStarted(address indexed newAdmin, uint48 delayedUntil); | ||
|
|
||
| /** | ||
| * @dev Returns the address of the current `DEFAULT_ADMIN_ROLE` owner. | ||
| */ | ||
| function owner() external view returns (address); | ||
|
|
||
| /** | ||
| * @dev Returns the timestamp in which the pending admin can claim the | ||
| * `DEFAULT_ADMIN_ROLE` role. | ||
| */ | ||
| function delayedUntil() external view returns (uint48); | ||
|
|
||
| /** | ||
| * @dev Returns the address of the pending `DEFAULT_ADMIN_ROLE` owner. | ||
| */ | ||
| function pendingAdmin() external view returns (address); | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @dev Starts a `DEFAULT_ADMIN_ROLE` transfer by setting a pending admin and | ||
| * a timer to be met. | ||
| * | ||
| * Requirements: | ||
| * | ||
| * - There shouldn't be another admin transfer in progress. See {cancelAdminTransfer}. | ||
| * - Can only be called by the current `DEFAULT_ADMIN_ROLE` owner. | ||
| * | ||
| * Emits an {AdminRoleChangeStarted}. | ||
| */ | ||
| function beginAdminTransfer(address newAdmin) external; | ||
|
|
||
| /** | ||
| * @dev Completes a `DEFAULT_ADMIN_ROLE` transfer. | ||
| * | ||
| * Requirements: | ||
| * | ||
| * - Caller should be the pending admin. | ||
| * - `DEFAULT_ADMIN_ROLE` should be granted to the caller. | ||
| * - `DEFAULT_ADMIN_ROLE` should be revoked from the previous owner. | ||
| * - Should allow to call {beginAdminTransfer} again. | ||
| */ | ||
| function acceptAdminTransfer() external; | ||
|
|
||
| /** | ||
| * @dev Cancels a `DEFAULT_ADMIN_ROLE` transfer. | ||
| * | ||
| * Requirements: | ||
| * | ||
| * - Should allow to call {beginAdminTransfer} again. | ||
| * - Can be called even after the timer is met. | ||
| * - Can only be called by the current `DEFAULT_ADMIN_ROLE` owner. | ||
| */ | ||
| function cancelAdminTransfer() external; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.