Skip to content
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

Make some private functions internal to allow the developpement of "withSignature" functions (like permit) #2568

Merged
merged 12 commits into from Sep 14, 2021
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 {
Amxx marked this conversation as resolved.
Show resolved Hide resolved
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 {
Amxx marked this conversation as resolved.
Show resolved Hide resolved
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