AccessManager tests consolidation (#4655)

test/access/manager/AccessManaged.test.js

@@ -59,7 +59,7 @@ contract('AccessManaged', function (accounts) {
       });
 
       it('reverts if the operation is not scheduled', async function () {
-        const calldata = await this.managed.contract.methods[method]().encodeABI();
+        const calldata = this.managed.contract.methods[method]().encodeABI();
         const opId = await this.authority.hashOperation(roleMember, this.managed.address, calldata);
 
         await expectRevertCustomError(this.managed.methods[method]({ from: roleMember }), 'AccessManagerNotScheduled', [
@@ -70,7 +70,7 @@ contract('AccessManaged', function (accounts) {
       it('succeeds if the operation is scheduled', async function () {
         // Arguments
         const delay = time.duration.hours(12);
-        const calldata = await this.managed.contract.methods[method]().encodeABI();
+        const calldata = this.managed.contract.methods[method]().encodeABI();
 
         // Schedule
         const timestamp = await time.latest();

test/access/manager/AccessManager.behavior.js

@@ -1,507 +1,45 @@
-const { time } = require('@openzeppelin/test-helpers');
-const {
-  time: { setNextBlockTimestamp },
-  setStorageAt,
-  mine,
-} = require('@nomicfoundation/hardhat-network-helpers');
-const { impersonate } = require('../../helpers/account');
+const { mine } = require('@nomicfoundation/hardhat-network-helpers');
 const { expectRevertCustomError } = require('../../helpers/customError');
-const { EXPIRATION, EXECUTION_ID_STORAGE_SLOT } = require('../../helpers/access-manager');
-
-// ============ COMMON PATHS ============
-
-const COMMON_IS_EXECUTING_PATH = {
-  executing() {
-    it('succeeds', async function () {
-      await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
-    });
-  },
-  notExecuting() {
-    it('reverts as AccessManagerUnauthorizedAccount', async function () {
-      await expectRevertCustomError(
-        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-        'AccessManagerUnauthorizedAccount',
-        [this.caller, this.role.id],
-      );
-    });
-  },
-};
-
-const COMMON_GET_ACCESS_PATH = {
-  requiredRoleIsGranted: {
-    roleGrantingIsDelayed: {
-      callerHasAnExecutionDelay: {
-        beforeGrantDelay() {
-          it('reverts as AccessManagerUnauthorizedAccount', async function () {
-            await expectRevertCustomError(
-              web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-              'AccessManagerUnauthorizedAccount',
-              [this.caller, this.role.id],
-            );
-          });
-        },
-        afterGrantDelay: undefined, // Diverges if there's an operation delay or not
-      },
-      callerHasNoExecutionDelay: {
-        beforeGrantDelay() {
-          it('reverts as AccessManagerUnauthorizedAccount', async function () {
-            await expectRevertCustomError(
-              web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-              'AccessManagerUnauthorizedAccount',
-              [this.caller, this.role.id],
-            );
-          });
-        },
-        afterGrantDelay() {
-          it('succeeds called directly', async function () {
-            await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
-          });
-
-          it('succeeds via execute', async function () {
-            await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
-          });
-        },
-      },
-    },
-    roleGrantingIsNotDelayed: {
-      callerHasAnExecutionDelay: undefined, // Diverges if there's an operation to schedule or not
-      callerHasNoExecutionDelay() {
-        it('succeeds called directly', async function () {
-          await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
-        });
-
-        it('succeeds via execute', async function () {
-          await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
-        });
-      },
-    },
-  },
-  requiredRoleIsNotGranted() {
-    it('reverts as AccessManagerUnauthorizedAccount', async function () {
-      await expectRevertCustomError(
-        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-        'AccessManagerUnauthorizedAccount',
-        [this.caller, this.role.id],
-      );
-    });
-  },
-};
-
-const COMMON_SCHEDULABLE_PATH = {
-  scheduled: {
-    before() {
-      it('reverts as AccessManagerNotReady', async function () {
-        await expectRevertCustomError(
-          web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-          'AccessManagerNotReady',
-          [this.operationId],
-        );
-      });
-    },
-    after() {
-      it('succeeds called directly', async function () {
-        await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
-      });
-
-      it('succeeds via execute', async function () {
-        await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
-      });
-    },
-    expired() {
-      it('reverts as AccessManagerExpired', async function () {
-        await expectRevertCustomError(
-          web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-          'AccessManagerExpired',
-          [this.operationId],
-        );
-      });
-    },
-  },
-  notScheduled() {
-    it('reverts as AccessManagerNotScheduled', async function () {
-      await expectRevertCustomError(
-        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
-        'AccessManagerNotScheduled',
-        [this.operationId],
-      );
-    });
-  },
-};
-
-const COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY = {
-  scheduled: {
-    before() {
-      it.skip('is not reachable without a delay');
-    },
-    after() {
-      it.skip('is not reachable without a delay');
-    },
-    expired() {
-      it.skip('is not reachable without a delay');
-    },
-  },
-  notScheduled() {
-    it('succeeds', async function () {
-      await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
-    });
-  },
-};
-
-// ============ MODE HELPERS ============
-
-/**
- * @requires this.{manager,target}
- */
-function shouldBehaveLikeClosable({ closed, open }) {
-  describe('when the manager is closed', function () {
-    beforeEach('close', async function () {
-      await this.manager.$_setTargetClosed(this.target.address, true);
-    });
-
-    closed();
-  });
-
-  describe('when the manager is open', function () {
-    beforeEach('open', async function () {
-      await this.manager.$_setTargetClosed(this.target.address, false);
-    });
-
-    open();
-  });
-}
-
-// ============ DELAY HELPERS ============
-
-/**
- * @requires this.{delay}
- */
-function shouldBehaveLikeDelay(type, { before, after }) {
-  beforeEach('define timestamp when delay takes effect', async function () {
-    const timestamp = await time.latest();
-    this.delayEffect = timestamp.add(this.delay);
-  });
-
-  describe(`when ${type} delay has not taken effect yet`, function () {
-    beforeEach(`set next block timestamp before ${type} takes effect`, async function () {
-      await setNextBlockTimestamp(this.delayEffect.subn(1));
-    });
-
-    before();
-  });
-
-  describe(`when ${type} delay has taken effect`, function () {
-    beforeEach(`set next block timestamp when ${type} takes effect`, async function () {
-      await setNextBlockTimestamp(this.delayEffect);
-    });
-
-    after();
-  });
-}
-
-// ============ OPERATION HELPERS ============
-
-/**
- * @requires this.{manager,scheduleIn,caller,target,calldata}
- */
-function shouldBehaveLikeSchedulableOperation({ scheduled: { before, after, expired }, notScheduled }) {
-  describe('when operation is scheduled', function () {
-    beforeEach('schedule operation', async function () {
-      await impersonate(this.caller); // May be a contract
-      const { operationId } = await scheduleOperation(this.manager, {
-        caller: this.caller,
-        target: this.target.address,
-        calldata: this.calldata,
-        delay: this.scheduleIn,
-      });
-      this.operationId = operationId;
-    });
-
-    describe('when operation is not ready for execution', function () {
-      beforeEach('set next block time before operation is ready', async function () {
-        this.scheduledAt = await time.latest();
-        const schedule = await this.manager.getSchedule(this.operationId);
-        await setNextBlockTimestamp(schedule.subn(1));
-      });
-
-      before();
-    });
-
-    describe('when operation is ready for execution', function () {
-      beforeEach('set next block time when operation is ready for execution', async function () {
-        this.scheduledAt = await time.latest();
-        const schedule = await this.manager.getSchedule(this.operationId);
-        await setNextBlockTimestamp(schedule);
-      });
-
-      after();
-    });
-
-    describe('when operation has expired', function () {
-      beforeEach('set next block time when operation expired', async function () {
-        this.scheduledAt = await time.latest();
-        const schedule = await this.manager.getSchedule(this.operationId);
-        await setNextBlockTimestamp(schedule.add(EXPIRATION));
-      });
-
-      expired();
-    });
-  });
-
-  describe('when operation is not scheduled', function () {
-    beforeEach('set expected operationId', async function () {
-      this.operationId = await this.manager.hashOperation(this.caller, this.target.address, this.calldata);
-
-      // Assert operation is not scheduled
-      expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal(web3.utils.toBN(0));
-    });
-
-    notScheduled();
-  });
-}
-
-/**
- * @requires this.{manager,roles,target,calldata}
- */
-function shouldBehaveLikeARestrictedOperation({ callerIsNotTheManager, callerIsTheManager }) {
-  describe('when the call comes from the manager (msg.sender == manager)', function () {
-    beforeEach('define caller as manager', async function () {
-      this.caller = this.manager.address;
-      await impersonate(this.caller);
-    });
-
-    shouldBehaveLikeCanCallExecuting(callerIsTheManager);
-  });
-
-  describe('when the call does not come from the manager (msg.sender != manager)', function () {
-    beforeEach('define non manager caller', function () {
-      this.caller = this.roles.SOME.members[0];
-    });
-
-    callerIsNotTheManager();
-  });
-}
-
-/**
- * @requires this.{manager,roles,executionDelay,operationDelay,target}
- */
-function shouldBehaveLikeDelayedOperation() {
-  describe('with operation delay', function () {
-    describe('when operation delay is greater than execution delay', function () {
-      beforeEach('set operation delay', async function () {
-        this.operationDelay = this.executionDelay.add(time.duration.hours(1));
-        await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
-        this.scheduleIn = this.operationDelay; // For shouldBehaveLikeSchedulableOperation
-      });
-
-      shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
-    });
-
-    describe('when operation delay is shorter than execution delay', function () {
-      beforeEach('set operation delay', async function () {
-        this.operationDelay = this.executionDelay.sub(time.duration.hours(1));
-        await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
-        this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
-      });
-
-      shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
-    });
-  });
-
-  describe('without operation delay', function () {
-    beforeEach('set operation delay', async function () {
-      this.operationDelay = web3.utils.toBN(0);
-      await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
-    });
-
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
-  });
-}
-
-// ============ METHOD HELPERS ============
-
-/**
- * @requires this.{manager,roles,role,target,calldata}
- */
-function shouldBehaveLikeCanCall({
-  closed,
-  open: {
-    callerIsTheManager,
-    callerIsNotTheManager: { publicRoleIsRequired, specificRoleIsRequired },
-  },
-}) {
-  shouldBehaveLikeClosable({
-    closed,
-    open() {
-      shouldBehaveLikeARestrictedOperation({
-        callerIsTheManager,
-        callerIsNotTheManager() {
-          shouldBehaveLikeHasRole({
-            publicRoleIsRequired,
-            specificRoleIsRequired,
-          });
-        },
-      });
-    },
-  });
-}
-
-/**
- * @requires this.{target,calldata}
- */
-function shouldBehaveLikeCanCallExecuting({ executing, notExecuting }) {
-  describe('when _executionId is in storage for target and selector', function () {
-    beforeEach('set _executionId flag from calldata and target', async function () {
-      const executionId = await web3.utils.keccak256(
-        web3.eth.abi.encodeParameters(['address', 'bytes4'], [this.target.address, this.calldata.substring(0, 10)]),
-      );
-      await setStorageAt(this.manager.address, EXECUTION_ID_STORAGE_SLOT, executionId);
-    });
-
-    executing();
-  });
-
-  describe('when _executionId does not match target and selector', notExecuting);
-}
-
-/**
- * @requires this.{target,calldata,roles,role}
- */
-function shouldBehaveLikeHasRole({ publicRoleIsRequired, specificRoleIsRequired }) {
-  describe('when the function requires the caller to be granted with the PUBLIC_ROLE', function () {
-    beforeEach('set target function role as PUBLIC_ROLE', async function () {
-      this.role = this.roles.PUBLIC;
-      await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, {
-        from: this.roles.ADMIN.members[0],
-      });
-    });
-
-    publicRoleIsRequired();
-  });
-
-  describe('when the function requires the caller to be granted with a role other than PUBLIC_ROLE', function () {
-    beforeEach('set target function role as PUBLIC_ROLE', async function () {
-      await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, {
-        from: this.roles.ADMIN.members[0],
-      });
-    });
-
-    shouldBehaveLikeGetAccess(specificRoleIsRequired);
-  });
-}
-
-/**
- * @requires this.{manager,role,caller}
- */
-function shouldBehaveLikeGetAccess({
-  requiredRoleIsGranted: {
-    roleGrantingIsDelayed: {
-      // Because both grant and execution delay are set within the same $_grantRole call
-      // it's not possible to create a set of tests that diverge between grant and execution delay.
-      // Therefore, the shouldBehaveLikeDelay arguments are renamed for clarity:
-      // before => beforeGrantDelay
-      // after => afterGrantDelay
-      callerHasAnExecutionDelay: { beforeGrantDelay: case1, afterGrantDelay: case2 },
-      callerHasNoExecutionDelay: { beforeGrantDelay: case3, afterGrantDelay: case4 },
-    },
-    roleGrantingIsNotDelayed: { callerHasAnExecutionDelay: case5, callerHasNoExecutionDelay: case6 },
-  },
-  requiredRoleIsNotGranted,
-}) {
-  describe('when the required role is granted to the caller', function () {
-    describe('when role granting is delayed', function () {
-      beforeEach('define delay', function () {
-        this.grantDelay = time.duration.minutes(3);
-        this.delay = this.grantDelay; // For shouldBehaveLikeDelay
-      });
-
-      describe('when caller has an execution delay', function () {
-        beforeEach('set role and delay', async function () {
-          this.executionDelay = time.duration.hours(10);
-          this.delay = this.grantDelay;
-          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
-        });
-
-        shouldBehaveLikeDelay('grant', { before: case1, after: case2 });
-      });
-
-      describe('when caller has no execution delay', function () {
-        beforeEach('set role and delay', async function () {
-          this.executionDelay = web3.utils.toBN(0);
-          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
-        });
-
-        shouldBehaveLikeDelay('grant', { before: case3, after: case4 });
-      });
-    });
-
-    describe('when role granting is not delayed', function () {
-      beforeEach('define delay', function () {
-        this.grantDelay = web3.utils.toBN(0);
-      });
-
-      describe('when caller has an execution delay', function () {
-        beforeEach('set role and delay', async function () {
-          this.executionDelay = time.duration.hours(10);
-          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
-        });
-
-        case5();
-      });
-
-      describe('when caller has no execution delay', function () {
-        beforeEach('set role and delay', async function () {
-          this.executionDelay = web3.utils.toBN(0);
-          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
-        });
-
-        case6();
-      });
-    });
-  });
-
-  describe('when role is not granted', function () {
-    // Because this helper can be composed with other helpers, it's possible
-    // that role has been set already by another helper.
-    // Although this is highly unlikely, we check for it here to avoid false positives.
-    beforeEach('assert role is unset', async function () {
-      const { since } = await this.manager.getAccess(this.role.id, this.caller);
-      expect(since).to.be.bignumber.equal(web3.utils.toBN(0));
-    });
-
-    requiredRoleIsNotGranted();
-  });
-}
-
-// ============ ADMIN OPERATION HELPERS ============
+const {
+  LIKE_COMMON_IS_EXECUTING,
+  LIKE_COMMON_GET_ACCESS,
+  LIKE_COMMON_SCHEDULABLE,
+  testAsSchedulableOperation,
+  testAsRestrictedOperation,
+  testAsDelayedOperation,
+  testAsCanCall,
+  testAsHasRole,
+} = require('./AccessManager.predicate');
+
+// ============ ADMIN OPERATION ============
 
 /**
  * @requires this.{manager,roles,calldata,role}
  */
 function shouldBehaveLikeDelayedAdminOperation() {
-  const getAccessPath = COMMON_GET_ACCESS_PATH;
+  const getAccessPath = LIKE_COMMON_GET_ACCESS;
   getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () {
     beforeEach('consume previously set grant delay', async function () {
       // Consume previously set delay
       await mine();
     });
-    shouldBehaveLikeDelayedOperation();
+    testAsDelayedOperation();
   };
   getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () {
     beforeEach('set execution delay', async function () {
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeDelayedOperation
+      this.scheduleIn = this.executionDelay; // For testAsDelayedOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
 
   beforeEach('set target as manager', function () {
     this.target = this.manager;
   });
 
-  shouldBehaveLikeARestrictedOperation({
-    callerIsTheManager: COMMON_IS_EXECUTING_PATH,
+  testAsRestrictedOperation({
+    callerIsTheManager: LIKE_COMMON_IS_EXECUTING,
     callerIsNotTheManager() {
-      shouldBehaveLikeHasRole({
+      testAsHasRole({
         publicRoleIsRequired() {
           it('reverts as AccessManagerUnauthorizedAccount', async function () {
             await expectRevertCustomError(
@@ -524,29 +62,29 @@ function shouldBehaveLikeDelayedAdminOperation() {
  * @requires this.{manager,roles,calldata,role}
  */
 function shouldBehaveLikeNotDelayedAdminOperation() {
-  const getAccessPath = COMMON_GET_ACCESS_PATH;
+  const getAccessPath = LIKE_COMMON_GET_ACCESS;
   getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () {
     beforeEach('set execution delay', async function () {
       await mine();
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
   getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () {
     beforeEach('set execution delay', async function () {
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
 
   beforeEach('set target as manager', function () {
     this.target = this.manager;
   });
 
-  shouldBehaveLikeARestrictedOperation({
-    callerIsTheManager: COMMON_IS_EXECUTING_PATH,
+  testAsRestrictedOperation({
+    callerIsTheManager: LIKE_COMMON_IS_EXECUTING,
     callerIsNotTheManager() {
-      shouldBehaveLikeHasRole({
+      testAsHasRole({
         publicRoleIsRequired() {
           it('reverts as AccessManagerUnauthorizedAccount', async function () {
             await expectRevertCustomError(
@@ -566,29 +104,29 @@ function shouldBehaveLikeNotDelayedAdminOperation() {
  * @requires this.{manager,roles,calldata,role}
  */
 function shouldBehaveLikeRoleAdminOperation(roleAdmin) {
-  const getAccessPath = COMMON_GET_ACCESS_PATH;
+  const getAccessPath = LIKE_COMMON_GET_ACCESS;
   getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () {
     beforeEach('set operation delay', async function () {
       await mine();
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
   getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () {
     beforeEach('set execution delay', async function () {
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
 
   beforeEach('set target as manager', function () {
     this.target = this.manager;
   });
 
-  shouldBehaveLikeARestrictedOperation({
-    callerIsTheManager: COMMON_IS_EXECUTING_PATH,
+  testAsRestrictedOperation({
+    callerIsTheManager: LIKE_COMMON_IS_EXECUTING,
     callerIsNotTheManager() {
-      shouldBehaveLikeHasRole({
+      testAsHasRole({
         publicRoleIsRequired() {
           it('reverts as AccessManagerUnauthorizedAccount', async function () {
             await expectRevertCustomError(
@@ -604,7 +142,7 @@ function shouldBehaveLikeRoleAdminOperation(roleAdmin) {
   });
 }
 
-// ============ RESTRICTED OPERATION HELPERS ============
+// ============ RESTRICTED OPERATION ============
 
 /**
  * @requires this.{manager,roles,calldata,role}
@@ -620,7 +158,7 @@ function shouldBehaveLikeAManagedRestrictedOperation() {
     });
   }
 
-  const getAccessPath = COMMON_GET_ACCESS_PATH;
+  const getAccessPath = LIKE_COMMON_GET_ACCESS;
 
   getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.beforeGrantDelay =
     revertUnauthorized;
@@ -632,21 +170,21 @@ function shouldBehaveLikeAManagedRestrictedOperation() {
     beforeEach('consume previously set grant delay', async function () {
       // Consume previously set delay
       await mine();
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
   getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () {
     beforeEach('consume previously set grant delay', async function () {
-      this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
     });
-    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
   };
 
-  const isExecutingPath = COMMON_IS_EXECUTING_PATH;
+  const isExecutingPath = LIKE_COMMON_IS_EXECUTING;
   isExecutingPath.notExecuting = revertUnauthorized;
 
-  shouldBehaveLikeCanCall({
+  testAsCanCall({
     closed: revertUnauthorized,
     open: {
       callerIsTheManager: isExecutingPath,
@@ -666,46 +204,9 @@ function shouldBehaveLikeAManagedRestrictedOperation() {
   });
 }
 
-// ============ HELPERS ============
-
-/**
- * @requires this.{manager, caller, target, calldata}
- */
-async function scheduleOperation(manager, { caller, target, calldata, delay }) {
-  const timestamp = await time.latest();
-  const scheduledAt = timestamp.addn(1);
-  await setNextBlockTimestamp(scheduledAt); // Fix next block timestamp for predictability
-  const { receipt } = await manager.schedule(target, calldata, scheduledAt.add(delay), {
-    from: caller,
-  });
-
-  return {
-    receipt,
-    scheduledAt,
-    operationId: await manager.hashOperation(caller, target, calldata),
-  };
-}
-
 module.exports = {
-  // COMMON PATHS
-  COMMON_SCHEDULABLE_PATH,
-  COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY,
-  // MODE HELPERS
-  shouldBehaveLikeClosable,
-  // DELAY HELPERS
-  shouldBehaveLikeDelay,
-  // OPERATION HELPERS
-  shouldBehaveLikeSchedulableOperation,
-  // METHOD HELPERS
-  shouldBehaveLikeCanCall,
-  shouldBehaveLikeGetAccess,
-  shouldBehaveLikeHasRole,
-  // ADMIN OPERATION HELPERS
   shouldBehaveLikeDelayedAdminOperation,
   shouldBehaveLikeNotDelayedAdminOperation,
   shouldBehaveLikeRoleAdminOperation,
-  // RESTRICTED OPERATION HELPERS
   shouldBehaveLikeAManagedRestrictedOperation,
-  // HELPERS
-  scheduleOperation,
 };

test/access/manager/AccessManager.predicate.js

@@ -0,0 +1,461 @@
+const { setStorageAt } = require('@nomicfoundation/hardhat-network-helpers');
+const {
+  time: { setNextBlockTimestamp },
+} = require('@nomicfoundation/hardhat-network-helpers');
+const { time } = require('@openzeppelin/test-helpers');
+const { EXECUTION_ID_STORAGE_SLOT, EXPIRATION, scheduleOperation } = require('../../helpers/access-manager');
+const { impersonate } = require('../../helpers/account');
+const { expectRevertCustomError } = require('../../helpers/customError');
+
+// ============ COMMON PREDICATES ============
+
+const LIKE_COMMON_IS_EXECUTING = {
+  executing() {
+    it('succeeds', async function () {
+      await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
+    });
+  },
+  notExecuting() {
+    it('reverts as AccessManagerUnauthorizedAccount', async function () {
+      await expectRevertCustomError(
+        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+        'AccessManagerUnauthorizedAccount',
+        [this.caller, this.role.id],
+      );
+    });
+  },
+};
+
+const LIKE_COMMON_GET_ACCESS = {
+  requiredRoleIsGranted: {
+    roleGrantingIsDelayed: {
+      callerHasAnExecutionDelay: {
+        beforeGrantDelay() {
+          it('reverts as AccessManagerUnauthorizedAccount', async function () {
+            await expectRevertCustomError(
+              web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+              'AccessManagerUnauthorizedAccount',
+              [this.caller, this.role.id],
+            );
+          });
+        },
+        afterGrantDelay: undefined, // Diverges if there's an operation delay or not
+      },
+      callerHasNoExecutionDelay: {
+        beforeGrantDelay() {
+          it('reverts as AccessManagerUnauthorizedAccount', async function () {
+            await expectRevertCustomError(
+              web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+              'AccessManagerUnauthorizedAccount',
+              [this.caller, this.role.id],
+            );
+          });
+        },
+        afterGrantDelay() {
+          it('succeeds called directly', async function () {
+            await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
+          });
+
+          it('succeeds via execute', async function () {
+            await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+          });
+        },
+      },
+    },
+    roleGrantingIsNotDelayed: {
+      callerHasAnExecutionDelay: undefined, // Diverges if there's an operation to schedule or not
+      callerHasNoExecutionDelay() {
+        it('succeeds called directly', async function () {
+          await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
+        });
+
+        it('succeeds via execute', async function () {
+          await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+        });
+      },
+    },
+  },
+  requiredRoleIsNotGranted() {
+    it('reverts as AccessManagerUnauthorizedAccount', async function () {
+      await expectRevertCustomError(
+        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+        'AccessManagerUnauthorizedAccount',
+        [this.caller, this.role.id],
+      );
+    });
+  },
+};
+
+const LIKE_COMMON_SCHEDULABLE = {
+  scheduled: {
+    before() {
+      it('reverts as AccessManagerNotReady', async function () {
+        await expectRevertCustomError(
+          web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+          'AccessManagerNotReady',
+          [this.operationId],
+        );
+      });
+    },
+    after() {
+      it('succeeds called directly', async function () {
+        await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller });
+      });
+
+      it('succeeds via execute', async function () {
+        await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+      });
+    },
+    expired() {
+      it('reverts as AccessManagerExpired', async function () {
+        await expectRevertCustomError(
+          web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+          'AccessManagerExpired',
+          [this.operationId],
+        );
+      });
+    },
+  },
+  notScheduled() {
+    it('reverts as AccessManagerNotScheduled', async function () {
+      await expectRevertCustomError(
+        web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }),
+        'AccessManagerNotScheduled',
+        [this.operationId],
+      );
+    });
+  },
+};
+
+// ============ MODE ============
+
+/**
+ * @requires this.{manager,target}
+ */
+function testAsClosable({ closed, open }) {
+  describe('when the manager is closed', function () {
+    beforeEach('close', async function () {
+      await this.manager.$_setTargetClosed(this.target.address, true);
+    });
+
+    closed();
+  });
+
+  describe('when the manager is open', function () {
+    beforeEach('open', async function () {
+      await this.manager.$_setTargetClosed(this.target.address, false);
+    });
+
+    open();
+  });
+}
+
+// ============ DELAY ============
+
+/**
+ * @requires this.{delay}
+ */
+function testAsDelay(type, { before, after }) {
+  beforeEach('define timestamp when delay takes effect', async function () {
+    const timestamp = await time.latest();
+    this.delayEffect = timestamp.add(this.delay);
+  });
+
+  describe(`when ${type} delay has not taken effect yet`, function () {
+    beforeEach(`set next block timestamp before ${type} takes effect`, async function () {
+      await setNextBlockTimestamp(this.delayEffect.subn(1));
+    });
+
+    before();
+  });
+
+  describe(`when ${type} delay has taken effect`, function () {
+    beforeEach(`set next block timestamp when ${type} takes effect`, async function () {
+      await setNextBlockTimestamp(this.delayEffect);
+    });
+
+    after();
+  });
+}
+
+// ============ OPERATION ============
+
+/**
+ * @requires this.{manager,scheduleIn,caller,target,calldata}
+ */
+function testAsSchedulableOperation({ scheduled: { before, after, expired }, notScheduled }) {
+  describe('when operation is scheduled', function () {
+    beforeEach('schedule operation', async function () {
+      await impersonate(this.caller); // May be a contract
+      const { operationId } = await scheduleOperation(this.manager, {
+        caller: this.caller,
+        target: this.target.address,
+        calldata: this.calldata,
+        delay: this.scheduleIn,
+      });
+      this.operationId = operationId;
+    });
+
+    describe('when operation is not ready for execution', function () {
+      beforeEach('set next block time before operation is ready', async function () {
+        this.scheduledAt = await time.latest();
+        const schedule = await this.manager.getSchedule(this.operationId);
+        await setNextBlockTimestamp(schedule.subn(1));
+      });
+
+      before();
+    });
+
+    describe('when operation is ready for execution', function () {
+      beforeEach('set next block time when operation is ready for execution', async function () {
+        this.scheduledAt = await time.latest();
+        const schedule = await this.manager.getSchedule(this.operationId);
+        await setNextBlockTimestamp(schedule);
+      });
+
+      after();
+    });
+
+    describe('when operation has expired', function () {
+      beforeEach('set next block time when operation expired', async function () {
+        this.scheduledAt = await time.latest();
+        const schedule = await this.manager.getSchedule(this.operationId);
+        await setNextBlockTimestamp(schedule.add(EXPIRATION));
+      });
+
+      expired();
+    });
+  });
+
+  describe('when operation is not scheduled', function () {
+    beforeEach('set expected operationId', async function () {
+      this.operationId = await this.manager.hashOperation(this.caller, this.target.address, this.calldata);
+
+      // Assert operation is not scheduled
+      expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal(web3.utils.toBN(0));
+    });
+
+    notScheduled();
+  });
+}
+
+/**
+ * @requires this.{manager,roles,target,calldata}
+ */
+function testAsRestrictedOperation({ callerIsTheManager: { executing, notExecuting }, callerIsNotTheManager }) {
+  describe('when the call comes from the manager (msg.sender == manager)', function () {
+    beforeEach('define caller as manager', async function () {
+      this.caller = this.manager.address;
+      await impersonate(this.caller);
+    });
+
+    describe('when _executionId is in storage for target and selector', function () {
+      beforeEach('set _executionId flag from calldata and target', async function () {
+        const executionId = web3.utils.keccak256(
+          web3.eth.abi.encodeParameters(['address', 'bytes4'], [this.target.address, this.calldata.substring(0, 10)]),
+        );
+        await setStorageAt(this.manager.address, EXECUTION_ID_STORAGE_SLOT, executionId);
+      });
+
+      executing();
+    });
+
+    describe('when _executionId does not match target and selector', notExecuting);
+  });
+
+  describe('when the call does not come from the manager (msg.sender != manager)', function () {
+    beforeEach('define non manager caller', function () {
+      this.caller = this.roles.SOME.members[0];
+    });
+
+    callerIsNotTheManager();
+  });
+}
+
+/**
+ * @requires this.{manager,scheduleIn,caller,target,calldata,executionDelay}
+ */
+function testAsDelayedOperation() {
+  describe('with operation delay', function () {
+    describe('when operation delay is greater than execution delay', function () {
+      beforeEach('set operation delay', async function () {
+        this.operationDelay = this.executionDelay.add(time.duration.hours(1));
+        await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
+        this.scheduleIn = this.operationDelay; // For testAsSchedulableOperation
+      });
+
+      testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
+    });
+
+    describe('when operation delay is shorter than execution delay', function () {
+      beforeEach('set operation delay', async function () {
+        this.operationDelay = this.executionDelay.sub(time.duration.hours(1));
+        await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
+        this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
+      });
+
+      testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
+    });
+  });
+
+  describe('without operation delay', function () {
+    beforeEach('set operation delay', async function () {
+      this.operationDelay = web3.utils.toBN(0);
+      await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay);
+      this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
+    });
+
+    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
+  });
+}
+
+// ============ METHOD ============
+
+/**
+ * @requires this.{manager,roles,role,target,calldata}
+ */
+function testAsCanCall({
+  closed,
+  open: {
+    callerIsTheManager,
+    callerIsNotTheManager: { publicRoleIsRequired, specificRoleIsRequired },
+  },
+}) {
+  testAsClosable({
+    closed,
+    open() {
+      testAsRestrictedOperation({
+        callerIsTheManager,
+        callerIsNotTheManager() {
+          testAsHasRole({
+            publicRoleIsRequired,
+            specificRoleIsRequired,
+          });
+        },
+      });
+    },
+  });
+}
+
+/**
+ * @requires this.{target,calldata,roles,role}
+ */
+function testAsHasRole({ publicRoleIsRequired, specificRoleIsRequired }) {
+  describe('when the function requires the caller to be granted with the PUBLIC_ROLE', function () {
+    beforeEach('set target function role as PUBLIC_ROLE', async function () {
+      this.role = this.roles.PUBLIC;
+      await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, {
+        from: this.roles.ADMIN.members[0],
+      });
+    });
+
+    publicRoleIsRequired();
+  });
+
+  describe('when the function requires the caller to be granted with a role other than PUBLIC_ROLE', function () {
+    beforeEach('set target function role as PUBLIC_ROLE', async function () {
+      await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, {
+        from: this.roles.ADMIN.members[0],
+      });
+    });
+
+    testAsGetAccess(specificRoleIsRequired);
+  });
+}
+
+/**
+ * @requires this.{manager,role,caller}
+ */
+function testAsGetAccess({
+  requiredRoleIsGranted: {
+    roleGrantingIsDelayed: {
+      // Because both grant and execution delay are set within the same $_grantRole call
+      // it's not possible to create a set of tests that diverge between grant and execution delay.
+      // Therefore, the testAsDelay arguments are renamed for clarity:
+      // before => beforeGrantDelay
+      // after => afterGrantDelay
+      callerHasAnExecutionDelay: { beforeGrantDelay: case1, afterGrantDelay: case2 },
+      callerHasNoExecutionDelay: { beforeGrantDelay: case3, afterGrantDelay: case4 },
+    },
+    roleGrantingIsNotDelayed: { callerHasAnExecutionDelay: case5, callerHasNoExecutionDelay: case6 },
+  },
+  requiredRoleIsNotGranted,
+}) {
+  describe('when the required role is granted to the caller', function () {
+    describe('when role granting is delayed', function () {
+      beforeEach('define delay', function () {
+        this.grantDelay = time.duration.minutes(3);
+        this.delay = this.grantDelay; // For testAsDelay
+      });
+
+      describe('when caller has an execution delay', function () {
+        beforeEach('set role and delay', async function () {
+          this.executionDelay = time.duration.hours(10);
+          this.delay = this.grantDelay;
+          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
+        });
+
+        testAsDelay('grant', { before: case1, after: case2 });
+      });
+
+      describe('when caller has no execution delay', function () {
+        beforeEach('set role and delay', async function () {
+          this.executionDelay = web3.utils.toBN(0);
+          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
+        });
+
+        testAsDelay('grant', { before: case3, after: case4 });
+      });
+    });
+
+    describe('when role granting is not delayed', function () {
+      beforeEach('define delay', function () {
+        this.grantDelay = web3.utils.toBN(0);
+      });
+
+      describe('when caller has an execution delay', function () {
+        beforeEach('set role and delay', async function () {
+          this.executionDelay = time.duration.hours(10);
+          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
+        });
+
+        case5();
+      });
+
+      describe('when caller has no execution delay', function () {
+        beforeEach('set role and delay', async function () {
+          this.executionDelay = web3.utils.toBN(0);
+          await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay);
+        });
+
+        case6();
+      });
+    });
+  });
+
+  describe('when role is not granted', function () {
+    // Because this helper can be composed with other helpers, it's possible
+    // that role has been set already by another helper.
+    // Although this is highly unlikely, we check for it here to avoid false positives.
+    beforeEach('assert role is unset', async function () {
+      const { since } = await this.manager.getAccess(this.role.id, this.caller);
+      expect(since).to.be.bignumber.equal(web3.utils.toBN(0));
+    });
+
+    requiredRoleIsNotGranted();
+  });
+}
+
+module.exports = {
+  LIKE_COMMON_IS_EXECUTING,
+  LIKE_COMMON_GET_ACCESS,
+  LIKE_COMMON_SCHEDULABLE,
+  testAsClosable,
+  testAsDelay,
+  testAsSchedulableOperation,
+  testAsRestrictedOperation,
+  testAsDelayedOperation,
+  testAsCanCall,
+  testAsHasRole,
+  testAsGetAccess,
+};

test/access/manager/AccessManager.test.js

@@ -10,30 +10,24 @@ const {
   MINSETBACK,
   EXECUTION_ID_STORAGE_SLOT,
   CONSUMING_SCHEDULE_STORAGE_SLOT,
+  scheduleOperation,
+  hashOperation,
 } = require('../../helpers/access-manager');
 const {
-  // COMMON PATHS
-  COMMON_SCHEDULABLE_PATH,
-  COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY,
-  // MODE HELPERS
-  shouldBehaveLikeClosable,
-  // DELAY HELPERS
-  shouldBehaveLikeDelay,
-  // OPERATION HELPERS
-  shouldBehaveLikeSchedulableOperation,
-  // METHOD HELPERS
-  shouldBehaveLikeCanCall,
-  shouldBehaveLikeGetAccess,
-  shouldBehaveLikeHasRole,
-  // ADMIN OPERATION HELPERS
   shouldBehaveLikeDelayedAdminOperation,
   shouldBehaveLikeNotDelayedAdminOperation,
   shouldBehaveLikeRoleAdminOperation,
-  // RESTRICTED OPERATION HELPERS
   shouldBehaveLikeAManagedRestrictedOperation,
-  // HELPERS
-  scheduleOperation,
 } = require('./AccessManager.behavior');
+const {
+  LIKE_COMMON_SCHEDULABLE,
+  testAsClosable,
+  testAsDelay,
+  testAsSchedulableOperation,
+  testAsCanCall,
+  testAsHasRole,
+  testAsGetAccess,
+} = require('./AccessManager.predicate');
 const { default: Wallet } = require('ethereumjs-wallet');
 const {
   mine,
@@ -49,6 +43,20 @@ const Ownable = artifacts.require('$Ownable');
 
 const someAddress = Wallet.generate().getChecksumAddressString();
 
+// This test suite is made using the following tools:
+//
+// * Predicates: Functions with common conditional setups without assertions.
+// * Behaviors: Functions with common assertions.
+//
+// The behavioral tests are built by composing predicates and are used as templates
+// for testing access to restricted functions.
+//
+// Similarly, unit tests in this suite will use predicates to test subsets of these
+// behaviors and are helped by common assertions provided for some of the predicates.
+//
+// The predicates can be identified by the `testAs*` prefix while the behaviors
+// are prefixed with `shouldBehave*`. The common assertions for predicates are
+// defined as constants.
 contract('AccessManager', function (accounts) {
   const [admin, manager, guardian, member, user, other] = accounts;
 
@@ -120,7 +128,7 @@ contract('AccessManager', function (accounts) {
         this.role = { id: web3.utils.toBN(379204) };
       });
 
-      shouldBehaveLikeCanCall({
+      testAsCanCall({
         closed() {
           it('should return false and no delay', async function () {
             const { immediate, delay } = await this.manager.canCall(
@@ -193,10 +201,10 @@ contract('AccessManager', function (accounts) {
                       beforeEach('consume previously set grant delay', async function () {
                         // Consume previously set delay
                         await mine();
-                        this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation
+                        this.scheduleIn = this.executionDelay; // For testAsSchedulableOperation
                       });
 
-                      shouldBehaveLikeSchedulableOperation({
+                      testAsSchedulableOperation({
                         scheduled: {
                           before() {
                             beforeEach('consume previously set delay', async function () {
@@ -350,7 +358,7 @@ contract('AccessManager', function (accounts) {
     });
 
     describe('#isTargetClosed', function () {
-      shouldBehaveLikeClosable({
+      testAsClosable({
         closed() {
           it('returns true', async function () {
             expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(true);
@@ -390,10 +398,10 @@ contract('AccessManager', function (accounts) {
           this.newDelay = time.duration.days(10);
 
           await this.manager.$_setTargetAdminDelay(this.target.address, this.newDelay);
-          this.delay = MINSETBACK; // For shouldBehaveLikeDelay
+          this.delay = MINSETBACK; // For testAsDelay
         });
 
-        shouldBehaveLikeDelay('effect', {
+        testAsDelay('effect', {
           before() {
             beforeEach('consume previously set grant delay', async function () {
               // Consume previously set delay
@@ -463,10 +471,10 @@ contract('AccessManager', function (accounts) {
           this.newDelay = time.duration.days(11);
 
           await this.manager.$_setGrantDelay(roleId, this.newDelay);
-          this.delay = MINSETBACK; // For shouldBehaveLikeDelay
+          this.delay = MINSETBACK; // For testAsDelay
         });
 
-        shouldBehaveLikeDelay('grant', {
+        testAsDelay('grant', {
           before() {
             beforeEach('consume previously set grant delay', async function () {
               // Consume previously set delay
@@ -501,7 +509,7 @@ contract('AccessManager', function (accounts) {
         this.caller = user;
       });
 
-      shouldBehaveLikeGetAccess({
+      testAsGetAccess({
         requiredRoleIsGranted: {
           roleGrantingIsDelayed: {
             callerHasAnExecutionDelay: {
@@ -618,13 +626,13 @@ contract('AccessManager', function (accounts) {
     });
 
     describe('#hasRole', function () {
-      beforeEach('setup shouldBehaveLikeHasRole', function () {
+      beforeEach('setup testAsHasRole', function () {
         this.role = { id: web3.utils.toBN(49832) };
         this.calldata = '0x1234';
         this.caller = user;
       });
 
-      shouldBehaveLikeHasRole({
+      testAsHasRole({
         publicRoleIsRequired() {
           it('has PUBLIC role', async function () {
             const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller);
@@ -724,11 +732,11 @@ contract('AccessManager', function (accounts) {
         await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id);
         await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay
 
-        this.calldata = await this.target.contract.methods[method]().encodeABI();
-        this.scheduleIn = time.duration.days(10); // For shouldBehaveLikeSchedulableOperation
+        this.calldata = this.target.contract.methods[method]().encodeABI();
+        this.scheduleIn = time.duration.days(10); // For testAsSchedulableOperation
       });
 
-      shouldBehaveLikeSchedulableOperation({
+      testAsSchedulableOperation({
         scheduled: {
           before() {
             beforeEach('consume previously set grant delay', async function () {
@@ -782,7 +790,7 @@ contract('AccessManager', function (accounts) {
           await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id);
           await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay
 
-          this.calldata = await this.target.contract.methods[method]().encodeABI();
+          this.calldata = this.target.contract.methods[method]().encodeABI();
           this.delay = time.duration.days(10);
 
           const { operationId } = await scheduleOperation(this.manager, {
@@ -813,9 +821,7 @@ contract('AccessManager', function (accounts) {
 
         const args = [user, address, calldata];
 
-        expect(await this.manager.hashOperation(...args)).to.be.bignumber.eq(
-          await web3.utils.keccak256(web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], args)),
-        );
+        expect(await this.manager.hashOperation(...args)).to.be.bignumber.eq(hashOperation(...args));
       });
     });
   });
@@ -1317,10 +1323,10 @@ contract('AccessManager', function (accounts) {
                 });
 
                 this.receipt = receipt;
-                this.delay = this.grantDelay; // For shouldBehaveLikeDelay
+                this.delay = this.grantDelay; // For testAsDelay
               });
 
-              shouldBehaveLikeDelay('grant', {
+              testAsDelay('grant', {
                 before() {
                   beforeEach('consume previously set grant delay', async function () {
                     // Consume previously set delay
@@ -1505,7 +1511,7 @@ contract('AccessManager', function (accounts) {
                   this.grantTimestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
 
                   this.receipt = receipt;
-                  this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For shouldBehaveLikeDelay
+                  this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For testAsDelay
                 });
 
                 it('emits event', function () {
@@ -1518,7 +1524,7 @@ contract('AccessManager', function (accounts) {
                   });
                 });
 
-                shouldBehaveLikeDelay('execution delay effect', {
+                testAsDelay('execution delay effect', {
                   before() {
                     beforeEach('consume effect delay', async function () {
                       // Consume previously set delay
@@ -1631,7 +1637,7 @@ contract('AccessManager', function (accounts) {
                   this.grantTimestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
 
                   this.receipt = receipt;
-                  this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For shouldBehaveLikeDelay
+                  this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For testAsDelay
                 });
 
                 it('emits event', function () {
@@ -1644,7 +1650,7 @@ contract('AccessManager', function (accounts) {
                   });
                 });
 
-                shouldBehaveLikeDelay('execution delay effect', {
+                testAsDelay('execution delay effect', {
                   before() {
                     beforeEach('consume effect delay', async function () {
                       // Consume previously set delay
@@ -1713,10 +1719,10 @@ contract('AccessManager', function (accounts) {
               this.grantDelay = time.duration.weeks(1);
               await this.manager.$_grantRole(ANOTHER_ROLE, user, this.grantDelay, 0);
 
-              this.delay = this.grantDelay; // For shouldBehaveLikeDelay
+              this.delay = this.grantDelay; // For testAsDelay
             });
 
-            shouldBehaveLikeDelay('grant', {
+            testAsDelay('grant', {
               before() {
                 beforeEach('consume previously set grant delay', async function () {
                   // Consume previously set delay
@@ -1915,7 +1921,7 @@ contract('AccessManager', function (accounts) {
     });
 
     describe('restrictions', function () {
-      shouldBehaveLikeCanCall({
+      testAsCanCall({
         closed() {
           it('reverts as AccessManagerUnauthorizedCall', async function () {
             await expectRevertCustomError(
@@ -2103,12 +2109,7 @@ contract('AccessManager', function (accounts) {
 
     it('increases the nonce of an operation scheduled more than once', async function () {
       // Setup and check initial nonce
-      const expectedOperationId = await web3.utils.keccak256(
-        web3.eth.abi.encodeParameters(
-          ['address', 'address', 'bytes'],
-          [this.caller, this.target.address, this.calldata],
-        ),
-      );
+      const expectedOperationId = hashOperation(this.caller, this.target.address, this.calldata);
       expect(await this.manager.getNonce(expectedOperationId)).to.be.bignumber.eq('0');
 
       // Schedule
@@ -2244,7 +2245,7 @@ contract('AccessManager', function (accounts) {
     });
 
     describe('restrictions', function () {
-      shouldBehaveLikeCanCall({
+      testAsCanCall({
         closed() {
           it('reverts as AccessManagerUnauthorizedCall', async function () {
             await expectRevertCustomError(
@@ -2273,7 +2274,9 @@ contract('AccessManager', function (accounts) {
           },
           callerIsNotTheManager: {
             publicRoleIsRequired() {
-              shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY);
+              it('succeeds', async function () {
+                await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+              });
             },
             specificRoleIsRequired: {
               requiredRoleIsGranted: {
@@ -2295,7 +2298,7 @@ contract('AccessManager', function (accounts) {
                         this.scheduleIn = time.duration.days(21);
                       });
 
-                      shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+                      testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
                     },
                   },
                   callerHasNoExecutionDelay: {
@@ -2314,7 +2317,9 @@ contract('AccessManager', function (accounts) {
                         await mine();
                       });
 
-                      shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY);
+                      it('succeeds', async function () {
+                        await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+                      });
                     },
                   },
                 },
@@ -2324,10 +2329,12 @@ contract('AccessManager', function (accounts) {
                       this.scheduleIn = time.duration.days(15);
                     });
 
-                    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH);
+                    testAsSchedulableOperation(LIKE_COMMON_SCHEDULABLE);
                   },
                   callerHasNoExecutionDelay() {
-                    shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY);
+                    it('succeeds', async function () {
+                      await this.manager.execute(this.target.address, this.calldata, { from: this.caller });
+                    });
                   },
                 },
               },
@@ -2427,7 +2434,7 @@ contract('AccessManager', function (accounts) {
       await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id);
       await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay
 
-      this.scheduleIn = time.duration.hours(10); // For shouldBehaveLikeSchedulableOperation
+      this.scheduleIn = time.duration.hours(10); // For testAsSchedulableOperation
     });
 
     describe('when caller is not consuming scheduled operation', function () {
@@ -2450,7 +2457,7 @@ contract('AccessManager', function (accounts) {
         await this.target.setIsConsumingScheduledOp(true, `0x${CONSUMING_SCHEDULE_STORAGE_SLOT.toString(16)}`);
       });
 
-      shouldBehaveLikeSchedulableOperation({
+      testAsSchedulableOperation({
         scheduled: {
           before() {
             it('reverts as AccessManagerNotReady', async function () {
@@ -2511,11 +2518,11 @@ contract('AccessManager', function (accounts) {
       await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.roles.SOME.id);
       await this.manager.$_grantRole(this.roles.SOME.id, this.caller, 0, 1); // nonzero execution delay
 
-      this.calldata = await this.target.contract.methods[method]().encodeABI();
-      this.scheduleIn = time.duration.days(10); // For shouldBehaveLikeSchedulableOperation
+      this.calldata = this.target.contract.methods[method]().encodeABI();
+      this.scheduleIn = time.duration.days(10); // For testAsSchedulableOperation
     });
 
-    shouldBehaveLikeSchedulableOperation({
+    testAsSchedulableOperation({
       scheduled: {
         before() {
           describe('when caller is the scheduler', function () {

test/access/manager/AuthorityUtils.test.js

@@ -53,22 +53,19 @@ contract('AuthorityUtils', function (accounts) {
     describe('when authority replies with a delay', function () {
       beforeEach(async function () {
         this.authority = await AuthorityDelayMock.new();
-        this.immediate = true;
-        this.delay = web3.utils.toBN(42);
-        await this.authority._setImmediate(this.immediate);
-        await this.authority._setDelay(this.delay);
       });
 
-      it('returns (immediate, delay)', async function () {
-        const { immediate, delay } = await this.mock.$canCallWithDelay(
-          this.authority.address,
-          user,
-          other,
-          '0x12345678',
-        );
-        expect(immediate).to.equal(this.immediate);
-        expect(delay).to.be.bignumber.equal(this.delay);
-      });
+      for (const immediate of [true, false]) {
+        for (const delay of ['0', '42']) {
+          it(`returns (immediate=${immediate}, delay=${delay})`, async function () {
+            await this.authority._setImmediate(immediate);
+            await this.authority._setDelay(delay);
+            const result = await this.mock.$canCallWithDelay(this.authority.address, user, other, '0x12345678');
+            expect(result.immediate).to.equal(immediate);
+            expect(result.delay).to.be.bignumber.equal(delay);
+          });
+        }
+      }
     });
 
     describe('when authority replies with empty data', function () {

test/governance/extensions/GovernorTimelockAccess.test.js

@@ -6,19 +6,18 @@ const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/govern
 const { expectRevertCustomError } = require('../../helpers/customError');
 const { clockFromReceipt } = require('../../helpers/time');
 const { selector } = require('../../helpers/methods');
+const { hashOperation } = require('../../helpers/access-manager');
 
 const AccessManager = artifacts.require('$AccessManager');
 const Governor = artifacts.require('$GovernorTimelockAccessMock');
 const AccessManagedTarget = artifacts.require('$AccessManagedTarget');
+const Ownable = artifacts.require('$Ownable');
 
 const TOKENS = [
   { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' },
   { Token: artifacts.require('$ERC20VotesTimestampMock'), mode: 'timestamp' },
 ];
 
-const hashOperation = (caller, target, data) =>
-  web3.utils.keccak256(web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [caller, target, data]));
-
 contract('GovernorTimelockAccess', function (accounts) {
   const [admin, voter1, voter2, voter3, voter4, other] = accounts;
 
@@ -245,6 +244,144 @@ contract('GovernorTimelockAccess', function (accounts) {
         }
       });
 
+      it('does not need to queue proposals with no delay', async function () {
+        const roleId = '1';
+
+        const executionDelay = web3.utils.toBN(0);
+        const baseDelay = web3.utils.toBN(0);
+
+        // Set execution delay
+        await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+          from: admin,
+        });
+        await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+        // Set base delay
+        await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+        this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr');
+        await this.helper.propose();
+        expect(await this.mock.proposalNeedsQueuing(this.helper.currentProposal.id)).to.be.false;
+      });
+
+      it('needs to queue proposals with any delay', async function () {
+        const roleId = '1';
+
+        const delays = [
+          [time.duration.hours(1), time.duration.hours(2)],
+          [time.duration.hours(2), time.duration.hours(1)],
+        ];
+
+        for (const [executionDelay, baseDelay] of delays) {
+          // Set execution delay
+          await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+            from: admin,
+          });
+          await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+          // Set base delay
+          await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+          const helper = new GovernorHelper(this.mock, mode);
+          this.proposal = await helper.setProposal(
+            [this.restricted.operation],
+            `executionDelay=${executionDelay.toString()}}baseDelay=${baseDelay.toString()}}`,
+          );
+          await helper.propose();
+          expect(await this.mock.proposalNeedsQueuing(helper.currentProposal.id)).to.be.true;
+        }
+      });
+
+      describe('execution plan', function () {
+        it('returns plan for delayed operations', async function () {
+          const roleId = '1';
+
+          const delays = [
+            [time.duration.hours(1), time.duration.hours(2)],
+            [time.duration.hours(2), time.duration.hours(1)],
+          ];
+
+          for (const [executionDelay, baseDelay] of delays) {
+            // Set execution delay
+            await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+              from: admin,
+            });
+            await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+            // Set base delay
+            await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+            const helper = new GovernorHelper(this.mock, mode);
+            this.proposal = await helper.setProposal(
+              [this.restricted.operation],
+              `executionDelay=${executionDelay.toString()}}baseDelay=${baseDelay.toString()}}`,
+            );
+            await helper.propose();
+            const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
+            const maxDelay = web3.utils.toBN(Math.max(baseDelay.toNumber(), executionDelay.toNumber()));
+            expect(planDelay).to.be.bignumber.eq(maxDelay);
+            expect(indirect).to.deep.eq([true]);
+            expect(withDelay).to.deep.eq([true]);
+          }
+        });
+
+        it('returns plan for not delayed operations', async function () {
+          const roleId = '1';
+
+          const executionDelay = web3.utils.toBN(0);
+          const baseDelay = web3.utils.toBN(0);
+
+          // Set execution delay
+          await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+            from: admin,
+          });
+          await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+          // Set base delay
+          await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+          this.proposal = await this.helper.setProposal([this.restricted.operation], `descr`);
+          await this.helper.propose();
+          const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
+          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(0));
+          expect(indirect).to.deep.eq([true]);
+          expect(withDelay).to.deep.eq([false]);
+        });
+
+        it('returns plan for an operation ignoring the manager', async function () {
+          await this.mock.$_setAccessManagerIgnored(this.receiver.address, this.restricted.selector, true);
+
+          const roleId = '1';
+
+          const delays = [
+            [time.duration.hours(1), time.duration.hours(2)],
+            [time.duration.hours(2), time.duration.hours(1)],
+          ];
+
+          for (const [executionDelay, baseDelay] of delays) {
+            // Set execution delay
+            await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+              from: admin,
+            });
+            await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+            // Set base delay
+            await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+            const helper = new GovernorHelper(this.mock, mode);
+            this.proposal = await helper.setProposal(
+              [this.restricted.operation],
+              `executionDelay=${executionDelay.toString()}}baseDelay=${baseDelay.toString()}}`,
+            );
+            await helper.propose();
+            const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
+            expect(planDelay).to.be.bignumber.eq(baseDelay);
+            expect(indirect).to.deep.eq([false]);
+            expect(withDelay).to.deep.eq([false]);
+          }
+        });
+      });
+
       describe('base delay only', function () {
         for (const [delay, queue] of [
           [0, true],
@@ -257,10 +394,6 @@ contract('GovernorTimelockAccess', function (accounts) {
             this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr');
 
             await this.helper.propose();
-            const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-            expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-            expect(indirect).to.deep.eq([false]);
-            expect(withDelay).to.deep.eq([false]);
 
             await this.helper.waitForSnapshot();
             await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
@@ -286,12 +419,6 @@ contract('GovernorTimelockAccess', function (accounts) {
         this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr');
 
         await this.helper.propose();
-        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true);
-        const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-        expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-        expect(indirect).to.deep.eq([false]);
-        expect(withDelay).to.deep.eq([false]);
-
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
         await this.helper.waitForDeadline();
@@ -317,15 +444,6 @@ contract('GovernorTimelockAccess', function (accounts) {
 
         // Go through all the governance process
         await original.propose();
-        expect(await this.mock.proposalNeedsQueuing(original.currentProposal.id)).to.be.eq(true);
-        const {
-          delay: planDelay,
-          indirect,
-          withDelay,
-        } = await this.mock.proposalExecutionPlan(original.currentProposal.id);
-        expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-        expect(indirect).to.deep.eq([true, false]);
-        expect(withDelay).to.deep.eq([true, false]);
         await original.waitForSnapshot();
         await original.vote({ support: Enums.VoteType.For }, { from: voter1 });
         await original.waitForDeadline();
@@ -367,12 +485,6 @@ contract('GovernorTimelockAccess', function (accounts) {
         this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr');
 
         await this.helper.propose();
-        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true);
-        const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-        expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-        expect(indirect).to.deep.eq([true]);
-        expect(withDelay).to.deep.eq([true]);
-
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
         await this.helper.waitForDeadline();
@@ -417,12 +529,6 @@ contract('GovernorTimelockAccess', function (accounts) {
         );
 
         await this.helper.propose();
-        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true);
-        const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-        expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(baseDelay));
-        expect(indirect).to.deep.eq([true, false, false]);
-        expect(withDelay).to.deep.eq([true, false, false]);
-
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
         await this.helper.waitForDeadline();
@@ -465,12 +571,6 @@ contract('GovernorTimelockAccess', function (accounts) {
           this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr');
 
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true);
-          const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-          expect(indirect).to.deep.eq([true]);
-          expect(withDelay).to.deep.eq([true]);
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -498,15 +598,6 @@ contract('GovernorTimelockAccess', function (accounts) {
 
           // Go through all the governance process
           await original.propose();
-          expect(await this.mock.proposalNeedsQueuing(original.currentProposal.id)).to.be.eq(true);
-          const {
-            delay: planDelay,
-            indirect,
-            withDelay,
-          } = await this.mock.proposalExecutionPlan(original.currentProposal.id);
-          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay));
-          expect(indirect).to.deep.eq([true]);
-          expect(withDelay).to.deep.eq([true]);
           await original.waitForSnapshot();
           await original.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await original.waitForDeadline();
@@ -548,12 +639,6 @@ contract('GovernorTimelockAccess', function (accounts) {
           this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr');
 
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false);
-          const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0'));
-          expect(indirect).to.deep.eq([false]);
-          expect(withDelay).to.deep.eq([false]);
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -575,12 +660,6 @@ contract('GovernorTimelockAccess', function (accounts) {
           this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr');
 
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false);
-          const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0'));
-          expect(indirect).to.deep.eq([false]);
-          expect(withDelay).to.deep.eq([false]);
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -692,12 +771,6 @@ contract('GovernorTimelockAccess', function (accounts) {
           );
 
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false);
-          const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id);
-          expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0'));
-          expect(indirect).to.deep.eq([]); // Governor operations ignore access manager
-          expect(withDelay).to.deep.eq([]); // Governor operations ignore access manager
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -738,14 +811,8 @@ contract('GovernorTimelockAccess', function (accounts) {
           await this.manager.setTargetFunctionRole(target, [selector], roleId, { from: admin });
           await this.manager.grantRole(roleId, this.mock.address, 0, { from: admin });
 
-          const proposal = await this.helper.setProposal([{ target, data, value: '0' }], '1');
+          this.proposal = await this.helper.setProposal([{ target, data, value: '0' }], '1');
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false);
-          const plan = await this.mock.proposalExecutionPlan(proposal.id);
-          expect(plan.delay).to.be.bignumber.eq(web3.utils.toBN('0'));
-          expect(plan.indirect).to.deep.eq([true]);
-          expect(plan.withDelay).to.deep.eq([false]);
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -757,14 +824,8 @@ contract('GovernorTimelockAccess', function (accounts) {
 
           await this.mock.$_setAccessManagerIgnored(target, selector, true);
 
-          const proposalIgnored = await this.helper.setProposal([{ target, data, value: '0' }], '2');
+          await this.helper.setProposal([{ target, data, value: '0' }], '2');
           await this.helper.propose();
-          expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false);
-          const planIgnored = await this.mock.proposalExecutionPlan(proposalIgnored.id);
-          expect(planIgnored.delay).to.be.bignumber.eq(web3.utils.toBN('0'));
-          expect(planIgnored.indirect).to.deep.eq([false]);
-          expect(planIgnored.withDelay).to.deep.eq([false]);
-
           await this.helper.waitForSnapshot();
           await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
           await this.helper.waitForDeadline();
@@ -772,6 +833,65 @@ contract('GovernorTimelockAccess', function (accounts) {
           expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.mock.address });
         });
       });
+
+      describe('operating on an Ownable contract', function () {
+        const method = selector('$_checkOwner()');
+
+        beforeEach(async function () {
+          this.ownable = await Ownable.new(this.manager.address);
+          this.operation = {
+            target: this.ownable.address,
+            value: '0',
+            data: this.ownable.contract.methods.$_checkOwner().encodeABI(),
+          };
+        });
+
+        it('succeeds with delay', async function () {
+          const roleId = '1';
+          const executionDelay = time.duration.hours(2);
+          const baseDelay = time.duration.hours(1);
+
+          // Set execution delay
+          await this.manager.setTargetFunctionRole(this.ownable.address, [method], roleId, {
+            from: admin,
+          });
+          await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+          // Set base delay
+          await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+          this.proposal = await this.helper.setProposal([this.operation], `descr`);
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          await this.helper.queue();
+          await this.helper.waitForEta();
+          await this.helper.execute(); // Don't revert
+        });
+
+        it('succeeds without delay', async function () {
+          const roleId = '1';
+          const executionDelay = web3.utils.toBN(0);
+          const baseDelay = web3.utils.toBN(0);
+
+          // Set execution delay
+          await this.manager.setTargetFunctionRole(this.ownable.address, [method], roleId, {
+            from: admin,
+          });
+          await this.manager.grantRole(roleId, this.mock.address, executionDelay, { from: admin });
+
+          // Set base delay
+          await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+          this.proposal = await this.helper.setProposal([this.operation], `descr`);
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          await this.helper.execute(); // Don't revert
+        });
+      });
     });
   }
 });

test/helpers/access-manager.js

@@ -1,6 +1,9 @@
 const { time } = require('@openzeppelin/test-helpers');
 const { MAX_UINT64 } = require('./constants');
-const { artifacts } = require('hardhat');
+const { namespaceSlot } = require('./namespaced-storage');
+const {
+  time: { setNextBlockTimestamp },
+} = require('@nomicfoundation/hardhat-network-helpers');
 
 function buildBaseRoles() {
   const roles = {
@@ -44,21 +47,30 @@ const formatAccess = access => [access[0], access[1].toString()];
 const MINSETBACK = time.duration.days(5);
 const EXPIRATION = time.duration.weeks(1);
 
-let EXECUTION_ID_STORAGE_SLOT = 3n;
-let CONSUMING_SCHEDULE_STORAGE_SLOT = 0n;
-try {
-  // Try to get the artifact paths, will throw if it doesn't exist
-  artifacts._getArtifactPathSync('AccessManagerUpgradeable');
-  artifacts._getArtifactPathSync('AccessManagedUpgradeable');
+const EXECUTION_ID_STORAGE_SLOT = namespaceSlot('AccessManager', 3n);
+const CONSUMING_SCHEDULE_STORAGE_SLOT = namespaceSlot('AccessManaged', 0n);
 
-  // ERC-7201 namespace location for AccessManager
-  EXECUTION_ID_STORAGE_SLOT += 0x40c6c8c28789853c7efd823ab20824bbd71718a8a5915e855f6f288c9a26ad00n;
-  // ERC-7201 namespace location for AccessManaged
-  CONSUMING_SCHEDULE_STORAGE_SLOT += 0xf3177357ab46d8af007ab3fdb9af81da189e1068fefdc0073dca88a2cab40a00n;
-} catch (_) {
-  // eslint-disable-next-line no-empty
+/**
+ * @requires this.{manager, caller, target, calldata}
+ */
+async function scheduleOperation(manager, { caller, target, calldata, delay }) {
+  const timestamp = await time.latest();
+  const scheduledAt = timestamp.addn(1);
+  await setNextBlockTimestamp(scheduledAt); // Fix next block timestamp for predictability
+  const { receipt } = await manager.schedule(target, calldata, scheduledAt.add(delay), {
+    from: caller,
+  });
+
+  return {
+    receipt,
+    scheduledAt,
+    operationId: hashOperation(caller, target, calldata),
+  };
 }
 
+const hashOperation = (caller, target, data) =>
+  web3.utils.keccak256(web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [caller, target, data]));
+
 module.exports = {
   buildBaseRoles,
   formatAccess,
@@ -66,4 +78,6 @@ module.exports = {
   EXPIRATION,
   EXECUTION_ID_STORAGE_SLOT,
   CONSUMING_SCHEDULE_STORAGE_SLOT,
+  scheduleOperation,
+  hashOperation,
 };

test/helpers/namespaced-storage.js

@@ -0,0 +1,30 @@
+const { artifacts } = require('hardhat');
+
+function namespaceId(contractName) {
+  return `openzeppelin.storage.${contractName}`;
+}
+
+function namespaceLocation(id) {
+  const hashIdBN = web3.utils.toBN(web3.utils.keccak256(id)).subn(1); // keccak256(id) - 1
+  const hashIdHex = web3.utils.padLeft(web3.utils.numberToHex(hashIdBN), 64);
+
+  const mask = BigInt(web3.utils.padLeft('0x00', 64, 'f')); // ~0xff
+
+  return BigInt(web3.utils.keccak256(hashIdHex)) & mask;
+}
+
+function namespaceSlot(contractName, offset) {
+  try {
+    // Try to get the artifact paths, will throw if it doesn't exist
+    artifacts._getArtifactPathSync(`${contractName}Upgradeable`);
+    return offset + namespaceLocation(namespaceId(contractName));
+  } catch (_) {
+    return offset;
+  }
+}
+
+module.exports = {
+  namespaceSlot,
+  namespaceLocation,
+  namespaceId,
+};