Remove 'external' functions (#2162)

* Remove _grantRole and _revokeRole, replace with _setupRole

* Make all external AccessControl functions public

* Remove Ownable._transferOwnership

* Rename ERC721's _safeTransferFrom and _transferFrom to _safeTransfer and _transfer

* Make all ERC721 external functions public

* Make all miscelaneous external functions public instead

* Add changelog entry

* Move calldata arguments to memory

* Update contracts/access/AccessControl.sol

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Restrict setupRole to the constructor

* Replace isConstructor for !isContract

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
This commit is contained in:
Nicolás Venturo
2020-04-02 15:29:58 -03:00
committed by GitHub
parent 1bc923b6a2
commit 5b5d91c9d4
13 changed files with 77 additions and 63 deletions

View File

@ -1,6 +1,7 @@
pragma solidity ^0.6.0;
import "../utils/EnumerableSet.sol";
import "../utils/Address.sol";
import "../GSN/Context.sol";
/**
@ -25,10 +26,7 @@ import "../GSN/Context.sol";
* }
* ```
*
* Roles can be granted and revoked programatically by calling the `internal`
* {_grantRole} and {_revokeRole} functions.
*
* This can also be achieved dynamically via the `external` {grantRole} and
* Roles can be granted and revoked dynamically via the {grantRole} and
* {revokeRole} functions. Each role has an associated admin role, and only
* accounts that have a role's admin role can call {grantRole} and {revokeRoke}.
*
@ -39,6 +37,7 @@ import "../GSN/Context.sol";
*/
abstract contract AccessControl is Context {
using EnumerableSet for EnumerableSet.AddressSet;
using Address for address;
struct RoleData {
EnumerableSet.AddressSet members;
@ -52,9 +51,8 @@ abstract contract AccessControl is Context {
/**
* @dev Emitted when `account` is granted `role`.
*
* `sender` is the account that originated the contract call:
* - if using `grantRole`, it is the admin role bearer
* - if using `_grantRole`, its meaning is system-dependent
* `sender` is the account that originated the contract call, an admin role
* bearer except when using {_setupRole}.
*/
event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
@ -64,7 +62,6 @@ abstract contract AccessControl is Context {
* `sender` is the account that originated the contract call:
* - if using `revokeRole`, it is the admin role bearer
* - if using `renounceRole`, it is the role bearer (i.e. `account`)
* - if using `_renounceRole`, its meaning is system-dependent
*/
event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
@ -105,20 +102,21 @@ abstract contract AccessControl is Context {
*
* To change a role's admin, use {_setRoleAdmin}.
*/
function getRoleAdmin(bytes32 role) external view returns (bytes32) {
function getRoleAdmin(bytes32 role) public view returns (bytes32) {
return _roles[role].adminRole;
}
/**
* @dev Grants `role` to `account`.
*
* Calls {_grantRole} internally.
* If `account` had not been already granted `role`, emits a {RoleGranted}
* event.
*
* Requirements:
*
* - the caller must have `role`'s admin role.
*/
function grantRole(bytes32 role, address account) external virtual {
function grantRole(bytes32 role, address account) public virtual {
require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to grant");
_grantRole(role, account);
@ -127,13 +125,13 @@ abstract contract AccessControl is Context {
/**
* @dev Revokes `role` from `account`.
*
* Calls {_revokeRole} internally.
* If `account` had been granted `role`, emits a {RoleRevoked} event.
*
* Requirements:
*
* - the caller must have `role`'s admin role.
*/
function revokeRole(bytes32 role, address account) external virtual {
function revokeRole(bytes32 role, address account) public virtual {
require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to revoke");
_revokeRole(role, account);
@ -146,11 +144,14 @@ abstract contract AccessControl is Context {
* purpose is to provide a mechanism for accounts to lose their privileges
* if they are compromised (such as when a trusted device is misplaced).
*
* If the calling account had been granted `role`, emits a {RoleRevoked}
* event.
*
* Requirements:
*
* - the caller must be `account`.
*/
function renounceRole(bytes32 role, address account) external virtual {
function renounceRole(bytes32 role, address account) public virtual {
require(account == _msgSender(), "AccessControl: can only renounce roles for self");
_revokeRole(role, account);
@ -160,23 +161,16 @@ abstract contract AccessControl is Context {
* @dev Grants `role` to `account`.
*
* If `account` had not been already granted `role`, emits a {RoleGranted}
* event.
*/
function _grantRole(bytes32 role, address account) internal virtual {
if (_roles[role].members.add(account)) {
emit RoleGranted(role, account, _msgSender());
}
}
/**
* @dev Revokes `role` from `account`.
* event. Note that unlike {grantRole}, this function doesn't perform any
* checks on the calling account.
*
* If `account` had been granted `role`, emits a {RoleRevoked} event.
* Requirements:
*
* - this function can only be called from a constructor.
*/
function _revokeRole(bytes32 role, address account) internal virtual {
if (_roles[role].members.remove(account)) {
emit RoleRevoked(role, account, _msgSender());
}
function _setupRole(bytes32 role, address account) internal virtual {
require(!address(this).isContract(), "AccessControl: roles cannot be setup after construction");
_grantRole(role, account);
}
/**
@ -185,4 +179,16 @@ abstract contract AccessControl is Context {
function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
_roles[role].adminRole = adminRole;
}
function _grantRole(bytes32 role, address account) private {
if (_roles[role].members.add(account)) {
emit RoleGranted(role, account, _msgSender());
}
}
function _revokeRole(bytes32 role, address account) private {
if (_roles[role].members.remove(account)) {
emit RoleRevoked(role, account, _msgSender());
}
}
}