From 5fd9a986218410aff54108ee0e079c5f22942fa0 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 8 Aug 2023 02:04:10 -0300
Subject: [PATCH 01/36] add back GovernorTimelock

---
 .../extensions/GovernorTimelock.sol           | 211 ++++++++++++++++++
 1 file changed, 211 insertions(+)
 create mode 100644 contracts/governance/extensions/GovernorTimelock.sol

diff --git a/contracts/governance/extensions/GovernorTimelock.sol b/contracts/governance/extensions/GovernorTimelock.sol
new file mode 100644
index 00000000000..db3b0320de4
--- /dev/null
+++ b/contracts/governance/extensions/GovernorTimelock.sol
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IGovernorTimelock} from "./IGovernorTimelock.sol";
+import {IGovernor, Governor} from "../Governor.sol";
+import {IManaged} from "../../access/manager/IManaged.sol";
+import {IAuthority, safeCanCall} from "../../access/manager/IAuthority.sol";
+import {IAccessManager} from "../../access/manager/IAccessManager.sol";
+import {Address} from "../../utils/Address.sol";
+import {Math} from "../../utils/math/Math.sol";
+
+/**
+ * @dev TODO
+ *
+ * _Available since v5.0._
+ */
+abstract contract GovernorTimelock is IGovernorTimelock, Governor {
+    struct ExecutionDetail {
+        address authority;
+        uint32 delay;
+    }
+
+    mapping(uint256 => ExecutionDetail[]) private _executionDetails;
+    mapping(uint256 => uint256) private _proposalEta;
+
+    /**
+     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` and `Expired` state.
+     */
+    function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) {
+        ProposalState currentState = super.state(proposalId);
+
+        if (currentState == ProposalState.Succeeded && proposalEta(proposalId) != 0) {
+            return ProposalState.Queued;
+        } else {
+            return currentState;
+        }
+    }
+
+    /**
+     * @dev Public accessor to check the eta of a queued proposal.
+     */
+    function proposalEta(uint256 proposalId) public view virtual override returns (uint256) {
+        return _proposalEta[proposalId];
+    }
+
+    /**
+     * @dev Public accessor to check the execution details.
+     */
+    function proposalExecutionDetails(uint256 proposalId) public view returns (ExecutionDetail[] memory) {
+        return _executionDetails[proposalId];
+    }
+
+    /**
+     * @dev See {IGovernor-propose}
+     */
+    function propose(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        string memory description
+    ) public virtual override(IGovernor, Governor) returns (uint256) {
+        uint256 proposalId = super.propose(targets, values, calldatas, description);
+
+        ExecutionDetail[] storage details = _executionDetails[proposalId];
+        for (uint256 i = 0; i < targets.length; ++i) {
+            details.push(_detectExecutionDetails(targets[i], bytes4(calldatas[i])));
+        }
+
+        return proposalId;
+    }
+
+    /**
+     * @dev Function to queue a proposal to the timelock.
+     *
+     * NOTE: execution delay is estimated based on the delay information retrieved in {proposal}. This value may be
+     * off if the delay were updated during the vote.
+     */
+    function queue(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) public virtual override returns (uint256) {
+        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
+
+        ProposalState currentState = state(proposalId);
+        if (currentState != ProposalState.Succeeded) {
+            revert GovernorUnexpectedProposalState(
+                proposalId,
+                currentState,
+                _encodeStateBitmap(ProposalState.Succeeded)
+            );
+        }
+
+        ExecutionDetail[] storage details = _executionDetails[proposalId];
+        ExecutionDetail memory detail;
+        uint32 setback = 0;
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            detail = details[i];
+            if (detail.authority != address(0)) {
+                IAccessManager(detail.authority).schedule(targets[i], calldatas[i]);
+            }
+            setback = uint32(Math.max(setback, detail.delay)); // cast is safe, both parameters are uint32
+        }
+
+        uint256 eta = block.timestamp + setback;
+        _proposalEta[proposalId] = eta;
+
+        return eta;
+    }
+
+    /**
+     * @dev See {IGovernor-_execute}
+     */
+    function _execute(
+        uint256 proposalId,
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 /*descriptionHash*/
+    ) internal virtual override {
+        ExecutionDetail[] storage details = _executionDetails[proposalId];
+        ExecutionDetail memory detail;
+
+        // TODO: enforce ETA (might include some _defaultDelaySeconds that are not enforced by any authority)
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            detail = details[i];
+            if (detail.authority != address(0)) {
+                IAccessManager(detail.authority).relay{value: values[i]}(targets[i], calldatas[i]);
+            } else {
+                (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
+                Address.verifyCallResult(success, returndata);
+            }
+        }
+
+        delete _executionDetails[proposalId];
+    }
+
+    /**
+     * @dev See {IGovernor-_cancel}
+     */
+    function _cancel(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) internal virtual override returns (uint256) {
+        uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash);
+
+        ExecutionDetail[] storage details = _executionDetails[proposalId];
+        ExecutionDetail memory detail;
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            detail = details[i];
+            if (detail.authority != address(0)) {
+                IAccessManager(detail.authority).cancel(address(this), targets[i], calldatas[i]);
+            }
+        }
+
+        delete _executionDetails[proposalId];
+
+        return proposalId;
+    }
+
+    /**
+     * @dev Default delay to apply to function calls that are not (scheduled and) executed through an AccessManager.
+     *
+     * NOTE: execution delays are processed by the AccessManager contracts. We expect these to always be in seconds.
+     * Therefore, the default delay that is enforced for calls that don't go through an access manager is also in
+     * seconds, regardless of the Governor's clock mode.
+     */
+    function _defaultDelaySeconds() internal view virtual returns (uint32) {
+        return 0;
+    }
+
+    /**
+     * @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: _defaultDelaySeconds() } if:
+     * - target does not have code
+     * - target does not implement IManaged
+     * - 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 _detectExecutionDetails(address target, bytes4 selector) private view returns (ExecutionDetail memory) {
+        bool success;
+        bytes memory returndata;
+
+        // Get authority
+        (success, returndata) = target.staticcall(abi.encodeCall(IManaged.authority, ()));
+        if (success && returndata.length >= 0x20) {
+            address authority = abi.decode(returndata, (address));
+
+            // Check if governor can call, and try to detect a delay
+            (bool authorized, uint32 delay) = safeCanCall(authority, address(this), target, selector);
+
+            // If direct call is not authorized, and delayed call is possible
+            if (!authorized && delay > 0) {
+                return ExecutionDetail({authority: authority, delay: delay});
+            }
+        }
+
+        return ExecutionDetail({authority: address(0), delay: _defaultDelaySeconds()});
+    }
+}

From 1498539a759b76b2956b4c228617bd0aa61ebb18 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 8 Aug 2023 02:45:28 -0300
Subject: [PATCH 02/36] WIP

---
 .../extensions/GovernorTimelock.sol           | 154 +++++++-----------
 1 file changed, 63 insertions(+), 91 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelock.sol b/contracts/governance/extensions/GovernorTimelock.sol
index db3b0320de4..3097968b7d3 100644
--- a/contracts/governance/extensions/GovernorTimelock.sol
+++ b/contracts/governance/extensions/GovernorTimelock.sol
@@ -2,53 +2,38 @@
 
 pragma solidity ^0.8.20;
 
-import {IGovernorTimelock} from "./IGovernorTimelock.sol";
 import {IGovernor, Governor} from "../Governor.sol";
-import {IManaged} from "../../access/manager/IManaged.sol";
-import {IAuthority, safeCanCall} from "../../access/manager/IAuthority.sol";
+import {IAuthority} from "../../access/manager/IAuthority.sol";
+import {AuthorityUtils} from "../../access/manager/AuthorityUtils.sol";
 import {IAccessManager} from "../../access/manager/IAccessManager.sol";
+import {IAccessManaged} from "../../access/manager/IAccessManaged.sol";
 import {Address} from "../../utils/Address.sol";
 import {Math} from "../../utils/math/Math.sol";
+import {SafeCast} from "../../utils/math/SafeCast.sol";
+import {Time} from "../../utils/types/Time.sol";
 
 /**
  * @dev TODO
  *
  * _Available since v5.0._
  */
