Skip to content

Commit

Permalink
Make some private functions internal to allow the developpement of "w…
Browse files Browse the repository at this point in the history
…ithSignature" functions (like permit) (#2568)

* add internal _setOwner in Ownable

* address issues raised in #2567

* updte changelog entry

* improve changelog and documentation

* rephrasing doc

* add cahngelog improvement lost in merge

* notify deprecation of _setupRole in changelog

* Demote caution to note

* Update CHANGELOG.md

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
  • Loading branch information
Amxx and frangio committed Sep 14, 2021
1 parent 5e34a84 commit 7237b16
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* `Ownable`: add an internal `_transferOwnership(address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568))
* `AccessControl`: add internal `_grantRole(bytes32,address)` and `_revokeRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568))
* `AccessControl`: mark `_setupRole(bytes32,address)` as deprecated in favor of `_grantRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568))

## 4.3.1

* `TimelockController`: Add additional isOperationReady check.
Expand Down
16 changes: 14 additions & 2 deletions contracts/access/AccessControl.sol
Expand Up @@ -178,6 +178,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* Using this function in any other way is effectively circumventing the admin
* system imposed by {AccessControl}.
* ====
*
* NOTE: This function is deprecated in favor of {_grantRole}.
*/
function _setupRole(bytes32 role, address account) internal virtual {
_grantRole(role, account);
Expand All @@ -194,14 +196,24 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
emit RoleAdminChanged(role, previousAdminRole, adminRole);
}

function _grantRole(bytes32 role, address account) private {
/**
* @dev Grants `role` to `account`.
*
* Internal function without access restriction.
*/
function _grantRole(bytes32 role, address account) internal virtual {
if (!hasRole(role, account)) {
_roles[role].members[account] = true;
emit RoleGranted(role, account, _msgSender());
}
}

function _revokeRole(bytes32 role, address account) private {
/**
* @dev Revokes `role` from `account`.
*
* Internal function without access restriction.
*/
function _revokeRole(bytes32 role, address account) internal virtual {
if (hasRole(role, account)) {
_roles[role].members[account] = false;
emit RoleRevoked(role, account, _msgSender());
Expand Down
12 changes: 8 additions & 4 deletions contracts/access/Ownable.sol
Expand Up @@ -25,7 +25,7 @@ abstract contract Ownable is Context {
* @dev Initializes the contract setting the deployer as the initial owner.
*/
constructor() {
_setOwner(_msgSender());
_transferOwnership(_msgSender());
}

/**
Expand All @@ -51,7 +51,7 @@ abstract contract Ownable is Context {
* thereby removing any functionality that is only available to the owner.
*/
function renounceOwnership() public virtual onlyOwner {
_setOwner(address(0));
_transferOwnership(address(0));
}

/**
Expand All @@ -60,10 +60,14 @@ abstract contract Ownable is Context {
*/
function transferOwnership(address newOwner) public virtual onlyOwner {
require(newOwner != address(0), "Ownable: new owner is the zero address");
_setOwner(newOwner);
_transferOwnership(newOwner);
}

function _setOwner(address newOwner) private {
/**
* @dev Transfers ownership of the contract to a new account (`newOwner`).
* Internal function without access restriction.
*/
function _transferOwnership(address newOwner) internal virtual {
address oldOwner = _owner;
_owner = newOwner;
emit OwnershipTransferred(oldOwner, newOwner);
Expand Down

0 comments on commit 7237b16

Please sign in to comment.