Skip to content

Commit

Permalink
Refactor time helper and remove custom error helper. (#4803)
Browse files Browse the repository at this point in the history
Co-authored-by: ernestognw <ernestognw@gmail.com>
  • Loading branch information
Amxx and ernestognw authored Dec 22, 2023
1 parent be0572a commit 015ef69
Show file tree
Hide file tree
Showing 32 changed files with 158 additions and 209 deletions.
54 changes: 26 additions & 28 deletions test/access/AccessControl.behavior.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { ethers } = require('hardhat');
const { expect } = require('chai');

const { bigint: time } = require('../helpers/time');

const { shouldSupportInterfaces } = require('../utils/introspection/SupportsInterface.behavior');
Expand Down Expand Up @@ -279,8 +280,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
await this.mock.connect(this.defaultAdmin).beginDefaultAdminTransfer(this.newDefaultAdmin);

// Wait for acceptance
const acceptSchedule = (await time.clock.timestamp()) + this.delay;
await time.forward.timestamp(acceptSchedule + 1n, false);
await time.increaseBy.timestamp(this.delay + 1n, false);
await this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer();

const value = await this.mock[getter]();
Expand Down Expand Up @@ -309,7 +309,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`returns pending admin and schedule ${tag} it passes if not accepted`, async function () {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdmin();
await time.forward.timestamp(firstSchedule + fromSchedule);
await time.increaseTo.timestamp(firstSchedule + fromSchedule);

const { newAdmin, schedule } = await this.mock.pendingDefaultAdmin();
expect(newAdmin).to.equal(this.newDefaultAdmin.address);
Expand All @@ -320,7 +320,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it('returns 0 after schedule passes and the transfer was accepted', async function () {
// Wait after schedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdmin();
await time.forward.timestamp(firstSchedule + 1n, false);
await time.increaseTo.timestamp(firstSchedule + 1n, false);

// Accepts
await this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer();
Expand Down Expand Up @@ -352,7 +352,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`returns ${delayTag} delay ${tag} delay schedule passes`, async function () {
// Wait until schedule + fromSchedule
const { schedule } = await this.mock.pendingDefaultAdminDelay();
await time.forward.timestamp(schedule + fromSchedule);
await time.increaseTo.timestamp(schedule + fromSchedule);

const currentDelay = await this.mock.defaultAdminDelay();
expect(currentDelay).to.equal(expectNew ? newDelay : this.delay);
Expand Down Expand Up @@ -383,7 +383,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`returns ${delayTag} delay ${tag} delay schedule passes`, async function () {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdminDelay();
await time.forward.timestamp(firstSchedule + fromSchedule);
await time.increaseTo.timestamp(firstSchedule + fromSchedule);

const { newDelay, schedule } = await this.mock.pendingDefaultAdminDelay();
expect(newDelay).to.equal(expectedDelay);
Expand Down Expand Up @@ -437,7 +437,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
const nextBlockTimestamp = (await time.clock.timestamp()) + 1n;
const acceptSchedule = nextBlockTimestamp + this.delay;

await time.forward.timestamp(nextBlockTimestamp, false); // set timestamp but don't mine the block yet
await time.increaseTo.timestamp(nextBlockTimestamp, false); // set timestamp but don't mine the block yet
await expect(this.mock.connect(this.defaultAdmin).beginDefaultAdminTransfer(this.newDefaultAdmin))
.to.emit(this.mock, 'DefaultAdminTransferScheduled')
.withArgs(this.newDefaultAdmin.address, acceptSchedule);
Expand All @@ -461,7 +461,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
]) {
it(`should be able to begin a transfer again ${tag} acceptSchedule passes`, async function () {
// Wait until schedule + fromSchedule
await time.forward.timestamp(this.acceptSchedule + fromSchedule, false);
await time.increaseTo.timestamp(this.acceptSchedule + fromSchedule, false);

// defaultAdmin changes its mind and begin again to another address
await expect(this.mock.connect(this.defaultAdmin).beginDefaultAdminTransfer(this.other)).to.emit(
Expand All @@ -477,7 +477,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {

it('should not emit a cancellation event if the new default admin accepted', async function () {
// Wait until the acceptSchedule has passed
await time.forward.timestamp(this.acceptSchedule + 1n, false);
await time.increaseTo.timestamp(this.acceptSchedule + 1n, false);

// Accept and restart
await this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer();
Expand Down Expand Up @@ -506,7 +506,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
} delay and apply it to next default admin transfer schedule ${schedulePassed} effectSchedule passed`, async function () {
// Wait until the expected fromSchedule time
const nextBlockTimestamp = this.effectSchedule + fromSchedule;
await time.forward.timestamp(nextBlockTimestamp, false);
await time.increaseTo.timestamp(nextBlockTimestamp, false);

// Start the new default admin transfer and get its schedule
const expectedDelay = expectNewDelay ? newDelay : this.delay;
Expand All @@ -531,15 +531,15 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
});

it('should revert if caller is not pending default admin', async function () {
await time.forward.timestamp(this.acceptSchedule + 1n, false);
await time.increaseTo.timestamp(this.acceptSchedule + 1n, false);
await expect(this.mock.connect(this.other).acceptDefaultAdminTransfer())
.to.be.revertedWithCustomError(this.mock, 'AccessControlInvalidDefaultAdmin')
.withArgs(this.other.address);
});

describe('when caller is pending default admin and delay has passed', function () {
beforeEach(async function () {
await time.forward.timestamp(this.acceptSchedule + 1n, false);
await time.increaseTo.timestamp(this.acceptSchedule + 1n, false);
});

it('accepts a transfer and changes default admin', async function () {
Expand Down Expand Up @@ -568,7 +568,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
[0n, 'equal'],
]) {
it(`should revert if block.timestamp is ${tag} to schedule`, async function () {
await time.forward.timestamp(this.acceptSchedule + fromSchedule, false);
await time.increaseTo.timestamp(this.acceptSchedule + fromSchedule, false);
expect(this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer())
.to.be.revertedWithCustomError(this.mock, 'AccessControlEnforcedDefaultAdminDelay')
.withArgs(this.acceptSchedule);
Expand Down Expand Up @@ -597,7 +597,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
]) {
it(`resets pending default admin and schedule ${tag} transfer schedule passes`, async function () {
// Advance until passed delay
await time.forward.timestamp(this.acceptSchedule + fromSchedule, false);
await time.increaseTo.timestamp(this.acceptSchedule + fromSchedule, false);

await expect(this.mock.connect(this.defaultAdmin).cancelDefaultAdminTransfer()).to.emit(
this.mock,
Expand All @@ -614,7 +614,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
await this.mock.connect(this.defaultAdmin).cancelDefaultAdminTransfer();

// Advance until passed delay
await time.forward.timestamp(this.acceptSchedule + 1n, false);
await time.increaseTo.timestamp(this.acceptSchedule + 1n, false);

// Previous pending default admin should not be able to accept after cancellation.
await expect(this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer())
Expand All @@ -641,19 +641,17 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
beforeEach(async function () {
await this.mock.connect(this.defaultAdmin).beginDefaultAdminTransfer(ethers.ZeroAddress);
this.expectedSchedule = (await time.clock.timestamp()) + this.delay;
this.delayNotPassed = this.expectedSchedule;
this.delayPassed = this.expectedSchedule + 1n;
});

it('reverts if caller is not default admin', async function () {
await time.forward.timestamp(this.delayPassed, false);
await time.increaseBy.timestamp(this.delay + 1n, false);
await expect(
this.mock.connect(this.defaultAdmin).renounceRole(DEFAULT_ADMIN_ROLE, this.other),
).to.be.revertedWithCustomError(this.mock, 'AccessControlBadConfirmation');
});

it("renouncing the admin role when not an admin doesn't affect the schedule", async function () {
await time.forward.timestamp(this.delayPassed, false);
await time.increaseBy.timestamp(this.delay + 1n, false);
await this.mock.connect(this.other).renounceRole(DEFAULT_ADMIN_ROLE, this.other);

const { newAdmin, schedule } = await this.mock.pendingDefaultAdmin();
Expand All @@ -662,7 +660,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
});

it('keeps defaultAdmin consistent with hasRole if another non-defaultAdmin user renounces the DEFAULT_ADMIN_ROLE', async function () {
await time.forward.timestamp(this.delayPassed, false);
await time.increaseBy.timestamp(this.delay + 1n, false);

// This passes because it's a noop
await this.mock.connect(this.other).renounceRole(DEFAULT_ADMIN_ROLE, this.other);
Expand All @@ -672,7 +670,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
});

it('renounces role', async function () {
await time.forward.timestamp(this.delayPassed, false);
await time.increaseBy.timestamp(this.delay + 1n, false);
await expect(this.mock.connect(this.defaultAdmin).renounceRole(DEFAULT_ADMIN_ROLE, this.defaultAdmin))
.to.emit(this.mock, 'RoleRevoked')
.withArgs(DEFAULT_ADMIN_ROLE, this.defaultAdmin.address, this.defaultAdmin.address);
Expand All @@ -687,7 +685,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
});

it('allows to recover access using the internal _grantRole', async function () {
await time.forward.timestamp(this.delayPassed, false);
await time.increaseBy.timestamp(this.delay + 1n, false);
await this.mock.connect(this.defaultAdmin).renounceRole(DEFAULT_ADMIN_ROLE, this.defaultAdmin);

await expect(this.mock.connect(this.defaultAdmin).$_grantRole(DEFAULT_ADMIN_ROLE, this.other))
Expand All @@ -701,7 +699,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
[0n, 'equal'],
]) {
it(`reverts if block.timestamp is ${tag} to schedule`, async function () {
await time.forward.timestamp(this.delayNotPassed + fromSchedule, false);
await time.increaseBy.timestamp(this.delay + fromSchedule, false);
await expect(this.mock.connect(this.defaultAdmin).renounceRole(DEFAULT_ADMIN_ROLE, this.defaultAdmin))
.to.be.revertedWithCustomError(this.mock, 'AccessControlEnforcedDefaultAdminDelay')
.withArgs(this.expectedSchedule);
Expand Down Expand Up @@ -736,7 +734,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
const nextBlockTimestamp = (await time.clock.timestamp()) + 1n;
const effectSchedule = nextBlockTimestamp + changeDelay;

await time.forward.timestamp(nextBlockTimestamp, false);
await time.increaseTo.timestamp(nextBlockTimestamp, false);

// Begins the change
await expect(this.mock.connect(this.defaultAdmin).changeDefaultAdminDelay(this.newDefaultAdminDelay))
Expand Down Expand Up @@ -765,7 +763,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdminDelay();
const nextBlockTimestamp = firstSchedule + fromSchedule;
await time.forward.timestamp(nextBlockTimestamp, false);
await time.increaseTo.timestamp(nextBlockTimestamp, false);

// Calculate expected values
const anotherNewDefaultAdminDelay = this.newDefaultAdminDelay + time.duration.hours(2);
Expand All @@ -788,7 +786,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`should ${emit} a cancellation event ${tag} the delay schedule passes`, async function () {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdminDelay();
await time.forward.timestamp(firstSchedule + fromSchedule, false);
await time.increaseTo.timestamp(firstSchedule + fromSchedule, false);

// Default admin changes its mind and begins another delay change
const anotherNewDefaultAdminDelay = this.newDefaultAdminDelay + time.duration.hours(2);
Expand Down Expand Up @@ -830,7 +828,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`resets pending delay and schedule ${tag} delay change schedule passes`, async function () {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdminDelay();
await time.forward.timestamp(firstSchedule + fromSchedule, false);
await time.increaseTo.timestamp(firstSchedule + fromSchedule, false);

await this.mock.connect(this.defaultAdmin).rollbackDefaultAdminDelay();

Expand All @@ -843,7 +841,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
it(`should ${emit} a cancellation event ${tag} the delay schedule passes`, async function () {
// Wait until schedule + fromSchedule
const { schedule: firstSchedule } = await this.mock.pendingDefaultAdminDelay();
await time.forward.timestamp(firstSchedule + fromSchedule, false);
await time.increaseTo.timestamp(firstSchedule + fromSchedule, false);

const expected = expect(this.mock.connect(this.defaultAdmin).rollbackDefaultAdminDelay());
if (passed) {
Expand Down
12 changes: 6 additions & 6 deletions test/access/manager/AccessManaged.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
const { bigint: time } = require('../../helpers/time');
const { ethers } = require('hardhat');

const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { impersonate } = require('../../helpers/account');
const { ethers } = require('hardhat');
const { bigint: time } = require('../../helpers/time');

async function fixture() {
const [admin, roleMember, other] = await ethers.getSigners();
Expand Down Expand Up @@ -84,14 +85,13 @@ describe('AccessManaged', function () {
const calldata = this.managed.interface.encodeFunctionData(fn, []);

// Schedule
const timestamp = await time.clock.timestamp();
const scheduledAt = timestamp + 1n;
const scheduledAt = (await time.clock.timestamp()) + 1n;
const when = scheduledAt + delay;
await time.forward.timestamp(scheduledAt, false);
await time.increaseTo.timestamp(scheduledAt, false);
await this.authority.connect(this.roleMember).schedule(this.managed, calldata, when);

// Set execution date
await time.forward.timestamp(when, false);
await time.increaseTo.timestamp(when, false);

// Shouldn't revert
await this.managed.connect(this.roleMember)[this.selector]();
Expand Down
13 changes: 7 additions & 6 deletions test/access/manager/AccessManager.predicate.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
const { ethers } = require('hardhat');
const { setStorageAt } = require('@nomicfoundation/hardhat-network-helpers');

const { EXECUTION_ID_STORAGE_SLOT, EXPIRATION, prepareOperation } = require('../../helpers/access-manager');
const { impersonate } = require('../../helpers/account');
const { bigint: time } = require('../../helpers/time');
const { ethers } = require('hardhat');

// ============ COMMON PREDICATES ============

Expand Down Expand Up @@ -146,15 +147,15 @@ function testAsDelay(type, { before, after }) {

describe(`when ${type} delay has not taken effect yet`, function () {
beforeEach(`set next block timestamp before ${type} takes effect`, async function () {
await time.forward.timestamp(this.delayEffect - 1n, !!before.mineDelay);
await time.increaseTo.timestamp(this.delayEffect - 1n, !!before.mineDelay);
});

before();
});

describe(`when ${type} delay has taken effect`, function () {
beforeEach(`set next block timestamp when ${type} takes effect`, async function () {
await time.forward.timestamp(this.delayEffect, !!after.mineDelay);
await time.increaseTo.timestamp(this.delayEffect, !!after.mineDelay);
});

after();
Expand Down Expand Up @@ -187,7 +188,7 @@ function testAsSchedulableOperation({ scheduled: { before, after, expired }, not
beforeEach('set next block time before operation is ready', async function () {
this.scheduledAt = await time.clock.timestamp();
const schedule = await this.manager.getSchedule(this.operationId);
await time.forward.timestamp(schedule - 1n, !!before.mineDelay);
await time.increaseTo.timestamp(schedule - 1n, !!before.mineDelay);
});

before();
Expand All @@ -197,7 +198,7 @@ function testAsSchedulableOperation({ scheduled: { before, after, expired }, not
beforeEach('set next block time when operation is ready for execution', async function () {
this.scheduledAt = await time.clock.timestamp();
const schedule = await this.manager.getSchedule(this.operationId);
await time.forward.timestamp(schedule, !!after.mineDelay);
await time.increaseTo.timestamp(schedule, !!after.mineDelay);
});

after();
Expand All @@ -207,7 +208,7 @@ function testAsSchedulableOperation({ scheduled: { before, after, expired }, not
beforeEach('set next block time when operation expired', async function () {
this.scheduledAt = await time.clock.timestamp();
const schedule = await this.manager.getSchedule(this.operationId);
await time.forward.timestamp(schedule + EXPIRATION, !!expired.mineDelay);
await time.increaseTo.timestamp(schedule + EXPIRATION, !!expired.mineDelay);
});

expired();
Expand Down
Loading

0 comments on commit 015ef69

Please sign in to comment.