diff --git a/contracts/access/AccessControlDefaultAdminRules.sol b/contracts/access/AccessControlDefaultAdminRules.sol index 43fca9350..0c640fb99 100644 --- a/contracts/access/AccessControlDefaultAdminRules.sol +++ b/contracts/access/AccessControlDefaultAdminRules.sol @@ -136,7 +136,7 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu * @dev See {AccessControl-_revokeRole}. */ function _revokeRole(bytes32 role, address account) internal virtual override { - if (role == DEFAULT_ADMIN_ROLE) { + if (role == DEFAULT_ADMIN_ROLE && account == _currentDefaultAdmin) { delete _currentDefaultAdmin; } super._revokeRole(role, account); diff --git a/test/access/AccessControl.behavior.js b/test/access/AccessControl.behavior.js index 6c88aa274..49ab44b58 100644 --- a/test/access/AccessControl.behavior.js +++ b/test/access/AccessControl.behavior.js @@ -513,15 +513,12 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa }); describe('when caller is pending default admin and delay has passed', function () { - let from; - beforeEach(async function () { await time.setNextBlockTimestamp(acceptSchedule.addn(1)); - from = newDefaultAdmin; }); it('accepts a transfer and changes default admin', async function () { - const receipt = await this.accessControl.acceptDefaultAdminTransfer({ from }); + const receipt = await this.accessControl.acceptDefaultAdminTransfer({ from: newDefaultAdmin }); // Storage changes expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, defaultAdmin)).to.be.false; @@ -625,10 +622,9 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa describe('renounces admin', function () { let delayPassed; - let from = defaultAdmin; beforeEach(async function () { - await this.accessControl.beginDefaultAdminTransfer(constants.ZERO_ADDRESS, { from }); + await this.accessControl.beginDefaultAdminTransfer(constants.ZERO_ADDRESS, { from: defaultAdmin }); delayPassed = web3.utils .toBN(await time.latest()) .add(delay) @@ -638,27 +634,37 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa it('reverts if caller is not default admin', async function () { await time.setNextBlockTimestamp(delayPassed); await expectRevert( - this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, other, { from }), + this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, other, { from: defaultAdmin }), `${errorPrefix}: can only renounce roles for self`, ); }); + it('keeps defaultAdmin consistent with hasRole if another non-defaultAdmin user renounces the DEFAULT_ADMIN_ROLE', async function () { + await time.setNextBlockTimestamp(delayPassed); + + // This passes because it's a noop + await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, other, { from: other }); + + expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, defaultAdmin)).to.be.true; + expect(await this.accessControl.defaultAdmin()).to.be.equal(defaultAdmin); + }); + it('renounces role', async function () { await time.setNextBlockTimestamp(delayPassed); - const receipt = await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, from, { from }); + const receipt = await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from: defaultAdmin }); expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, defaultAdmin)).to.be.false; - expect(await this.accessControl.hasRole(constants.ZERO_ADDRESS, defaultAdmin)).to.be.false; + expect(await this.accessControl.defaultAdmin()).to.be.equal(constants.ZERO_ADDRESS); expectEvent(receipt, 'RoleRevoked', { role: DEFAULT_ADMIN_ROLE, - account: from, + account: defaultAdmin, }); expect(await this.accessControl.owner()).to.equal(constants.ZERO_ADDRESS); }); it('allows to recover access using the internal _grantRole', async function () { await time.setNextBlockTimestamp(delayPassed); - await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, from, { from }); + await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from: defaultAdmin }); const grantRoleReceipt = await this.accessControl.$_grantRole(DEFAULT_ADMIN_ROLE, other); expectEvent(grantRoleReceipt, 'RoleGranted', { @@ -681,7 +687,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa it(`reverts if block.timestamp is ${tag} to schedule`, async function () { await time.setNextBlockTimestamp(delayNotPassed.toNumber() + fromSchedule); await expectRevert( - this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from }), + this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from: defaultAdmin }), `${errorPrefix}: only can renounce in two delayed steps`, ); });