-abstract contract GovernorTimelock is IGovernorTimelock, Governor {
-    struct ExecutionDetail {
-        address authority;
+abstract contract GovernorTimelockAccess is Governor {
+    struct ExecutionPlan {
         uint32 delay;
+        IAccessManager[] managers;
     }
 
-    mapping(uint256 => ExecutionDetail[]) private _executionDetails;
-    mapping(uint256 => uint256) private _proposalEta;
+    mapping(uint256 => ExecutionPlan) private _executionPlan;
+    mapping(address target => address) private _authorityOverride;
 
-    /**
-     * @dev Overridden version of the {Governor-state} function with added support for the `Queued` and `Expired` state.
-     */
-    function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) {
-        ProposalState currentState = super.state(proposalId);
-
-        if (currentState == ProposalState.Succeeded && proposalEta(proposalId) != 0) {
-            return ProposalState.Queued;
-        } else {
-            return currentState;
-        }
-    }
-
-    /**
-     * @dev Public accessor to check the eta of a queued proposal.
-     */
-    function proposalEta(uint256 proposalId) public view virtual override returns (uint256) {
-        return _proposalEta[proposalId];
+    constructor(uint32 defaultDelay) {
     }
 
     /**
      * @dev Public accessor to check the execution details.
      */
-    function proposalExecutionDetails(uint256 proposalId) public view returns (ExecutionDetail[] memory) {
-        return _executionDetails[proposalId];
+    function proposalExecutionPlan(uint256 proposalId) public view returns (ExecutionPlan memory) {
+        return _executionPlan[proposalId];
     }
 
     /**
@@ -59,14 +44,22 @@ abstract contract GovernorTimelock is IGovernorTimelock, Governor {
         uint256[] memory values,
         bytes[] memory calldatas,
         string memory description
-    ) public virtual override(IGovernor, Governor) returns (uint256) {
+    ) public virtual override returns (uint256) {
         uint256 proposalId = super.propose(targets, values, calldatas, description);
 
-        ExecutionDetail[] storage details = _executionDetails[proposalId];
+        uint32 maxDelay = 0;
+
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
         for (uint256 i = 0; i < targets.length; ++i) {
-            details.push(_detectExecutionDetails(targets[i], bytes4(calldatas[i])));
+            (address manager, uint32 delay) = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
+            plan.managers.push(IAccessManager(manager));
+            // downcast is safe because both arguments are uint32
+            maxDelay = uint32(Math.max(delay, maxDelay));
         }
 
+        plan.delay = maxDelay;
+
         return proposalId;
     }
 
@@ -76,67 +69,38 @@ abstract contract GovernorTimelock is IGovernorTimelock, Governor {
      * NOTE: execution delay is estimated based on the delay information retrieved in {proposal}. This value may be
      * off if the delay were updated during the vote.
      */
-    function queue(
+    function _queueOperations(
+        uint256 proposalId,
         address[] memory targets,
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public virtual override returns (uint256) {
+    ) internal virtual override returns (uint48) {
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
-        ProposalState currentState = state(proposalId);
-        if (currentState != ProposalState.Succeeded) {
-            revert GovernorUnexpectedProposalState(
-                proposalId,
-                currentState,
-                _encodeStateBitmap(ProposalState.Succeeded)
-            );
-        }
-
-        ExecutionDetail[] storage details = _executionDetails[proposalId];
-        ExecutionDetail memory detail;
-        uint32 setback = 0;
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+        uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            detail = details[i];
-            if (detail.authority != address(0)) {
-                IAccessManager(detail.authority).schedule(targets[i], calldatas[i]);
+            IAccessManager manager = plan.managers[i];
+            if (address(manager) != address(0)) {
+                manager.schedule(targets[i], calldatas[i], eta);
             }
-            setback = uint32(Math.max(setback, detail.delay)); // cast is safe, both parameters are uint32
         }
 
-        uint256 eta = block.timestamp + setback;
-        _proposalEta[proposalId] = eta;
-
         return eta;
     }
 
-    /**
-     * @dev See {IGovernor-_execute}
-     */
-    function _execute(
+    function _executeOperations(
         uint256 proposalId,
         address[] memory targets,
         uint256[] memory values,
         bytes[] memory calldatas,
-        bytes32 /*descriptionHash*/
+        bytes32 descriptionHash
     ) internal virtual override {
-        ExecutionDetail[] storage details = _executionDetails[proposalId];
-        ExecutionDetail memory detail;
-
-        // TODO: enforce ETA (might include some _defaultDelaySeconds that are not enforced by any authority)
-
-        for (uint256 i = 0; i < targets.length; ++i) {
-            detail = details[i];
-            if (detail.authority != address(0)) {
-                IAccessManager(detail.authority).relay{value: values[i]}(targets[i], calldatas[i]);
-            } else {
-                (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
-                Address.verifyCallResult(success, returndata);
-            }
-        }
-
-        delete _executionDetails[proposalId];
+        uint256 eta = proposalEta(proposalId);
+        require(block.timestamp >= eta);
+        super._executeOperations(proposalId, targets, values, calldatas, descriptionHash);
     }
 
     /**
@@ -150,17 +114,18 @@ abstract contract GovernorTimelock is IGovernorTimelock, Governor {
     ) internal virtual override returns (uint256) {
         uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash);
 
-        ExecutionDetail[] storage details = _executionDetails[proposalId];
-        ExecutionDetail memory detail;
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+        ExecutionPlan memory detail;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            detail = details[i];
-            if (detail.authority != address(0)) {
-                IAccessManager(detail.authority).cancel(address(this), targets[i], calldatas[i]);
+            IAccessManager manager = plan.managers[i];
+            if (address(manager) != address(0)) {
+                // Attempt to cancel, but allow to revert if a guardian cancelled part of the proposal
+                try manager.cancel(address(this), targets[i], calldatas[i]) {} catch {}
             }
         }
 
-        delete _executionDetails[proposalId];
+        delete _executionPlan[proposalId];
 
         return proposalId;
     }
@@ -182,30 +147,37 @@ abstract contract GovernorTimelock is IGovernorTimelock, Governor {
      *
      * Returns { manager: address(0), delay: _defaultDelaySeconds() } if:
      * - target does not have code
-     * - target does not implement IManaged
+     * - 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 _detectExecutionDetails(address target, bytes4 selector) private view returns (ExecutionDetail memory) {
-        bool success;
-        bytes memory returndata;
+    function _detectExecutionRequirements(address target, bytes4 selector) private view returns (address, uint32) {
+        address authority = _authorityOverride[target];
 
-        // Get authority
-        (success, returndata) = target.staticcall(abi.encodeCall(IManaged.authority, ()));
-        if (success && returndata.length >= 0x20) {
-            address authority = abi.decode(returndata, (address));
+        // Get authority from target contract if there is no override
+        if (authority == address(0)) {
+            bool success;
+            bytes memory returndata;
+
+            (success, returndata) = target.staticcall(abi.encodeCall(IAccessManaged.authority, ()));
+            if (success && returndata.length >= 0x20) {
+                authority = abi.decode(returndata, (address));
+            }
+        }
 
-            // Check if governor can call, and try to detect a delay
-            (bool authorized, uint32 delay) = safeCanCall(authority, address(this), target, selector);
+        if (authority != address(0)) {
+            // Check if governor can call according to authority, and try to detect a delay
+            (bool authorized, uint32 delay) = AuthorityUtils.canCallWithDelay(authority, address(this), target, selector);
 
             // If direct call is not authorized, and delayed call is possible
             if (!authorized && delay > 0) {
-                return ExecutionDetail({authority: authority, delay: delay});
+                return (authority, delay);
             }
         }
 
-        return ExecutionDetail({authority: address(0), delay: _defaultDelaySeconds()});
+        // Use internal delay mechanism
+        return (address(0), _defaultDelaySeconds());
     }
 }

From 36dc8841e6acd1c2e432bb2927275edafeaa7bd8 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 8 Aug 2023 15:21:28 -0300
Subject: [PATCH 03/36] rename GovernorTimelock -> GovernorTimelockAccess

---
 .../{GovernorTimelock.sol => GovernorTimelockAccess.sol}          | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename contracts/governance/extensions/{GovernorTimelock.sol => GovernorTimelockAccess.sol} (100%)

diff --git a/contracts/governance/extensions/GovernorTimelock.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
similarity index 100%
rename from contracts/governance/extensions/GovernorTimelock.sol
rename to contracts/governance/extensions/GovernorTimelockAccess.sol

From 2d2b087981df36c606efa972710595f0ef791c20 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Thu, 10 Aug 2023 16:42:52 -0300
Subject: [PATCH 04/36] make base delay modifiable

---
 .../extensions/GovernorTimelockAccess.sol     | 64 +++++++++++++------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 3097968b7d3..5651e962a58 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -26,7 +26,41 @@ abstract contract GovernorTimelockAccess is Governor {
     mapping(uint256 => ExecutionPlan) private _executionPlan;
     mapping(address target => address) private _authorityOverride;
 
-    constructor(uint32 defaultDelay) {
+    uint32 _baseDelay;
+
+    error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
+
+    event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
+
+    constructor(uint32 initialBaseDelay) {
+        _baseDelay = initialBaseDelay;
+    }
+
+    /**
+     * @dev Base delay that will be applied to all function calls. Some may be further delayed by their associated
+     * `AccessManager` authority; in this case the final delay will be the maximum of the base delay and the one
+     * demanded by the authority.
+     *
+     * NOTE: Execution delays are processed by the `AccessManager` contracts, and according to that contract are
+     * expressed in seconds. Therefore, the base delay is also in seconds, regardless of the governor's clock mode.
+     */
+    function baseDelaySeconds() public view virtual returns (uint32) {
+        return _baseDelay;
+    }
+
+    /**
+     * @dev Change the value of {baseDelaySeconds}. This operation can only be invoked through a governance proposal.
+     */
+    function setBaseDelaySeconds(uint32 newBaseDelay) public virtual onlyGovernance {
+        _setBaseDelaySeconds(newBaseDelay);
+    }
+
+    /**
+     * @dev Change the value of {baseDelaySeconds}. Internal function without access control.
+     */
+    function _setBaseDelaySeconds(uint32 newBaseDelay) internal virtual {
+        emit BaseDelaySet(_baseDelay, newBaseDelay);
+        _baseDelay = newBaseDelay;
     }
 
     /**
@@ -47,7 +81,7 @@ abstract contract GovernorTimelockAccess is Governor {
     ) public virtual override returns (uint256) {
         uint256 proposalId = super.propose(targets, values, calldatas, description);
 
-        uint32 maxDelay = 0;
+        uint32 maxDelay = baseDelaySeconds();
 
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
@@ -72,12 +106,10 @@ abstract contract GovernorTimelockAccess is Governor {
     function _queueOperations(
         uint256 proposalId,
         address[] memory targets,
-        uint256[] memory values,
+        uint256[] memory /* values */,
         bytes[] memory calldatas,
-        bytes32 descriptionHash
+        bytes32 /* descriptionHash */
     ) internal virtual override returns (uint48) {
-        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
-
         ExecutionPlan storage plan = _executionPlan[proposalId];
         uint48 eta = Time.timestamp() + plan.delay;
 
@@ -99,7 +131,9 @@ abstract contract GovernorTimelockAccess is Governor {
         bytes32 descriptionHash
     ) internal virtual override {
         uint256 eta = proposalEta(proposalId);
-        require(block.timestamp >= eta);
+        if (block.timestamp < eta) {
+            revert GovernorUnmetDelay(proposalId, eta);
+        }
         super._executeOperations(proposalId, targets, values, calldatas, descriptionHash);
     }
 
@@ -115,7 +149,6 @@ abstract contract GovernorTimelockAccess is Governor {
         uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash);
 
         ExecutionPlan storage plan = _executionPlan[proposalId];
-        ExecutionPlan memory detail;
 
         for (uint256 i = 0; i < targets.length; ++i) {
             IAccessManager manager = plan.managers[i];
@@ -130,22 +163,11 @@ abstract contract GovernorTimelockAccess is Governor {
         return proposalId;
     }
 
-    /**
-     * @dev Default delay to apply to function calls that are not (scheduled and) executed through an AccessManager.
-     *
-     * NOTE: execution delays are processed by the AccessManager contracts. We expect these to always be in seconds.
-     * Therefore, the default delay that is enforced for calls that don't go through an access manager is also in
-     * seconds, regardless of the Governor's clock mode.
-     */
-    function _defaultDelaySeconds() internal view virtual returns (uint32) {
-        return 0;
-    }
-
     /**
      * @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: _defaultDelaySeconds() } if:
+     * Returns { manager: address(0), delay: _baseDelaySeconds() } if:
      * - target does not have code
      * - target does not implement IAccessManaged
      * - calling canCall on the target's manager returns a 0 delay
@@ -178,6 +200,6 @@ abstract contract GovernorTimelockAccess is Governor {
         }
 
         // Use internal delay mechanism
-        return (address(0), _defaultDelaySeconds());
+        return (address(0), 0);
     }
 }

From 221261f08ddeb2b502bdbe93d3a2f41f32209168 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Thu, 10 Aug 2023 16:43:58 -0300
Subject: [PATCH 05/36] remove available since

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 5651e962a58..18af689abc3 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -14,8 +14,6 @@ import {Time} from "../../utils/types/Time.sol";
 
 /**
  * @dev TODO
- *
- * _Available since v5.0._
  */
 abstract contract GovernorTimelockAccess is Governor {
     struct ExecutionPlan {

From 499a2a184b697979b42a37d1f2b0bc8dfebcf82a Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Thu, 10 Aug 2023 16:54:10 -0300
Subject: [PATCH 06/36] use relay to call the target contract

---
 .../extensions/GovernorTimelockAccess.sol         | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 18af689abc3..3358b6b3935 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -132,7 +132,20 @@ abstract contract GovernorTimelockAccess is Governor {
         if (block.timestamp < eta) {
             revert GovernorUnmetDelay(proposalId, eta);
         }
-        super._executeOperations(proposalId, targets, values, calldatas, descriptionHash);
+
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            IAccessManager manager = plan.managers[i];
+            if (address(manager) != address(0)) {
+                manager.relay{value: values[i]}(targets[i], calldatas[i]);
+            } else {
+                (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
+                Address.verifyCallResult(success, returndata);
+            }
+        }
+
+        delete _executionPlan[proposalId];
     }
 
     /**

From d5ca8be325032d22a61f1936aec2d15a444f898f Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Thu, 10 Aug 2023 16:55:50 -0300
Subject: [PATCH 07/36] fix warning

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 3358b6b3935..ff6e3a88f92 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -126,7 +126,7 @@ abstract contract GovernorTimelockAccess is Governor {
         address[] memory targets,
         uint256[] memory values,
         bytes[] memory calldatas,
-        bytes32 descriptionHash
+        bytes32 /* descriptionHash */
     ) internal virtual override {
         uint256 eta = proposalEta(proposalId);
         if (block.timestamp < eta) {

From 7ed7c1ca1d126cfc96835fb7e2ca223066243796 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Thu, 10 Aug 2023 22:17:25 -0300
Subject: [PATCH 08/36] Update
 contracts/governance/extensions/GovernorTimelockAccess.sol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index ff6e3a88f92..014bc3cd9da 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -178,7 +178,7 @@ abstract contract GovernorTimelockAccess is Governor {
      * @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: _baseDelaySeconds() } if:
+     * 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

From bf9fdf919dcff8e6487b8b1052e617611f2c7a65 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Thu, 10 Aug 2023 22:18:19 -0300
Subject: [PATCH 09/36] fix initial set of base delay

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 014bc3cd9da..64d1c293609 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -31,7 +31,7 @@ abstract contract GovernorTimelockAccess is Governor {
     event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
 
     constructor(uint32 initialBaseDelay) {
-        _baseDelay = initialBaseDelay;
+        _setBaseDelaySeconds(initialBaseDelay);
     }
 
     /**

From 5ced769081d002266b62510ee02966ef2395c3e3 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 11 Aug 2023 11:47:18 -0300
Subject: [PATCH 10/36] rename maxDelay -> neededDelay

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 64d1c293609..5b0b31a38b7 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -79,7 +79,7 @@ abstract contract GovernorTimelockAccess is Governor {
     ) public virtual override returns (uint256) {
         uint256 proposalId = super.propose(targets, values, calldatas, description);
 
-        uint32 maxDelay = baseDelaySeconds();
+        uint32 neededDelay = baseDelaySeconds();
 
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
@@ -87,10 +87,10 @@ abstract contract GovernorTimelockAccess is Governor {
             (address manager, uint32 delay) = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
             plan.managers.push(IAccessManager(manager));
             // downcast is safe because both arguments are uint32
-            maxDelay = uint32(Math.max(delay, maxDelay));
+            neededDelay = uint32(Math.max(delay, neededDelay));
         }
 
-        plan.delay = maxDelay;
+        plan.delay = neededDelay;
 
         return proposalId;
     }

From 3820637fb66a00ae0f6bfa03cac3a3c22199b410 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 11 Aug 2023 18:37:17 -0300
Subject: [PATCH 11/36] address guardian cancellation risk

---
 .../extensions/GovernorTimelockAccess.sol     | 44 ++++++++++++++-----
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 5b0b31a38b7..046fe2b2a93 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -18,7 +18,12 @@ import {Time} from "../../utils/types/Time.sol";
 abstract contract GovernorTimelockAccess is Governor {
     struct ExecutionPlan {
         uint32 delay;
-        IAccessManager[] managers;
+        OperationDetails[] operations;
+    }
+
+    struct OperationDetails {
+        IAccessManager manager;
+        bytes32 id;
     }
 
     mapping(uint256 => ExecutionPlan) private _executionPlan;
@@ -85,7 +90,8 @@ abstract contract GovernorTimelockAccess is Governor {
 
         for (uint256 i = 0; i < targets.length; ++i) {
             (address manager, uint32 delay) = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
-            plan.managers.push(IAccessManager(manager));
+            plan.operations.push();
+            plan.operations[i].manager = IAccessManager(manager);
             // downcast is safe because both arguments are uint32
             neededDelay = uint32(Math.max(delay, neededDelay));
         }
@@ -112,9 +118,9 @@ abstract contract GovernorTimelockAccess is Governor {
         uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            IAccessManager manager = plan.managers[i];
+            IAccessManager manager = plan.operations[i].manager;
             if (address(manager) != address(0)) {
-                manager.schedule(targets[i], calldatas[i], eta);
+                plan.operations[i].id = manager.schedule(targets[i], calldatas[i], eta);
             }
         }
 
@@ -128,7 +134,7 @@ abstract contract GovernorTimelockAccess is Governor {
         bytes[] memory calldatas,
         bytes32 /* descriptionHash */
     ) internal virtual override {
-        uint256 eta = proposalEta(proposalId);
+        uint48 eta = SafeCast.toUint48(proposalEta(proposalId));
         if (block.timestamp < eta) {
             revert GovernorUnmetDelay(proposalId, eta);
         }
@@ -136,8 +142,15 @@ abstract contract GovernorTimelockAccess is Governor {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            IAccessManager manager = plan.managers[i];
+            IAccessManager manager = plan.operations[i].manager;
             if (address(manager) != address(0)) {
+                bytes32 operationId = plan.operations[i].id;
+
+                uint48 schedule = manager.getSchedule(operationId);
+                if (schedule != eta) {
+                    revert GovernorNotQueuedProposal(proposalId);
+                }
+
                 manager.relay{value: values[i]}(targets[i], calldatas[i]);
             } else {
                 (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
@@ -159,13 +172,22 @@ abstract contract GovernorTimelockAccess is Governor {
     ) internal virtual override returns (uint256) {
         uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash);
 
+        uint48 eta = SafeCast.toUint48(proposalEta(proposalId));
+
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
-        for (uint256 i = 0; i < targets.length; ++i) {
-            IAccessManager manager = plan.managers[i];
-            if (address(manager) != address(0)) {
-                // Attempt to cancel, but allow to revert if a guardian cancelled part of the proposal
-                try manager.cancel(address(this), targets[i], calldatas[i]) {} catch {}
+        // If the proposal has been scheduled it will have an ETA and we have to externally cancel
+        if (eta != 0) {
+            for (uint256 i = 0; i < targets.length; ++i) {
+                IAccessManager manager = plan.operations[i].manager;
+                if (address(manager) != address(0)) {
+                    bytes32 operationId = plan.operations[i].id;
+                    // Attempt to cancel considering the operation could have been cancelled and rescheduled already
+                    uint48 schedule = manager.getSchedule(operationId);
+                    if (schedule == eta) {
+                        manager.cancel(address(this), targets[i], calldatas[i]);
+                    }
+                }
             }
         }
 

From 31e20a12847d3332ae056e2120b2ca7b33646130 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 11 Aug 2023 22:31:56 -0300
Subject: [PATCH 12/36] use single manager per governor

---
 .../extensions/GovernorTimelockAccess.sol     | 67 +++++++------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 046fe2b2a93..84e78244b25 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -22,7 +22,7 @@ abstract contract GovernorTimelockAccess is Governor {
     }
 
     struct OperationDetails {
-        IAccessManager manager;
+        bool delayed;
         bytes32 id;
     }
 
@@ -31,14 +31,24 @@ abstract contract GovernorTimelockAccess is Governor {
 
     uint32 _baseDelay;
 
+    IAccessManager immutable private _manager;
+
     error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
 
     event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
 
-    constructor(uint32 initialBaseDelay) {
+    constructor(address manager, uint32 initialBaseDelay) {
+        _manager = IAccessManager(manager);
         _setBaseDelaySeconds(initialBaseDelay);
     }
 
+    /**
+     * @dev Returns the {AccessManager} instance associated to this governor.
+     */
+    function accessManager() public view virtual returns (IAccessManager) {
+        return _manager;
+    }
+
     /**
      * @dev Base delay that will be applied to all function calls. Some may be further delayed by their associated
      * `AccessManager` authority; in this case the final delay will be the maximum of the base delay and the one
@@ -89,9 +99,11 @@ abstract contract GovernorTimelockAccess is Governor {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            (address manager, uint32 delay) = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
             plan.operations.push();
-            plan.operations[i].manager = IAccessManager(manager);
+            uint32 delay = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
+            if (delay > 0) {
+                plan.operations[i].delayed = true;
+            }
             // downcast is safe because both arguments are uint32
             neededDelay = uint32(Math.max(delay, neededDelay));
         }
@@ -118,9 +130,8 @@ abstract contract GovernorTimelockAccess is Governor {
         uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            IAccessManager manager = plan.operations[i].manager;
-            if (address(manager) != address(0)) {
-                plan.operations[i].id = manager.schedule(targets[i], calldatas[i], eta);
+            if (plan.operations[i].delayed) {
+                plan.operations[i].id = _manager.schedule(targets[i], calldatas[i], eta);
             }
         }
 
@@ -142,16 +153,15 @@ abstract contract GovernorTimelockAccess is Governor {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            IAccessManager manager = plan.operations[i].manager;
-            if (address(manager) != address(0)) {
+            if (plan.operations[i].delayed) {
                 bytes32 operationId = plan.operations[i].id;
 
-                uint48 schedule = manager.getSchedule(operationId);
+                uint48 schedule = _manager.getSchedule(operationId);
                 if (schedule != eta) {
                     revert GovernorNotQueuedProposal(proposalId);
                 }
 
-                manager.relay{value: values[i]}(targets[i], calldatas[i]);
+                _manager.relay{value: values[i]}(targets[i], calldatas[i]);
             } else {
                 (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
                 Address.verifyCallResult(success, returndata);
@@ -179,13 +189,12 @@ abstract contract GovernorTimelockAccess is Governor {
         // If the proposal has been scheduled it will have an ETA and we have to externally cancel
         if (eta != 0) {
             for (uint256 i = 0; i < targets.length; ++i) {
-                IAccessManager manager = plan.operations[i].manager;
-                if (address(manager) != address(0)) {
+                if (plan.operations[i].delayed) {
                     bytes32 operationId = plan.operations[i].id;
                     // Attempt to cancel considering the operation could have been cancelled and rescheduled already
-                    uint48 schedule = manager.getSchedule(operationId);
+                    uint48 schedule = _manager.getSchedule(operationId);
                     if (schedule == eta) {
-                        manager.cancel(address(this), targets[i], calldatas[i]);
+                        _manager.cancel(address(this), targets[i], calldatas[i]);
                     }
                 }
             }
@@ -208,31 +217,7 @@ abstract contract GovernorTimelockAccess is Governor {
      * 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 (address, uint32) {
-        address authority = _authorityOverride[target];
-
-        // Get authority from target contract if there is no override
-        if (authority == address(0)) {
-            bool success;
-            bytes memory returndata;
-
-            (success, returndata) = target.staticcall(abi.encodeCall(IAccessManaged.authority, ()));
-            if (success && returndata.length >= 0x20) {
-                authority = abi.decode(returndata, (address));
-            }
-        }
-
-        if (authority != address(0)) {
-            // Check if governor can call according to authority, and try to detect a delay
-            (bool authorized, uint32 delay) = AuthorityUtils.canCallWithDelay(authority, address(this), target, selector);
-
-            // If direct call is not authorized, and delayed call is possible
-            if (!authorized && delay > 0) {
-                return (authority, delay);
-            }
-        }
-
-        // Use internal delay mechanism
-        return (address(0), 0);
+    function _detectExecutionRequirements(address target, bytes4 selector) private view returns (uint32 delay) {
+        (, delay) = AuthorityUtils.canCallWithDelay(address(_manager), address(this), target, selector);
     }
 }

From 56ed5a45ddf28d4c0bbcb738bacbca6270bb36b2 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Sat, 12 Aug 2023 01:08:00 -0300
Subject: [PATCH 13/36] add nonces

---
 contracts/access/manager/AccessManager.sol    | 71 +++++++++-----
 contracts/access/manager/IAccessManager.sol   | 14 +--
 .../extensions/GovernorTimelockAccess.sol     | 93 +++++++++++++------
 3 files changed, 125 insertions(+), 53 deletions(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index 929a6736bca..00d0831c328 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -89,7 +89,14 @@ contract AccessManager is Context, Multicall, IAccessManager {
     mapping(address target => AccessMode mode) private _contractMode;
     mapping(uint64 classId => Class) private _classes;
     mapping(uint64 groupId => Group) private _groups;
-    mapping(bytes32 operationId => uint48 schedule) private _schedules;
+
+    struct Schedule {
+        uint48 timepoint;
+        uint32 nonce;
+    }
+
+    mapping(bytes32 operationId => Schedule) private _schedules;
+
     mapping(bytes4 selector => Time.Delay delay) private _adminDelays;
 
     // This should be transcient storage when supported by the EVM.
@@ -568,10 +575,18 @@ contract AccessManager is Context, Multicall, IAccessManager {
      * operation is not yet scheduled, has expired, was executed, or was canceled.
      */
     function getSchedule(bytes32 id) public view virtual returns (uint48) {
-        uint48 timepoint = _schedules[id];
+        uint48 timepoint = _schedules[id].timepoint;
         return _isExpired(timepoint) ? 0 : timepoint;
     }
 
+    /**
+     * @dev Return the nonce for the latest scheduled operation with a given id. Returns 0 if the operation has never
+     * been scheduled.
+     */
+    function getNonce(bytes32 id) public view virtual returns (uint32) {
+        return _schedules[id].nonce;
+    }
+
     /**
      * @dev Schedule a delayed operation for future execution, and return the operation identifier. It is possible to
      * choose the timestamp at which the operation becomes executable as long as it satisfies the execution delays
@@ -579,7 +594,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
      *
      * Emits a {OperationScheduled} event.
      */
-    function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32) {
+    function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32 operationId, uint32 nonce) {
         address caller = _msgSender();
 
         // Fetch restriction to that apply to the caller on the targeted function
@@ -587,25 +602,30 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
         uint48 minWhen = Time.timestamp() + setback;
 
+        if (when == 0) {
+            when = minWhen;
+        }
+
         // If caller is not authorised, revert
-        if (!allowed && (setback == 0 || when.isSetAndPast(minWhen - 1))) {
+        if (!allowed && (setback == 0 || when < minWhen)) {
             revert AccessManagerUnauthorizedCall(caller, target, bytes4(data[0:4]));
         }
 
         // If caller is authorised, schedule operation
-        bytes32 operationId = _hashOperation(caller, target, data);
+        operationId = _hashOperation(caller, target, data);
 
         // Cannot reschedule unless the operation has expired
-        uint48 prevTimepoint = _schedules[operationId];
+        uint48 prevTimepoint = _schedules[operationId].timepoint;
         if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) {
             revert AccessManagerAlreadyScheduled(operationId);
         }
 
-        uint48 timepoint = when == 0 ? minWhen : when;
-        _schedules[operationId] = timepoint;
-        emit OperationScheduled(operationId, timepoint, caller, target, data);
+        nonce = _schedules[operationId].nonce + 1;
+        _schedules[operationId].timepoint = when;
+        _schedules[operationId].nonce = nonce;
+        emit OperationScheduled(operationId, nonce, when, caller, target, data);
 
-        return operationId;
+        // Using named return values because otherwise we get stack too deep
     }
 
     /**
@@ -617,7 +637,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     // Reentrancy is not an issue because permissions are checked on msg.sender. Additionally,
     // _consumeScheduledOp guarantees a scheduled operation is only executed once.
     // slither-disable-next-line reentrancy-no-eth
-    function relay(address target, bytes calldata data) public payable virtual {
+    function relay(address target, bytes calldata data) public payable virtual returns (uint32) {
         address caller = _msgSender();
 
         // Fetch restriction to that apply to the caller on the targeted function
@@ -630,9 +650,10 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
         // If caller is authorised, check operation was scheduled early enough
         bytes32 operationId = _hashOperation(caller, target, data);
+        uint32 nonce;
 
         if (setback != 0) {
-            _consumeScheduledOp(operationId);
+            nonce = _consumeScheduledOp(operationId);
         }
 
         // Mark the target and selector as authorised
@@ -644,6 +665,8 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
         // Reset relay identifier
         _relayIdentifier = relayIdentifierBefore;
+
+        return nonce;
     }
 
     /**
@@ -664,8 +687,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
     /**
      * @dev Internal variant of {consumeScheduledOp} that operates on bytes32 operationId.
      */
-    function _consumeScheduledOp(bytes32 operationId) internal virtual {
-        uint48 timepoint = _schedules[operationId];
+    function _consumeScheduledOp(bytes32 operationId) internal virtual returns (uint32) {
+        uint48 timepoint = _schedules[operationId].timepoint;
+        uint32 nonce = _schedules[operationId].nonce;
 
         if (timepoint == 0) {
             revert AccessManagerNotScheduled(operationId);
@@ -676,7 +700,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
 
         delete _schedules[operationId];
-        emit OperationExecuted(operationId, timepoint);
+        emit OperationExecuted(operationId, nonce, timepoint);
+
+        return nonce;
     }
 
     /**
@@ -688,12 +714,12 @@ contract AccessManager is Context, Multicall, IAccessManager {
      *
      * Emits a {OperationCanceled} event.
      */
-    function cancel(address caller, address target, bytes calldata data) public virtual {
+    function cancel(address caller, address target, bytes calldata data) public virtual returns (uint32) {
         address msgsender = _msgSender();
         bytes4 selector = bytes4(data[0:4]);
 
         bytes32 operationId = _hashOperation(caller, target, data);
-        if (_schedules[operationId] == 0) {
+        if (_schedules[operationId].timepoint == 0) {
             revert AccessManagerNotScheduled(operationId);
         } else if (caller != msgsender) {
             // calls can only be canceled by the account that scheduled them, a global admin, or by a guardian of the required group.
@@ -705,15 +731,18 @@ contract AccessManager is Context, Multicall, IAccessManager {
             }
         }
 
-        uint48 timepoint = _schedules[operationId];
-        delete _schedules[operationId];
-        emit OperationCanceled(operationId, timepoint);
+        uint32 nonce = _schedules[operationId].nonce;
+        uint48 timepoint = _schedules[operationId].timepoint;
+        delete _schedules[operationId].timepoint;
+        emit OperationCanceled(operationId, nonce, timepoint);
+
+        return nonce;
     }
 
     /**
      * @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) internal pure virtual returns (bytes32) {
         return keccak256(abi.encode(caller, target, data));
     }
 
diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol
index 312b4da740a..fcf78ba3f70 100644
--- a/contracts/access/manager/IAccessManager.sol
+++ b/contracts/access/manager/IAccessManager.sol
@@ -9,17 +9,17 @@ interface IAccessManager {
     /**
      * @dev A delayed operation was scheduled.
      */
-    event OperationScheduled(bytes32 indexed operationId, uint48 schedule, address caller, address target, bytes data);
+    event OperationScheduled(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule, address caller, address target, bytes data);
 
     /**
      * @dev A scheduled operation was executed.
      */
-    event OperationExecuted(bytes32 indexed operationId, uint48 schedule);
+    event OperationExecuted(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule);
 
     /**
      * @dev A scheduled operation was canceled.
      */
-    event OperationCanceled(bytes32 indexed operationId, uint48 schedule);
+    event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule);
 
     event GroupLabel(uint64 indexed groupId, string label);
     event GroupGranted(uint64 indexed groupId, address indexed account, uint32 delay, uint48 since);
@@ -94,11 +94,13 @@ interface IAccessManager {
 
     function getSchedule(bytes32 id) external returns (uint48);
 
-    function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32);
+    function getNonce(bytes32 id) external returns (uint32);
 
-    function relay(address target, bytes calldata data) external payable;
+    function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32, uint32);
 
-    function cancel(address caller, address target, bytes calldata data) external;
+    function relay(address target, bytes calldata data) external payable returns (uint32);
+
+    function cancel(address caller, address target, bytes calldata data) external returns (uint32);
 
     function consumeScheduledOp(address caller, bytes calldata data) external;
 
diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 84e78244b25..86ee9f93f76 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -17,16 +17,18 @@ import {Time} from "../../utils/types/Time.sol";
  */
 abstract contract GovernorTimelockAccess is Governor {
     struct ExecutionPlan {
+        uint16 length;
         uint32 delay;
-        OperationDetails[] operations;
+        // We use mappings instead of arrays because it allows us to pack values in storage more tightly without storing
+        // the length redundantly.
+        // We pack 8 operations' data in each bucket. Each uint32 value is set to 1 upon proposal creation if it has to
+        // be scheduled and relayed through the manager. Upon queuing, the value is set to nonce + 1, where the nonce is
+        // that which we get back from the manager when scheduling the operation.
+        mapping (uint256 bucket => uint32[8]) managerData;
     }
 
-    struct OperationDetails {
-        bool delayed;
-        bytes32 id;
-    }
+    mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
 
-    mapping(uint256 => ExecutionPlan) private _executionPlan;
     mapping(address target => address) private _authorityOverride;
 
     uint32 _baseDelay;
@@ -77,10 +79,21 @@ abstract contract GovernorTimelockAccess is Governor {
     }
 
     /**
-     * @dev Public accessor to check the execution details.
+     * @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}.
      */
-    function proposalExecutionPlan(uint256 proposalId) public view returns (ExecutionPlan memory) {
-        return _executionPlan[proposalId];
+    function proposalExecutionPlan(uint256 proposalId) public view returns (uint32, bool[] memory) {
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
+        uint32 delay = plan.delay;
+        uint32 length = plan.length;
+        bool[] memory indirect = new bool[](length);
+        for (uint256 i = 0; i < length; ++i) {
+            (indirect[i], ) = _getManagerData(plan, i);
+        }
+
+        return (delay, indirect);
     }
 
     /**
@@ -97,12 +110,12 @@ abstract contract GovernorTimelockAccess is Governor {
         uint32 neededDelay = baseDelaySeconds();
 
         ExecutionPlan storage plan = _executionPlan[proposalId];
+        plan.length = SafeCast.toUint16(targets.length);
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            plan.operations.push();
             uint32 delay = _detectExecutionRequirements(targets[i], bytes4(calldatas[i]));
             if (delay > 0) {
-                plan.operations[i].delayed = true;
+                _setManagerData(plan, i, 0);
             }
             // downcast is safe because both arguments are uint32
             neededDelay = uint32(Math.max(delay, neededDelay));
@@ -130,8 +143,10 @@ abstract contract GovernorTimelockAccess is Governor {
         uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            if (plan.operations[i].delayed) {
-                plan.operations[i].id = _manager.schedule(targets[i], calldatas[i], eta);
+            (bool delayed,) = _getManagerData(plan, i);
+            if (delayed) {
+                (, uint32 nonce) = _manager.schedule(targets[i], calldatas[i], eta);
+                _setManagerData(plan, i, nonce);
             }
         }
 
@@ -153,15 +168,13 @@ abstract contract GovernorTimelockAccess is Governor {
         ExecutionPlan storage plan = _executionPlan[proposalId];
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            if (plan.operations[i].delayed) {
-                bytes32 operationId = plan.operations[i].id;
-
-                uint48 schedule = _manager.getSchedule(operationId);
-                if (schedule != eta) {
-                    revert GovernorNotQueuedProposal(proposalId);
+            (bool delayed, uint32 nonce) = _getManagerData(plan, i);
+            if (delayed) {
+                uint32 relayedNonce = _manager.relay{value: values[i]}(targets[i], calldatas[i]);
+                if (relayedNonce != nonce) {
+                    // TODO: custom error
+                    revert();
                 }
-
-                _manager.relay{value: values[i]}(targets[i], calldatas[i]);
             } else {
                 (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
                 Address.verifyCallResult(success, returndata);
@@ -189,12 +202,13 @@ abstract contract GovernorTimelockAccess is Governor {
         // If the proposal has been scheduled it will have an ETA and we have to externally cancel
         if (eta != 0) {
             for (uint256 i = 0; i < targets.length; ++i) {
-                if (plan.operations[i].delayed) {
-                    bytes32 operationId = plan.operations[i].id;
+                (bool delayed, uint32 nonce) = _getManagerData(plan, i);
+                if (delayed) {
                     // Attempt to cancel considering the operation could have been cancelled and rescheduled already
-                    uint48 schedule = _manager.getSchedule(operationId);
-                    if (schedule == eta) {
-                        _manager.cancel(address(this), targets[i], calldatas[i]);
+                    uint32 canceledNonce = _manager.cancel(address(this), targets[i], calldatas[i]);
+                    if (canceledNonce != nonce) {
+                        // TODO: custom error
+                        revert();
                     }
                 }
             }
@@ -220,4 +234,31 @@ abstract contract GovernorTimelockAccess is Governor {
     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) {
+        (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
+        uint32 nonce = plan.managerData[bucket][subindex];
+        unchecked {
+            return nonce > 0 ? (true, nonce - 1) : (false, 0);
+        }
+    }
+
+    /**
+     * @dev Marks an operation at an index as delayed by the manager, and sets its scheduling nonce.
+     */
+    function _setManagerData(ExecutionPlan storage plan, uint256 index, uint32 nonce) private {
+        (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
+        plan.managerData[bucket][subindex] = nonce + 1;
+    }
+
+    /**
+     * @dev Returns bucket and subindex for reading manager data from the packed array mapping.
+     */
+    function _getManagerDataIndices(uint256 index) private pure returns (uint256 bucket, uint256 subindex) {
+        bucket = index >> 3;  // index / 8
+        subindex = index & 7; // index % 8
+    }
 }

From 6ca99ef1f138bac3a0d06d89cf771830593f701a Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Mon, 14 Aug 2023 21:03:33 -0300
Subject: [PATCH 14/36] make _hashOperation private

---
 contracts/access/manager/AccessManager.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index 00d0831c328..13f7ee1de1b 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -742,7 +742,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     /**
      * @dev Hashing function for delayed operations
      */
-    function _hashOperation(address caller, address target, bytes calldata data) internal pure virtual returns (bytes32) {
+    function _hashOperation(address caller, address target, bytes calldata data) private pure returns (bytes32) {
         return keccak256(abi.encode(caller, target, data));
     }
 

From e34c093c76fba4cdf490fdda3d785cab05dddd53 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Mon, 14 Aug 2023 23:21:32 -0300
Subject: [PATCH 15/36] add docs for nonce

---
 contracts/access/manager/AccessManager.sol | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index 13f7ee1de1b..fb3c78665d5 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -592,6 +592,10 @@ contract AccessManager is Context, Multicall, IAccessManager {
      * choose the timestamp at which the operation becomes executable as long as it satisfies the execution delays
      * required for the caller. The special value zero will automatically set the earliest possible time.
      *
+     * Returns the `operationId` that was scheduled. Since this value is a hash of the parameters, it can reoccur when
+     * the same parameters are used; if this is relevant, the returned `nonce` can be used to uniquely identify this
+     * scheduled operation from other occurences of the same `operationId` in invocations of {relay} and {cancel}.
+     *
      * Emits a {OperationScheduled} event.
      */
     function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32 operationId, uint32 nonce) {
@@ -632,6 +636,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
      * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the
      * execution delay is 0.
      *
+     * Returns the nonce that identifies the previously scheduled operation that is relayed, or 0 if the
+     * operation wasn't previously scheduled (if the caller doesn't have an execution delay).
+     *
      * Emits an {OperationExecuted} event only if the call was scheduled and delayed.
      */
     // Reentrancy is not an issue because permissions are checked on msg.sender. Additionally,
@@ -686,6 +693,8 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
     /**
      * @dev Internal variant of {consumeScheduledOp} that operates on bytes32 operationId.
+     *
+     * Returns the nonce of the scheduled operation that is consumed.
      */
     function _consumeScheduledOp(bytes32 operationId) internal virtual returns (uint32) {
         uint48 timepoint = _schedules[operationId].timepoint;
@@ -706,7 +715,8 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     /**
-     * @dev Cancel a scheduled (delayed) operation.
+     * @dev Cancel a scheduled (delayed) operation. Returns the nonce that identifies the previously scheduled
+     * operation that is cancelled.
      *
      * Requirements:
      *

From 40adbb73157194be5bf90aeb77d842e79511b6af Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 12:48:54 -0300
Subject: [PATCH 16/36] typo

---
 contracts/access/manager/AccessManager.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index fb3c78665d5..5c2c28df5d6 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -594,7 +594,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
      *
      * Returns the `operationId` that was scheduled. Since this value is a hash of the parameters, it can reoccur when
      * the same parameters are used; if this is relevant, the returned `nonce` can be used to uniquely identify this
-     * scheduled operation from other occurences of the same `operationId` in invocations of {relay} and {cancel}.
+     * scheduled operation from other occurrences of the same `operationId` in invocations of {relay} and {cancel}.
      *
      * Emits a {OperationScheduled} event.
      */

From 2c41d41015ce9b2e26a8c49b3bb4f68a29d2198f Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Tue, 15 Aug 2023 15:35:22 -0300
Subject: [PATCH 17/36] Update
 contracts/governance/extensions/GovernorTimelockAccess.sol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 86ee9f93f76..2e854345441 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -130,7 +130,7 @@ abstract contract GovernorTimelockAccess is Governor {
      * @dev Function to queue a proposal to the timelock.
      *
      * NOTE: execution delay is estimated based on the delay information retrieved in {proposal}. This value may be
-     * off if the delay were updated during the vote.
+     * off if the delay was updated during the vote.
      */
     function _queueOperations(
         uint256 proposalId,

From 1446af0a532ebbf24c53e771ba3c829fe6f6cc37 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Tue, 15 Aug 2023 15:35:45 -0300
Subject: [PATCH 18/36] Update
 contracts/governance/extensions/GovernorTimelockAccess.sol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 2e854345441..10b4e064560 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -24,7 +24,7 @@ abstract contract GovernorTimelockAccess is Governor {
         // We pack 8 operations' data in each bucket. Each uint32 value is set to 1 upon proposal creation if it has to
         // be scheduled and relayed through the manager. Upon queuing, the value is set to nonce + 1, where the nonce is
         // that which we get back from the manager when scheduling the operation.
-        mapping (uint256 bucket => uint32[8]) managerData;
+        mapping (uint256 operationBucket => uint32[8]) managerData;
     }
 
     mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;

From bc2bfa563bba5a8e0cf6ba021d53d6701a777119 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Tue, 15 Aug 2023 15:43:08 -0300
Subject: [PATCH 19/36] Update contracts/access/manager/AccessManager.sol

---
 contracts/access/manager/AccessManager.sol | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index 5c2c28df5d6..c3d944e913d 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -624,7 +624,10 @@ contract AccessManager is Context, Multicall, IAccessManager {
             revert AccessManagerAlreadyScheduled(operationId);
         }
 
-        nonce = _schedules[operationId].nonce + 1;
+        unchecked {
+            // It's not feasible to overflow the nonce in less than 1000 years
+            nonce = _schedules[operationId].nonce + 1;
+        }
         _schedules[operationId].timepoint = when;
         _schedules[operationId].nonce = nonce;
         emit OperationScheduled(operationId, nonce, when, caller, target, data);

From af274b1f2ecd659edfc76ca1e009d4603ef66cf0 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 17:38:35 -0300
Subject: [PATCH 20/36] add proposalNeedsQueuing

---
 contracts/governance/Governor.sol                         | 7 +++++++
 contracts/governance/IGovernor.sol                        | 6 ++++++
 .../governance/extensions/GovernorTimelockAccess.sol      | 8 ++++++++
 .../governance/extensions/GovernorTimelockCompound.sol    | 7 +++++++
 .../governance/extensions/GovernorTimelockControl.sol     | 7 +++++++
 contracts/mocks/docs/governance/MyGovernor.sol            | 4 ++++
 contracts/mocks/governance/GovernorStorageMock.sol        | 4 ++++
 .../mocks/governance/GovernorTimelockCompoundMock.sol     | 4 ++++
 .../mocks/governance/GovernorTimelockControlMock.sol      | 4 ++++
 test/utils/introspection/SupportsInterface.behavior.js    | 1 +
 10 files changed, 52 insertions(+)

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index 9d9e2eb5907..8c68d1d5bda 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -208,6 +208,13 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         return _proposals[proposalId].eta;
     }
 
+    /**
+     * @dev See {IGovernor-proposalEta}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual returns (bool) {
+        return false;
+    }
+
     /**
      * @dev Reverts if the `msg.sender` is not the executor. In case the executor is not this contract
      * itself, the function reverts if `msg.data` is not whitelisted as a result of an {execute}
diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol
index 3b31bc502f8..a3f6fec5228 100644
--- a/contracts/governance/IGovernor.sol
+++ b/contracts/governance/IGovernor.sol
@@ -245,6 +245,12 @@ interface IGovernor is IERC165, IERC6372 {
      */
     function proposalEta(uint256 proposalId) external view returns (uint256);
 
+    /**
+     * @notice module:core
+     * @dev Whether a proposal needs to be queued before execution.
+     */
+    function proposalNeedsQueuing(uint256 proposalId) external view returns (bool);
+
     /**
      * @notice module:user-config
      * @dev Delay, between the proposal is created and the vote starts. The unit this duration is expressed in depends
diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 10b4e064560..9f5ef6a35ef 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -96,6 +96,14 @@ abstract contract GovernorTimelockAccess is Governor {
         return (delay, indirect);
     }
 
+    /**
+     * @dev See {IGovernor-proposalEta}.
+     */
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override returns (bool) {
+        return _executionPlan[proposalId].delay > 0;
+    }
+
+
     /**
      * @dev See {IGovernor-propose}
      */
diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol
index fe554dad5d2..b9dffbc9ccc 100644
--- a/contracts/governance/extensions/GovernorTimelockCompound.sol
+++ b/contracts/governance/extensions/GovernorTimelockCompound.sol
@@ -54,6 +54,13 @@ abstract contract GovernorTimelockCompound is Governor {
         return address(_timelock);
     }
 
+    /**
+     * @dev See {IGovernor-proposalEta}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
+        return true;
+    }
+
     /**
      * @dev Function to queue a proposal to the timelock.
      */
diff --git a/contracts/governance/extensions/GovernorTimelockControl.sol b/contracts/governance/extensions/GovernorTimelockControl.sol
index c468f328b31..d732a82ed33 100644
--- a/contracts/governance/extensions/GovernorTimelockControl.sol
+++ b/contracts/governance/extensions/GovernorTimelockControl.sol
@@ -67,6 +67,13 @@ abstract contract GovernorTimelockControl is Governor {
         return address(_timelock);
     }
 
+    /**
+     * @dev See {IGovernor-proposalEta}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
+        return true;
+    }
+
     /**
      * @dev Function to queue a proposal to the timelock.
      */
diff --git a/contracts/mocks/docs/governance/MyGovernor.sol b/contracts/mocks/docs/governance/MyGovernor.sol
index 7cd5f3c2c74..87b399d1893 100644
--- a/contracts/mocks/docs/governance/MyGovernor.sol
+++ b/contracts/mocks/docs/governance/MyGovernor.sol
@@ -40,6 +40,10 @@ contract MyGovernor is
         return super.state(proposalId);
     }
 
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+        return super.proposalNeedsQueuing(proposalId);
+    }
+
     function _queueOperations(
         uint256 proposalId,
         address[] memory targets,
diff --git a/contracts/mocks/governance/GovernorStorageMock.sol b/contracts/mocks/governance/GovernorStorageMock.sol
index 3d08a003170..18860fa26d6 100644
--- a/contracts/mocks/governance/GovernorStorageMock.sol
+++ b/contracts/mocks/governance/GovernorStorageMock.sol
@@ -28,6 +28,10 @@ abstract contract GovernorStorageMock is
         return super.proposalThreshold();
     }
 
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+        return super.proposalNeedsQueuing(proposalId);
+    }
+
     function _propose(
         address[] memory targets,
         uint256[] memory values,
diff --git a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
index 124b0346fbf..c7b225a56e7 100644
--- a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
+++ b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
@@ -28,6 +28,10 @@ abstract contract GovernorTimelockCompoundMock is
         return super.proposalThreshold();
     }
 
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockCompound) returns (bool) {
+        return super.proposalNeedsQueuing(proposalId);
+    }
+
     function _queueOperations(
         uint256 proposalId,
         address[] memory targets,
diff --git a/contracts/mocks/governance/GovernorTimelockControlMock.sol b/contracts/mocks/governance/GovernorTimelockControlMock.sol
index 125aa37df11..a351bf58cb3 100644
--- a/contracts/mocks/governance/GovernorTimelockControlMock.sol
+++ b/contracts/mocks/governance/GovernorTimelockControlMock.sol
@@ -26,6 +26,10 @@ abstract contract GovernorTimelockControlMock is
         return super.proposalThreshold();
     }
 
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+        return super.proposalNeedsQueuing(proposalId);
+    }
+
     function _queueOperations(
         uint256 proposalId,
         address[] memory targets,
diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js
index d254082d2da..49a30e75576 100644
--- a/test/utils/introspection/SupportsInterface.behavior.js
+++ b/test/utils/introspection/SupportsInterface.behavior.js
@@ -61,6 +61,7 @@ const INTERFACES = {
     'proposalDeadline(uint256)',
     'proposalProposer(uint256)',
     'proposalEta(uint256)',
+    'proposalNeedsQueuing(uint256)',
     'votingDelay()',
     'votingPeriod()',
     'quorum(uint256)',

From bac912eac0358627ef148316b2b894fa8f306ab1 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 17:39:13 -0300
Subject: [PATCH 21/36] remove timepoint from executed and canceled events

---
 contracts/access/manager/AccessManager.sol  | 7 +++----
 contracts/access/manager/IAccessManager.sol | 4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index c3d944e913d..14be9838644 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -712,7 +712,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
 
         delete _schedules[operationId];
-        emit OperationExecuted(operationId, nonce, timepoint);
+        emit OperationExecuted(operationId, nonce);
 
         return nonce;
     }
@@ -744,10 +744,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
             }
         }
 
-        uint32 nonce = _schedules[operationId].nonce;
-        uint48 timepoint = _schedules[operationId].timepoint;
         delete _schedules[operationId].timepoint;
-        emit OperationCanceled(operationId, nonce, timepoint);
+        uint32 nonce = _schedules[operationId].nonce;
+        emit OperationCanceled(operationId, nonce);
 
         return nonce;
     }
diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol
index fcf78ba3f70..e8ac7069321 100644
--- a/contracts/access/manager/IAccessManager.sol
+++ b/contracts/access/manager/IAccessManager.sol
@@ -14,12 +14,12 @@ interface IAccessManager {
     /**
      * @dev A scheduled operation was executed.
      */
-    event OperationExecuted(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule);
+    event OperationExecuted(bytes32 indexed operationId, uint32 indexed nonce);
 
     /**
      * @dev A scheduled operation was canceled.
      */
-    event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule);
+    event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce);
 
     event GroupLabel(uint64 indexed groupId, string label);
     event GroupGranted(uint64 indexed groupId, address indexed account, uint32 delay, uint48 since);

From a1cbc8324709e6e3f98d200665ca908d612b1847 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 18:02:46 -0300
Subject: [PATCH 22/36] add tests for proposalNeedsQueuing

---
 test/governance/Governor.test.js               |  6 ++++++
 .../GovernorTimelockCompound.test.js           | 13 +++++++++++--
 .../extensions/GovernorTimelockControl.test.js | 18 ++++++++++++++----
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
index 9dfa53e7bbd..45cceba9a89 100644
--- a/test/governance/Governor.test.js
+++ b/test/governance/Governor.test.js
@@ -100,6 +100,9 @@ contract('Governor', function (accounts) {
         expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(value);
         expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal('0');
 
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0');
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(false);
+
         // Run proposal
         const txPropose = await this.helper.propose({ from: proposer });
 
@@ -164,6 +167,9 @@ contract('Governor', function (accounts) {
         expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(true);
         expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal('0');
         expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal(value);
+
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0');
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(false);
       });
 
       it('send ethers', async function () {
diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js
index 0f7fd635255..78179dbabe0 100644
--- a/test/governance/extensions/GovernorTimelockCompound.test.js
+++ b/test/governance/extensions/GovernorTimelockCompound.test.js
@@ -29,6 +29,8 @@ contract('GovernorTimelockCompound', function (accounts) {
   const votingPeriod = web3.utils.toBN(16);
   const value = web3.utils.toWei('1');
 
+  const defaultDelay = 2 * 86400;
+
   for (const { mode, Token } of TOKENS) {
     describe(`using ${Token._json.contractName}`, function () {
       beforeEach(async function () {
@@ -40,7 +42,7 @@ contract('GovernorTimelockCompound', function (accounts) {
         const nonce = await web3.eth.getTransactionCount(deployer);
         const predictGovernor = computeCreateAddress(deployer, nonce + 1);
 
-        this.timelock = await Timelock.new(predictGovernor, 2 * 86400);
+        this.timelock = await Timelock.new(predictGovernor, defaultDelay);
         this.mock = await Governor.new(
           name,
           votingDelay,
@@ -91,6 +93,9 @@ contract('GovernorTimelockCompound', function (accounts) {
       });
 
       it('nominal', async function () {
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0');
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true);
+
         await this.helper.propose();
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
@@ -99,7 +104,11 @@ contract('GovernorTimelockCompound', function (accounts) {
         await this.helper.vote({ support: Enums.VoteType.Abstain }, { from: voter4 });
         await this.helper.waitForDeadline();
         const txQueue = await this.helper.queue();
-        const eta = await this.mock.proposalEta(this.proposal.id);
+
+        const eta = web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(defaultDelay);
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal(eta);
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true);
+
         await this.helper.waitForEta();
         const txExecute = await this.helper.execute();
 
diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js
index fe40d30e9e1..ec03d614475 100644
--- a/test/governance/extensions/GovernorTimelockControl.test.js
+++ b/test/governance/extensions/GovernorTimelockControl.test.js
@@ -4,6 +4,7 @@ const { expect } = require('chai');
 const Enums = require('../../helpers/enums');
 const { GovernorHelper, proposalStatesToBitMap, timelockSalt } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
+const { clockFromReceipt } = require('../../helpers/time');
 
 const Timelock = artifacts.require('TimelockController');
 const Governor = artifacts.require('$GovernorTimelockControlMock');
@@ -33,13 +34,15 @@ contract('GovernorTimelockControl', function (accounts) {
   const votingPeriod = web3.utils.toBN(16);
   const value = web3.utils.toWei('1');
 
+  const delay = 3600;
+
   for (const { mode, Token } of TOKENS) {
     describe(`using ${Token._json.contractName}`, function () {
       beforeEach(async function () {
         const [deployer] = await web3.eth.getAccounts();
 
         this.token = await Token.new(tokenName, tokenSymbol, tokenName, version);
-        this.timelock = await Timelock.new(3600, [], [], deployer);
+        this.timelock = await Timelock.new(delay, [], [], deployer);
         this.mock = await Governor.new(
           name,
           votingDelay,
@@ -107,6 +110,9 @@ contract('GovernorTimelockControl', function (accounts) {
       });
 
       it('nominal', async function () {
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0');
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true);
+
         await this.helper.propose();
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
@@ -116,6 +122,11 @@ contract('GovernorTimelockControl', function (accounts) {
         await this.helper.waitForDeadline();
         const txQueue = await this.helper.queue();
         await this.helper.waitForEta();
+
+        const eta = web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(delay);
+        expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal(eta);
+        expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true);
+
         const txExecute = await this.helper.execute();
 
         expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id });
@@ -352,11 +363,10 @@ contract('GovernorTimelockControl', function (accounts) {
             const data = this.mock.contract.methods.relay(constants.ZERO_ADDRESS, 0, '0x').encodeABI();
             const predecessor = constants.ZERO_BYTES32;
             const salt = constants.ZERO_BYTES32;
-            const delay = 3600;
 
             await this.timelock.schedule(target, value, data, predecessor, salt, delay, { from: owner });
 
-            await time.increase(3600);
+            await time.increase(delay);
 
             await expectRevertCustomError(
               this.timelock.execute(target, value, data, predecessor, salt, { from: owner }),
@@ -369,7 +379,7 @@ contract('GovernorTimelockControl', function (accounts) {
         describe('updateTimelock', function () {
           beforeEach(async function () {
             this.newTimelock = await Timelock.new(
-              3600,
+              delay,
               [this.mock.address],
               [this.mock.address],
               constants.ZERO_ADDRESS,

From ebd6dcd6d933c68969f97123aafa8913fcc1d986 Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Tue, 15 Aug 2023 18:23:58 -0300
Subject: [PATCH 23/36] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/governance/Governor.sol                          | 2 +-
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index 8c68d1d5bda..f77a4881461 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -209,7 +209,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     }
 
     /**
-     * @dev See {IGovernor-proposalEta}.
+     * @dev See {IGovernor-proposalNeedsQueuing}.
      */
     function proposalNeedsQueuing(uint256) public view virtual returns (bool) {
         return false;
diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 9f5ef6a35ef..726e46e5ecc 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -97,7 +97,7 @@ abstract contract GovernorTimelockAccess is Governor {
     }
 
     /**
-     * @dev See {IGovernor-proposalEta}.
+     * @dev See {IGovernor-proposalNeedsQueuing}.
      */
     function proposalNeedsQueuing(uint256 proposalId) public view virtual override returns (bool) {
         return _executionPlan[proposalId].delay > 0;

From 6902ad7c5ed90577e17d90d396c93de71924c787 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 18:25:34 -0300
Subject: [PATCH 24/36] fix docs

---
 contracts/governance/extensions/GovernorTimelockCompound.sol | 2 +-
 contracts/governance/extensions/GovernorTimelockControl.sol  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol
index b9dffbc9ccc..a3d47bea122 100644
--- a/contracts/governance/extensions/GovernorTimelockCompound.sol
+++ b/contracts/governance/extensions/GovernorTimelockCompound.sol
@@ -55,7 +55,7 @@ abstract contract GovernorTimelockCompound is Governor {
     }
 
     /**
-     * @dev See {IGovernor-proposalEta}.
+     * @dev See {IGovernor-proposalNeedsQueuing}.
      */
     function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
         return true;
diff --git a/contracts/governance/extensions/GovernorTimelockControl.sol b/contracts/governance/extensions/GovernorTimelockControl.sol
index d732a82ed33..5f1e58ded6e 100644
--- a/contracts/governance/extensions/GovernorTimelockControl.sol
+++ b/contracts/governance/extensions/GovernorTimelockControl.sol
@@ -68,7 +68,7 @@ abstract contract GovernorTimelockControl is Governor {
     }
 
     /**
-     * @dev See {IGovernor-proposalEta}.
+     * @dev See {IGovernor-proposalNeedsQueuing}.
      */
     function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
         return true;

From e96474c05969039a841005c96caa3d5202251b6a Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 18:39:11 -0300
Subject: [PATCH 25/36] remove unnecessary override

---
 contracts/governance/Governor.sol | 40 +++++++++++++++----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index f77a4881461..e661b568ec7 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -98,14 +98,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     /**
      * @dev See {IGovernor-name}.
      */
-    function name() public view virtual override returns (string memory) {
+    function name() public view virtual returns (string memory) {
         return _name;
     }
 
     /**
      * @dev See {IGovernor-version}.
      */
-    function version() public view virtual override returns (string memory) {
+    function version() public view virtual returns (string memory) {
         return "1";
     }
 
@@ -127,14 +127,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public pure virtual override returns (uint256) {
+    ) public pure virtual returns (uint256) {
         return uint256(keccak256(abi.encode(targets, values, calldatas, descriptionHash)));
     }
 
     /**
      * @dev See {IGovernor-state}.
      */
-    function state(uint256 proposalId) public view virtual override returns (ProposalState) {
+    function state(uint256 proposalId) public view virtual returns (ProposalState) {
         // ProposalCore is just one slot. We can load it from storage to stack with a single sload
         ProposalCore storage proposal = _proposals[proposalId];
         bool proposalExecuted = proposal.executed;
@@ -176,35 +176,35 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     /**
      * @dev See {IGovernor-proposalThreshold}.
      */
-    function proposalThreshold() public view virtual override returns (uint256) {
+    function proposalThreshold() public view virtual returns (uint256) {
         return 0;
     }
 
     /**
      * @dev See {IGovernor-proposalSnapshot}.
      */
-    function proposalSnapshot(uint256 proposalId) public view virtual override returns (uint256) {
+    function proposalSnapshot(uint256 proposalId) public view virtual returns (uint256) {
         return _proposals[proposalId].voteStart;
     }
 
     /**
      * @dev See {IGovernor-proposalDeadline}.
      */
-    function proposalDeadline(uint256 proposalId) public view virtual override returns (uint256) {
+    function proposalDeadline(uint256 proposalId) public view virtual returns (uint256) {
         return _proposals[proposalId].voteStart + _proposals[proposalId].voteDuration;
     }
 
     /**
      * @dev See {IGovernor-proposalProposer}.
      */
-    function proposalProposer(uint256 proposalId) public view virtual override returns (address) {
+    function proposalProposer(uint256 proposalId) public view virtual returns (address) {
         return _proposals[proposalId].proposer;
     }
 
     /**
      * @dev See {IGovernor-proposalEta}.
      */
-    function proposalEta(uint256 proposalId) public view virtual override returns (uint256) {
+    function proposalEta(uint256 proposalId) public view virtual returns (uint256) {
         return _proposals[proposalId].eta;
     }
 
@@ -277,7 +277,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         string memory description
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         address proposer = _msgSender();
 
         // check description restriction
@@ -347,7 +347,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Succeeded));
@@ -395,7 +395,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public payable virtual override returns (uint256) {
+    ) public payable virtual returns (uint256) {
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(
@@ -455,7 +455,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         // The proposalId will be recomputed in the `_cancel` call further down. However we need the value before we
         // do the internal call, because we need to check the proposal state BEFORE the internal `_cancel` call
         // changes it. The `hashProposal` duplication has a cost that is limited, and that we accept.
@@ -501,7 +501,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     /**
      * @dev See {IGovernor-getVotes}.
      */
-    function getVotes(address account, uint256 timepoint) public view virtual override returns (uint256) {
+    function getVotes(address account, uint256 timepoint) public view virtual returns (uint256) {
         return _getVotes(account, timepoint, _defaultParams());
     }
 
@@ -512,14 +512,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         address account,
         uint256 timepoint,
         bytes memory params
-    ) public view virtual override returns (uint256) {
+    ) public view virtual returns (uint256) {
         return _getVotes(account, timepoint, params);
     }
 
     /**
      * @dev See {IGovernor-castVote}.
      */
-    function castVote(uint256 proposalId, uint8 support) public virtual override returns (uint256) {
+    function castVote(uint256 proposalId, uint8 support) public virtual returns (uint256) {
         address voter = _msgSender();
         return _castVote(proposalId, voter, support, "");
     }
@@ -531,7 +531,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256 proposalId,
         uint8 support,
         string calldata reason
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         address voter = _msgSender();
         return _castVote(proposalId, voter, support, reason);
     }
@@ -544,7 +544,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint8 support,
         string calldata reason,
         bytes memory params
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         address voter = _msgSender();
         return _castVote(proposalId, voter, support, reason, params);
     }
@@ -557,7 +557,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint8 support,
         address voter,
         bytes memory signature
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         bool valid = SignatureChecker.isValidSignatureNow(
             voter,
             _hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, _useNonce(voter)))),
@@ -581,7 +581,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         string calldata reason,
         bytes memory params,
         bytes memory signature
-    ) public virtual override returns (uint256) {
+    ) public virtual returns (uint256) {
         bool valid = SignatureChecker.isValidSignatureNow(
             voter,
             _hashTypedDataV4(

From 452c65e649f33166979f9d0d7a729894662fff99 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 19:11:48 -0300
Subject: [PATCH 26/36] remove _authorityOverride

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 726e46e5ecc..96b26c7c467 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -29,8 +29,6 @@ abstract contract GovernorTimelockAccess is Governor {
 
     mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
 
-    mapping(address target => address) private _authorityOverride;
-
     uint32 _baseDelay;
 
     IAccessManager immutable private _manager;

From f7b3b93f7d2e05ce30c92b88bd5913fda9e64866 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 19:17:16 -0300
Subject: [PATCH 27/36] add missing docs

---
 .../extensions/GovernorTimelockAccess.sol     | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 96b26c7c467..8e2bf4e6eeb 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -13,9 +13,23 @@ import {SafeCast} from "../../utils/math/SafeCast.sol";
 import {Time} from "../../utils/types/Time.sol";
 
 /**
- * @dev TODO
+ * @dev This module connects a {Governor} instance to an {AccessManager} instance, allowing the governor to make calls
+ * that are delay-restricted by the manager using the normal {queue} workflow. An optional base delay is applied to
+ * operations that are not delayed externally by the manager. Execution of a proposal will be delayed as much as
+ * necessary to meet the required delays of all of its operations.
+ *
+ * This extension allows the governor to hold and use its own assets and permissions, unlike {GovernorTimelockControl}
+ * and {GovernorTimelockCompound}, where the timelock is a separate contract that must be the one to hold assets and
+ * permissions. Operations that are delay-restricted by the manager, however, will be executed through the
+ * {AccessManager-relay} 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 cancelation by a guardian of a single
+ * operation in a proposal will cause all of it to become unable to execute.
  */
 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
+    // execution semantics of the proposal, namely whether a call will go through {AccessManager-relay}.
     struct ExecutionPlan {
         uint16 length;
         uint32 delay;
@@ -37,6 +51,9 @@ abstract contract GovernorTimelockAccess is Governor {
 
     event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
 
+    /**
+     * @dev Initialize the governor with an {AccessManager} and initial base delay.
+     */
     constructor(address manager, uint32 initialBaseDelay) {
         _manager = IAccessManager(manager);
         _setBaseDelaySeconds(initialBaseDelay);
@@ -133,10 +150,10 @@ abstract contract GovernorTimelockAccess is Governor {
     }
 
     /**
-     * @dev Function to queue a proposal to the timelock.
+     * @dev Mechanism to queue a proposal, potentially scheduling some of its operations in the AccessManager.
      *
-     * NOTE: execution delay is estimated based on the delay information retrieved in {proposal}. This value may be
-     * off if the delay was updated during the vote.
+     * NOTE: The execution delay is chosen based on the delay information retrieved in {propose}. This value may be
+     * off if the delay was updated since proposal creation. In this case, the proposal needs to be recreated.
      */
     function _queueOperations(
         uint256 proposalId,
@@ -159,6 +176,9 @@ abstract contract GovernorTimelockAccess is Governor {
         return eta;
     }
 
+    /**
+     * @dev Mechanism to execute a proposal, potentially going through {AccessManager-relay} for delayed operations.
+     */
     function _executeOperations(
         uint256 proposalId,
         address[] memory targets,

From 3b470387dfa53cc41121ee4057bae95a9313c18e Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 19:18:25 -0300
Subject: [PATCH 28/36] remove unused imports

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 8e2bf4e6eeb..699b2a6870f 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -2,11 +2,9 @@
 
 pragma solidity ^0.8.20;
 
-import {IGovernor, Governor} from "../Governor.sol";
-import {IAuthority} from "../../access/manager/IAuthority.sol";
+import {Governor} from "../Governor.sol";
 import {AuthorityUtils} from "../../access/manager/AuthorityUtils.sol";
 import {IAccessManager} from "../../access/manager/IAccessManager.sol";
-import {IAccessManaged} from "../../access/manager/IAccessManaged.sol";
 import {Address} from "../../utils/Address.sol";
 import {Math} from "../../utils/math/Math.sol";
 import {SafeCast} from "../../utils/math/SafeCast.sol";

From 8d5e7345f2becf65b9f0ad13e62087a5126bf034 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 19:20:13 -0300
Subject: [PATCH 29/36] typo

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 699b2a6870f..8674616dbea 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -22,7 +22,7 @@ import {Time} from "../../utils/types/Time.sol";
  * {AccessManager-relay} 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 cancelation by a guardian of a single
+ * 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.
  */
 abstract contract GovernorTimelockAccess is Governor {

From 4f89411c162278d5342b741dd8b3eb6bd46a780f Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Tue, 15 Aug 2023 19:21:40 -0300
Subject: [PATCH 30/36] lint

---
 contracts/access/manager/AccessManager.sol               | 6 +++++-
 contracts/access/manager/IAccessManager.sol              | 9 ++++++++-
 .../governance/extensions/GovernorTimelockAccess.sol     | 9 ++++-----
 contracts/mocks/docs/governance/MyGovernor.sol           | 4 +++-
 contracts/mocks/governance/GovernorStorageMock.sol       | 4 +++-
 .../mocks/governance/GovernorTimelockCompoundMock.sol    | 4 +++-
 .../mocks/governance/GovernorTimelockControlMock.sol     | 4 +++-
 7 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index 14be9838644..01ac7664e00 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -598,7 +598,11 @@ contract AccessManager is Context, Multicall, IAccessManager {
      *
      * Emits a {OperationScheduled} event.
      */
-    function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32 operationId, uint32 nonce) {
+    function schedule(
+        address target,
+        bytes calldata data,
+        uint48 when
+    ) public virtual returns (bytes32 operationId, uint32 nonce) {
         address caller = _msgSender();
 
         // Fetch restriction to that apply to the caller on the targeted function
diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol
index e8ac7069321..f4a7da362ff 100644
--- a/contracts/access/manager/IAccessManager.sol
+++ b/contracts/access/manager/IAccessManager.sol
@@ -9,7 +9,14 @@ interface IAccessManager {
     /**
      * @dev A delayed operation was scheduled.
      */
-    event OperationScheduled(bytes32 indexed operationId, uint32 indexed nonce, uint48 schedule, address caller, address target, bytes data);
+    event OperationScheduled(
+        bytes32 indexed operationId,
+        uint32 indexed nonce,
+        uint48 schedule,
+        address caller,
+        address target,
+        bytes data
+    );
 
     /**
      * @dev A scheduled operation was executed.
diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 8674616dbea..e7f755f8433 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -36,14 +36,14 @@ abstract contract GovernorTimelockAccess is Governor {
         // We pack 8 operations' data in each bucket. Each uint32 value is set to 1 upon proposal creation if it has to
         // be scheduled and relayed through the manager. Upon queuing, the value is set to nonce + 1, where the nonce is
         // that which we get back from the manager when scheduling the operation.
-        mapping (uint256 operationBucket => uint32[8]) managerData;
+        mapping(uint256 operationBucket => uint32[8]) managerData;
     }
 
     mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
 
     uint32 _baseDelay;
 
-    IAccessManager immutable private _manager;
+    IAccessManager private immutable _manager;
 
     error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
 
@@ -116,7 +116,6 @@ abstract contract GovernorTimelockAccess is Governor {
         return _executionPlan[proposalId].delay > 0;
     }
 
-
     /**
      * @dev See {IGovernor-propose}
      */
@@ -164,7 +163,7 @@ abstract contract GovernorTimelockAccess is Governor {
         uint48 eta = Time.timestamp() + plan.delay;
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            (bool delayed,) = _getManagerData(plan, i);
+            (bool delayed, ) = _getManagerData(plan, i);
             if (delayed) {
                 (, uint32 nonce) = _manager.schedule(targets[i], calldatas[i], eta);
                 _setManagerData(plan, i, nonce);
@@ -282,7 +281,7 @@ abstract contract GovernorTimelockAccess is Governor {
      * @dev Returns bucket and subindex for reading manager data from the packed array mapping.
      */
     function _getManagerDataIndices(uint256 index) private pure returns (uint256 bucket, uint256 subindex) {
-        bucket = index >> 3;  // index / 8
+        bucket = index >> 3; // index / 8
         subindex = index & 7; // index % 8
     }
 }
diff --git a/contracts/mocks/docs/governance/MyGovernor.sol b/contracts/mocks/docs/governance/MyGovernor.sol
index 87b399d1893..d898fc71563 100644
--- a/contracts/mocks/docs/governance/MyGovernor.sol
+++ b/contracts/mocks/docs/governance/MyGovernor.sol
@@ -40,7 +40,9 @@ contract MyGovernor is
         return super.state(proposalId);
     }
 
-    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
         return super.proposalNeedsQueuing(proposalId);
     }
 
diff --git a/contracts/mocks/governance/GovernorStorageMock.sol b/contracts/mocks/governance/GovernorStorageMock.sol
index 18860fa26d6..88c6bf906bf 100644
--- a/contracts/mocks/governance/GovernorStorageMock.sol
+++ b/contracts/mocks/governance/GovernorStorageMock.sol
@@ -28,7 +28,9 @@ abstract contract GovernorStorageMock is
         return super.proposalThreshold();
     }
 
-    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
         return super.proposalNeedsQueuing(proposalId);
     }
 
diff --git a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
index c7b225a56e7..03ef62510f5 100644
--- a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
+++ b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol
@@ -28,7 +28,9 @@ abstract contract GovernorTimelockCompoundMock is
         return super.proposalThreshold();
     }
 
-    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockCompound) returns (bool) {
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) public view virtual override(Governor, GovernorTimelockCompound) returns (bool) {
         return super.proposalNeedsQueuing(proposalId);
     }
 
diff --git a/contracts/mocks/governance/GovernorTimelockControlMock.sol b/contracts/mocks/governance/GovernorTimelockControlMock.sol
index a351bf58cb3..edaccc0b769 100644
--- a/contracts/mocks/governance/GovernorTimelockControlMock.sol
+++ b/contracts/mocks/governance/GovernorTimelockControlMock.sol
@@ -26,7 +26,9 @@ abstract contract GovernorTimelockControlMock is
         return super.proposalThreshold();
     }
 
-    function proposalNeedsQueuing(uint256 proposalId) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) public view virtual override(Governor, GovernorTimelockControl) returns (bool) {
         return super.proposalNeedsQueuing(proposalId);
     }
 

From 40eea489bb8bf692b3f760f6ddd9ee36cbcb060e Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 01:52:20 -0300
Subject: [PATCH 31/36] add basic tests

---
 .../governance/GovernorTimelockAccessMock.sol |  70 +++++
 .../extensions/GovernorTimelockAccess.test.js | 258 ++++++++++++++++++
 2 files changed, 328 insertions(+)
 create mode 100644 contracts/mocks/governance/GovernorTimelockAccessMock.sol
 create mode 100644 test/governance/extensions/GovernorTimelockAccess.test.js

diff --git a/contracts/mocks/governance/GovernorTimelockAccessMock.sol b/contracts/mocks/governance/GovernorTimelockAccessMock.sol
new file mode 100644
index 00000000000..3d1bbeeef97
--- /dev/null
+++ b/contracts/mocks/governance/GovernorTimelockAccessMock.sol
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IGovernor, Governor} from "../../governance/Governor.sol";
+import {GovernorTimelockAccess} from "../../governance/extensions/GovernorTimelockAccess.sol";
+import {GovernorSettings} from "../../governance/extensions/GovernorSettings.sol";
+import {GovernorCountingSimple} from "../../governance/extensions/GovernorCountingSimple.sol";
+import {GovernorVotesQuorumFraction} from "../../governance/extensions/GovernorVotesQuorumFraction.sol";
+
+abstract contract GovernorTimelockAccessMock is
+    GovernorSettings,
+    GovernorTimelockAccess,
+    GovernorVotesQuorumFraction,
+    GovernorCountingSimple
+{
+    function nonGovernanceFunction() external {}
+
+    function quorum(uint256 blockNumber) public view override(Governor, GovernorVotesQuorumFraction) returns (uint256) {
+        return super.quorum(blockNumber);
+    }
+
+    function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) {
+        return super.proposalThreshold();
+    }
+
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) public view virtual override(Governor, GovernorTimelockAccess) returns (bool) {
+        return super.proposalNeedsQueuing(proposalId);
+    }
+
+    function propose(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        string memory description
+    ) public override(Governor, GovernorTimelockAccess) returns (uint256) {
+        return super.propose(targets, values, calldatas, description);
+    }
+
+    function _queueOperations(
+        uint256 proposalId,
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) internal override(Governor, GovernorTimelockAccess) returns (uint48) {
+        return super._queueOperations(proposalId, targets, values, calldatas, descriptionHash);
+    }
+
+    function _executeOperations(
+        uint256 proposalId,
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) internal override(Governor, GovernorTimelockAccess) {
+        super._executeOperations(proposalId, targets, values, calldatas, descriptionHash);
+    }
+
+    function _cancel(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) internal override(Governor, GovernorTimelockAccess) returns (uint256) {
+        return super._cancel(targets, values, calldatas, descriptionHash);
+    }
+}
diff --git a/test/governance/extensions/GovernorTimelockAccess.test.js b/test/governance/extensions/GovernorTimelockAccess.test.js
new file mode 100644
index 00000000000..0c92001c68d
--- /dev/null
+++ b/test/governance/extensions/GovernorTimelockAccess.test.js
@@ -0,0 +1,258 @@
+const { constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
+
+const Enums = require('../../helpers/enums');
+const { GovernorHelper, proposalStatesToBitMap, timelockSalt } = require('../../helpers/governance');
+const { expectRevertCustomError } = require('../../helpers/customError');
+const { clockFromReceipt } = require('../../helpers/time');
+
+const AccessManager = artifacts.require('$AccessManager');
+const Governor = artifacts.require('$GovernorTimelockAccessMock');
+const AccessManagedTarget = artifacts.require('$AccessManagedTarget');
+const ERC721 = artifacts.require('$ERC721');
+const ERC1155 = artifacts.require('$ERC1155');
+
+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;
+
+  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
+  const PROPOSER_ROLE = web3.utils.soliditySha3('PROPOSER_ROLE');
+  const EXECUTOR_ROLE = web3.utils.soliditySha3('EXECUTOR_ROLE');
+  const CANCELLER_ROLE = web3.utils.soliditySha3('CANCELLER_ROLE');
+
+  const name = 'OZ-Governor';
+  const version = '1';
+  const tokenName = 'MockToken';
+  const tokenSymbol = 'MTKN';
+  const tokenSupply = web3.utils.toWei('100');
+  const votingDelay = web3.utils.toBN(4);
+  const votingPeriod = web3.utils.toBN(16);
+  const value = web3.utils.toWei('1');
+
+  for (const { mode, Token } of TOKENS) {
+    describe(`using ${Token._json.contractName}`, function () {
+      beforeEach(async function () {
+        const [deployer] = await web3.eth.getAccounts();
+
+        this.token = await Token.new(tokenName, tokenSymbol, tokenName, version);
+        this.manager = await AccessManager.new(admin);
+        this.mock = await Governor.new(
+          name,
+          votingDelay,
+          votingPeriod,
+          0, // proposal threshold
+          this.manager.address,
+          0, // base delay
+          this.token.address,
+          0, // quorum
+        );
+        this.receiver = await AccessManagedTarget.new(this.manager.address);
+
+        this.helper = new GovernorHelper(this.mock, mode);
+
+        await web3.eth.sendTransaction({ from: admin, to: this.mock.address, value });
+
+        await this.token.$_mint(admin, tokenSupply);
+        await this.helper.delegate({ token: this.token, to: voter1, value: web3.utils.toWei('10') }, { from: admin });
+        await this.helper.delegate({ token: this.token, to: voter2, value: web3.utils.toWei('7') }, { from: admin });
+        await this.helper.delegate({ token: this.token, to: voter3, value: web3.utils.toWei('5') }, { from: admin });
+        await this.helper.delegate({ token: this.token, to: voter4, value: web3.utils.toWei('2') }, { from: admin });
+
+        // default proposals
+        this.restricted = {};
+        this.restricted.selector = this.receiver.contract.methods.fnRestricted().encodeABI();
+        this.restricted.operation = {
+          target: this.receiver.address,
+          value: '0',
+          data: this.restricted.selector,
+        };
+        this.restricted.operationId = hashOperation(
+          this.mock.address,
+          this.restricted.operation.target,
+          this.restricted.operation.data,
+        );
+
+        this.unrestricted = {};
+        this.unrestricted.selector = this.receiver.contract.methods.fnUnrestricted().encodeABI();
+        this.unrestricted.operation = {
+          target: this.receiver.address,
+          value: '0',
+          data: this.unrestricted.selector,
+        };
+        this.unrestricted.operationId = hashOperation(
+          this.mock.address,
+          this.unrestricted.operation.target,
+          this.unrestricted.operation.data,
+        );
+      });
+
+      it('accepts ether transfers', async function () {
+        await web3.eth.sendTransaction({ from: admin, to: this.mock.address, value: 1 });
+      });
+
+      it('post deployment check', async function () {
+        expect(await this.mock.name()).to.be.equal(name);
+        expect(await this.mock.token()).to.be.equal(this.token.address);
+        expect(await this.mock.votingDelay()).to.be.bignumber.equal(votingDelay);
+        expect(await this.mock.votingPeriod()).to.be.bignumber.equal(votingPeriod);
+        expect(await this.mock.quorum(0)).to.be.bignumber.equal('0');
+
+        expect(await this.mock.accessManager()).to.be.equal(this.manager.address);
+      });
+
+      describe('base delay only', function () {
+        for (const [delay, queue] of [
+          [0, true],
+          [0, false],
+          [1000, true],
+        ]) {
+          it(`delay ${delay}, ${queue ? 'with' : 'without'} queuing`, async function () {
+            await this.mock.$_setBaseDelaySeconds(delay);
+
+            this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr');
+
+            await this.helper.propose();
+            await this.helper.waitForSnapshot();
+            await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+            await this.helper.waitForDeadline();
+            if (queue) {
+              const txQueue = await this.helper.queue();
+              expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id });
+            }
+            if (delay > 0) {
+              await this.helper.waitForEta();
+            }
+            const txExecute = await this.helper.execute();
+            expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
+            expectEvent.inTransaction(txExecute, this.receiver, 'CalledUnrestricted');
+          });
+        }
+      });
+
+      it('single operation with access manager delay', async function () {
+        const delay = 1000;
+        const classId = '1';
+        const groupId = '1';
+
+        await this.manager.setContractClass(this.receiver.address, classId, { from: admin });
+        await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin });
+        await this.manager.grantGroup(groupId, this.mock.address, delay, { from: admin });
+
+        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();
+        const txQueue = await this.helper.queue();
+        await this.helper.waitForEta();
+        const txExecute = await this.helper.execute();
+
+        expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id });
+        await expectEvent.inTransaction(txQueue.tx, this.manager, 'OperationScheduled', {
+          operationId: this.restricted.operationId,
+          nonce: '1',
+          schedule: web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(delay),
+          caller: this.mock.address,
+          target: this.restricted.operation.target,
+          data: this.restricted.operation.data,
+        });
+
+        expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
+        await expectEvent.inTransaction(txExecute.tx, this.manager, 'OperationExecuted', {
+          operationId: this.restricted.operationId,
+          nonce: '1',
+        });
+        await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledRestricted');
+      });
+
+      it('bundle of varied operations', async function () {
+        const managerDelay = 1000;
+        const classId = '1';
+        const groupId = '1';
+
+        const baseDelay = managerDelay * 2;
+
+        await this.mock.$_setBaseDelaySeconds(baseDelay);
+
+        await this.manager.setContractClass(this.receiver.address, classId, { from: admin });
+        await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin });
+        await this.manager.grantGroup(groupId, this.mock.address, managerDelay, { from: admin });
+
+        this.proposal = await this.helper.setProposal([this.restricted.operation, this.unrestricted.operation], 'descr');
+
+        await this.helper.propose();
+        await this.helper.waitForSnapshot();
+        await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
+        await this.helper.waitForDeadline();
+        const txQueue = await this.helper.queue();
+        await this.helper.waitForEta();
+        const txExecute = await this.helper.execute();
+
+        expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id });
+        await expectEvent.inTransaction(txQueue.tx, this.manager, 'OperationScheduled', {
+          operationId: this.restricted.operationId,
+          nonce: '1',
+          schedule: web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(baseDelay),
+          caller: this.mock.address,
+          target: this.restricted.operation.target,
+          data: this.restricted.operation.data,
+        });
+
+        expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
+        await expectEvent.inTransaction(txExecute.tx, this.manager, 'OperationExecuted', {
+          operationId: this.restricted.operationId,
+          nonce: '1',
+        });
+        await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledRestricted');
+        await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledUnrestricted');
+      });
+
+      it('cancellation after queue (internal)', async function () {
+        const delay = 1000;
+        const classId = '1';
+        const groupId = '1';
+
+        await this.manager.setContractClass(this.receiver.address, classId, { from: admin });
+        await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin });
+        await this.manager.grantGroup(groupId, this.mock.address, delay, { from: admin });
+
+        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();
+        const txQueue = 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',
+        });
+
+        await this.helper.waitForEta();
+        await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [
+          this.proposal.id,
+          Enums.ProposalState.Canceled,
+          proposalStatesToBitMap(
+            [Enums.ProposalState.Succeeded, Enums.ProposalState.Queued],
+          ),
+        ]);
+      });
+    });
+  }
+});

From 0604f4df5a9619c5aa7f4091b3b8530553825596 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 02:04:05 -0300
Subject: [PATCH 32/36] add custom error

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index e7f755f8433..a026867b789 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -46,6 +46,7 @@ abstract contract GovernorTimelockAccess is Governor {
     IAccessManager private immutable _manager;
 
     error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
+    error GovernorMismatchedNonce(uint256 proposalId, uint256 expectedNonce, uint256 actualNonce);
 
     event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
 
@@ -195,8 +196,7 @@ abstract contract GovernorTimelockAccess is Governor {
             if (delayed) {
                 uint32 relayedNonce = _manager.relay{value: values[i]}(targets[i], calldatas[i]);
                 if (relayedNonce != nonce) {
-                    // TODO: custom error
-                    revert();
+                    revert GovernorMismatchedNonce(proposalId, nonce, relayedNonce);
                 }
             } else {
                 (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
@@ -230,8 +230,7 @@ abstract contract GovernorTimelockAccess is Governor {
                     // 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) {
-                        // TODO: custom error
-                        revert();
+                        revert GovernorMismatchedNonce(proposalId, nonce, canceledNonce);
                     }
                 }
             }

From 16519fefe02b025602c245e4c5581069e68677ee Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 02:08:21 -0300
Subject: [PATCH 33/36] lint

---
 .../extensions/GovernorTimelockAccess.test.js | 34 ++++++-------------
 .../GovernorTimelockCompound.test.js          |  1 +
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/test/governance/extensions/GovernorTimelockAccess.test.js b/test/governance/extensions/GovernorTimelockAccess.test.js
index 0c92001c68d..776ec9390e3 100644
--- a/test/governance/extensions/GovernorTimelockAccess.test.js
+++ b/test/governance/extensions/GovernorTimelockAccess.test.js
@@ -1,36 +1,25 @@
-const { constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers');
+const { expectEvent } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
 
 const Enums = require('../../helpers/enums');
-const { GovernorHelper, proposalStatesToBitMap, timelockSalt } = require('../../helpers/governance');
+const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
 const { clockFromReceipt } = require('../../helpers/time');
 
 const AccessManager = artifacts.require('$AccessManager');
 const Governor = artifacts.require('$GovernorTimelockAccessMock');
 const AccessManagedTarget = artifacts.require('$AccessManagedTarget');
-const ERC721 = artifacts.require('$ERC721');
-const ERC1155 = artifacts.require('$ERC1155');
 
 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],
-  )
-);
+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;
-
-  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
-  const PROPOSER_ROLE = web3.utils.soliditySha3('PROPOSER_ROLE');
-  const EXECUTOR_ROLE = web3.utils.soliditySha3('EXECUTOR_ROLE');
-  const CANCELLER_ROLE = web3.utils.soliditySha3('CANCELLER_ROLE');
+  const [admin, voter1, voter2, voter3, voter4] = accounts;
 
   const name = 'OZ-Governor';
   const version = '1';
@@ -44,8 +33,6 @@ contract('GovernorTimelockAccess', function (accounts) {
   for (const { mode, Token } of TOKENS) {
     describe(`using ${Token._json.contractName}`, function () {
       beforeEach(async function () {
-        const [deployer] = await web3.eth.getAccounts();
-
         this.token = await Token.new(tokenName, tokenSymbol, tokenName, version);
         this.manager = await AccessManager.new(admin);
         this.mock = await Governor.new(
@@ -191,7 +178,10 @@ contract('GovernorTimelockAccess', function (accounts) {
         await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin });
         await this.manager.grantGroup(groupId, this.mock.address, managerDelay, { from: admin });
 
-        this.proposal = await this.helper.setProposal([this.restricted.operation, this.unrestricted.operation], 'descr');
+        this.proposal = await this.helper.setProposal(
+          [this.restricted.operation, this.unrestricted.operation],
+          'descr',
+        );
 
         await this.helper.propose();
         await this.helper.waitForSnapshot();
@@ -235,7 +225,7 @@ contract('GovernorTimelockAccess', function (accounts) {
         await this.helper.waitForSnapshot();
         await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 });
         await this.helper.waitForDeadline();
-        const txQueue = await this.helper.queue();
+        await this.helper.queue();
 
         const txCancel = await this.helper.cancel('internal');
         expectEvent(txCancel, 'ProposalCanceled', { proposalId: this.proposal.id });
@@ -248,9 +238,7 @@ contract('GovernorTimelockAccess', function (accounts) {
         await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [
           this.proposal.id,
           Enums.ProposalState.Canceled,
-          proposalStatesToBitMap(
-            [Enums.ProposalState.Succeeded, Enums.ProposalState.Queued],
-          ),
+          proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]),
         ]);
       });
     });
diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js
index 78179dbabe0..e9d6f83736a 100644
--- a/test/governance/extensions/GovernorTimelockCompound.test.js
+++ b/test/governance/extensions/GovernorTimelockCompound.test.js
@@ -5,6 +5,7 @@ const Enums = require('../../helpers/enums');
 const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
 const { computeCreateAddress } = require('../../helpers/create');
+const { clockFromReceipt } = require('../../helpers/time');
 
 const Timelock = artifacts.require('CompTimelock');
 const Governor = artifacts.require('$GovernorTimelockCompoundMock');

From 85148a980f37f9231f001f689a3e75b0165a2ccb Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 02:15:05 -0300
Subject: [PATCH 34/36] make variable private

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index a026867b789..116722d7dfd 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -41,7 +41,7 @@ abstract contract GovernorTimelockAccess is Governor {
 
     mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
 
-    uint32 _baseDelay;
+    uint32 private _baseDelay;
 
     IAccessManager private immutable _manager;
 

From a36618bd16332ce5bdf456014796a628038029b8 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 02:18:58 -0300
Subject: [PATCH 35/36] add changeset

---
 .changeset/violet-melons-press.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/violet-melons-press.md

diff --git a/.changeset/violet-melons-press.md b/.changeset/violet-melons-press.md
new file mode 100644
index 00000000000..18fd70b5851
--- /dev/null
+++ b/.changeset/violet-melons-press.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`GovernorTimelockAccess`: Added a module to connect a governor with an instance of `AccessManager`, allowing the governor to make calls that are delay-restricted by the manager using the normal `queue` workflow.

From b37ed302dd55a3aa17476fc286ae729678820f9e Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Wed, 16 Aug 2023 02:27:08 -0300
Subject: [PATCH 36/36] do not delete executionPlan

---
 contracts/governance/extensions/GovernorTimelockAccess.sol | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol
index 116722d7dfd..6ebb3f1d6cb 100644
--- a/contracts/governance/extensions/GovernorTimelockAccess.sol
+++ b/contracts/governance/extensions/GovernorTimelockAccess.sol
@@ -203,8 +203,6 @@ abstract contract GovernorTimelockAccess is Governor {
                 Address.verifyCallResult(success, returndata);
             }
         }
-
-        delete _executionPlan[proposalId];
     }
 
     /**
@@ -236,8 +234,6 @@ abstract contract GovernorTimelockAccess is Governor {
             }
         }
 
-        delete _executionPlan[proposalId];
-
         return proposalId;
     }