diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol index 00f2efc98..cd7ebd9ca 100644 --- a/contracts/access/AccessControl.sol +++ b/contracts/access/AccessControl.sol @@ -164,12 +164,16 @@ abstract contract AccessControl is Context { * event. Note that unlike {grantRole}, this function doesn't perform any * checks on the calling account. * - * Requirements: - * - * - this function can only be called from a constructor. + * [WARNING] + * ==== + * This function should only be called from the constructor when setting + * up the initial roles for the system. + * + * Using this function in any other way is effectively circumventing the admin + * system imposed by {AccessControl}. + * ==== */ function _setupRole(bytes32 role, address account) internal virtual { - require(!address(this).isContract(), "AccessControl: roles cannot be setup after construction"); _grantRole(role, account); } diff --git a/contracts/mocks/AccessControlMock.sol b/contracts/mocks/AccessControlMock.sol index 876e78a2a..4f96be5d5 100644 --- a/contracts/mocks/AccessControlMock.sol +++ b/contracts/mocks/AccessControlMock.sol @@ -10,8 +10,4 @@ contract AccessControlMock is AccessControl { function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public { _setRoleAdmin(roleId, adminRoleId); } - - function setupRole(bytes32 roleId, address account) public { - _setupRole(roleId, account); - } } diff --git a/contracts/mocks/ERC20DecimalsMock.sol b/contracts/mocks/ERC20DecimalsMock.sol index d4d25c52f..afb941305 100644 --- a/contracts/mocks/ERC20DecimalsMock.sol +++ b/contracts/mocks/ERC20DecimalsMock.sol @@ -6,8 +6,4 @@ contract ERC20DecimalsMock is ERC20 { constructor (string memory name, string memory symbol, uint8 decimals) public ERC20(name, symbol) { _setupDecimals(decimals); } - - function setupDecimals(uint8 decimals) public { - _setupDecimals(decimals); - } } diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index ac514c483..9503a0a80 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -279,12 +279,11 @@ contract ERC20 is Context, IERC20 { /** * @dev Sets {decimals} to a value other than the default one of 18. * - * Requirements: - * - * - this function can only be called from a constructor. + * WARNING: This function should only be called from the constructor. Most + * applications that interact with token contracts will not expect + * {decimals} to ever change, and may work incorrectly if it does. */ function _setupDecimals(uint8 decimals_) internal { - require(!address(this).isContract(), "ERC20: decimals cannot be changed after construction"); _decimals = decimals_; } diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js index db4cf47c0..c49276f92 100644 --- a/test/access/AccessControl.test.js +++ b/test/access/AccessControl.test.js @@ -17,15 +17,6 @@ describe('AccessControl', function () { this.accessControl = await AccessControlMock.new({ from: admin }); }); - describe('_setupRole', function () { - it('cannot be called outside the constructor', async function () { - 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); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 7996fa39e..ba1701074 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -44,12 +44,6 @@ describe('ERC20', function () { const token = await ERC20DecimalsMock.new(name, symbol, decimals); expect(await token.decimals()).to.be.bignumber.equal(decimals); }); - - it('reverts if setting decimals after construction', async function () { - const token = await ERC20DecimalsMock.new(name, symbol, decimals); - - await expectRevert(token.setupDecimals(decimals.addn(1)), 'ERC20: decimals cannot be changed after construction'); - }); }); shouldBehaveLikeERC20('ERC20', initialSupply, initialHolder, recipient, anotherAccount);