Migrate AccessManager tests to ethers (OpenZeppelin#4710)

Co-authored-by: Hadrien Croubois <[email protected]>

contracts/mocks/AuthorityMock.sol

@@ -52,7 +52,7 @@ contract AuthorityNoResponse {
     function canCall(address /* caller */, address /* target */, bytes4 /* selector */) external view {}
 }
 
-contract AuthoritiyObserveIsConsuming {
+contract AuthorityObserveIsConsuming {
     event ConsumeScheduledOpCalled(address caller, bytes data, bytes4 isConsuming);
 
     function canCall(

test/access/Ownable.test.js

@@ -13,6 +13,12 @@ describe('Ownable', function () {
     Object.assign(this, await loadFixture(fixture));
   });
 
+  it('emits ownership transfer events during construction', async function () {
+    await expect(await this.ownable.deploymentTransaction())
+      .to.emit(this.ownable, 'OwnershipTransferred')
+      .withArgs(ethers.ZeroAddress, this.owner.address);
+  });
+
   it('rejects zero address for initialOwner', async function () {
     await expect(ethers.deployContract('$Ownable', [ethers.ZeroAddress]))
       .to.be.revertedWithCustomError({ interface: this.ownable.interface }, 'OwnableInvalidOwner')

test/access/manager/AccessManaged.test.js

@@ -1,142 +1,145 @@
-const { expectEvent, time, expectRevert } = require('@openzeppelin/test-helpers');
-const { selector } = require('../../helpers/methods');
-const { expectRevertCustomError } = require('../../helpers/customError');
-const {
-  time: { setNextBlockTimestamp },
-} = require('@nomicfoundation/hardhat-network-helpers');
+const { bigint: time } = require('../../helpers/time');
+const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { impersonate } = require('../../helpers/account');
+const { ethers } = require('hardhat');
 
-const AccessManaged = artifacts.require('$AccessManagedTarget');
-const AccessManager = artifacts.require('$AccessManager');
+async function fixture() {
+  const [admin, roleMember, other] = await ethers.getSigners();
 
-const AuthoritiyObserveIsConsuming = artifacts.require('$AuthoritiyObserveIsConsuming');
+  const authority = await ethers.deployContract('$AccessManager', [admin]);
+  const managed = await ethers.deployContract('$AccessManagedTarget', [authority]);
 
-contract('AccessManaged', function (accounts) {
-  const [admin, roleMember, other] = accounts;
+  const anotherAuthority = await ethers.deployContract('$AccessManager', [admin]);
+  const authorityObserveIsConsuming = await ethers.deployContract('$AuthorityObserveIsConsuming');
 
+  await impersonate(authority.target);
+  const authorityAsSigner = await ethers.getSigner(authority.target);
+
+  return {
+    roleMember,
+    other,
+    authorityAsSigner,
+    authority,
+    managed,
+    authorityObserveIsConsuming,
+    anotherAuthority,
+  };
+}
+
+describe('AccessManaged', function () {
   beforeEach(async function () {
-    this.authority = await AccessManager.new(admin);
-    this.managed = await AccessManaged.new(this.authority.address);
+    Object.assign(this, await loadFixture(fixture));
   });
 
   it('sets authority and emits AuthorityUpdated event during construction', async function () {
-    await expectEvent.inConstruction(this.managed, 'AuthorityUpdated', {
-      authority: this.authority.address,
-    });
-    expect(await this.managed.authority()).to.eq(this.authority.address);
+    await expect(await this.managed.deploymentTransaction())
+      .to.emit(this.managed, 'AuthorityUpdated')
+      .withArgs(this.authority.target);
   });
 
   describe('restricted modifier', function () {
-    const method = 'fnRestricted()';
-
     beforeEach(async function () {
-      this.selector = selector(method);
-      this.role = web3.utils.toBN(42);
-      await this.authority.$_setTargetFunctionRole(this.managed.address, this.selector, this.role);
-      await this.authority.$_grantRole(this.role, roleMember, 0, 0);
+      this.selector = this.managed.fnRestricted.getFragment().selector;
+      this.role = 42n;
+      await this.authority.$_setTargetFunctionRole(this.managed, this.selector, this.role);
+      await this.authority.$_grantRole(this.role, this.roleMember, 0, 0);
     });
 
     it('succeeds when role is granted without execution delay', async function () {
-      await this.managed.methods[method]({ from: roleMember });
+      await this.managed.connect(this.roleMember)[this.selector]();
     });
 
     it('reverts when role is not granted', async function () {
-      await expectRevertCustomError(this.managed.methods[method]({ from: other }), 'AccessManagedUnauthorized', [
-        other,
-      ]);
+      await expect(this.managed.connect(this.other)[this.selector]())
+        .to.be.revertedWithCustomError(this.managed, 'AccessManagedUnauthorized')
+        .withArgs(this.other.address);
     });
 
     it('panics in short calldata', async function () {
       // We avoid adding the `restricted` modifier to the fallback function because other tests may depend on it
       // being accessible without restrictions. We check for the internal `_checkCanCall` instead.
-      await expectRevert.unspecified(this.managed.$_checkCanCall(other, '0x1234'));
+      await expect(this.managed.$_checkCanCall(this.roleMember, '0x1234')).to.be.reverted;
     });
 
     describe('when role is granted with execution delay', function () {
       beforeEach(async function () {
-        const executionDelay = web3.utils.toBN(911);
-        await this.authority.$_grantRole(this.role, roleMember, 0, executionDelay);
+        const executionDelay = 911n;
+        await this.authority.$_grantRole(this.role, this.roleMember, 0, executionDelay);
       });
 
       it('reverts if the operation is not scheduled', async function () {
-        const calldata = this.managed.contract.methods[method]().encodeABI();
-        const opId = await this.authority.hashOperation(roleMember, this.managed.address, calldata);
+        const fn = this.managed.interface.getFunction(this.selector);
+        const calldata = this.managed.interface.encodeFunctionData(fn, []);
+        const opId = await this.authority.hashOperation(this.roleMember, this.managed, calldata);
 
-        await expectRevertCustomError(this.managed.methods[method]({ from: roleMember }), 'AccessManagerNotScheduled', [
-          opId,
-        ]);
+        await expect(this.managed.connect(this.roleMember)[this.selector]())
+          .to.be.revertedWithCustomError(this.authority, 'AccessManagerNotScheduled')
+          .withArgs(opId);
       });
 
       it('succeeds if the operation is scheduled', async function () {
         // Arguments
         const delay = time.duration.hours(12);
-        const calldata = this.managed.contract.methods[method]().encodeABI();
+        const fn = this.managed.interface.getFunction(this.selector);
+        const calldata = this.managed.interface.encodeFunctionData(fn, []);
 
         // Schedule
-        const timestamp = await time.latest();
-        const scheduledAt = timestamp.addn(1);
-        const when = scheduledAt.add(delay);
-        await setNextBlockTimestamp(scheduledAt);
-        await this.authority.schedule(this.managed.address, calldata, when, {
-          from: roleMember,
-        });
+        const timestamp = await time.clock.timestamp();
+        const scheduledAt = timestamp + 1n;
+        const when = scheduledAt + delay;
+        await time.forward.timestamp(scheduledAt, false);
+        await this.authority.connect(this.roleMember).schedule(this.managed, calldata, when);
 
         // Set execution date
-        await setNextBlockTimestamp(when);
+        await time.forward.timestamp(when, false);
 
         // Shouldn't revert
-        await this.managed.methods[method]({ from: roleMember });
+        await this.managed.connect(this.roleMember)[this.selector]();
       });
     });
   });
 
   describe('setAuthority', function () {
-    beforeEach(async function () {
-      this.newAuthority = await AccessManager.new(admin);
-    });
-
     it('reverts if the caller is not the authority', async function () {
-      await expectRevertCustomError(this.managed.setAuthority(other, { from: other }), 'AccessManagedUnauthorized', [
-        other,
-      ]);
+      await expect(this.managed.connect(this.other).setAuthority(this.other))
+        .to.be.revertedWithCustomError(this.managed, 'AccessManagedUnauthorized')
+        .withArgs(this.other.address);
     });
 
     it('reverts if the new authority is not a valid authority', async function () {
-      await impersonate(this.authority.address);
-      await expectRevertCustomError(
-        this.managed.setAuthority(other, { from: this.authority.address }),
-        'AccessManagedInvalidAuthority',
-        [other],
-      );
+      await expect(this.managed.connect(this.authorityAsSigner).setAuthority(this.other))
+        .to.be.revertedWithCustomError(this.managed, 'AccessManagedInvalidAuthority')
+        .withArgs(this.other.address);
     });
 
     it('sets authority and emits AuthorityUpdated event', async function () {
-      await impersonate(this.authority.address);
-      const { receipt } = await this.managed.setAuthority(this.newAuthority.address, { from: this.authority.address });
-      await expectEvent(receipt, 'AuthorityUpdated', {
-        authority: this.newAuthority.address,
-      });
-      expect(await this.managed.authority()).to.eq(this.newAuthority.address);
+      await expect(this.managed.connect(this.authorityAsSigner).setAuthority(this.anotherAuthority))
+        .to.emit(this.managed, 'AuthorityUpdated')
+        .withArgs(this.anotherAuthority.target);
+
+      expect(await this.managed.authority()).to.equal(this.anotherAuthority.target);
     });
   });
 
   describe('isConsumingScheduledOp', function () {
     beforeEach(async function () {
-      this.authority = await AuthoritiyObserveIsConsuming.new();
-      this.managed = await AccessManaged.new(this.authority.address);
+      await this.managed.connect(this.authorityAsSigner).setAuthority(this.authorityObserveIsConsuming);
     });
 
     it('returns bytes4(0) when not consuming operation', async function () {
-      expect(await this.managed.isConsumingScheduledOp()).to.eq('0x00000000');
+      expect(await this.managed.isConsumingScheduledOp()).to.equal('0x00000000');
     });
 
     it('returns isConsumingScheduledOp selector when consuming operation', async function () {
-      const receipt = await this.managed.fnRestricted({ from: other });
-      await expectEvent.inTransaction(receipt.tx, this.authority, 'ConsumeScheduledOpCalled', {
-        caller: other,
-        data: this.managed.contract.methods.fnRestricted().encodeABI(),
-        isConsuming: selector('isConsumingScheduledOp()'),
-      });
+      const isConsumingScheduledOp = this.managed.interface.getFunction('isConsumingScheduledOp()');
+      const fnRestricted = this.managed.fnRestricted.getFragment();
+      await expect(this.managed.connect(this.other).fnRestricted())
+        .to.emit(this.authorityObserveIsConsuming, 'ConsumeScheduledOpCalled')
+        .withArgs(
+          this.other.address,
+          this.managed.interface.encodeFunctionData(fnRestricted, []),
+          isConsumingScheduledOp.selector,
+        );
     });
   });
 });