Add AccessControlDefaultAdminRules (#4009)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>

test/access/AccessControl.behavior.js

@@ -1,5 +1,7 @@
 const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
+const { ZERO_ADDRESS } = require('@openzeppelin/test-helpers/src/constants');
 const { expect } = require('chai');
+const { time } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { shouldSupportInterfaces } = require('../utils/introspection/SupportsInterface.behavior');
 
@@ -210,8 +212,290 @@ function shouldBehaveLikeAccessControlEnumerable(errorPrefix, admin, authorized,
   });
 }
 
+function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defaultAdmin, newDefaultAdmin, other) {
+  shouldSupportInterfaces(['AccessControlDefaultAdminRules']);
+
+  it('has a default disabled delayed until', async function () {
+    expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(web3.utils.toBN(0));
+  });
+
+  it('has a default pending default admin', async function () {
+    expect(await this.accessControl.pendingDefaultAdmin()).to.equal(ZERO_ADDRESS);
+  });
+
+  it('has a default current owner set to the initial default admin', async function () {
+    const owner = await this.accessControl.owner();
+    expect(owner).to.equal(defaultAdmin);
+    expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, owner)).to.be.true;
+  });
+
+  it('should revert if granting default admin role', async function () {
+    await expectRevert(
+      this.accessControl.grantRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from: defaultAdmin }),
+      `${errorPrefix}: can't directly grant default admin role`,
+    );
+  });
+
+  it('should revert if revoking default admin role', async function () {
+    await expectRevert(
+      this.accessControl.revokeRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from: defaultAdmin }),
+      `${errorPrefix}: can't directly revoke default admin role`,
+    );
+  });
+
+  it("should revert if defaultAdmin's admin is changed", async function () {
+    await expectRevert(
+      this.accessControl.$_setRoleAdmin(DEFAULT_ADMIN_ROLE, defaultAdmin),
+      `${errorPrefix}: can't violate default admin rules`,
+    );
+  });
+
+  it('should not grant the default admin role twice', async function () {
+    await expectRevert(
+      this.accessControl.$_grantRole(DEFAULT_ADMIN_ROLE, defaultAdmin),
+      `${errorPrefix}: default admin already granted`,
+    );
+  });
+
+  describe('begins transfer of default admin', function () {
+    let receipt;
+    let defaultAdminTransferDelayedUntil;
+
+    beforeEach('begins admin transfer', async function () {
+      receipt = await this.accessControl.beginDefaultAdminTransfer(newDefaultAdmin, { from: defaultAdmin });
+      defaultAdminTransferDelayedUntil = web3.utils.toBN(await time.latest()).add(delay);
+    });
+
+    it('should set pending default admin and delayed until', async function () {
+      expect(await this.accessControl.pendingDefaultAdmin()).to.equal(newDefaultAdmin);
+      expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(
+        defaultAdminTransferDelayedUntil,
+      );
+      expectEvent(receipt, 'DefaultAdminRoleChangeStarted', {
+        newDefaultAdmin,
+        defaultAdminTransferDelayedUntil,
+      });
+    });
+
+    it('should be able to begin a transfer again before delay pass', async function () {
+      // Time passes just before delay
+      await time.setNextBlockTimestamp(defaultAdminTransferDelayedUntil.subn(1));
+
+      // defaultAdmin changes its mind and begin again to another address
+      await this.accessControl.beginDefaultAdminTransfer(other, { from: defaultAdmin });
+      const newDelayedUntil = web3.utils.toBN(await time.latest()).add(delay);
+      expect(await this.accessControl.pendingDefaultAdmin()).to.equal(other);
+      expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(newDelayedUntil);
+    });
+
+    it('should be able to begin a transfer again after delay pass if not accepted', async function () {
+      // Time passes after delay without acceptance
+      await time.setNextBlockTimestamp(defaultAdminTransferDelayedUntil.addn(1));
+
+      // defaultAdmin changes its mind and begin again to another address
+      await this.accessControl.beginDefaultAdminTransfer(other, { from: defaultAdmin });
+      const newDelayedUntil = web3.utils.toBN(await time.latest()).add(delay);
+      expect(await this.accessControl.pendingDefaultAdmin()).to.equal(other);
+      expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(newDelayedUntil);
+    });
+
+    it('should revert if it is called by non-admin accounts', async function () {
+      await expectRevert(
+        this.accessControl.beginDefaultAdminTransfer(newDefaultAdmin, { from: other }),
+        `${errorPrefix}: account ${other.toLowerCase()} is missing role ${DEFAULT_ADMIN_ROLE}`,
+      );
+    });
+  });
+
+  describe('accepts transfer admin', function () {
+    let delayPassed;
+
+    beforeEach(async function () {
+      await this.accessControl.beginDefaultAdminTransfer(newDefaultAdmin, { from: defaultAdmin });
+      delayPassed = web3.utils
+        .toBN(await time.latest())
+        .add(delay)
+        .addn(1);
+    });
+
+    describe('caller is pending default admin and delay has passed', function () {
+      let from;
+
+      beforeEach(async function () {
+        await time.setNextBlockTimestamp(delayPassed);
+        from = newDefaultAdmin;
+      });
+
+      it('accepts a transfer and changes default admin', async function () {
+        const receipt = await this.accessControl.acceptDefaultAdminTransfer({ from });
+
+        // Storage changes
+        expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, defaultAdmin)).to.be.false;
+        expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, newDefaultAdmin)).to.be.true;
+        expect(await this.accessControl.owner()).to.equal(newDefaultAdmin);
+
+        // Emit events
+        expectEvent(receipt, 'RoleRevoked', {
+          role: DEFAULT_ADMIN_ROLE,
+          account: defaultAdmin,
+        });
+        expectEvent(receipt, 'RoleGranted', {
+          role: DEFAULT_ADMIN_ROLE,
+          account: newDefaultAdmin,
+        });
+
+        // Resets pending default admin and delayed until
+        expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(web3.utils.toBN(0));
+        expect(await this.accessControl.pendingDefaultAdmin()).to.equal(ZERO_ADDRESS);
+      });
+    });
+
+    it('should revert if caller is not pending default admin', async function () {
+      await time.setNextBlockTimestamp(delayPassed);
+      await expectRevert(
+        this.accessControl.acceptDefaultAdminTransfer({ from: other }),
+        `${errorPrefix}: pending admin must accept`,
+      );
+    });
+
+    describe('delayedUntil not passed', function () {
+      let delayNotPassed;
+
+      beforeEach(function () {
+        delayNotPassed = delayPassed.subn(1);
+      });
+
+      it('should revert if block.timestamp is equal to delayed until', async function () {
+        await time.setNextBlockTimestamp(delayNotPassed);
+        await expectRevert(
+          this.accessControl.acceptDefaultAdminTransfer({ from: newDefaultAdmin }),
+          `${errorPrefix}: transfer delay not passed`,
+        );
+      });
+
+      it('should revert if block.timestamp is less than delayed until', async function () {
+        await expectRevert(
+          this.accessControl.acceptDefaultAdminTransfer({ from: newDefaultAdmin }),
+          `${errorPrefix}: transfer delay not passed`,
+        );
+      });
+    });
+  });
+
+  describe('cancel transfer default admin', function () {
+    let delayPassed;
+
+    beforeEach(async function () {
+      await this.accessControl.beginDefaultAdminTransfer(newDefaultAdmin, { from: defaultAdmin });
+      delayPassed = web3.utils
+        .toBN(await time.latest())
+        .add(delay)
+        .addn(1);
+    });
+
+    it('resets pending default admin and delayed until', async function () {
+      await this.accessControl.cancelDefaultAdminTransfer({ from: defaultAdmin });
+      expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(web3.utils.toBN(0));
+      expect(await this.accessControl.pendingDefaultAdmin()).to.equal(ZERO_ADDRESS);
+
+      // Advance until passed delay
+      await time.setNextBlockTimestamp(delayPassed);
+
+      // Previous pending default admin should not be able to accept after cancellation.
+      await expectRevert(
+        this.accessControl.acceptDefaultAdminTransfer({ from: newDefaultAdmin }),
+        `${errorPrefix}: pending admin must accept`,
+      );
+    });
+
+    it('cancels even after delay has passed', async function () {
+      await this.accessControl.cancelDefaultAdminTransfer({ from: defaultAdmin });
+      await time.setNextBlockTimestamp(delayPassed);
+      expect(await this.accessControl.defaultAdminTransferDelayedUntil()).to.be.bignumber.equal(web3.utils.toBN(0));
+      expect(await this.accessControl.pendingDefaultAdmin()).to.equal(ZERO_ADDRESS);
+    });
+
+    it('reverts if called by non default admin accounts', async function () {
+      await expectRevert(
+        this.accessControl.cancelDefaultAdminTransfer({ from: other }),
+        `${errorPrefix}: account ${other.toLowerCase()} is missing role ${DEFAULT_ADMIN_ROLE}`,
+      );
+    });
+  });
+
+  describe('renouncing admin', function () {
+    let delayPassed;
+    let from = defaultAdmin;
+
+    beforeEach(async function () {
+      await this.accessControl.beginDefaultAdminTransfer(ZERO_ADDRESS, { from });
+      delayPassed = web3.utils
+        .toBN(await time.latest())
+        .add(delay)
+        .addn(1);
+    });
+
+    it('it renounces role', async function () {
+      await time.setNextBlockTimestamp(delayPassed);
+      const receipt = await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, from, { from });
+
+      expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, defaultAdmin)).to.be.false;
+      expect(await this.accessControl.hasRole(ZERO_ADDRESS, defaultAdmin)).to.be.false;
+      expectEvent(receipt, 'RoleRevoked', {
+        role: DEFAULT_ADMIN_ROLE,
+        account: from,
+      });
+      expect(await this.accessControl.owner()).to.equal(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 });
+
+      const grantRoleReceipt = await this.accessControl.$_grantRole(DEFAULT_ADMIN_ROLE, other);
+      expectEvent(grantRoleReceipt, 'RoleGranted', {
+        role: DEFAULT_ADMIN_ROLE,
+        account: other,
+      });
+    });
+
+    it('reverts if caller is not default admin', async function () {
+      await time.setNextBlockTimestamp(delayPassed);
+      await expectRevert(
+        this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, other, { from }),
+        `${errorPrefix}: can only renounce roles for self`,
+      );
+    });
+
+    describe('delayed until not passed', function () {
+      let delayNotPassed;
+
+      beforeEach(function () {
+        delayNotPassed = delayPassed.subn(1);
+      });
+
+      it('reverts if block.timestamp is equal to delayed until', async function () {
+        await time.setNextBlockTimestamp(delayNotPassed);
+        await expectRevert(
+          this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from }),
+          `${errorPrefix}: only can renounce in two delayed steps`,
+        );
+      });
+
+      it('reverts if block.timestamp is less than delayed until', async function () {
+        await time.setNextBlockTimestamp(delayNotPassed.subn(1));
+        await expectRevert(
+          this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, defaultAdmin, { from }),
+          `${errorPrefix}: only can renounce in two delayed steps`,
+        );
+      });
+    });
+  });
+}
+
 module.exports = {
   DEFAULT_ADMIN_ROLE,
   shouldBehaveLikeAccessControl,
   shouldBehaveLikeAccessControlEnumerable,
+  shouldBehaveLikeAccessControlDefaultAdminRules,
 };

