Improve GovernorTimelockAccess (#4591)

Co-authored-by: Francisco <fg@frang.io>

contracts/access/manager/AccessManager.sol

@@ -582,13 +582,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
 
         // If caller is authorised, schedule operation
-        operationId = _hashOperation(caller, target, data);
+        operationId = hashOperation(caller, target, data);
 
-        // Cannot reschedule unless the operation has expired
-        uint48 prevTimepoint = _schedules[operationId].timepoint;
-        if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) {
-            revert AccessManagerAlreadyScheduled(operationId);
-        }
+        _checkNotScheduled(operationId);
 
         unchecked {
             // It's not feasible to overflow the nonce in less than 1000 years
@@ -601,6 +597,17 @@ contract AccessManager is Context, Multicall, IAccessManager {
         // Using named return values because otherwise we get stack too deep
     }
 
+    /**
+     * @dev Reverts if the operation is currently scheduled and has not expired.
+     * (Note: This function was introduced due to stack too deep errors in schedule.)
+     */
+    function _checkNotScheduled(bytes32 operationId) private view {
+        uint48 prevTimepoint = _schedules[operationId].timepoint;
+        if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) {
+            revert AccessManagerAlreadyScheduled(operationId);
+        }
+    }
+
     /**
      * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the
      * execution delay is 0.
@@ -625,7 +632,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
 
         // If caller is authorised, check operation was scheduled early enough
-        bytes32 operationId = _hashOperation(caller, target, data);
+        bytes32 operationId = hashOperation(caller, target, data);
         uint32 nonce;
 
         if (setback != 0) {
@@ -659,7 +666,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         if (IAccessManaged(target).isConsumingScheduledOp() != IAccessManaged.isConsumingScheduledOp.selector) {
             revert AccessManagerUnauthorizedConsume(target);
         }
-        _consumeScheduledOp(_hashOperation(caller, target, data));
+        _consumeScheduledOp(hashOperation(caller, target, data));
     }
 
     /**
@@ -699,7 +706,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         address msgsender = _msgSender();
         bytes4 selector = bytes4(data[0:4]);
 
-        bytes32 operationId = _hashOperation(caller, target, data);
+        bytes32 operationId = hashOperation(caller, target, data);
         if (_schedules[operationId].timepoint == 0) {
             revert AccessManagerNotScheduled(operationId);
         } else if (caller != msgsender) {
@@ -721,7 +728,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     /**
      * @dev Hashing function for delayed operations
      */
-    function _hashOperation(address caller, address target, bytes calldata data) private pure returns (bytes32) {
+    function hashOperation(address caller, address target, bytes calldata data) public view virtual returns (bytes32) {
         return keccak256(abi.encode(caller, target, data));
     }
 
@@ -756,7 +763,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
                 (, uint64 requiredRole, ) = _getAdminRestrictions(_msgData());
                 revert AccessManagerUnauthorizedAccount(caller, requiredRole);
             } else {
-                _consumeScheduledOp(_hashOperation(caller, address(this), _msgData()));
+                _consumeScheduledOp(hashOperation(caller, address(this), _msgData()));
             }
         }
     }

contracts/access/manager/IAccessManager.sol

@@ -57,6 +57,8 @@ interface IAccessManager {
         bytes4 selector
     ) external view returns (bool allowed, uint32 delay);
 
+    function hashOperation(address caller, address target, bytes calldata data) external view returns (bytes32);
+
     function expiration() external view returns (uint32);
 
     function isTargetClosed(address target) external view returns (bool);

contracts/governance/extensions/GovernorTimelockAccess.sol

@@ -21,9 +21,19 @@ import {Time} from "../../utils/types/Time.sol";
  * permissions. Operations that are delay-restricted by the manager, however, will be executed through the
  * {AccessManager-execute} function.
  *
