Remove in-constructor requirements (#2195)

* Remove isConstructor requirement from _setupRole

* Remove isConstructor requirement from _setupDecimals

* Update contracts/access/AccessControl.sol

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

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>

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:
+     * [WARNING]
+     * ====
+     * This function should only be called from the constructor when setting
+     * up the initial roles for the system.
      *
-     * - this function can only be called from a constructor.
+     * 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);
     }
 

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);
-    }
 }

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);
-    }
 }

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_;
     }
 

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);

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);