test/access/AccessControlDefaultAdminRules.test.js

@@ -0,0 +1,18 @@
+const { time } = require('@openzeppelin/test-helpers');
+const {
+  shouldBehaveLikeAccessControl,
+  shouldBehaveLikeAccessControlDefaultAdminRules,
+} = require('./AccessControl.behavior.js');
+
+const AccessControlDefaultAdminRules = artifacts.require('$AccessControlDefaultAdminRules');
+
+contract('AccessControlDefaultAdminRules', function (accounts) {
+  const delay = web3.utils.toBN(time.duration.days(10));
+
+  beforeEach(async function () {
+    this.accessControl = await AccessControlDefaultAdminRules.new(delay, accounts[0], { from: accounts[0] });
+  });
+
+  shouldBehaveLikeAccessControl('AccessControl', ...accounts);
+  shouldBehaveLikeAccessControlDefaultAdminRules('AccessControl', delay, ...accounts);
+});

test/access/Ownable.test.js

@@ -37,7 +37,7 @@ contract('Ownable', function (accounts) {
   });
 
   describe('renounce ownership', function () {
-    it('loses owner after renouncement', async function () {
+    it('loses ownership after renouncement', async function () {
       const receipt = await this.ownable.renounceOwnership({ from: owner });
       expectEvent(receipt, 'OwnershipTransferred');
 
@@ -47,5 +47,13 @@ contract('Ownable', function (accounts) {
     it('prevents non-owners from renouncement', async function () {
       await expectRevert(this.ownable.renounceOwnership({ from: other }), 'Ownable: caller is not the owner');
     });
+
+    it('allows to recover access using the internal _transferOwnership', async function () {
+      await this.ownable.renounceOwnership({ from: owner });
+      const receipt = await this.ownable.$_transferOwnership(other);
+      expectEvent(receipt, 'OwnershipTransferred');
+
+      expect(await this.ownable.owner()).to.equal(other);
+    });
   });
 });

test/access/Ownable2Step.test.js

@@ -27,6 +27,16 @@ contract('Ownable2Step', function (accounts) {
       expect(await this.ownable2Step.pendingOwner()).to.not.equal(accountA);
     });
 
+    it('guards transfer against invalid user', async function () {
+      await this.ownable2Step.transferOwnership(accountA, { from: owner });
+      await expectRevert(
+        this.ownable2Step.acceptOwnership({ from: accountB }),
+        'Ownable2Step: caller is not the new owner',
+      );
+    });
+  });
+
+  it('renouncing ownership', async function () {
     it('changes owner after renouncing ownership', async function () {
       await this.ownable2Step.renounceOwnership({ from: owner });
       // If renounceOwnership is removed from parent an alternative is needed ...
@@ -46,12 +56,12 @@ contract('Ownable2Step', function (accounts) {
       );
     });
 
-    it('guards transfer against invalid user', async function () {
-      await this.ownable2Step.transferOwnership(accountA, { from: owner });
-      await expectRevert(
-        this.ownable2Step.acceptOwnership({ from: accountB }),
-        'Ownable2Step: caller is not the new owner',
-      );
+    it('allows to recover access using the internal _transferOwnership', async function () {
+      await this.ownable.renounceOwnership({ from: owner });
+      const receipt = await this.ownable.$_transferOwnership(accountA);
+      expectEvent(receipt, 'OwnershipTransferred');
+
+      expect(await this.ownable.owner()).to.equal(accountA);
     });
   });
 });

test/utils/introspection/SupportsInterface.behavior.js

@@ -37,6 +37,15 @@ const INTERFACES = {
     'renounceRole(bytes32,address)',
   ],
   AccessControlEnumerable: ['getRoleMember(bytes32,uint256)', 'getRoleMemberCount(bytes32)'],
+  AccessControlDefaultAdminRules: [
+    'defaultAdminDelay()',
+    'defaultAdmin()',
+    'defaultAdminTransferDelayedUntil()',
+    'pendingDefaultAdmin()',
+    'beginDefaultAdminTransfer(address)',
+    'acceptDefaultAdminTransfer()',
+    'cancelDefaultAdminTransfer()',
+  ],
   Governor: [
     'name()',
     'version()',