- * Note that some operations may be cancelable in the {AccessManager} by the admin or a set of guardians, depending on
- * the restricted operation being invoked. Since proposals are atomic, the cancellation by a guardian of a single
- * operation in a proposal will cause all of it to become unable to execute.
+ * ==== Security Considerations
+ *
+ * Some operations may be cancelable in the `AccessManager` by the admin or a set of guardians, depending on the
+ * restricted function being invoked. Since proposals are atomic, the cancellation by a guardian of a single operation
+ * in a proposal will cause all of the proposal to become unable to execute. Consider proposing cancellable operations
+ * separately.
+ *
+ * By default, function calls will be routed through the associated `AccessManager` whenever it claims the target
+ * function to be restricted by it. However, admins may configure the manager to make that claim for functions that a
+ * governor would want to call directly (e.g., token transfers) in an attempt to deny it access to those functions. To
+ * mitigate this attack vector, the governor is able to ignore the restrictions claimed by the `AccessManager` using
+ * {setAccessManagerIgnored}. While permanent denial of service is mitigated, temporary DoS may still be technically
+ * possible. All of the governor's own functions (e.g., {setBaseDelaySeconds}) ignore the `AccessManager` by default.
  */
 abstract contract GovernorTimelockAccess is Governor {
     // An execution plan is produced at the moment a proposal is created, in order to fix at that point the exact
@@ -39,6 +49,11 @@ abstract contract GovernorTimelockAccess is Governor {
         mapping(uint256 operationBucket => uint32[8]) managerData;
     }
 
+    // The meaning of the "toggle" set to true depends on the target contract.
+    // If target == address(this), the manager is ignored by default, and a true toggle means it won't be ignored.
+    // For all other target contracts, the manager is used by default, and a true toggle means it will be ignored.
+    mapping(address target => mapping(bytes4 selector => bool)) private _ignoreToggle;
+
     mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
 
     uint32 private _baseDelay;
@@ -47,8 +62,10 @@ abstract contract GovernorTimelockAccess is Governor {
 
     error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
     error GovernorMismatchedNonce(uint256 proposalId, uint256 expectedNonce, uint256 actualNonce);
+    error GovernorLockedIgnore();
 
     event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
+    event AccessManagerIgnoredSet(address target, bytes4 selector, bool ignored);
 
     /**
      * @dev Initialize the governor with an {AccessManager} and initial base delay.
@@ -92,22 +109,62 @@ abstract contract GovernorTimelockAccess is Governor {
         _baseDelay = newBaseDelay;
     }
 
+    /**
+     * @dev Check if restrictions from the associated {AccessManager} are ignored for a target function. Returns true
+     * when the target function will be invoked directly regardless of `AccessManager` settings for the function.
+     * See {setAccessManagerIgnored} and Security Considerations above.
+     */
+    function isAccessManagerIgnored(address target, bytes4 selector) public view virtual returns (bool) {
+        bool isGovernor = target == address(this);
+        return _ignoreToggle[target][selector] != isGovernor; // equivalent to: isGovernor ? !toggle : toggle
+    }
+
+    /**
+     * @dev Configure whether restrictions from the associated {AccessManager} are ignored for a target function.
+     * See Security Considerations above.
+     */
+    function setAccessManagerIgnored(
+        address target,
+        bytes4[] calldata selectors,
+        bool ignored
+    ) public virtual onlyGovernance {
+        for (uint256 i = 0; i < selectors.length; ++i) {
+            _setAccessManagerIgnored(target, selectors[i], ignored);
+        }
+    }
+
+    /**
+     * @dev Internal version of {setAccessManagerIgnored} without access restriction.
+     */
+    function _setAccessManagerIgnored(address target, bytes4 selector, bool ignored) internal virtual {
+        bool isGovernor = target == address(this);
+        if (isGovernor && selector == this.setAccessManagerIgnored.selector) {
+            revert GovernorLockedIgnore();
+        }
+        _ignoreToggle[target][selector] = ignored != isGovernor; // equivalent to: isGovernor ? !ignored : ignored
+        emit AccessManagerIgnoredSet(target, selector, ignored);
+    }
+
     /**
      * @dev Public accessor to check the execution plan, including the number of seconds that the proposal will be
-     * delayed since queuing, and an array indicating which of the proposal actions will be executed indirectly through
-     * the associated {AccessManager}.
+     * delayed since queuing, an array indicating which of the proposal actions will be executed indirectly through
+     * the associated {AccessManager}, and another indicating which will be scheduled in {queue}. Note that
+     * those that must be scheduled are cancellable by `AccessManager` guardians.
      */
-    function proposalExecutionPlan(uint256 proposalId) public view returns (uint32, bool[] memory) {
+    function proposalExecutionPlan(
+        uint256 proposalId
+    ) public view returns (uint32 delay, bool[] memory indirect, bool[] memory withDelay) {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
-        uint32 delay = plan.delay;
         uint32 length = plan.length;
-        bool[] memory indirect = new bool[](length);
+        delay = plan.delay;
+        indirect = new bool[](length);
+        withDelay = new bool[](length);
         for (uint256 i = 0; i < length; ++i) {
-            (indirect[i], ) = _getManagerData(plan, i);
+            (indirect[i], withDelay[i], ) = _getManagerData(plan, i);
         }
 
-        return (delay, indirect);
+        return (delay, indirect, withDelay);
     }
 
     /**
@@ -134,12 +191,19 @@ abstract contract GovernorTimelockAccess is Governor {
         plan.length = SafeCast.toUint16(targets.length);
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            uint32 delay = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
-            if (delay > 0) {
-                _setManagerData(plan, i, 0);
+            address target = targets[i];
+            bytes4 selector = bytes4(calldatas[i]);
+            (bool immediate, uint32 delay) = AuthorityUtils.canCallWithDelay(
+                address(_manager),
+                address(this),
+                target,
+                selector
+            );
+            if ((immediate || delay > 0) && !isAccessManagerIgnored(target, selector)) {
+                _setManagerData(plan, i, !immediate, 0);
+                // downcast is safe because both arguments are uint32
+                neededDelay = uint32(Math.max(delay, neededDelay));
             }
-            // downcast is safe because both arguments are uint32
-            neededDelay = uint32(Math.max(delay, neededDelay));
         }
 
         plan.delay = neededDelay;
@@ -164,10 +228,10 @@ abstract contract GovernorTimelockAccess is Governor {
         uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            (bool delayed, ) = _getManagerData(plan, i);
-            if (delayed) {
+            (, bool withDelay, ) = _getManagerData(plan, i);
+            if (withDelay) {
                 (, uint32 nonce) = _manager.schedule(targets[i], calldatas[i], eta);
-                _setManagerData(plan, i, nonce);
+                _setManagerData(plan, i, true, nonce);
             }
         }
 
@@ -192,10 +256,10 @@ abstract contract GovernorTimelockAccess is Governor {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            (bool delayed, uint32 nonce) = _getManagerData(plan, i);
-            if (delayed) {
+            (bool controlled, bool withDelay, uint32 nonce) = _getManagerData(plan, i);
+            if (controlled) {
                 uint32 executedNonce = _manager.execute{value: values[i]}(targets[i], calldatas[i]);
-                if (executedNonce != nonce) {
+                if (withDelay && executedNonce != nonce) {
                     revert GovernorMismatchedNonce(proposalId, nonce, executedNonce);
                 }
             } else {
@@ -220,15 +284,23 @@ abstract contract GovernorTimelockAccess is Governor {
 
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
-        // If the proposal has been scheduled it will have an ETA and we have to externally cancel
+        // If the proposal has been scheduled it will have an ETA and we may have to externally cancel
         if (eta != 0) {
             for (uint256 i = 0; i < targets.length; ++i) {
-                (bool delayed, uint32 nonce) = _getManagerData(plan, i);
-                if (delayed) {
-                    // Attempt to cancel considering the operation could have been cancelled and rescheduled already
-                    uint32 canceledNonce = _manager.cancel(address(this), targets[i], calldatas[i]);
-                    if (canceledNonce != nonce) {
-                        revert GovernorMismatchedNonce(proposalId, nonce, canceledNonce);
+                (, bool withDelay, uint32 nonce) = _getManagerData(plan, i);
+                // Only attempt to cancel if the execution plan included a delay
+                if (withDelay) {
+                    bytes32 operationId = _manager.hashOperation(address(this), targets[i], calldatas[i]);
+                    // Check first if the current operation nonce is the one that we observed previously. It could
+                    // already have been cancelled and rescheduled. We don't want to cancel unless it is exactly the
+                    // instance that we previously scheduled.
+                    if (nonce == _manager.getNonce(operationId)) {
+                        // It is important that all calls have an opportunity to be cancelled. We chose to ignore
+                        // potential failures of some of the cancel operations to give the other operations a chance to
+                        // be properly cancelled. In particular cancel might fail if the operation was already cancelled
+                        // by guardians previously. We don't match on the revert reason to avoid encoding assumptions
+                        // about specific errors.
+                        try _manager.cancel(address(this), targets[i], calldatas[i]) {} catch {}
                     }
                 }
             }
@@ -237,39 +309,27 @@ abstract contract GovernorTimelockAccess is Governor {
         return proposalId;
     }
 
-    /**
-     * @dev Check if the execution of a call needs to be performed through an AccessManager and what delay should be
-     * applied to this call.
-     *
-     * Returns { manager: address(0), delay: 0 } if:
-     * - target does not have code
-     * - target does not implement IAccessManaged
-     * - calling canCall on the target's manager returns a 0 delay
-     * - calling canCall on the target's manager reverts
-     * Otherwise (calling canCall on the target's manager returns a non 0 delay), return the address of the
-     * AccessManager to use, and the delay for this call.
-     */
-    function _detectExecutionRequirements(address target, bytes4 selector) private view returns (uint32 delay) {
-        (, delay) = AuthorityUtils.canCallWithDelay(address(_manager), address(this), target, selector);
-    }
-
     /**
      * @dev Returns whether the operation at an index is delayed by the manager, and its scheduling nonce once queued.
      */
-    function _getManagerData(ExecutionPlan storage plan, uint256 index) private view returns (bool, uint32) {
+    function _getManagerData(
+        ExecutionPlan storage plan,
+        uint256 index
+    ) private view returns (bool controlled, bool withDelay, uint32 nonce) {
         (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
-        uint32 nonce = plan.managerData[bucket][subindex];
+        uint32 value = plan.managerData[bucket][subindex];
         unchecked {
-            return nonce > 0 ? (true, nonce - 1) : (false, 0);
+            return (value > 0, value > 1, value > 1 ? value - 2 : 0);
         }
     }
 
     /**
-     * @dev Marks an operation at an index as delayed by the manager, and sets its scheduling nonce.
+     * @dev Marks an operation at an index as permissioned by the manager, potentially delayed, and
+     * when delayed sets its scheduling nonce.
      */
-    function _setManagerData(ExecutionPlan storage plan, uint256 index, uint32 nonce) private {
+    function _setManagerData(ExecutionPlan storage plan, uint256 index, bool withDelay, uint32 nonce) private {
         (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
-        plan.managerData[bucket][subindex] = nonce + 1;
+        plan.managerData[bucket][subindex] = withDelay ? nonce + 2 : 1;
     }
 
     /**

test/governance/extensions/GovernorTimelockAccess.test.js

@@ -5,6 +5,7 @@ const Enums = require('../../helpers/enums');
 const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
 const { clockFromReceipt } = require('../../helpers/time');
+const { selector } = require('../../helpers/methods');
 
 const AccessManager = artifacts.require('$AccessManager');
 const Governor = artifacts.require('$GovernorTimelockAccessMock');
@@ -210,36 +211,196 @@ contract('GovernorTimelockAccess', function (accounts) {
         await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledUnrestricted');
       });
 
-      it('cancellation after queue (internal)', async function () {
+      describe('cancel', function () {
         const delay = 1000;
         const roleId = '1';
 
-        await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
-          from: admin,
+        beforeEach(async function () {
+          await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, {
+            from: admin,
+          });
+          await this.manager.grantRole(roleId, this.mock.address, delay, { from: admin });
         });
-        await this.manager.grantRole(roleId, this.mock.address, delay, { from: admin });
 
-        this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr');
+        it('cancellation after queue (internal)', async function () {
+          this.proposal = await this.helper.setProposal([this.restricted.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.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          await this.helper.queue();
 
-        const txCancel = await this.helper.cancel('internal');
-        expectEvent(txCancel, 'ProposalCanceled', { proposalId: this.proposal.id });
-        await expectEvent.inTransaction(txCancel.tx, this.manager, 'OperationCanceled', {
-          operationId: this.restricted.operationId,
-          nonce: '1',
+          const txCancel = await this.helper.cancel('internal');
+          expectEvent(txCancel, 'ProposalCanceled', { proposalId: this.proposal.id });
+          await expectEvent.inTransaction(txCancel.tx, this.manager, 'OperationCanceled', {
+            operationId: this.restricted.operationId,
+            nonce: '1',
+          });
+
+          await this.helper.waitForEta();
+          await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [
+            this.proposal.id,
+            Enums.ProposalState.Canceled,
+            proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]),
+          ]);
         });
 
-        await this.helper.waitForEta();
-        await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [
-          this.proposal.id,
-          Enums.ProposalState.Canceled,
-          proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]),
-        ]);
+        it('cancel calls already canceled by guardian', async function () {
+          const operationA = { target: this.receiver.address, data: this.restricted.selector + '00' };
+          const operationB = { target: this.receiver.address, data: this.restricted.selector + '01' };
+          const operationC = { target: this.receiver.address, data: this.restricted.selector + '02' };
+          const operationAId = hashOperation(this.mock.address, operationA.target, operationA.data);
+          const operationBId = hashOperation(this.mock.address, operationB.target, operationB.data);
+
+          const proposal1 = new GovernorHelper(this.mock, mode);
+          const proposal2 = new GovernorHelper(this.mock, mode);
+          proposal1.setProposal([operationA, operationB], 'proposal A+B');
+          proposal2.setProposal([operationA, operationC], 'proposal A+C');
+
+          for (const p of [proposal1, proposal2]) {
+            await p.propose();
+            await p.waitForSnapshot();
+            await p.vote({ support: Enums.VoteType.For }, { from: voter1 });
+            await p.waitForDeadline();
+          }
+
+          // Can queue the first proposal
+          await proposal1.queue();
+
+          // Cannot queue the second proposal: operation A already scheduled with delay
+          await expectRevertCustomError(proposal2.queue(), 'AccessManagerAlreadyScheduled', [operationAId]);
+
+          // Admin cancels operation B on the manager
+          await this.manager.cancel(this.mock.address, operationB.target, operationB.data, { from: admin });
+
+          // Still cannot queue the second proposal: operation A already scheduled with delay
+          await expectRevertCustomError(proposal2.queue(), 'AccessManagerAlreadyScheduled', [operationAId]);
+
+          await proposal1.waitForEta();
+
+          // Cannot execute first proposal: operation B has been canceled
+          await expectRevertCustomError(proposal1.execute(), 'AccessManagerNotScheduled', [operationBId]);
+
+          // Cancel the first proposal to release operation A
+          await proposal1.cancel('internal');
+
+          // can finally queue the second proposal
+          await proposal2.queue();
+
+          await proposal2.waitForEta();
+
+          // Can execute second proposal
+          await proposal2.execute();
+        });
+      });
+
+      describe('ignore AccessManager', function () {
+        it('defaults', async function () {
+          expect(await this.mock.isAccessManagerIgnored(this.receiver.address, this.restricted.selector)).to.equal(
+            false,
+          );
+          expect(await this.mock.isAccessManagerIgnored(this.mock.address, '0x12341234')).to.equal(true);
+        });
+
+        it('internal setter', async function () {
+          const p1 = { target: this.receiver.address, selector: this.restricted.selector, ignored: true };
+          const tx1 = await this.mock.$_setAccessManagerIgnored(p1.target, p1.selector, p1.ignored);
+          expect(await this.mock.isAccessManagerIgnored(p1.target, p1.selector)).to.equal(p1.ignored);
+          expectEvent(tx1, 'AccessManagerIgnoredSet', p1);
+
+          const p2 = { target: this.mock.address, selector: '0x12341234', ignored: false };
+          const tx2 = await this.mock.$_setAccessManagerIgnored(p2.target, p2.selector, p2.ignored);
+          expect(await this.mock.isAccessManagerIgnored(p2.target, p2.selector)).to.equal(p2.ignored);
+          expectEvent(tx2, 'AccessManagerIgnoredSet', p2);
+        });
+
+        it('external setter', async function () {
+          const setAccessManagerIgnored = (...args) =>
+            this.mock.contract.methods.setAccessManagerIgnored(...args).encodeABI();
+
+          await this.helper.setProposal(
+            [
+              {
+                target: this.mock.address,
+                data: setAccessManagerIgnored(
+                  this.receiver.address,
+                  [this.restricted.selector, this.unrestricted.selector],
+                  true,
+                ),
+                value: '0',
+              },
+              {
+                target: this.mock.address,
+                data: setAccessManagerIgnored(this.mock.address, ['0x12341234', '0x67896789'], false),
+                value: '0',
+              },
+            ],
+            'descr',
+          );
+
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          const tx = await this.helper.execute();
+
+          expectEvent(tx, 'AccessManagerIgnoredSet');
+
+          expect(await this.mock.isAccessManagerIgnored(this.receiver.address, this.restricted.selector)).to.equal(
+            true,
+          );
+          expect(await this.mock.isAccessManagerIgnored(this.receiver.address, this.unrestricted.selector)).to.equal(
+            true,
+          );
+
+          expect(await this.mock.isAccessManagerIgnored(this.mock.address, '0x12341234')).to.equal(false);
+          expect(await this.mock.isAccessManagerIgnored(this.mock.address, '0x67896789')).to.equal(false);
+        });
+
+        it('locked function', async function () {
+          const setAccessManagerIgnored = selector('setAccessManagerIgnored(address,bytes4[],bool)');
+          await expectRevertCustomError(
+            this.mock.$_setAccessManagerIgnored(this.mock.address, setAccessManagerIgnored, true),
+            'GovernorLockedIgnore',
+            [],
+          );
+          await this.mock.$_setAccessManagerIgnored(this.receiver.address, setAccessManagerIgnored, true);
+        });
+
+        it('ignores access manager', async function () {
+          const amount = 100;
+
+          const target = this.token.address;
+          const data = this.token.contract.methods.transfer(voter4, amount).encodeABI();
+          const selector = data.slice(0, 10);
+          await this.token.$_mint(this.mock.address, amount);
+
+          const roleId = '1';
+          await this.manager.setTargetFunctionRole(target, [selector], roleId, { from: admin });
+          await this.manager.grantRole(roleId, this.mock.address, 0, { from: admin });
+
+          await this.helper.setProposal([{ target, data, value: '0' }], '1');
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          await expectRevertCustomError(this.helper.execute(), 'ERC20InsufficientBalance', [
+            this.manager.address,
+            0,
+            amount,
+          ]);
+
+          await this.mock.$_setAccessManagerIgnored(target, selector, true);
+
+          await this.helper.setProposal([{ target, data, value: '0' }], '2');
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+          await this.helper.waitForDeadline();
+          const tx = await this.helper.execute();
+          expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.mock.address });
+        });
       });
     });
   }