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

Remove 'external' functions #2162

Merged
merged 11 commits into from
Apr 2, 2020
24 changes: 20 additions & 4 deletions contracts/access/AccessControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,15 @@ abstract contract AccessControl is Context {
* @dev Grants `role` to `account`.
*
* If `account` had not been already granted `role`, emits a {RoleGranted}
* event.
* event. Note that unlike {grantRole}, this function doesn't perform any
* checks on the calling account.
*
* Requirements:
*
* NOTE: Unlike {grantRole}, this function doesn't perform any checks on the
* calling account. Its usage should be restricted to granting the initial
* set of rules during contract construction.
* - this function can only be called from a constructor.
*/
function _setupRole(bytes32 role, address account) internal virtual {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
require(_isConstructor(), "AccessControl: roles cannot be setup after construction");
_grantRole(role, account);
}

Expand All @@ -187,4 +189,18 @@ abstract contract AccessControl is Context {
emit RoleRevoked(role, account, _msgSender());
}
}

// @dev Returns true if and only if the function is running in the constructor
function _isConstructor() private view returns (bool) {
// extcodesize checks the size of the code stored in an address, and
// address returns the current address. Since the code is still not
// deployed when running a constructor, any checks on its code size will
// yield zero, making it an effective way to detect if a contract is
// under construction or not.
address self = address(this);
uint256 cs;
// solhint-disable-next-line no-inline-assembly
assembly { cs := extcodesize(self) }
nventuro marked this conversation as resolved.
Show resolved Hide resolved
return cs == 0;
}
}
4 changes: 4 additions & 0 deletions contracts/mocks/AccessControlMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ contract AccessControlMock is AccessControl {
function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public {
_setRoleAdmin(roleId, adminRoleId);
}

function setupRole(bytes32 roleId, address account) public {
_setupRole(roleId, account);
}
}
9 changes: 9 additions & 0 deletions test/access/AccessControl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ describe('AccessControl', function () {
this.accessControl = await AccessControlMock.new({ from: admin });
});

describe('_setupRole', function () {
it('cannot be called outside the constructor', async function () {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
await expectRevert(
this.accessControl.setupRole(OTHER_ROLE, other),
'AccessControl: roles cannot be setup after construction'
);
});
});

describe('default admin', function () {
it('deployer has default admin role', async function () {
expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
Expand Down