Browse Source

Merge branch 'feat/access-manager'

Francisco Giordano 2 years ago
parent
commit
f7db0bea31
86 changed files with 3987 additions and 370 deletions
  1. 5 0
      .changeset/empty-cheetahs-hunt.md
  2. 5 0
      .changeset/fair-humans-peel.md
  3. 5 0
      .changeset/large-humans-remain.md
  4. 5 0
      .changeset/lazy-rice-joke.md
  5. 5 0
      .changeset/proud-spiders-attend.md
  6. 5 0
      .changeset/quiet-trainers-kick.md
  7. 5 0
      .changeset/violet-melons-press.md
  8. 2 0
      .github/workflows/checks.yml
  9. 10 0
      contracts/access/README.adoc
  10. 121 0
      contracts/access/manager/AccessManaged.sol
  11. 881 0
      contracts/access/manager/AccessManager.sol
  12. 31 0
      contracts/access/manager/AuthorityUtils.sol
  13. 17 0
      contracts/access/manager/IAccessManaged.sol
  14. 113 0
      contracts/access/manager/IAccessManager.sol
  15. 13 0
      contracts/access/manager/IAuthority.sol
  16. 2 2
      contracts/finance/VestingWallet.sol
  17. 41 37
      contracts/governance/Governor.sol
  18. 7 1
      contracts/governance/IGovernor.sol
  19. 2 2
      contracts/governance/TimelockController.sol
  20. 3 3
      contracts/governance/extensions/GovernorPreventLateQuorum.sol
  21. 0 1
      contracts/governance/extensions/GovernorSettings.sol
  22. 3 2
      contracts/governance/extensions/GovernorStorage.sol
  23. 342 0
      contracts/governance/extensions/GovernorTimelockAccess.sol
  24. 22 14
      contracts/governance/extensions/GovernorTimelockCompound.sol
  25. 18 8
      contracts/governance/extensions/GovernorTimelockControl.sol
  26. 4 2
      contracts/governance/extensions/GovernorVotes.sol
  27. 0 1
      contracts/governance/extensions/GovernorVotesQuorumFraction.sol
  28. 3 2
      contracts/governance/utils/Votes.sol
  29. 18 0
      contracts/mocks/AccessManagedTarget.sol
  30. 6 0
      contracts/mocks/docs/governance/MyGovernor.sol
  31. 6 0
      contracts/mocks/governance/GovernorStorageMock.sol
  32. 70 0
      contracts/mocks/governance/GovernorTimelockAccessMock.sol
  33. 6 0
      contracts/mocks/governance/GovernorTimelockCompoundMock.sol
  34. 6 0
      contracts/mocks/governance/GovernorTimelockControlMock.sol
  35. 2 1
      contracts/mocks/token/ERC20VotesLegacyMock.sol
  36. 4 4
      contracts/proxy/ERC1967/ERC1967Proxy.sol
  37. 2 2
      contracts/proxy/ERC1967/ERC1967Utils.sol
  38. 2 2
      contracts/proxy/Proxy.sol
  39. 6 6
      contracts/proxy/beacon/BeaconProxy.sol
  40. 19 20
      contracts/proxy/transparent/TransparentUpgradeableProxy.sol
  41. 19 8
      contracts/proxy/utils/Initializable.sol
  42. 5 4
      contracts/token/ERC1155/ERC1155.sol
  43. 5 4
      contracts/token/ERC20/ERC20.sol
  44. 2 2
      contracts/token/ERC20/utils/SafeERC20.sol
  45. 18 16
      contracts/token/ERC721/ERC721.sol
  46. 6 3
      contracts/token/ERC721/IERC721.sol
  47. 2 1
      contracts/token/ERC721/IERC721Receiver.sol
  48. 5 5
      contracts/token/ERC721/extensions/ERC721Consecutive.sol
  49. 2 15
      contracts/token/ERC721/extensions/ERC721Royalty.sol
  50. 6 25
      contracts/token/ERC721/extensions/ERC721URIStorage.sol
  51. 3 3
      contracts/token/ERC721/extensions/ERC721Wrapper.sol
  52. 2 1
      contracts/token/ERC721/utils/ERC721Holder.sol
  53. 1 2
      contracts/utils/Nonces.sol
  54. 2 1
      contracts/utils/ShortStrings.sol
  55. 2 1
      contracts/utils/Strings.sol
  56. 5 2
      contracts/utils/cryptography/ECDSA.sol
  57. 17 11
      contracts/utils/cryptography/MerkleProof.sol
  58. 3 6
      contracts/utils/cryptography/MessageHashUtils.sol
  59. 8 7
      contracts/utils/math/Math.sol
  60. 48 27
      contracts/utils/structs/Checkpoints.sol
  61. 8 7
      contracts/utils/structs/DoubleEndedQueue.sol
  62. 4 8
      contracts/utils/structs/EnumerableMap.sol
  63. 16 16
      contracts/utils/structs/EnumerableSet.sol
  64. 125 0
      contracts/utils/types/Time.sol
  65. 1 1
      contracts/vendor/compound/ICompoundTimelock.sol
  66. 6 6
      package-lock.json
  67. 16 9
      scripts/generate/templates/Checkpoints.js
  68. 4 8
      scripts/generate/templates/EnumerableMap.js
  69. 16 16
      scripts/generate/templates/EnumerableSet.js
  70. 1161 0
      test/access/manager/AccessManager.test.js
  71. 6 0
      test/governance/Governor.test.js
  72. 407 0
      test/governance/extensions/GovernorTimelockAccess.test.js
  73. 12 2
      test/governance/extensions/GovernorTimelockCompound.test.js
  74. 14 4
      test/governance/extensions/GovernorTimelockControl.test.js
  75. 16 0
      test/helpers/iterate.js
  76. 0 7
      test/helpers/map-values.js
  77. 5 0
      test/helpers/methods.js
  78. 10 10
      test/proxy/utils/Initializable.test.js
  79. 17 11
      test/token/ERC721/extensions/ERC721Royalty.test.js
  80. 20 6
      test/token/ERC721/extensions/ERC721URIStorage.test.js
  81. 1 2
      test/utils/Nonces.test.js
  82. 1 1
      test/utils/cryptography/EIP712.test.js
  83. 1 0
      test/utils/introspection/SupportsInterface.behavior.js
  84. 1 1
      test/utils/structs/EnumerableMap.test.js
  85. 1 1
      test/utils/structs/EnumerableSet.test.js
  86. 140 0
      test/utils/types/Time.test.js

+ 5 - 0
.changeset/empty-cheetahs-hunt.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`ERC721URIStorage`: Allow setting the token URI prior to minting.

+ 5 - 0
.changeset/fair-humans-peel.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`ERC721URIStorage`, `ERC721Royalty`: Stop resetting token-specific URI and royalties when burning.

+ 5 - 0
.changeset/large-humans-remain.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`MerkleProof`: Use custom error to report invalid multiproof instead of reverting with overflow panic.

+ 5 - 0
.changeset/lazy-rice-joke.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Initializable`: Use intermediate variables to improve readability.

+ 5 - 0
.changeset/proud-spiders-attend.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`ERC721`: Renamed `_requireMinted` to `_requireOwned` and added a return value with the current owner. Implemented `ownerOf` in terms of `_requireOwned`.

+ 5 - 0
.changeset/quiet-trainers-kick.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`AccessManager`: Added a new contract for managing access control of complex systems in a consolidated location.

+ 5 - 0
.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.

+ 2 - 0
.github/workflows/checks.yml

@@ -42,6 +42,7 @@ jobs:
         run: npm run test:generation
       - name: Compare gas costs
         uses: ./.github/actions/gas-compare
+        if: github.base_ref == 'master'
         with:
           token: ${{ github.token }}
 
@@ -63,6 +64,7 @@ jobs:
         run: npm run test:inheritance
       - name: Check storage layout
         uses: ./.github/actions/storage-layout
+        if: github.base_ref == 'master'
         continue-on-error: ${{ contains(github.event.pull_request.labels.*.name, 'breaking change') }}
         with:
           token: ${{ github.token }}

+ 10 - 0
contracts/access/README.adoc

@@ -27,3 +27,13 @@ This directory provides ways to restrict who can access the functions of a contr
 {{IAccessControlDefaultAdminRules}}
 
 {{AccessControlDefaultAdminRules}}
+
+== AccessManager
+
+{{IAuthority}}
+
+{{AccessManager}}
+
+{{AccessManaged}}
+
+{{AccessManagerAdapter}}

+ 121 - 0
contracts/access/manager/AccessManaged.sol

@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IAuthority} from "./IAuthority.sol";
+import {AuthorityUtils} from "./AuthorityUtils.sol";
+import {IAccessManager} from "./IAccessManager.sol";
+import {IAccessManaged} from "./IAccessManaged.sol";
+import {Context} from "../../utils/Context.sol";
+
+/**
+ * @dev This contract module makes available a {restricted} modifier. Functions decorated with this modifier will be
+ * permissioned according to an "authority": a contract like {AccessManager} that follows the {IAuthority} interface,
+ * implementing a policy that allows certain callers to access certain functions.
+ *
+ * IMPORTANT: The `restricted` modifier should never be used on `internal` functions, judiciously used in `public`
+ * functions, and ideally only used in `external` functions. See {restricted}.
+ */
+abstract contract AccessManaged is Context, IAccessManaged {
+    address private _authority;
+
+    bool private _consumingSchedule;
+
+    /**
+     * @dev Initializes the contract connected to an initial authority.
+     */
+    constructor(address initialAuthority) {
+        _setAuthority(initialAuthority);
+    }
+
+    /**
+     * @dev Restricts access to a function as defined by the connected Authority for this contract and the
+     * caller and selector of the function that entered the contract.
+     *
+     * [IMPORTANT]
+     * ====
+     * In general, this modifier should only be used on `external` functions. It is okay to use it on `public`
+     * functions that are used as external entry points and are not called internally. Unless you know what you're
+     * doing, it should never be used on `internal` functions. Failure to follow these rules can have critical security
+     * implications! This is because the permissions are determined by the function that entered the contract, i.e. the
+     * function at the bottom of the call stack, and not the function where the modifier is visible in the source code.
+     * ====
+     *
+     * [NOTE]
+     * ====
+     * Selector collisions are mitigated by scoping permissions per contract, but some edge cases must be considered:
+     *
+     * * If the https://docs.soliditylang.org/en/v0.8.20/contracts.html#receive-ether-function[`receive()`] function
+     * is restricted, any other function with a `0x00000000` selector will share permissions with `receive()`.
+     * * Similarly, if there's no `receive()` function but a `fallback()` instead, the fallback might be called with
+     * empty `calldata`, sharing the `0x00000000` selector permissions as well.
+     * * For any other selector, if the restricted function is set on an upgradeable contract, an upgrade may remove
+     * the restricted function and replace it with a new method whose selector replaces the last one, keeping the
+     * previous permissions.
+     * ====
+     */
+    modifier restricted() {
+        _checkCanCall(_msgSender(), _msgData());
+        _;
+    }
+
+    /**
+     * @dev Returns the current authority.
+     */
+    function authority() public view virtual returns (address) {
+        return _authority;
+    }
+
+    /**
+     * @dev Transfers control to a new authority. The caller must be the current authority.
+     */
+    function setAuthority(address newAuthority) public virtual {
+        address caller = _msgSender();
+        if (caller != authority()) {
+            revert AccessManagedUnauthorized(caller);
+        }
+        if (newAuthority.code.length == 0) {
+            revert AccessManagedInvalidAuthority(newAuthority);
+        }
+        _setAuthority(newAuthority);
+    }
+
+    /**
+     * @dev Returns true only in the context of a delayed restricted call, at the moment that the scheduled operation is
+     * being consumed. Prevents denial of service for delayed restricted calls in the case that the contract performs
+     * attacker controlled calls.
+     */
+    function isConsumingScheduledOp() public view returns (bytes4) {
+        return _consumingSchedule ? this.isConsumingScheduledOp.selector : bytes4(0);
+    }
+
+    /**
+     * @dev Transfers control to a new authority. Internal function with no access restriction. Allows bypassing the
+     * permissions set by the current authority.
+     */
+    function _setAuthority(address newAuthority) internal virtual {
+        _authority = newAuthority;
+        emit AuthorityUpdated(newAuthority);
+    }
+
+    /**
+     * @dev Reverts if the caller is not allowed to call the function identified by a selector.
+     */
+    function _checkCanCall(address caller, bytes calldata data) internal virtual {
+        (bool immediate, uint32 delay) = AuthorityUtils.canCallWithDelay(
+            authority(),
+            caller,
+            address(this),
+            bytes4(data)
+        );
+        if (!immediate) {
+            if (delay > 0) {
+                _consumingSchedule = true;
+                IAccessManager(authority()).consumeScheduledOp(caller, data);
+                _consumingSchedule = false;
+            } else {
+                revert AccessManagedUnauthorized(caller);
+            }
+        }
+    }
+}

+ 881 - 0
contracts/access/manager/AccessManager.sol

@@ -0,0 +1,881 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IAccessManager} from "./IAccessManager.sol";
+import {IAccessManaged} from "./IAccessManaged.sol";
+import {Address} from "../../utils/Address.sol";
+import {Context} from "../../utils/Context.sol";
+import {Multicall} from "../../utils/Multicall.sol";
+import {Math} from "../../utils/math/Math.sol";
+import {Time} from "../../utils/types/Time.sol";
+
+/**
+ * @dev AccessManager is a central contract to store the permissions of a system.
+ *
+ * The smart contracts under the control of an AccessManager instance will have a set of "restricted" functions, and the
+ * exact details of how access is restricted for each of those functions is configurable by the admins of the instance.
+ * These restrictions are expressed in terms of "roles".
+ *
+ * An AccessManager instance will define a set of roles. Accounts can be added into any number of these roles. Each of
+ * them defines a role, and may confer access to some of the restricted functions in the system, as configured by admins
+ * through the use of {setFunctionAllowedRoles}.
+ *
+ * Note that a function in a target contract may become permissioned in this way only when: 1) said contract is
+ * {AccessManaged} and is connected to this contract as its manager, and 2) said function is decorated with the
+ * `restricted` modifier.
+ *
+ * There is a special role defined by default named "public" which all accounts automatically have.
+ *
+ * In addition to the access rules defined by each target's functions being assigned to roles, then entire target can
+ * be "closed". This "closed" mode is set/unset by the admin using {setTargetClosed} and can be used to lock a contract
+ * while permissions are being (re-)configured.
+ *
+ * Since all the permissions of the managed system can be modified by the admins of this instance, it is expected that
+ * they will be highly secured (e.g., a multisig or a well-configured DAO).
+ *
+ * NOTE: This contract implements a form of the {IAuthority} interface, but {canCall} has additional return data so it
+ * doesn't inherit `IAuthority`. It is however compatible with the `IAuthority` interface since the first 32 bytes of
+ * the return data are a boolean as expected by that interface.
+ *
+ * NOTE: Systems that implement other access control mechanisms (for example using {Ownable}) can be paired with an
+ * {AccessManager} by transferring permissions (ownership in the case of {Ownable}) directly to the {AccessManager}.
+ * Users will be able to interact with these contracts through the {execute} function, following the access rules
+ * registered in the {AccessManager}. Keep in mind that in that context, the msg.sender seen by restricted functions
+ * will be {AccessManager} itself.
+ *
+ * WARNING: When granting permissions over an {Ownable} or {AccessControl} contract to an {AccessManager}, be very
+ * mindful of the danger associated with functions such as {{Ownable-renounceOwnership}} or
+ * {{AccessControl-renounceRole}}.
+ */
+contract AccessManager is Context, Multicall, IAccessManager {
+    using Time for *;
+
+    // Structure that stores the details for a target contract.
+    struct TargetConfig {
+        mapping(bytes4 selector => uint64 roleId) allowedRoles;
+        Time.Delay adminDelay;
+        bool closed;
+    }
+
+    // Structure that stores the details for a role/account pair. This structures fit into a single slot.
+    struct Access {
+        // Timepoint at which the user gets the permission. If this is either 0, or in the future, the role
+        // permission is not available.
+        uint48 since;
+        // Delay for execution. Only applies to restricted() / execute() calls.
+        Time.Delay delay;
+    }
+
+    // Structure that stores the details of a role, including:
+    // - the members of the role
+    // - the admin role (that can grant or revoke permissions)
+    // - the guardian role (that can cancel operations targeting functions that need this role)
+    // - the grand delay
+    struct Role {
+        mapping(address user => Access access) members;
+        uint64 admin;
+        uint64 guardian;
+        Time.Delay grantDelay;
+    }
+
+    // Structure that stores the details for a scheduled operation. This structure fits into a single slot.
+    struct Schedule {
+        uint48 timepoint;
+        uint32 nonce;
+    }
+
+    uint64 public constant ADMIN_ROLE = type(uint64).min; // 0
+    uint64 public constant PUBLIC_ROLE = type(uint64).max; // 2**64-1
+
+    mapping(address target => TargetConfig mode) private _targets;
+    mapping(uint64 roleId => Role) private _roles;
+    mapping(bytes32 operationId => Schedule) private _schedules;
+
+    // This should be transient storage when supported by the EVM.
+    bytes32 private _executionId;
+
+    /**
+     * @dev Check that the caller is authorized to perform the operation, following the restrictions encoded in
+     * {_getAdminRestrictions}.
+     */
+    modifier onlyAuthorized() {
+        _checkAuthorized();
+        _;
+    }
+
+    constructor(address initialAdmin) {
+        if (initialAdmin == address(0)) {
+            revert AccessManagerInvalidInitialAdmin(address(0));
+        }
+
+        // admin is active immediately and without any execution delay.
+        _grantRole(ADMIN_ROLE, initialAdmin, 0, 0);
+    }
+
+    // =================================================== GETTERS ====================================================
+    /**
+     * @dev Check if an address (`caller`) is authorised to call a given function on a given contract directly (with
+     * no restriction). Additionally, it returns the delay needed to perform the call indirectly through the {schedule}
+     * & {execute} workflow.
+     *
+     * This function is usually called by the targeted contract to control immediate execution of restricted functions.
+     * Therefore we only return true is the call can be performed without any delay. If the call is subject to a delay,
+     * then the function should return false, and the caller should schedule the operation for future execution.
+     *
+     * We may be able to hash the operation, and check if the call was scheduled, but we would not be able to cleanup
+     * the schedule, leaving the possibility of multiple executions. Maybe this function should not be view?
+     *
+     * NOTE: The IAuthority interface does not include the `uint32` delay. This is an extension of that interface that
+     * is backward compatible. Some contracts may thus ignore the second return argument. In that case they will fail
+     * to identify the indirect workflow, and will consider calls that require a delay to be forbidden.
+     */
+    function canCall(address caller, address target, bytes4 selector) public view virtual returns (bool, uint32) {
+        if (isTargetClosed(target)) {
+            return (false, 0);
+        } else if (caller == address(this)) {
+            // Caller is AccessManager, this means the call was sent through {execute} and it already checked
+            // permissions. We verify that the call "identifier", which is set during {execute}, is correct.
+            return (_isExecuting(target, selector), 0);
+        } else {
+            uint64 roleId = getTargetFunctionRole(target, selector);
+            (bool isMember, uint32 currentDelay) = hasRole(roleId, caller);
+            return isMember ? (currentDelay == 0, currentDelay) : (false, 0);
+        }
+    }
+
+    /**
+     * @dev Expiration delay for scheduled proposals. Defaults to 1 week.
+     */
+    function expiration() public view virtual returns (uint32) {
+        return 1 weeks;
+    }
+
+    /**
+     * @dev Minimum setback for all delay updates, with the exception of execution delays, which
+     * can be increased without setback (and in the event of an accidental increase can be reset
+     * via {revokeRole}). Defaults to 5 days.
+     */
+    function minSetback() public view virtual returns (uint32) {
+        return 5 days;
+    }
+
+    /**
+     * @dev Get the mode under which a contract is operating.
+     */
+    function isTargetClosed(address target) public view virtual returns (bool) {
+        return _targets[target].closed;
+    }
+
+    /**
+     * @dev Get the role required to call a function.
+     */
+    function getTargetFunctionRole(address target, bytes4 selector) public view virtual returns (uint64) {
+        return _targets[target].allowedRoles[selector];
+    }
+
+    /**
+     * @dev Get the admin delay for a target contract. Changes to contract configuration are subject to this delay.
+     */
+    function getTargetAdminDelay(address target) public view virtual returns (uint32) {
+        return _targets[target].adminDelay.get();
+    }
+
+    /**
+     * @dev Get the id of the role that acts as an admin for given role.
+     *
+     * The admin permission is required to grant the role, revoke the role and update the execution delay to execute
+     * an operation that is restricted to this role.
+     */
+    function getRoleAdmin(uint64 roleId) public view virtual returns (uint64) {
+        return _roles[roleId].admin;
+    }
+
+    /**
+     * @dev Get the role that acts as a guardian for a given role.
+     *
+     * The guardian permission allows canceling operations that have been scheduled under the role.
+     */
+    function getRoleGuardian(uint64 roleId) public view virtual returns (uint64) {
+        return _roles[roleId].guardian;
+    }
+
+    /**
+     * @dev Get the role current grant delay, that value may change at any point, without an event emitted, following
+     * a call to {setGrantDelay}. Changes to this value, including effect timepoint are notified by the
+     * {RoleGrantDelayChanged} event.
+     */
+    function getRoleGrantDelay(uint64 roleId) public view virtual returns (uint32) {
+        return _roles[roleId].grantDelay.get();
+    }
+
+    /**
+     * @dev Get the access details for a given account for a given role. These details include the timepoint at which
+     * membership becomes active, and the delay applied to all operation by this user that requires this permission
+     * level.
+     *
+     * Returns:
+     * [0] Timestamp at which the account membership becomes valid. 0 means role is not granted.
+     * [1] Current execution delay for the account.
+     * [2] Pending execution delay for the account.
+     * [3] Timestamp at which the pending execution delay will become active. 0 means no delay update is scheduled.
+     */
+    function getAccess(uint64 roleId, address account) public view virtual returns (uint48, uint32, uint32, uint48) {
+        Access storage access = _roles[roleId].members[account];
+
+        uint48 since = access.since;
+        (uint32 currentDelay, uint32 pendingDelay, uint48 effect) = access.delay.getFull();
+
+        return (since, currentDelay, pendingDelay, effect);
+    }
+
+    /**
+     * @dev Check if a given account currently had the permission level corresponding to a given role. Note that this
+     * permission might be associated with a delay. {getAccess} can provide more details.
+     */
+    function hasRole(uint64 roleId, address account) public view virtual returns (bool, uint32) {
+        if (roleId == PUBLIC_ROLE) {
+            return (true, 0);
+        } else {
+            (uint48 hasRoleSince, uint32 currentDelay, , ) = getAccess(roleId, account);
+            return (hasRoleSince != 0 && hasRoleSince <= Time.timestamp(), currentDelay);
+        }
+    }
+
+    // =============================================== ROLE MANAGEMENT ===============================================
+    /**
+     * @dev Give a label to a role, for improved role discoverabily by UIs.
+     *
+     * Emits a {RoleLabel} event.
+     */
+    function labelRole(uint64 roleId, string calldata label) public virtual onlyAuthorized {
+        if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+        emit RoleLabel(roleId, label);
+    }
+
+    /**
+     * @dev Add `account` to `roleId`, or change its execution delay.
+     *
+     * This gives the account the authorization to call any function that is restricted to this role. An optional
+     * execution delay (in seconds) can be set. If that delay is non 0, the user is required to schedule any operation
+     * that is restricted to members this role. The user will only be able to execute the operation after the delay has
+     * passed, before it has expired. During this period, admin and guardians can cancel the operation (see {cancel}).
+     *
+     * If the account has already been granted this role, the execution delay will be updated. This update is not
+     * immediate and follows the delay rules. For example, If a user currently has a delay of 3 hours, and this is
+     * called to reduce that delay to 1 hour, the new delay will take some time to take effect, enforcing that any
+     * operation executed in the 3 hours that follows this update was indeed scheduled before this update.
+     *
+     * Requirements:
+     *
+     * - the caller must be an admin for the role (see {getRoleAdmin})
+     *
+     * Emits a {RoleGranted} event
+     */
+    function grantRole(uint64 roleId, address account, uint32 executionDelay) public virtual onlyAuthorized {
+        _grantRole(roleId, account, getRoleGrantDelay(roleId), executionDelay);
+    }
+
+    /**
+     * @dev Remove an account from a role, with immediate effect. If the account does not have the role, this call has
+     * no effect.
+     *
+     * Requirements:
+     *
+     * - the caller must be an admin for the role (see {getRoleAdmin})
+     *
+     * Emits a {RoleRevoked} event if the account had the role.
+     */
+    function revokeRole(uint64 roleId, address account) public virtual onlyAuthorized {
+        _revokeRole(roleId, account);
+    }
+
+    /**
+     * @dev Renounce role permissions for the calling account, with immediate effect. If the sender is not in
+     * the role, this call has no effect.
+     *
+     * Requirements:
+     *
+     * - the caller must be `callerConfirmation`.
+     *
+     * Emits a {RoleRevoked} event if the account had the role.
+     */
+    function renounceRole(uint64 roleId, address callerConfirmation) public virtual {
+        if (callerConfirmation != _msgSender()) {
+            revert AccessManagerBadConfirmation();
+        }
+        _revokeRole(roleId, callerConfirmation);
+    }
+
+    /**
+     * @dev Change admin role for a given role.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {RoleAdminChanged} event
+     */
+    function setRoleAdmin(uint64 roleId, uint64 admin) public virtual onlyAuthorized {
+        _setRoleAdmin(roleId, admin);
+    }
+
+    /**
+     * @dev Change guardian role for a given role.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {RoleGuardianChanged} event
+     */
+    function setRoleGuardian(uint64 roleId, uint64 guardian) public virtual onlyAuthorized {
+        _setRoleGuardian(roleId, guardian);
+    }
+
+    /**
+     * @dev Update the delay for granting a `roleId`.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {RoleGrantDelayChanged} event.
+     */
+    function setGrantDelay(uint64 roleId, uint32 newDelay) public virtual onlyAuthorized {
+        _setGrantDelay(roleId, newDelay);
+    }
+
+    /**
+     * @dev Internal version of {grantRole} without access control. Returns true if the role was newly granted.
+     *
+     * Emits a {RoleGranted} event.
+     */
+    function _grantRole(
+        uint64 roleId,
+        address account,
+        uint32 grantDelay,
+        uint32 executionDelay
+    ) internal virtual returns (bool) {
+        if (roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+
+        bool newMember = _roles[roleId].members[account].since == 0;
+        uint48 since;
+
+        if (newMember) {
+            since = Time.timestamp() + grantDelay;
+            _roles[roleId].members[account] = Access({since: since, delay: executionDelay.toDelay()});
+        } else {
+            // No setback here. Value can be reset by doing revoke + grant, effectively allowing the admin to perform
+            // any change to the execution delay within the duration of the role admin delay.
+            (_roles[roleId].members[account].delay, since) = _roles[roleId].members[account].delay.withUpdate(
+                executionDelay,
+                0
+            );
+        }
+
+        emit RoleGranted(roleId, account, executionDelay, since, newMember);
+        return newMember;
+    }
+
+    /**
+     * @dev Internal version of {revokeRole} without access control. This logic is also used by {renounceRole}.
+     * Returns true if the role was previously granted.
+     *
+     * Emits a {RoleRevoked} event if the account had the role.
+     */
+    function _revokeRole(uint64 roleId, address account) internal virtual returns (bool) {
+        if (roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+
+        if (_roles[roleId].members[account].since == 0) {
+            return false;
+        }
+
+        delete _roles[roleId].members[account];
+
+        emit RoleRevoked(roleId, account);
+        return true;
+    }
+
+    /**
+     * @dev Internal version of {setRoleAdmin} without access control.
+     *
+     * Emits a {RoleAdminChanged} event
+     */
+    function _setRoleAdmin(uint64 roleId, uint64 admin) internal virtual {
+        if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+
+        _roles[roleId].admin = admin;
+
+        emit RoleAdminChanged(roleId, admin);
+    }
+
+    /**
+     * @dev Internal version of {setRoleGuardian} without access control.
+     *
+     * Emits a {RoleGuardianChanged} event
+     */
+    function _setRoleGuardian(uint64 roleId, uint64 guardian) internal virtual {
+        if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+
+        _roles[roleId].guardian = guardian;
+
+        emit RoleGuardianChanged(roleId, guardian);
+    }
+
+    /**
+     * @dev Internal version of {setGrantDelay} without access control.
+     *
+     * Emits a {RoleGrantDelayChanged} event
+     */
+    function _setGrantDelay(uint64 roleId, uint32 newDelay) internal virtual {
+        if (roleId == PUBLIC_ROLE) {
+            revert AccessManagerLockedRole(roleId);
+        }
+
+        uint48 effect;
+        (_roles[roleId].grantDelay, effect) = _roles[roleId].grantDelay.withUpdate(newDelay, minSetback());
+
+        emit RoleGrantDelayChanged(roleId, newDelay, effect);
+    }
+
+    // ============================================= FUNCTION MANAGEMENT ==============================================
+    /**
+     * @dev Set the role required to call functions identified by the `selectors` in the `target` contract.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {TargetFunctionRoleUpdated} event per selector.
+     */
+    function setTargetFunctionRole(
+        address target,
+        bytes4[] calldata selectors,
+        uint64 roleId
+    ) public virtual onlyAuthorized {
+        for (uint256 i = 0; i < selectors.length; ++i) {
+            _setTargetFunctionRole(target, selectors[i], roleId);
+        }
+    }
+
+    /**
+     * @dev Internal version of {setFunctionAllowedRole} without access control.
+     *
+     * Emits a {TargetFunctionRoleUpdated} event
+     */
+    function _setTargetFunctionRole(address target, bytes4 selector, uint64 roleId) internal virtual {
+        _targets[target].allowedRoles[selector] = roleId;
+        emit TargetFunctionRoleUpdated(target, selector, roleId);
+    }
+
+    /**
+     * @dev Set the delay for changing the configuration of a given target contract.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {TargetAdminDelayUpdated} event per selector
+     */
+    function setTargetAdminDelay(address target, uint32 newDelay) public virtual onlyAuthorized {
+        _setTargetAdminDelay(target, newDelay);
+    }
+
+    /**
+     * @dev Internal version of {setTargetAdminDelay} without access control.
+     *
+     * Emits a {TargetAdminDelayUpdated} event
+     */
+    function _setTargetAdminDelay(address target, uint32 newDelay) internal virtual {
+        uint48 effect;
+        (_targets[target].adminDelay, effect) = _targets[target].adminDelay.withUpdate(newDelay, minSetback());
+
+        emit TargetAdminDelayUpdated(target, newDelay, effect);
+    }
+
+    // =============================================== MODE MANAGEMENT ================================================
+    /**
+     * @dev Set the closed flag for a contract.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {TargetClosed} event.
+     */
+    function setTargetClosed(address target, bool closed) public virtual onlyAuthorized {
+        _setTargetClosed(target, closed);
+    }
+
+    /**
+     * @dev Set the closed flag for a contract. This is an internal setter with no access restrictions.
+     *
+     * Emits a {TargetClosed} event.
+     */
+    function _setTargetClosed(address target, bool closed) internal virtual {
+        if (target == address(this)) {
+            revert AccessManagerLockedAccount(target);
+        }
+        _targets[target].closed = closed;
+        emit TargetClosed(target, closed);
+    }
+
+    // ============================================== DELAYED OPERATIONS ==============================================
+    /**
+     * @dev Return the timepoint at which a scheduled operation will be ready for execution. This returns 0 if the
+     * 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].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
+     * 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 occurrences of the same `operationId` in invocations of {execute} and {cancel}.
+     *
+     * Emits a {OperationScheduled} event.
+     *
+     * NOTE: It is not possible to concurrently schedule more than one operation with the same `target` and `data`. If
+     * this is necessary, a random byte can be appended to `data` to act as a salt that will be ignored by the target
+     * contract if it is using standard Solidity ABI encoding.
+     */
+    function schedule(
+        address target,
+        bytes calldata data,
+        uint48 when
+    ) public virtual returns (bytes32 operationId, uint32 nonce) {
+        address caller = _msgSender();
+
+        // Fetch restrictions that apply to the caller on the targeted function
+        (bool immediate, uint32 setback) = _canCallExtended(caller, target, data);
+
+        uint48 minWhen = Time.timestamp() + setback;
+
+        // if call is not authorized, or if requested timing is too soon
+        if ((!immediate && setback == 0) || (when > 0 && when < minWhen)) {
+            revert AccessManagerUnauthorizedCall(caller, target, bytes4(data[0:4]));
+        }
+
+        // Reuse variable due to stack too deep
+        when = uint48(Math.max(when, minWhen)); // cast is safe: both inputs are uint48
+
+        // If caller is authorised, schedule operation
+        operationId = hashOperation(caller, target, data);
+
+        _checkNotScheduled(operationId);
+
+        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);
+
+        // Using named return values because otherwise we get stack too deep
+    }
+
+    /**
+     * @dev Reverts if the operation is currently scheduled and has not expired.
+     * (Note: This function was introduced due to stack too deep errors in schedule.)
+     */
+    function _checkNotScheduled(bytes32 operationId) private view {
+        uint48 prevTimepoint = _schedules[operationId].timepoint;
+        if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) {
+            revert AccessManagerAlreadyScheduled(operationId);
+        }
+    }
+
+    /**
+     * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the
+     * execution delay is 0.
+     *
+     * Returns the nonce that identifies the previously scheduled operation that is executed, 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,
+    // _consumeScheduledOp guarantees a scheduled operation is only executed once.
+    // slither-disable-next-line reentrancy-no-eth
+    function execute(address target, bytes calldata data) public payable virtual returns (uint32) {
+        address caller = _msgSender();
+
+        // Fetch restrictions that apply to the caller on the targeted function
+        (bool immediate, uint32 setback) = _canCallExtended(caller, target, data);
+
+        // If caller is not authorised, revert
+        if (!immediate && setback == 0) {
+            revert AccessManagerUnauthorizedCall(caller, target, bytes4(data));
+        }
+
+        // If caller is authorised, check operation was scheduled early enough
+        bytes32 operationId = hashOperation(caller, target, data);
+        uint32 nonce;
+
+        if (setback != 0) {
+            nonce = _consumeScheduledOp(operationId);
+        }
+
+        // Mark the target and selector as authorised
+        bytes32 executionIdBefore = _executionId;
+        _executionId = _hashExecutionId(target, bytes4(data));
+
+        // Perform call
+        Address.functionCallWithValue(target, data, msg.value);
+
+        // Reset execute identifier
+        _executionId = executionIdBefore;
+
+        return nonce;
+    }
+
+    /**
+     * @dev Consume a scheduled operation targeting the caller. If such an operation exists, mark it as consumed
+     * (emit an {OperationExecuted} event and clean the state). Otherwise, throw an error.
+     *
+     * This is useful for contract that want to enforce that calls targeting them were scheduled on the manager,
+     * with all the verifications that it implies.
+     *
+     * Emit a {OperationExecuted} event
+     */
+    function consumeScheduledOp(address caller, bytes calldata data) public virtual {
+        address target = _msgSender();
+        if (IAccessManaged(target).isConsumingScheduledOp() != IAccessManaged.isConsumingScheduledOp.selector) {
+            revert AccessManagerUnauthorizedConsume(target);
+        }
+        _consumeScheduledOp(hashOperation(caller, target, data));
+    }
+
+    /**
+     * @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;
+        uint32 nonce = _schedules[operationId].nonce;
+
+        if (timepoint == 0) {
+            revert AccessManagerNotScheduled(operationId);
+        } else if (timepoint > Time.timestamp()) {
+            revert AccessManagerNotReady(operationId);
+        } else if (_isExpired(timepoint)) {
+            revert AccessManagerExpired(operationId);
+        }
+
+        delete _schedules[operationId].timepoint; // reset the timepoint, keep the nonce
+        emit OperationExecuted(operationId, nonce);
+
+        return nonce;
+    }
+
+    /**
+     * @dev Cancel a scheduled (delayed) operation. Returns the nonce that identifies the previously scheduled
+     * operation that is cancelled.
+     *
+     * Requirements:
+     *
+     * - the caller must be the proposer, a guardian of the targeted function, or a global admin
+     *
+     * Emits a {OperationCanceled} event.
+     */
+    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].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 role.
+            (bool isAdmin, ) = hasRole(ADMIN_ROLE, msgsender);
+            (bool isGuardian, ) = hasRole(getRoleGuardian(getTargetFunctionRole(target, selector)), msgsender);
+            if (!isAdmin && !isGuardian) {
+                revert AccessManagerUnauthorizedCancel(msgsender, caller, target, selector);
+            }
+        }
+
+        delete _schedules[operationId].timepoint; // reset the timepoint, keep the nonce
+        uint32 nonce = _schedules[operationId].nonce;
+        emit OperationCanceled(operationId, nonce);
+
+        return nonce;
+    }
+
+    /**
+     * @dev Hashing function for delayed operations
+     */
+    function hashOperation(address caller, address target, bytes calldata data) public view virtual returns (bytes32) {
+        return keccak256(abi.encode(caller, target, data));
+    }
+
+    /**
+     * @dev Hashing function for execute protection
+     */
+    function _hashExecutionId(address target, bytes4 selector) private pure returns (bytes32) {
+        return keccak256(abi.encode(target, selector));
+    }
+
+    // ==================================================== OTHERS ====================================================
+    /**
+     * @dev Change the AccessManager instance used by a contract that correctly uses this instance.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     */
+    function updateAuthority(address target, address newAuthority) public virtual onlyAuthorized {
+        IAccessManaged(target).setAuthority(newAuthority);
+    }
+
+    // ================================================= ADMIN LOGIC ==================================================
+    /**
+     * @dev Check if the current call is authorized according to admin logic.
+     */
+    function _checkAuthorized() private {
+        address caller = _msgSender();
+        (bool immediate, uint32 delay) = _canCallSelf(caller, _msgData());
+        if (!immediate) {
+            if (delay == 0) {
+                (, uint64 requiredRole, ) = _getAdminRestrictions(_msgData());
+                revert AccessManagerUnauthorizedAccount(caller, requiredRole);
+            } else {
+                _consumeScheduledOp(hashOperation(caller, address(this), _msgData()));
+            }
+        }
+    }
+
+    /**
+     * @dev Get the admin restrictions of a given function call based on the function and arguments involved.
+     *
+     * Returns:
+     * - bool restricted: does this data match a restricted operation
+     * - uint64: which role is this operation restricted to
+     * - uint32: minimum delay to enforce for that operation (on top of the admin's execution delay)
+     */
+    function _getAdminRestrictions(bytes calldata data) private view returns (bool, uint64, uint32) {
+        bytes4 selector = bytes4(data);
+
+        if (data.length < 4) {
+            return (false, 0, 0);
+        }
+
+        // Restricted to ADMIN with no delay beside any execution delay the caller may have
+        if (
+            selector == this.labelRole.selector ||
+            selector == this.setRoleAdmin.selector ||
+            selector == this.setRoleGuardian.selector ||
+            selector == this.setGrantDelay.selector ||
+            selector == this.setTargetAdminDelay.selector
+        ) {
+            return (true, ADMIN_ROLE, 0);
+        }
+
+        // Restricted to ADMIN with the admin delay corresponding to the target
+        if (
+            selector == this.updateAuthority.selector ||
+            selector == this.setTargetClosed.selector ||
+            selector == this.setTargetFunctionRole.selector
+        ) {
+            // First argument is a target.
+            address target = abi.decode(data[0x04:0x24], (address));
+            uint32 delay = getTargetAdminDelay(target);
+            return (true, ADMIN_ROLE, delay);
+        }
+
+        // Restricted to that role's admin with no delay beside any execution delay the caller may have.
+        if (selector == this.grantRole.selector || selector == this.revokeRole.selector) {
+            // First argument is a roleId.
+            uint64 roleId = abi.decode(data[0x04:0x24], (uint64));
+            uint64 roleAdminId = getRoleAdmin(roleId);
+            return (true, roleAdminId, 0);
+        }
+
+        return (false, 0, 0);
+    }
+
+    // =================================================== HELPERS ====================================================
+    /**
+     * @dev An extended version of {canCall} for internal use that considers restrictions for admin functions.
+     *
+     * Returns:
+     * - bool immediate: whether the operation can be executed immediately (with no delay)
+     * - uint32 delay: the execution delay
+     *
+     * If immediate is true, the delay can be disregarded and the operation can be immediately executed.
+     * If immediate is false, the operation can be executed if and only if delay is greater than 0.
+     */
+    function _canCallExtended(address caller, address target, bytes calldata data) private view returns (bool, uint32) {
+        if (target == address(this)) {
+            return _canCallSelf(caller, data);
+        } else {
+            bytes4 selector = bytes4(data);
+            return canCall(caller, target, selector);
+        }
+    }
+
+    /**
+     * @dev A version of {canCall} that checks for admin restrictions in this contract.
+     */
+    function _canCallSelf(address caller, bytes calldata data) private view returns (bool immediate, uint32 delay) {
+        if (caller == address(this)) {
+            // Caller is AccessManager, this means the call was sent through {execute} and it already checked
+            // permissions. We verify that the call "identifier", which is set during {execute}, is correct.
+            return (_isExecuting(address(this), bytes4(data)), 0);
+        }
+
+        (bool enabled, uint64 roleId, uint32 operationDelay) = _getAdminRestrictions(data);
+        if (!enabled) {
+            return (false, 0);
+        }
+
+        (bool inRole, uint32 executionDelay) = hasRole(roleId, caller);
+        if (!inRole) {
+            return (false, 0);
+        }
+
+        // downcast is safe because both options are uint32
+        delay = uint32(Math.max(operationDelay, executionDelay));
+        return (delay == 0, delay);
+    }
+
+    /**
+     * @dev Returns true if a call with `target` and `selector` is being executed via {executed}.
+     */
+    function _isExecuting(address target, bytes4 selector) private view returns (bool) {
+        return _executionId == _hashExecutionId(target, selector);
+    }
+
+    /**
+     * @dev Returns true if a schedule timepoint is past its expiration deadline.
+     */
+    function _isExpired(uint48 timepoint) private view returns (bool) {
+        return timepoint + expiration() <= Time.timestamp();
+    }
+}

+ 31 - 0
contracts/access/manager/AuthorityUtils.sol

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IAuthority} from "./IAuthority.sol";
+
+library AuthorityUtils {
+    /**
+     * @dev Since `AccessManager` implements an extended IAuthority interface, invoking `canCall` with backwards compatibility
+     * for the preexisting `IAuthority` interface requires special care to avoid reverting on insufficient return data.
+     * This helper function takes care of invoking `canCall` in a backwards compatible way without reverting.
+     */
+    function canCallWithDelay(
+        address authority,
+        address caller,
+        address target,
+        bytes4 selector
+    ) internal view returns (bool immediate, uint32 delay) {
+        (bool success, bytes memory data) = authority.staticcall(
+            abi.encodeCall(IAuthority.canCall, (caller, target, selector))
+        );
+        if (success) {
+            if (data.length >= 0x40) {
+                (immediate, delay) = abi.decode(data, (bool, uint32));
+            } else if (data.length >= 0x20) {
+                immediate = abi.decode(data, (bool));
+            }
+        }
+        return (immediate, delay);
+    }
+}

+ 17 - 0
contracts/access/manager/IAccessManaged.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+interface IAccessManaged {
+    event AuthorityUpdated(address authority);
+
+    error AccessManagedUnauthorized(address caller);
+    error AccessManagedRequiredDelay(address caller, uint32 delay);
+    error AccessManagedInvalidAuthority(address authority);
+
+    function authority() external view returns (address);
+
+    function setAuthority(address) external;
+
+    function isConsumingScheduledOp() external view returns (bytes4);
+}

+ 113 - 0
contracts/access/manager/IAccessManager.sol

@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {IAccessManaged} from "./IAccessManaged.sol";
+import {Time} from "../../utils/types/Time.sol";
+
+interface IAccessManager {
+    /**
+     * @dev A delayed operation was scheduled.
+     */
+    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, uint32 indexed nonce);
+
+    /**
+     * @dev A scheduled operation was canceled.
+     */
+    event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce);
+
+    event RoleLabel(uint64 indexed roleId, string label);
+    event RoleGranted(uint64 indexed roleId, address indexed account, uint32 delay, uint48 since, bool newMember);
+    event RoleRevoked(uint64 indexed roleId, address indexed account);
+    event RoleAdminChanged(uint64 indexed roleId, uint64 indexed admin);
+    event RoleGuardianChanged(uint64 indexed roleId, uint64 indexed guardian);
+    event RoleGrantDelayChanged(uint64 indexed roleId, uint32 delay, uint48 since);
+    event TargetClosed(address indexed target, bool closed);
+    event TargetFunctionRoleUpdated(address indexed target, bytes4 selector, uint64 indexed roleId);
+    event TargetAdminDelayUpdated(address indexed target, uint32 delay, uint48 since);
+
+    error AccessManagerAlreadyScheduled(bytes32 operationId);
+    error AccessManagerNotScheduled(bytes32 operationId);
+    error AccessManagerNotReady(bytes32 operationId);
+    error AccessManagerExpired(bytes32 operationId);
+    error AccessManagerLockedAccount(address account);
+    error AccessManagerLockedRole(uint64 roleId);
+    error AccessManagerBadConfirmation();
+    error AccessManagerUnauthorizedAccount(address msgsender, uint64 roleId);
+    error AccessManagerUnauthorizedCall(address caller, address target, bytes4 selector);
+    error AccessManagerUnauthorizedConsume(address target);
+    error AccessManagerUnauthorizedCancel(address msgsender, address caller, address target, bytes4 selector);
+    error AccessManagerInvalidInitialAdmin(address initialAdmin);
+
+    function canCall(
+        address caller,
+        address target,
+        bytes4 selector
+    ) external view returns (bool allowed, uint32 delay);
+
+    function hashOperation(address caller, address target, bytes calldata data) external view returns (bytes32);
+
+    function expiration() external view returns (uint32);
+
+    function isTargetClosed(address target) external view returns (bool);
+
+    function getTargetFunctionRole(address target, bytes4 selector) external view returns (uint64);
+
+    function getTargetAdminDelay(address target) external view returns (uint32);
+
+    function getRoleAdmin(uint64 roleId) external view returns (uint64);
+
+    function getRoleGuardian(uint64 roleId) external view returns (uint64);
+
+    function getRoleGrantDelay(uint64 roleId) external view returns (uint32);
+
+    function getAccess(uint64 roleId, address account) external view returns (uint48, uint32, uint32, uint48);
+
+    function hasRole(uint64 roleId, address account) external view returns (bool, uint32);
+
+    function labelRole(uint64 roleId, string calldata label) external;
+
+    function grantRole(uint64 roleId, address account, uint32 executionDelay) external;
+
+    function revokeRole(uint64 roleId, address account) external;
+
+    function renounceRole(uint64 roleId, address callerConfirmation) external;
+
+    function setRoleAdmin(uint64 roleId, uint64 admin) external;
+
+    function setRoleGuardian(uint64 roleId, uint64 guardian) external;
+
+    function setGrantDelay(uint64 roleId, uint32 newDelay) external;
+
+    function setTargetFunctionRole(address target, bytes4[] calldata selectors, uint64 roleId) external;
+
+    function setTargetAdminDelay(address target, uint32 newDelay) external;
+
+    function setTargetClosed(address target, bool closed) external;
+
+    function getSchedule(bytes32 id) external view returns (uint48);
+
+    function getNonce(bytes32 id) external view returns (uint32);
+
+    function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32, uint32);
+
+    function execute(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;
+
+    function updateAuthority(address target, address newAuthority) external;
+}

+ 13 - 0
contracts/access/manager/IAuthority.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+/**
+ * @dev Standard interface for permissioning originally defined in Dappsys.
+ */
+interface IAuthority {
+    /**
+     * @dev Returns true if the caller can invoke on a target the function identified by a function selector.
+     */
+    function canCall(address caller, address target, bytes4 selector) external view returns (bool allowed);
+}

+ 2 - 2
contracts/finance/VestingWallet.sol

@@ -24,8 +24,8 @@ import {Ownable} from "../access/Ownable.sol";
  * counterfactually deployed contract, 2) there is likely to be a migration path for EOAs to become contracts in the
  * near future.
  *
- * NOTE: When using this contract with any token whose balance is adjusted automatically (i.e. a rebase token), make sure
- * to account the supply/balance adjustment in the vesting schedule to ensure the vested amount is as intended.
+ * NOTE: When using this contract with any token whose balance is adjusted automatically (i.e. a rebase token), make
+ * sure to account the supply/balance adjustment in the vesting schedule to ensure the vested amount is as intended.
  */
 contract VestingWallet is Context, Ownable {
     event EtherReleased(uint256 amount);

+ 41 - 37
contracts/governance/Governor.sol

@@ -16,7 +16,7 @@ import {Nonces} from "../utils/Nonces.sol";
 import {IGovernor, IERC6372} from "./IGovernor.sol";
 
 /**
- * @dev Core of the governance system, designed to be extended though various modules.
+ * @dev Core of the governance system, designed to be extended through various modules.
  *
  * This contract is abstract and requires several functions to be implemented in various modules:
  *
@@ -40,7 +40,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint32 voteDuration;
         bool executed;
         bool canceled;
-        uint48 eta;
+        uint48 etaSeconds;
     }
 
     bytes32 private constant ALL_PROPOSAL_STATES_BITMAP = bytes32((2 ** (uint8(type(ProposalState).max) + 1)) - 1);
@@ -48,9 +48,9 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
 
     mapping(uint256 proposalId => ProposalCore) private _proposals;
 
-    // This queue keeps track of the governor operating on itself. Calls to functions protected by the
-    // {onlyGovernance} modifier needs to be whitelisted in this queue. Whitelisting is set in {_beforeExecute},
-    // consumed by the {onlyGovernance} modifier and eventually reset in {_afterExecute}. This ensures that the
+    // This queue keeps track of the governor operating on itself. Calls to functions protected by the {onlyGovernance}
+    // modifier needs to be whitelisted in this queue. Whitelisting is set in {execute}, consumed by the
+    // {onlyGovernance} modifier and eventually reset after {_executeOperations} completes. This ensures that the
     // execution of {onlyGovernance} protected calls can only be achieved through successful proposals.
     DoubleEndedQueue.Bytes32Deque private _governanceCall;
 
@@ -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,15 +127,15 @@ 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) {
-        // ProposalCore is just one slot. We can load it from storage to stack with a single sload
+    function state(uint256 proposalId) public view virtual returns (ProposalState) {
+        // We read the struct fields into the stack at once so Solidity emits a single SLOAD
         ProposalCore storage proposal = _proposals[proposalId];
         bool proposalExecuted = proposal.executed;
         bool proposalCanceled = proposal.canceled;
@@ -176,36 +176,43 @@ 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) {
-        return _proposals[proposalId].eta;
+    function proposalEta(uint256 proposalId) public view virtual returns (uint256) {
+        return _proposals[proposalId].etaSeconds;
+    }
+
+    /**
+     * @dev See {IGovernor-proposalNeedsQueuing}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual returns (bool) {
+        return false;
     }
 
     /**
@@ -270,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
@@ -340,16 +347,16 @@ 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));
 
-        uint48 eta = _queueOperations(proposalId, targets, values, calldatas, descriptionHash);
+        uint48 etaSeconds = _queueOperations(proposalId, targets, values, calldatas, descriptionHash);
 
-        if (eta != 0) {
-            _proposals[proposalId].eta = eta;
-            emit ProposalQueued(proposalId, eta);
+        if (etaSeconds != 0) {
+            _proposals[proposalId].etaSeconds = etaSeconds;
+            emit ProposalQueued(proposalId, etaSeconds);
         } else {
             revert GovernorQueueNotImplemented();
         }
@@ -363,12 +370,12 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
      *
      * This is empty by default, and must be overridden to implement queuing.
      *
-     * This function returns a timestamp that describes the expected eta for execution. If the returned value is 0
+     * This function returns a timestamp that describes the expected ETA for execution. If the returned value is 0
      * (which is the default value), the core will consider queueing did not succeed, and the public {queue} function
      * will revert.
      *
      * NOTE: Calling this function directly will NOT check the current state of the proposal, or emit the
-     * `ProposalQueued` event. Queuing a proposal should be done using {queue} or {_queue}.
+     * `ProposalQueued` event. Queuing a proposal should be done using {queue}.
      */
     function _queueOperations(
         uint256 /*proposalId*/,
@@ -388,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(
@@ -448,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.
@@ -494,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());
     }
 
@@ -505,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, "");
     }
@@ -524,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);
     }
@@ -537,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);
     }
@@ -550,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)))),
@@ -574,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(
@@ -628,10 +635,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         string memory reason,
         bytes memory params
     ) internal virtual returns (uint256) {
-        ProposalState currentState = state(proposalId);
-        if (currentState != ProposalState.Active) {
-            revert GovernorUnexpectedProposalState(proposalId, currentState, _encodeStateBitmap(ProposalState.Active));
-        }
+        _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Active));
 
         uint256 weight = _getVotes(account, proposalSnapshot(proposalId), params);
         _countVote(proposalId, account, support, weight, params);

+ 7 - 1
contracts/governance/IGovernor.sol

@@ -122,7 +122,7 @@ interface IGovernor is IERC165, IERC6372 {
     /**
      * @dev Emitted when a proposal is queued.
      */
-    event ProposalQueued(uint256 proposalId, uint256 eta);
+    event ProposalQueued(uint256 proposalId, uint256 etaSeconds);
 
     /**
      * @dev Emitted when a proposal is executed.
@@ -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

+ 2 - 2
contracts/governance/TimelockController.sol

@@ -164,8 +164,8 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     }
 
     /**
-     * @dev Returns whether an id correspond to a registered operation. This
-     * includes both Pending, Ready and Done operations.
+     * @dev Returns whether an id corresponds to a registered operation. This
+     * includes both Waiting, Ready, and Done operations.
      */
     function isOperation(bytes32 id) public view returns (bool) {
         return getOperationState(id) != OperationState.Unset;

+ 3 - 3
contracts/governance/extensions/GovernorPreventLateQuorum.sol

@@ -27,9 +27,9 @@ abstract contract GovernorPreventLateQuorum is Governor {
     event LateQuorumVoteExtensionSet(uint64 oldVoteExtension, uint64 newVoteExtension);
 
     /**
-     * @dev Initializes the vote extension parameter: the time in either number of blocks or seconds (depending on the governor
-     * clock mode) that is required to pass since the moment a proposal reaches quorum until its voting period ends. If
-     * necessary the voting period will be extended beyond the one set during proposal creation.
+     * @dev Initializes the vote extension parameter: the time in either number of blocks or seconds (depending on the
+     * governor clock mode) that is required to pass since the moment a proposal reaches quorum until its voting period
+     * ends. If necessary the voting period will be extended beyond the one set during proposal creation.
      */
     constructor(uint48 initialVoteExtension) {
         _setLateQuorumVoteExtension(initialVoteExtension);

+ 0 - 1
contracts/governance/extensions/GovernorSettings.sol

@@ -93,7 +93,6 @@ abstract contract GovernorSettings is Governor {
      * Emits a {VotingPeriodSet} event.
      */
     function _setVotingPeriod(uint32 newVotingPeriod) internal virtual {
-        // voting period must be at least one block long
         if (newVotingPeriod == 0) {
             revert GovernorInvalidVotingPeriod(0);
         }

+ 3 - 2
contracts/governance/extensions/GovernorStorage.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-pragma solidity ^0.8.19;
+pragma solidity ^0.8.20;
 
 import {Governor} from "../Governor.sol";
 
@@ -10,7 +10,8 @@ import {Governor} from "../Governor.sol";
  *
  * Use cases for this module include:
  * - UIs that explore the proposal state without relying on event indexing.
- * - Using only the proposalId as an argument in the {Governor-queue} and {Governor-execute} functions for L2 chains where storage is cheap compared to calldata.
+ * - Using only the proposalId as an argument in the {Governor-queue} and {Governor-execute} functions for L2 chains
+ *   where storage is cheap compared to calldata.
  */
 abstract contract GovernorStorage is Governor {
     struct ProposalDetails {

+ 342 - 0
contracts/governance/extensions/GovernorTimelockAccess.sol

@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {Governor} from "../Governor.sol";
+import {AuthorityUtils} from "../../access/manager/AuthorityUtils.sol";
+import {IAccessManager} from "../../access/manager/IAccessManager.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 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-execute} function.
+ *
+ * ==== Security Considerations
+ *
+ * Some operations may be cancelable in the `AccessManager` by the admin or a set of guardians, depending on the
+ * restricted function being invoked. Since proposals are atomic, the cancellation by a guardian of a single operation
+ * in a proposal will cause all of the proposal to become unable to execute. Consider proposing cancellable operations
+ * separately.
+ *
+ * By default, function calls will be routed through the associated `AccessManager` whenever it claims the target
+ * function to be restricted by it. However, admins may configure the manager to make that claim for functions that a
+ * governor would want to call directly (e.g., token transfers) in an attempt to deny it access to those functions. To
+ * mitigate this attack vector, the governor is able to ignore the restrictions claimed by the `AccessManager` using
+ * {setAccessManagerIgnored}. While permanent denial of service is mitigated, temporary DoS may still be technically
+ * possible. All of the governor's own functions (e.g., {setBaseDelaySeconds}) ignore the `AccessManager` by default.
+ */
+abstract contract GovernorTimelockAccess is Governor {
+    // An execution plan is produced at the moment a proposal is created, in order to fix at that point the exact
+    // execution semantics of the proposal, namely whether a call will go through {AccessManager-execute}.
+    struct ExecutionPlan {
+        uint16 length;
+        uint32 delay;
+        // 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 executed through the manager. Upon queuing, the value is set to nonce + 2, where the
+        // nonce is received from the manager when scheduling the operation.
+        mapping(uint256 operationBucket => uint32[8]) managerData;
+    }
+
+    // The meaning of the "toggle" set to true depends on the target contract.
+    // If target == address(this), the manager is ignored by default, and a true toggle means it won't be ignored.
+    // For all other target contracts, the manager is used by default, and a true toggle means it will be ignored.
+    mapping(address target => mapping(bytes4 selector => bool)) private _ignoreToggle;
+
+    mapping(uint256 proposalId => ExecutionPlan) private _executionPlan;
+
+    uint32 private _baseDelay;
+
+    IAccessManager private immutable _manager;
+
+    error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp);
+    error GovernorMismatchedNonce(uint256 proposalId, uint256 expectedNonce, uint256 actualNonce);
+    error GovernorLockedIgnore();
+
+    event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds);
+    event AccessManagerIgnoredSet(address target, bytes4 selector, bool ignored);
+
+    /**
+     * @dev Initialize the governor with an {AccessManager} and initial base delay.
+     */
+    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
+     * 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;
+    }
+
+    /**
+     * @dev Check if restrictions from the associated {AccessManager} are ignored for a target function. Returns true
+     * when the target function will be invoked directly regardless of `AccessManager` settings for the function.
+     * See {setAccessManagerIgnored} and Security Considerations above.
+     */
+    function isAccessManagerIgnored(address target, bytes4 selector) public view virtual returns (bool) {
+        bool isGovernor = target == address(this);
+        return _ignoreToggle[target][selector] != isGovernor; // equivalent to: isGovernor ? !toggle : toggle
+    }
+
+    /**
+     * @dev Configure whether restrictions from the associated {AccessManager} are ignored for a target function.
+     * See Security Considerations above.
+     */
+    function setAccessManagerIgnored(
+        address target,
+        bytes4[] calldata selectors,
+        bool ignored
+    ) public virtual onlyGovernance {
+        for (uint256 i = 0; i < selectors.length; ++i) {
+            _setAccessManagerIgnored(target, selectors[i], ignored);
+        }
+    }
+
+    /**
+     * @dev Internal version of {setAccessManagerIgnored} without access restriction.
+     */
+    function _setAccessManagerIgnored(address target, bytes4 selector, bool ignored) internal virtual {
+        bool isGovernor = target == address(this);
+        if (isGovernor && selector == this.setAccessManagerIgnored.selector) {
+            revert GovernorLockedIgnore();
+        }
+        _ignoreToggle[target][selector] = ignored != isGovernor; // equivalent to: isGovernor ? !ignored : ignored
+        emit AccessManagerIgnoredSet(target, selector, ignored);
+    }
+
+    /**
+     * @dev Public accessor to check the execution plan, including the number of seconds that the proposal will be
+     * delayed since queuing, an array indicating which of the proposal actions will be executed indirectly through
+     * the associated {AccessManager}, and another indicating which will be scheduled in {queue}. Note that
+     * those that must be scheduled are cancellable by `AccessManager` guardians.
+     */
+    function proposalExecutionPlan(
+        uint256 proposalId
+    ) public view returns (uint32 delay, bool[] memory indirect, bool[] memory withDelay) {
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
+        uint32 length = plan.length;
+        delay = plan.delay;
+        indirect = new bool[](length);
+        withDelay = new bool[](length);
+        for (uint256 i = 0; i < length; ++i) {
+            (indirect[i], withDelay[i], ) = _getManagerData(plan, i);
+        }
+
+        return (delay, indirect, withDelay);
+    }
+
+    /**
+     * @dev See {IGovernor-proposalNeedsQueuing}.
+     */
+    function proposalNeedsQueuing(uint256 proposalId) public view virtual override returns (bool) {
+        return _executionPlan[proposalId].delay > 0;
+    }
+
+    /**
+     * @dev See {IGovernor-propose}
+     */
+    function propose(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        string memory description
+    ) public virtual override returns (uint256) {
+        uint256 proposalId = super.propose(targets, values, calldatas, description);
+
+        uint32 neededDelay = baseDelaySeconds();
+
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+        plan.length = SafeCast.toUint16(targets.length);
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            address target = targets[i];
+            bytes4 selector = bytes4(calldatas[i]);
+            (bool immediate, uint32 delay) = AuthorityUtils.canCallWithDelay(
+                address(_manager),
+                address(this),
+                target,
+                selector
+            );
+            if ((immediate || delay > 0) && !isAccessManagerIgnored(target, selector)) {
+                _setManagerData(plan, i, !immediate, 0);
+                // downcast is safe because both arguments are uint32
+                neededDelay = uint32(Math.max(delay, neededDelay));
+            }
+        }
+
+        plan.delay = neededDelay;
+
+        return proposalId;
+    }
+
+    /**
+     * @dev Mechanism to queue a proposal, potentially scheduling some of its operations in the AccessManager.
+     *
+     * 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,
+        address[] memory targets,
+        uint256[] memory /* values */,
+        bytes[] memory calldatas,
+        bytes32 /* descriptionHash */
+    ) internal virtual override returns (uint48) {
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+        uint48 etaSeconds = Time.timestamp() + plan.delay;
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            (, bool withDelay, ) = _getManagerData(plan, i);
+            if (withDelay) {
+                (, uint32 nonce) = _manager.schedule(targets[i], calldatas[i], etaSeconds);
+                _setManagerData(plan, i, true, nonce);
+            }
+        }
+
+        return etaSeconds;
+    }
+
+    /**
+     * @dev Mechanism to execute a proposal, potentially going through {AccessManager-execute} for delayed operations.
+     */
+    function _executeOperations(
+        uint256 proposalId,
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 /* descriptionHash */
+    ) internal virtual override {
+        uint48 etaSeconds = SafeCast.toUint48(proposalEta(proposalId));
+        if (block.timestamp < etaSeconds) {
+            revert GovernorUnmetDelay(proposalId, etaSeconds);
+        }
+
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
+        for (uint256 i = 0; i < targets.length; ++i) {
+            (bool controlled, bool withDelay, uint32 nonce) = _getManagerData(plan, i);
+            if (controlled) {
+                uint32 executedNonce = _manager.execute{value: values[i]}(targets[i], calldatas[i]);
+                if (withDelay && executedNonce != nonce) {
+                    revert GovernorMismatchedNonce(proposalId, nonce, executedNonce);
+                }
+            } else {
+                (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
+                Address.verifyCallResult(success, returndata);
+            }
+        }
+    }
+
+    /**
+     * @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);
+
+        uint48 etaSeconds = SafeCast.toUint48(proposalEta(proposalId));
+
+        ExecutionPlan storage plan = _executionPlan[proposalId];
+
+        // If the proposal has been scheduled it will have an ETA and we may have to externally cancel
+        if (etaSeconds != 0) {
+            for (uint256 i = 0; i < targets.length; ++i) {
+                (, bool withDelay, uint32 nonce) = _getManagerData(plan, i);
+                // Only attempt to cancel if the execution plan included a delay
+                if (withDelay) {
+                    bytes32 operationId = _manager.hashOperation(address(this), targets[i], calldatas[i]);
+                    // Check first if the current operation nonce is the one that we observed previously. It could
+                    // already have been cancelled and rescheduled. We don't want to cancel unless it is exactly the
+                    // instance that we previously scheduled.
+                    if (nonce == _manager.getNonce(operationId)) {
+                        // It is important that all calls have an opportunity to be cancelled. We chose to ignore
+                        // potential failures of some of the cancel operations to give the other operations a chance to
+                        // be properly cancelled. In particular cancel might fail if the operation was already cancelled
+                        // by guardians previously. We don't match on the revert reason to avoid encoding assumptions
+                        // about specific errors.
+                        try _manager.cancel(address(this), targets[i], calldatas[i]) {} catch {}
+                    }
+                }
+            }
+        }
+
+        return proposalId;
+    }
+
+    /**
+     * @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 controlled, bool withDelay, uint32 nonce) {
+        (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
+        uint32 value = plan.managerData[bucket][subindex];
+        unchecked {
+            return (value > 0, value > 1, value > 1 ? value - 2 : 0);
+        }
+    }
+
+    /**
+     * @dev Marks an operation at an index as permissioned by the manager, potentially delayed, and
+     * when delayed sets its scheduling nonce.
+     */
+    function _setManagerData(ExecutionPlan storage plan, uint256 index, bool withDelay, uint32 nonce) private {
+        (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index);
+        plan.managerData[bucket][subindex] = withDelay ? nonce + 2 : 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
+    }
+}

+ 22 - 14
contracts/governance/extensions/GovernorTimelockCompound.sol

@@ -5,7 +5,6 @@ pragma solidity ^0.8.20;
 
 import {IGovernor, Governor} from "../Governor.sol";
 import {ICompoundTimelock} from "../../vendor/compound/ICompoundTimelock.sol";
-import {IERC165} from "../../interfaces/IERC165.sol";
 import {Address} from "../../utils/Address.sol";
 import {SafeCast} from "../../utils/math/SafeCast.sol";
 
@@ -54,6 +53,13 @@ abstract contract GovernorTimelockCompound is Governor {
         return address(_timelock);
     }
 
+    /**
+     * @dev See {IGovernor-proposalNeedsQueuing}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
+        return true;
+    }
+
     /**
      * @dev Function to queue a proposal to the timelock.
      */
@@ -64,21 +70,23 @@ abstract contract GovernorTimelockCompound is Governor {
         bytes[] memory calldatas,
         bytes32 /*descriptionHash*/
     ) internal virtual override returns (uint48) {
-        uint48 eta = SafeCast.toUint48(block.timestamp + _timelock.delay());
+        uint48 etaSeconds = SafeCast.toUint48(block.timestamp + _timelock.delay());
 
         for (uint256 i = 0; i < targets.length; ++i) {
-            if (_timelock.queuedTransactions(keccak256(abi.encode(targets[i], values[i], "", calldatas[i], eta)))) {
+            if (
+                _timelock.queuedTransactions(keccak256(abi.encode(targets[i], values[i], "", calldatas[i], etaSeconds)))
+            ) {
                 revert GovernorAlreadyQueuedProposal(proposalId);
             }
-            _timelock.queueTransaction(targets[i], values[i], "", calldatas[i], eta);
+            _timelock.queueTransaction(targets[i], values[i], "", calldatas[i], etaSeconds);
         }
 
-        return eta;
+        return etaSeconds;
     }
 
     /**
-     * @dev Overridden version of the {Governor-_executeOperations} function that run the already queued proposal through
-     * the timelock.
+     * @dev Overridden version of the {Governor-_executeOperations} function that run the already queued proposal
+     * through the timelock.
      */
     function _executeOperations(
         uint256 proposalId,
@@ -87,18 +95,18 @@ abstract contract GovernorTimelockCompound is Governor {
         bytes[] memory calldatas,
         bytes32 /*descriptionHash*/
     ) internal virtual override {
-        uint256 eta = proposalEta(proposalId);
-        if (eta == 0) {
+        uint256 etaSeconds = proposalEta(proposalId);
+        if (etaSeconds == 0) {
             revert GovernorNotQueuedProposal(proposalId);
         }
         Address.sendValue(payable(_timelock), msg.value);
         for (uint256 i = 0; i < targets.length; ++i) {
-            _timelock.executeTransaction(targets[i], values[i], "", calldatas[i], eta);
+            _timelock.executeTransaction(targets[i], values[i], "", calldatas[i], etaSeconds);
         }
     }
 
     /**
-     * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already
+     * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it has already
      * been queued.
      */
     function _cancel(
@@ -109,11 +117,11 @@ abstract contract GovernorTimelockCompound is Governor {
     ) internal virtual override returns (uint256) {
         uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash);
 
-        uint256 eta = proposalEta(proposalId);
-        if (eta > 0) {
+        uint256 etaSeconds = proposalEta(proposalId);
+        if (etaSeconds > 0) {
             // do external call later
             for (uint256 i = 0; i < targets.length; ++i) {
-                _timelock.cancelTransaction(targets[i], values[i], "", calldatas[i], eta);
+                _timelock.cancelTransaction(targets[i], values[i], "", calldatas[i], etaSeconds);
             }
         }
 

+ 18 - 8
contracts/governance/extensions/GovernorTimelockControl.sol

@@ -15,12 +15,15 @@ import {SafeCast} from "../../utils/math/SafeCast.sol";
  *
  * Using this model means the proposal will be operated by the {TimelockController} and not by the {Governor}. Thus,
  * the assets and permissions must be attached to the {TimelockController}. Any asset sent to the {Governor} will be
- * inaccessible.
+ * inaccessible from a proposal, unless executed via {Governor-relay}.
  *
- * WARNING: Setting up the TimelockController to have additional proposers besides the governor is very risky, as it
- * grants them powers that they must be trusted or known not to use: 1) {onlyGovernance} functions like {relay} are
- * available to them through the timelock, and 2) approved governance proposals can be blocked by them, effectively
- * executing a Denial of Service attack. This risk will be mitigated in a future release.
+ * WARNING: Setting up the TimelockController to have additional proposers or cancellers besides the governor is very
+ * risky, as it grants them the ability to: 1) execute operations as the timelock, and thus possibly performing
+ * operations or accessing funds that are expected to only be accessible through a vote, and 2) block governance
+ * proposals that have been approved by the voters, effectively executing a Denial of Service attack.
+ *
+ * NOTE: `AccessManager` does not support scheduling more than one operation with the same target and calldata at
+ * the same time. See {AccessManager-schedule} for a workaround.
  */
 abstract contract GovernorTimelockControl is Governor {
     TimelockController private _timelock;
@@ -67,6 +70,13 @@ abstract contract GovernorTimelockControl is Governor {
         return address(_timelock);
     }
 
+    /**
+     * @dev See {IGovernor-proposalNeedsQueuing}.
+     */
+    function proposalNeedsQueuing(uint256) public view virtual override returns (bool) {
+        return true;
+    }
+
     /**
      * @dev Function to queue a proposal to the timelock.
      */
@@ -87,8 +97,8 @@ abstract contract GovernorTimelockControl is Governor {
     }
 
     /**
-     * @dev Overridden version of the {Governor-_executeOperations} function that runs the already queued proposal through
-     * the timelock.
+     * @dev Overridden version of the {Governor-_executeOperations} function that runs the already queued proposal
+     * through the timelock.
      */
     function _executeOperations(
         uint256 proposalId,
@@ -104,7 +114,7 @@ abstract contract GovernorTimelockControl is Governor {
     }
 
     /**
-     * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already
+     * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it has already
      * been queued.
      */
     // This function can reenter through the external call to the timelock, but we assume the timelock is trusted and

+ 4 - 2
contracts/governance/extensions/GovernorVotes.sol

@@ -7,9 +7,11 @@ import {Governor} from "../Governor.sol";
 import {IVotes} from "../utils/IVotes.sol";
 import {IERC5805} from "../../interfaces/IERC5805.sol";
 import {SafeCast} from "../../utils/math/SafeCast.sol";
+import {Time} from "../../utils/types/Time.sol";
 
 /**
- * @dev Extension of {Governor} for voting weight extraction from an {ERC20Votes} token, or since v4.5 an {ERC721Votes} token.
+ * @dev Extension of {Governor} for voting weight extraction from an {ERC20Votes} token, or since v4.5 an {ERC721Votes}
+ * token.
  */
 abstract contract GovernorVotes is Governor {
     IERC5805 private immutable _token;
@@ -33,7 +35,7 @@ abstract contract GovernorVotes is Governor {
         try token().clock() returns (uint48 timepoint) {
             return timepoint;
         } catch {
-            return SafeCast.toUint48(block.number);
+            return Time.blockNumber();
         }
     }
 

+ 0 - 1
contracts/governance/extensions/GovernorVotesQuorumFraction.sol

@@ -45,7 +45,6 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
      * @dev Returns the quorum numerator at a specific timepoint. See {quorumDenominator}.
      */
     function quorumNumerator(uint256 timepoint) public view virtual returns (uint256) {
-        // If history is empty, fallback to old storage
         uint256 length = _quorumNumeratorHistory._checkpoints.length;
 
         // Optimistic search, check the latest checkpoint

+ 3 - 2
contracts/governance/utils/Votes.sol

@@ -9,6 +9,7 @@ import {EIP712} from "../../utils/cryptography/EIP712.sol";
 import {Checkpoints} from "../../utils/structs/Checkpoints.sol";
 import {SafeCast} from "../../utils/math/SafeCast.sol";
 import {ECDSA} from "../../utils/cryptography/ECDSA.sol";
+import {Time} from "../../utils/types/Time.sol";
 
 /**
  * @dev This is a base abstract contract that tracks voting units, which are a measure of voting power that can be
@@ -55,7 +56,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
      * checkpoints (and voting), in which case {CLOCK_MODE} should be overridden as well to match.
      */
     function clock() public view virtual returns (uint48) {
-        return SafeCast.toUint48(block.number);
+        return Time.blockNumber();
     }
 
     /**
@@ -64,7 +65,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
     // solhint-disable-next-line func-name-mixedcase
     function CLOCK_MODE() public view virtual returns (string memory) {
         // Check that the clock was not modified
-        if (clock() != block.number) {
+        if (clock() != Time.blockNumber()) {
             revert ERC6372InconsistentClock();
         }
         return "mode=blocknumber&from=default";

+ 18 - 0
contracts/mocks/AccessManagedTarget.sol

@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {AccessManaged} from "../access/manager/AccessManaged.sol";
+
+abstract contract AccessManagedTarget is AccessManaged {
+    event CalledRestricted(address caller);
+    event CalledUnrestricted(address caller);
+
+    function fnRestricted() public restricted {
+        emit CalledRestricted(msg.sender);
+    }
+
+    function fnUnrestricted() public {
+        emit CalledUnrestricted(msg.sender);
+    }
+}

+ 6 - 0
contracts/mocks/docs/governance/MyGovernor.sol

@@ -40,6 +40,12 @@ 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,

+ 6 - 0
contracts/mocks/governance/GovernorStorageMock.sol

@@ -28,6 +28,12 @@ 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,

+ 70 - 0
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);
+    }
+}

+ 6 - 0
contracts/mocks/governance/GovernorTimelockCompoundMock.sol

@@ -28,6 +28,12 @@ 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,

+ 6 - 0
contracts/mocks/governance/GovernorTimelockControlMock.sol

@@ -26,6 +26,12 @@ 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,

+ 2 - 1
contracts/mocks/token/ERC20VotesLegacyMock.sol

@@ -88,7 +88,8 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit {
         //
         // Initially we check if the block is recent to narrow the search range.
         // During the loop, the index of the wanted checkpoint remains in the range [low-1, high).
-        // With each iteration, either `low` or `high` is moved towards the middle of the range to maintain the invariant.
+        // With each iteration, either `low` or `high` is moved towards the middle of the range to maintain the
+        // invariant.
         // - If the middle checkpoint is after `blockNumber`, we look in [low, mid)
         // - If the middle checkpoint is before or equal to `blockNumber`, we look in [mid+1, high)
         // Once we reach a single value (when low == high), we've found the right checkpoint at the index high-1, if not

+ 4 - 4
contracts/proxy/ERC1967/ERC1967Proxy.sol

@@ -16,8 +16,8 @@ contract ERC1967Proxy is Proxy {
     /**
      * @dev Initializes the upgradeable proxy with an initial implementation specified by `implementation`.
      *
-     * If `_data` is nonempty, it's used as data in a delegate call to `implementation`. This will typically be an encoded
-     * function call, and allows initializing the storage of the proxy like a Solidity constructor.
+     * If `_data` is nonempty, it's used as data in a delegate call to `implementation`. This will typically be an
+     * encoded function call, and allows initializing the storage of the proxy like a Solidity constructor.
      *
      * Requirements:
      *
@@ -30,8 +30,8 @@ contract ERC1967Proxy is Proxy {
     /**
      * @dev Returns the current implementation address.
      *
-     * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using the
-     * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
+     * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using
+     * the https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
      * `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc`
      */
     function _implementation() internal view virtual override returns (address) {

+ 2 - 2
contracts/proxy/ERC1967/ERC1967Utils.sol

@@ -101,8 +101,8 @@ library ERC1967Utils {
     /**
      * @dev Returns the current admin.
      *
-     * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using the
-     * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
+     * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using
+     * the https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
      * `0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103`
      */
     function getAdmin() internal view returns (address) {

+ 2 - 2
contracts/proxy/Proxy.sol

@@ -45,8 +45,8 @@ abstract contract Proxy {
     }
 
     /**
-     * @dev This is a virtual function that should be overridden so it returns the address to which the fallback function
-     * and {_fallback} should delegate.
+     * @dev This is a virtual function that should be overridden so it returns the address to which the fallback
+     * function and {_fallback} should delegate.
      */
     function _implementation() internal view virtual returns (address);
 

+ 6 - 6
contracts/proxy/beacon/BeaconProxy.sol

@@ -10,15 +10,15 @@ import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol";
 /**
  * @dev This contract implements a proxy that gets the implementation address for each call from an {UpgradeableBeacon}.
  *
- * The beacon address can only be set once during construction, and cannot be changed afterwards. It is stored in an immutable
- * variable to avoid unnecessary storage reads, and also in the beacon storage slot specified by
+ * The beacon address can only be set once during construction, and cannot be changed afterwards. It is stored in an
+ * immutable variable to avoid unnecessary storage reads, and also in the beacon storage slot specified by
  * https://eips.ethereum.org/EIPS/eip-1967[EIP1967] so that it can be accessed externally.
  *
- * CAUTION: Since the beacon address can never be changed, you must ensure that you either control the beacon, or trust the
- * beacon to not upgrade the implementation maliciously.
+ * CAUTION: Since the beacon address can never be changed, you must ensure that you either control the beacon, or trust
+ * the beacon to not upgrade the implementation maliciously.
  *
- * IMPORTANT: Do not use the implementation logic to modify the beacon storage slot. Doing so would leave the proxy in an
- * inconsistent state where the beacon storage slot does not match the beacon address.
+ * IMPORTANT: Do not use the implementation logic to modify the beacon storage slot. Doing so would leave the proxy in
+ * an inconsistent state where the beacon storage slot does not match the beacon address.
  */
 contract BeaconProxy is Proxy {
     // An immutable address for the beacon to avoid unnecessary SLOADs before each delegate call.

+ 19 - 20
contracts/proxy/transparent/TransparentUpgradeableProxy.sol

@@ -28,35 +28,34 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  *
  * 1. If any account other than the admin calls the proxy, the call will be forwarded to the implementation, even if
  * that call matches the {ITransparentUpgradeableProxy-upgradeToAndCall} function exposed by the proxy itself.
- * 2. If the admin calls the proxy, it can call the `upgradeToAndCall` function but any other call won't be forwarded to the
- * implementation. If the admin tries to call a function on the implementation it will fail with an error indicating the
- * proxy admin cannot fallback to the target implementation.
+ * 2. If the admin calls the proxy, it can call the `upgradeToAndCall` function but any other call won't be forwarded to
+ * the implementation. If the admin tries to call a function on the implementation it will fail with an error indicating
+ * the proxy admin cannot fallback to the target implementation.
  *
- * These properties mean that the admin account can only be used for upgrading the proxy, so it's best if it's a dedicated
- * account that is not used for anything else. This will avoid headaches due to sudden errors when trying to call a function
- * from the proxy implementation. For this reason, the proxy deploys an instance of {ProxyAdmin} and allows upgrades
- * only if they come through it.
- * You should think of the `ProxyAdmin` instance as the administrative interface of the proxy, including the ability to
- * change who can trigger upgrades by transferring ownership.
+ * These properties mean that the admin account can only be used for upgrading the proxy, so it's best if it's a
+ * dedicated account that is not used for anything else. This will avoid headaches due to sudden errors when trying to
+ * call a function from the proxy implementation. For this reason, the proxy deploys an instance of {ProxyAdmin} and
+ * allows upgrades only if they come through it. You should think of the `ProxyAdmin` instance as the administrative
+ * interface of the proxy, including the ability to change who can trigger upgrades by transferring ownership.
  *
  * NOTE: The real interface of this proxy is that defined in `ITransparentUpgradeableProxy`. This contract does not
- * inherit from that interface, and instead `upgradeToAndCall` is implicitly implemented using a custom dispatch mechanism
- * in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to fully
- * implement transparency without decoding reverts caused by selector clashes between the proxy and the
+ * inherit from that interface, and instead `upgradeToAndCall` is implicitly implemented using a custom dispatch
+ * mechanism in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to
+ * fully implement transparency without decoding reverts caused by selector clashes between the proxy and the
  * implementation.
  *
  * NOTE: This proxy does not inherit from {Context} deliberately. The {ProxyAdmin} of this contract won't send a
  * meta-transaction in any way, and any other meta-transaction setup should be made in the implementation contract.
  *
- * IMPORTANT: This contract avoids unnecessary storage reads by setting the admin only during construction as an immutable variable,
- * preventing any changes thereafter. However, the admin slot defined in ERC-1967 can still be overwritten by the implementation
- * logic pointed to by this proxy. In such cases, the contract may end up in an undesirable state where the admin slot is different
- * from the actual admin.
+ * IMPORTANT: This contract avoids unnecessary storage reads by setting the admin only during construction as an
+ * immutable variable, preventing any changes thereafter. However, the admin slot defined in ERC-1967 can still be
+ * overwritten by the implementation logic pointed to by this proxy. In such cases, the contract may end up in an
+ * undesirable state where the admin slot is different from the actual admin.
  *
- * WARNING: It is not recommended to extend this contract to add additional external functions. If you do so, the compiler
- * will not check that there are no selector conflicts, due to the note above. A selector clash between any new function
- * and the functions declared in {ITransparentUpgradeableProxy} will be resolved in favor of the new one. This could
- * render the `upgradeToAndCall` function inaccessible, preventing upgradeability and compromising transparency.
+ * WARNING: It is not recommended to extend this contract to add additional external functions. If you do so, the
+ * compiler will not check that there are no selector conflicts, due to the note above. A selector clash between any new
+ * function and the functions declared in {ITransparentUpgradeableProxy} will be resolved in favor of the new one. This
+ * could render the `upgradeToAndCall` function inaccessible, preventing upgradeability and compromising transparency.
  */
 contract TransparentUpgradeableProxy is ERC1967Proxy {
     // An immutable address for the admin to avoid unnecessary SLOADs before each call

+ 19 - 8
contracts/proxy/utils/Initializable.sol

@@ -79,7 +79,7 @@ abstract contract Initializable {
     /**
      * @dev The contract is already initialized.
      */
-    error AlreadyInitialized();
+    error InvalidInitialization();
 
     /**
      * @dev The contract is not initializing.
@@ -95,8 +95,9 @@ abstract contract Initializable {
      * @dev A modifier that defines a protected initializer function that can be invoked at most once. In its scope,
      * `onlyInitializing` functions can be used to initialize parent contracts.
      *
-     * Similar to `reinitializer(1)`, except that functions marked with `initializer` can be nested in the context of a
-     * constructor.
+     * Similar to `reinitializer(1)`, except that in the context of a constructor an `initializer` may be invoked any
+     * number of times. This behavior in the constructor can be useful during testing and is not expected to be used in
+     * production.
      *
      * Emits an {Initialized} event.
      */
@@ -104,10 +105,20 @@ abstract contract Initializable {
         // solhint-disable-next-line var-name-mixedcase
         InitializableStorage storage $ = _getInitializableStorage();
 
+        // Cache values to avoid duplicated sloads
         bool isTopLevelCall = !$._initializing;
         uint64 initialized = $._initialized;
-        if (!(isTopLevelCall && initialized < 1) && !(address(this).code.length == 0 && initialized == 1)) {
-            revert AlreadyInitialized();
+
+        // Allowed calls:
+        // - initialSetup: the contract is not in the initializing state and no previous version was
+        //                 initialized
+        // - construction: the contract is initialized at version 1 (no reininitialization) and the
+        //                 current contract is just being deployed
+        bool initialSetup = initialized == 0 && isTopLevelCall;
+        bool construction = initialized == 1 && address(this).code.length == 0;
+
+        if (!initialSetup && !construction) {
+            revert InvalidInitialization();
         }
         $._initialized = 1;
         if (isTopLevelCall) {
@@ -134,7 +145,7 @@ abstract contract Initializable {
      * Note that versions can jump in increments greater than 1; this implies that if multiple reinitializers coexist in
      * a contract, executing them in the right order is up to the developer or operator.
      *
-     * WARNING: setting the version to 255 will prevent any future reinitialization.
+     * WARNING: Setting the version to 2**64 - 1 will prevent any future reinitialization.
      *
      * Emits an {Initialized} event.
      */
@@ -143,7 +154,7 @@ abstract contract Initializable {
         InitializableStorage storage $ = _getInitializableStorage();
 
         if ($._initializing || $._initialized >= version) {
-            revert AlreadyInitialized();
+            revert InvalidInitialization();
         }
         $._initialized = version;
         $._initializing = true;
@@ -183,7 +194,7 @@ abstract contract Initializable {
         InitializableStorage storage $ = _getInitializableStorage();
 
         if ($._initializing) {
-            revert AlreadyInitialized();
+            revert InvalidInitialization();
         }
         if ($._initialized != type(uint64).max) {
             $._initialized = type(uint64).max;

+ 5 - 4
contracts/token/ERC1155/ERC1155.sol

@@ -132,7 +132,8 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
     }
 
     /**
-     * @dev Transfers a `value` amount of tokens of type `id` from `from` to `to`. Will mint (or burn) if `from` (or `to`) is the zero address.
+     * @dev Transfers a `value` amount of tokens of type `id` from `from` to `to`. Will mint (or burn) if `from`
+     * (or `to`) is the zero address.
      *
      * Emits a {TransferSingle} event if the arrays contain one element, and {TransferBatch} otherwise.
      *
@@ -181,9 +182,9 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
     }
 
     /**
-     * @dev Version of {_update} that performs the token acceptance check by calling {IERC1155Receiver-onERC1155Received}
-     * or {IERC1155Receiver-onERC1155BatchReceived} on the receiver address if it contains code (eg. is a smart contract
-     * at the moment of execution).
+     * @dev Version of {_update} that performs the token acceptance check by calling
+     * {IERC1155Receiver-onERC1155Received} or {IERC1155Receiver-onERC1155BatchReceived} on the receiver address if it
+     * contains code (eg. is a smart contract at the moment of execution).
      *
      * IMPORTANT: Overriding this function is discouraged because it poses a reentrancy risk from the receiver. So any
      * update to the contract state after this function would break the check-effect-interaction pattern. Consider

+ 5 - 4
contracts/token/ERC20/ERC20.sol

@@ -179,8 +179,9 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
     }
 
     /**
-     * @dev Transfers a `value` amount of tokens from `from` to `to`, or alternatively mints (or burns) if `from` (or `to`) is
-     * the zero address. All customizations to transfers, mints, and burns should be done by overriding this function.
+     * @dev Transfers a `value` amount of tokens from `from` to `to`, or alternatively mints (or burns) if `from`
+     * (or `to`) is the zero address. All customizations to transfers, mints, and burns should be done by overriding
+     * this function.
      *
      * Emits a {Transfer} event.
      */
@@ -270,8 +271,8 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
      * `_spendAllowance` during the `transferFrom` operation set the flag to false. This saves gas by not emitting any
      * `Approval` event during `transferFrom` operations.
      *
-     * Anyone who wishes to continue emitting `Approval` events on the`transferFrom` operation can force the flag to true
-     * using the following override:
+     * Anyone who wishes to continue emitting `Approval` events on the`transferFrom` operation can force the flag to
+     * true using the following override:
      * ```
      * function _approve(address owner, address spender, uint256 value, bool) internal virtual override {
      *     super._approve(owner, spender, value, true);

+ 2 - 2
contracts/token/ERC20/utils/SafeERC20.sol

@@ -55,8 +55,8 @@ library SafeERC20 {
     }
 
     /**
-     * @dev Decrease the calling contract's allowance toward `spender` by `requestedDecrease`. If `token` returns no value,
-     * non-reverting calls are assumed to be successful.
+     * @dev Decrease the calling contract's allowance toward `spender` by `requestedDecrease`. If `token` returns no
+     * value, non-reverting calls are assumed to be successful.
      */
     function safeDecreaseAllowance(IERC20 token, address spender, uint256 requestedDecrease) internal {
         unchecked {

+ 18 - 16
contracts/token/ERC721/ERC721.sol

@@ -65,11 +65,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721-ownerOf}.
      */
     function ownerOf(uint256 tokenId) public view virtual returns (address) {
-        address owner = _ownerOf(tokenId);
-        if (owner == address(0)) {
-            revert ERC721NonexistentToken(tokenId);
-        }
-        return owner;
+        return _requireOwned(tokenId);
     }
 
     /**
@@ -90,7 +86,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721Metadata-tokenURI}.
      */
     function tokenURI(uint256 tokenId) public view virtual returns (string memory) {
-        _requireMinted(tokenId);
+        _requireOwned(tokenId);
 
         string memory baseURI = _baseURI();
         return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString()) : "";
@@ -116,7 +112,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev See {IERC721-getApproved}.
      */
     function getApproved(uint256 tokenId) public view virtual returns (address) {
-        _requireMinted(tokenId);
+        _requireOwned(tokenId);
 
         return _getApproved(tokenId);
     }
@@ -188,8 +184,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
      * @dev Returns whether `spender` is allowed to manage `owner`'s tokens, or `tokenId` in
      * particular (ignoring whether it is owned by `owner`).
      *
-     * WARNING: This function assumes that `owner` is the actual owner of `tokenId` and does not
-     * verify this assumption.
+     * WARNING: This function assumes that `owner` is the actual owner of `tokenId` and does not verify this
+     * assumption.
      */
     function _isAuthorized(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) {
         return
@@ -199,10 +195,11 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
 
     /**
      * @dev Checks if `spender` can operate on `tokenId`, assuming the provided `owner` is the actual owner.
-     * Reverts if `spender` has not approval for all assets of the provided `owner` nor the actual owner approved the `spender` for the specific `tokenId`.
+     * Reverts if `spender` does not have approval from the provided `owner` for the given token or for all its assets
+     * the `spender` for the specific `tokenId`.
      *
-     * WARNING: This function relies on {_isAuthorized}, so it doesn't check whether `owner` is the
-     * actual owner of `tokenId`.
+     * WARNING: This function assumes that `owner` is the actual owner of `tokenId` and does not verify this
+     * assumption.
      */
     function _checkAuthorized(address owner, address spender, uint256 tokenId) internal view virtual {
         if (!_isAuthorized(owner, spender, tokenId)) {
@@ -411,7 +408,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     function _approve(address to, uint256 tokenId, address auth, bool emitEvent) internal virtual {
         // Avoid reading the owner unless necessary
         if (emitEvent || auth != address(0)) {
-            address owner = ownerOf(tokenId);
+            address owner = _requireOwned(tokenId);
 
             // We do not use _isAuthorized because single-token approvals should not be able to call approve
             if (auth != address(0) && owner != auth && !isApprovedForAll(owner, auth)) {
@@ -443,12 +440,17 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
     }
 
     /**
-     * @dev Reverts if the `tokenId` has not been minted yet.
+     * @dev Reverts if the `tokenId` doesn't have a current owner (it hasn't been minted, or it has been burned).
+     * Returns the owner.
+     *
+     * Overrides to ownership logic should be done to {_ownerOf}.
      */
-    function _requireMinted(uint256 tokenId) internal view virtual {
-        if (_ownerOf(tokenId) == address(0)) {
+    function _requireOwned(uint256 tokenId) internal view returns (address) {
+        address owner = _ownerOf(tokenId);
+        if (owner == address(0)) {
             revert ERC721NonexistentToken(tokenId);
         }
+        return owner;
     }
 
     /**

+ 6 - 3
contracts/token/ERC721/IERC721.sol

@@ -47,7 +47,8 @@ interface IERC721 is IERC165 {
      * - `to` cannot be the zero address.
      * - `tokenId` token must exist and be owned by `from`.
      * - If the caller is not `from`, it must be approved to move this token by either {approve} or {setApprovalForAll}.
-     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon
+     *   a safe transfer.
      *
      * Emits a {Transfer} event.
      */
@@ -62,8 +63,10 @@ interface IERC721 is IERC165 {
      * - `from` cannot be the zero address.
      * - `to` cannot be the zero address.
      * - `tokenId` token must exist and be owned by `from`.
-     * - If the caller is not `from`, it must have been allowed to move this token by either {approve} or {setApprovalForAll}.
-     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
+     * - If the caller is not `from`, it must have been allowed to move this token by either {approve} or
+     *   {setApprovalForAll}.
+     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon
+     *   a safe transfer.
      *
      * Emits a {Transfer} event.
      */

+ 2 - 1
contracts/token/ERC721/IERC721Receiver.sol

@@ -14,7 +14,8 @@ interface IERC721Receiver {
      * by `operator` from `from`, this function is called.
      *
      * It must return its Solidity selector to confirm the token transfer.
-     * If any other value is returned or the interface is not implemented by the recipient, the transfer will be reverted.
+     * If any other value is returned or the interface is not implemented by the recipient, the transfer will be
+     * reverted.
      *
      * The selector can be obtained in Solidity with `IERC721Receiver.onERC721Received.selector`.
      */

+ 5 - 5
contracts/token/ERC721/extensions/ERC721Consecutive.sol

@@ -19,12 +19,12 @@ import {Checkpoints} from "../../../utils/structs/Checkpoints.sol";
  * Using this extension removes the ability to mint single tokens during contract construction. This ability is
  * regained after construction. During construction, only batch minting is allowed.
  *
- * IMPORTANT: This extension bypasses the hooks {_beforeTokenTransfer} and {_afterTokenTransfer} for tokens minted in
- * batch. The hooks will be only called once per batch, so you should take `batchSize` parameter into consideration
- * when relying on hooks.
+ * IMPORTANT: This extension does not call the {_update} function for tokens minted in batch. Any logic added to this
+ * function through overrides will not be triggered when token are minted in batch. You may want to also override
+ * {_increaseBalance} or {_mintConsecutive} to account for these mints.
  *
- * IMPORTANT: When overriding {_afterTokenTransfer}, be careful about call ordering. {ownerOf} may return invalid
- * values during the {_afterTokenTransfer} execution if the super call is not called first. To be safe, execute the
+ * IMPORTANT: When overriding {_mintConsecutive}, be careful about call ordering. {ownerOf} may return invalid
+ * values during the {_mintConsecutive} execution if the super call is not called first. To be safe, execute the
  * super call before your custom logic.
  */
 abstract contract ERC721Consecutive is IERC2309, ERC721 {

+ 2 - 15
contracts/token/ERC721/extensions/ERC721Royalty.sol

@@ -10,8 +10,8 @@ import {ERC2981} from "../../common/ERC2981.sol";
  * @dev Extension of ERC721 with the ERC2981 NFT Royalty Standard, a standardized way to retrieve royalty payment
  * information.
  *
- * Royalty information can be specified globally for all token ids via {ERC2981-_setDefaultRoyalty}, and/or individually for
- * specific token ids via {ERC2981-_setTokenRoyalty}. The latter takes precedence over the first.
+ * Royalty information can be specified globally for all token ids via {ERC2981-_setDefaultRoyalty}, and/or individually
+ * for specific token ids via {ERC2981-_setTokenRoyalty}. The latter takes precedence over the first.
  *
  * IMPORTANT: ERC-2981 only specifies a way to signal royalty information and does not enforce its payment. See
  * https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. Marketplaces are expected to
@@ -24,17 +24,4 @@ abstract contract ERC721Royalty is ERC2981, ERC721 {
     function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC2981) returns (bool) {
         return super.supportsInterface(interfaceId);
     }
-
-    /**
-     * @dev See {ERC721-_update}. When burning, this override will additionally clear the royalty information for the token.
-     */
-    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
-        address previousOwner = super._update(to, tokenId, auth);
-
-        if (to == address(0)) {
-            _resetTokenRoyalty(tokenId);
-        }
-
-        return previousOwner;
-    }
 }

+ 6 - 25
contracts/token/ERC721/extensions/ERC721URIStorage.sol

@@ -14,6 +14,10 @@ import {IERC165} from "../../../interfaces/IERC165.sol";
 abstract contract ERC721URIStorage is IERC4906, ERC721 {
     using Strings for uint256;
 
+    // Interface ID as defined in ERC-4906. This does not correspond to a traditional interface ID as ERC-4906 only
+    // defines events and does not include any external function.
+    bytes4 private constant ERC4906_INTERFACE_ID = bytes4(0x49064906);
+
     // Optional mapping for token URIs
     mapping(uint256 tokenId => string) private _tokenURIs;
 
@@ -21,14 +25,14 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
      * @dev See {IERC165-supportsInterface}
      */
     function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) {
-        return interfaceId == bytes4(0x49064906) || super.supportsInterface(interfaceId);
+        return interfaceId == ERC4906_INTERFACE_ID || super.supportsInterface(interfaceId);
     }
 
     /**
      * @dev See {IERC721Metadata-tokenURI}.
      */
     function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
-        _requireMinted(tokenId);
+        _requireOwned(tokenId);
 
         string memory _tokenURI = _tokenURIs[tokenId];
         string memory base = _baseURI();
@@ -49,32 +53,9 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
      * @dev Sets `_tokenURI` as the tokenURI of `tokenId`.
      *
      * Emits {MetadataUpdate}.
-     *
-     * Requirements:
-     *
-     * - `tokenId` must exist.
      */
     function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
-        if (_ownerOf(tokenId) == address(0)) {
-            revert ERC721NonexistentToken(tokenId);
-        }
         _tokenURIs[tokenId] = _tokenURI;
-
         emit MetadataUpdate(tokenId);
     }
-
-    /**
-     * @dev See {ERC721-_update}. When burning, this override will additionally check if a
-     * token-specific URI was set for the token, and if so, it deletes the token URI from
-     * the storage mapping.
-     */
-    function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
-        address previousOwner = super._update(to, tokenId, auth);
-
-        if (to == address(0) && bytes(_tokenURIs[tokenId]).length != 0) {
-            delete _tokenURIs[tokenId];
-        }
-
-        return previousOwner;
-    }
 }

+ 3 - 3
contracts/token/ERC721/extensions/ERC721Wrapper.sol

@@ -9,9 +9,9 @@ import {IERC721Receiver} from "../IERC721Receiver.sol";
 /**
  * @dev Extension of the ERC721 token contract to support token wrapping.
  *
- * Users can deposit and withdraw an "underlying token" and receive a "wrapped token" with a matching tokenId. This is useful
- * in conjunction with other modules. For example, combining this wrapping mechanism with {ERC721Votes} will allow the
- * wrapping of an existing "basic" ERC721 into a governance token.
+ * Users can deposit and withdraw an "underlying token" and receive a "wrapped token" with a matching tokenId. This is
+ * useful in conjunction with other modules. For example, combining this wrapping mechanism with {ERC721Votes} will allow
+ * the wrapping of an existing "basic" ERC721 into a governance token.
  */
 abstract contract ERC721Wrapper is ERC721, IERC721Receiver {
     IERC721 private immutable _underlying;

+ 2 - 1
contracts/token/ERC721/utils/ERC721Holder.sol

@@ -9,7 +9,8 @@ import {IERC721Receiver} from "../IERC721Receiver.sol";
  * @dev Implementation of the {IERC721Receiver} interface.
  *
  * Accepts all token transfers.
- * Make sure the contract is able to use its token with {IERC721-safeTransferFrom}, {IERC721-approve} or {IERC721-setApprovalForAll}.
+ * Make sure the contract is able to use its token with {IERC721-safeTransferFrom}, {IERC721-approve} or
+ * {IERC721-setApprovalForAll}.
  */
 abstract contract ERC721Holder is IERC721Receiver {
     /**

+ 1 - 2
contracts/utils/Nonces.sol

@@ -36,11 +36,10 @@ abstract contract Nonces {
     /**
      * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`.
      */
-    function _useCheckedNonce(address owner, uint256 nonce) internal virtual returns (uint256) {
+    function _useCheckedNonce(address owner, uint256 nonce) internal virtual {
         uint256 current = _useNonce(owner);
         if (nonce != current) {
             revert InvalidAccountNonce(owner, current);
         }
-        return current;
     }
 }

+ 2 - 1
contracts/utils/ShortStrings.sol

@@ -107,7 +107,8 @@ library ShortStrings {
     }
 
     /**
-     * @dev Return the length of a string that was encoded to `ShortString` or written to storage using {setWithFallback}.
+     * @dev Return the length of a string that was encoded to `ShortString` or written to storage using
+     * {setWithFallback}.
      *
      * WARNING: This will return the "byte length" of the string. This may not reflect the actual length in terms of
      * actual characters as the UTF-8 encoding of a single character can span over multiple bytes.

+ 2 - 1
contracts/utils/Strings.sol

@@ -78,7 +78,8 @@ library Strings {
     }
 
     /**
-     * @dev Converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation.
+     * @dev Converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal
+     * representation.
      */
     function toHexString(address addr) internal pure returns (string memory) {
         return toHexString(uint256(uint160(addr)), ADDRESS_LENGTH);

+ 5 - 2
contracts/utils/cryptography/ECDSA.sol

@@ -33,8 +33,11 @@ library ECDSA {
     error ECDSAInvalidSignatureS(bytes32 s);
 
     /**
-     * @dev Returns the address that signed a hashed message (`hash`) with
-     * `signature` or error string. This address can then be used for verification purposes.
+     * @dev Returns the address that signed a hashed message (`hash`) with `signature` or an error. This will not
+     * return address(0) without also returning an error description. Errors are documented using an enum (error type)
+     * and a bytes32 providing additional information about the error.
+     *
+     * If no error is returned, then the address can be used for verification purposes.
      *
      * The `ecrecover` EVM precompile allows for malleable (non-unique) signatures:
      * this function rejects them by requiring the `s` value to be in the lower

+ 17 - 11
contracts/utils/cryptography/MerkleProof.sol

@@ -13,8 +13,8 @@ pragma solidity ^0.8.20;
  * WARNING: You should avoid using leaf values that are 64 bytes long prior to
  * hashing, or use a hash function other than keccak256 for hashing leaves.
  * This is because the concatenation of a sorted pair of internal nodes in
- * the merkle tree could be reinterpreted as a leaf value.
- * OpenZeppelin's JavaScript library generates merkle trees that are safe
+ * the Merkle tree could be reinterpreted as a leaf value.
+ * OpenZeppelin's JavaScript library generates Merkle trees that are safe
  * against this attack out of the box.
  */
 library MerkleProof {
@@ -66,10 +66,10 @@ library MerkleProof {
     }
 
     /**
-     * @dev Returns true if the `leaves` can be simultaneously proven to be a part of a merkle tree defined by
+     * @dev Returns true if the `leaves` can be simultaneously proven to be a part of a Merkle tree defined by
      * `root`, according to `proof` and `proofFlags` as described in {processMultiProof}.
      *
-     * CAUTION: Not all merkle trees admit multiproofs. See {processMultiProof} for details.
+     * CAUTION: Not all Merkle trees admit multiproofs. See {processMultiProof} for details.
      */
     function multiProofVerify(
         bytes32[] memory proof,
@@ -83,7 +83,7 @@ library MerkleProof {
     /**
      * @dev Calldata version of {multiProofVerify}
      *
-     * CAUTION: Not all merkle trees admit multiproofs. See {processMultiProof} for details.
+     * CAUTION: Not all Merkle trees admit multiproofs. See {processMultiProof} for details.
      */
     function multiProofVerifyCalldata(
         bytes32[] calldata proof,
@@ -100,7 +100,7 @@ library MerkleProof {
      * leaf/inner node or a proof sibling node, depending on whether each `proofFlags` item is true or false
      * respectively.
      *
-     * CAUTION: Not all merkle trees admit multiproofs. To use multiproofs, it is sufficient to ensure that: 1) the tree
+     * CAUTION: Not all Merkle trees admit multiproofs. To use multiproofs, it is sufficient to ensure that: 1) the tree
      * is complete (but not necessarily perfect), 2) the leaves to be proven are in the opposite order they are in the
      * tree (i.e., as seen from right to left starting at the deepest layer and continuing at the next layer).
      */
@@ -112,13 +112,13 @@ library MerkleProof {
         // This function rebuilds the root hash by traversing the tree up from the leaves. The root is rebuilt by
         // consuming and producing values on a queue. The queue starts with the `leaves` array, then goes onto the
         // `hashes` array. At the end of the process, the last hash in the `hashes` array should contain the root of
-        // the merkle tree.
+        // the Merkle tree.
         uint256 leavesLen = leaves.length;
         uint256 proofLen = proof.length;
         uint256 totalHashes = proofFlags.length;
 
         // Check proof validity.
-        if (leavesLen + proofLen - 1 != totalHashes) {
+        if (leavesLen + proofLen != totalHashes + 1) {
             revert MerkleProofInvalidMultiproof();
         }
 
@@ -158,7 +158,7 @@ library MerkleProof {
     /**
      * @dev Calldata version of {processMultiProof}.
      *
-     * CAUTION: Not all merkle trees admit multiproofs. See {processMultiProof} for details.
+     * CAUTION: Not all Merkle trees admit multiproofs. See {processMultiProof} for details.
      */
     function processMultiProofCalldata(
         bytes32[] calldata proof,
@@ -168,13 +168,13 @@ library MerkleProof {
         // This function rebuilds the root hash by traversing the tree up from the leaves. The root is rebuilt by
         // consuming and producing values on a queue. The queue starts with the `leaves` array, then goes onto the
         // `hashes` array. At the end of the process, the last hash in the `hashes` array should contain the root of
-        // the merkle tree.
+        // the Merkle tree.
         uint256 leavesLen = leaves.length;
         uint256 proofLen = proof.length;
         uint256 totalHashes = proofFlags.length;
 
         // Check proof validity.
-        if (leavesLen + proofLen - 1 != totalHashes) {
+        if (leavesLen + proofLen != totalHashes + 1) {
             revert MerkleProofInvalidMultiproof();
         }
 
@@ -211,10 +211,16 @@ library MerkleProof {
         }
     }
 
+    /**
+     * @dev Sorts the pair (a, b) and hashes the result.
+     */
     function _hashPair(bytes32 a, bytes32 b) private pure returns (bytes32) {
         return a < b ? _efficientHash(a, b) : _efficientHash(b, a);
     }
 
+    /**
+     * @dev Implementation of keccak256(abi.encode(a, b)) that doesn't allocate or expand memory.
+     */
     function _efficientHash(bytes32 a, bytes32 b) private pure returns (bytes32 value) {
         /// @solidity memory-safe-assembly
         assembly {

+ 3 - 6
contracts/utils/cryptography/MessageHashUtils.sol

@@ -20,7 +20,7 @@ library MessageHashUtils {
      * `"\x19Ethereum Signed Message:\n32"` and hashing the result. It corresponds with the
      * hash signed when using the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`] JSON-RPC method.
      *
-     * NOTE: The `hash` parameter is intended to be the result of hashing a raw message with
+     * NOTE: The `messageHash` parameter is intended to be the result of hashing a raw message with
      * keccak256, although any bytes32 value can be safely used because the final digest will
      * be re-hashed.
      *
@@ -45,7 +45,7 @@ library MessageHashUtils {
      *
      * See {ECDSA-recover}.
      */
-    function toEthSignedMessageHash(bytes memory message) internal pure returns (bytes32 digest) {
+    function toEthSignedMessageHash(bytes memory message) internal pure returns (bytes32) {
         return
             keccak256(bytes.concat("\x19Ethereum Signed Message:\n", bytes(Strings.toString(message.length)), message));
     }
@@ -59,10 +59,7 @@ library MessageHashUtils {
      *
      * See {ECDSA-recover}.
      */
-    function toDataWithIntendedValidatorHash(
-        address validator,
-        bytes memory data
-    ) internal pure returns (bytes32 digest) {
+    function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) {
         return keccak256(abi.encodePacked(hex"19_00", validator, data));
     }
 

+ 8 - 7
contracts/utils/math/Math.sol

@@ -115,9 +115,10 @@ library Math {
     }
 
     /**
-     * @notice Calculates floor(x * y / denominator) with full precision. Throws if result overflows a uint256 or denominator == 0
-     * @dev Original credit to Remco Bloemen under MIT license (https://xn--2-umb.com/21/muldiv)
-     * with further edits by Uniswap Labs also under MIT license.
+     * @notice Calculates floor(x * y / denominator) with full precision. Throws if result overflows a uint256 or
+     * denominator == 0.
+     * @dev Original credit to Remco Bloemen under MIT license (https://xn--2-umb.com/21/muldiv) with further edits by
+     * Uniswap Labs also under MIT license.
      */
     function mulDiv(uint256 x, uint256 y, uint256 denominator) internal pure returns (uint256 result) {
         unchecked {
@@ -159,8 +160,8 @@ library Math {
                 prod0 := sub(prod0, remainder)
             }
 
-            // Factor powers of two out of denominator and compute largest power of two divisor of denominator. Always >= 1.
-            // See https://cs.stackexchange.com/q/138556/92363.
+            // Factor powers of two out of denominator and compute largest power of two divisor of denominator.
+            // Always >= 1. See https://cs.stackexchange.com/q/138556/92363.
 
             uint256 twos = denominator & (0 - denominator);
             assembly {
@@ -182,8 +183,8 @@ library Math {
             // four bits. That is, denominator * inv = 1 mod 2^4.
             uint256 inverse = (3 * denominator) ^ 2;
 
-            // Use the Newton-Raphson iteration to improve the precision. Thanks to Hensel's lifting lemma, this also works
-            // in modular arithmetic, doubling the correct bits in each step.
+            // Use the Newton-Raphson iteration to improve the precision. Thanks to Hensel's lifting lemma, this also
+            // works in modular arithmetic, doubling the correct bits in each step.
             inverse *= 2 - denominator * inverse; // inverse mod 2^8
             inverse *= 2 - denominator * inverse; // inverse mod 2^16
             inverse *= 2 - denominator * inverse; // inverse mod 2^32

+ 48 - 27
contracts/utils/structs/Checkpoints.sol

@@ -33,14 +33,16 @@ library Checkpoints {
      *
      * Returns previous value and new value.
      *
-     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint32).max` key set will disable the library.
+     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint32).max` key set will disable the
+     * library.
      */
     function push(Trace224 storage self, uint32 key, uint224 value) internal returns (uint224, uint224) {
         return _insert(self._checkpoints, key, value);
     }
 
     /**
-     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if
+     * there is none.
      */
     function lowerLookup(Trace224 storage self, uint32 key) internal view returns (uint224) {
         uint256 len = self._checkpoints.length;
@@ -49,7 +51,8 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      */
     function upperLookup(Trace224 storage self, uint32 key) internal view returns (uint224) {
         uint256 len = self._checkpoints.length;
@@ -58,9 +61,11 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      *
-     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high keys).
+     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high
+     * keys).
      */
     function upperLookupRecent(Trace224 storage self, uint32 key) internal view returns (uint224) {
         uint256 len = self._checkpoints.length;
@@ -148,8 +153,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high`
+     * if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and exclusive
+     * `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */
@@ -171,8 +177,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or
+     * `high` if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and
+     * exclusive `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */
@@ -220,14 +227,16 @@ library Checkpoints {
      *
      * Returns previous value and new value.
      *
-     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint48).max` key set will disable the library.
+     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint48).max` key set will disable the
+     * library.
      */
     function push(Trace208 storage self, uint48 key, uint208 value) internal returns (uint208, uint208) {
         return _insert(self._checkpoints, key, value);
     }
 
     /**
-     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if
+     * there is none.
      */
     function lowerLookup(Trace208 storage self, uint48 key) internal view returns (uint208) {
         uint256 len = self._checkpoints.length;
@@ -236,7 +245,8 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      */
     function upperLookup(Trace208 storage self, uint48 key) internal view returns (uint208) {
         uint256 len = self._checkpoints.length;
@@ -245,9 +255,11 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      *
-     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high keys).
+     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high
+     * keys).
      */
     function upperLookupRecent(Trace208 storage self, uint48 key) internal view returns (uint208) {
         uint256 len = self._checkpoints.length;
@@ -335,8 +347,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high`
+     * if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and exclusive
+     * `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */
@@ -358,8 +371,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or
+     * `high` if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and
+     * exclusive `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */
@@ -407,14 +421,16 @@ library Checkpoints {
      *
      * Returns previous value and new value.
      *
-     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint96).max` key set will disable the library.
+     * IMPORTANT: Never accept `key` as a user input, since an arbitrary `type(uint96).max` key set will disable the
+     * library.
      */
     function push(Trace160 storage self, uint96 key, uint160 value) internal returns (uint160, uint160) {
         return _insert(self._checkpoints, key, value);
     }
 
     /**
-     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if
+     * there is none.
      */
     function lowerLookup(Trace160 storage self, uint96 key) internal view returns (uint160) {
         uint256 len = self._checkpoints.length;
@@ -423,7 +439,8 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      */
     function upperLookup(Trace160 storage self, uint96 key) internal view returns (uint160) {
         uint256 len = self._checkpoints.length;
@@ -432,9 +449,11 @@ library Checkpoints {
     }
 
     /**
-     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+     * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+     * if there is none.
      *
-     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high keys).
+     * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high
+     * keys).
      */
     function upperLookupRecent(Trace160 storage self, uint96 key) internal view returns (uint160) {
         uint256 len = self._checkpoints.length;
@@ -522,8 +541,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or `high`
+     * if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and exclusive
+     * `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */
@@ -545,8 +565,9 @@ library Checkpoints {
     }
 
     /**
-     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or `high` if there is none.
-     * `low` and `high` define a section where to do the search, with inclusive `low` and exclusive `high`.
+     * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or
+     * `high` if there is none. `low` and `high` define a section where to do the search, with inclusive `low` and
+     * exclusive `high`.
      *
      * WARNING: `high` should not be greater than the array's length.
      */

+ 8 - 7
contracts/utils/structs/DoubleEndedQueue.sol

@@ -31,16 +31,13 @@ library DoubleEndedQueue {
     error QueueOutOfBounds();
 
     /**
-     * @dev Indices are signed integers because the queue can grow in any direction. They are 128 bits so begin and end
-     * are packed in a single storage slot for efficient access. Since the items are added one at a time we can safely
-     * assume that these 128-bit indices will not overflow, and use unchecked arithmetic.
+     * @dev Indices are 128 bits so begin and end are packed in a single storage slot for efficient access.
      *
      * Struct members have an underscore prefix indicating that they are "private" and should not be read or written to
      * directly. Use the functions provided below instead. Modifying the struct manually may violate assumptions and
      * lead to unexpected behavior.
      *
-     * Indices are in the range [begin, end) which means the first item is at data[begin] and the last item is at
-     * data[end - 1].
+     * The first item is at data[begin] and the last item is at data[end - 1]. This range can wrap around.
      */
     struct Bytes32Deque {
         uint128 _begin;
@@ -50,6 +47,8 @@ library DoubleEndedQueue {
 
     /**
      * @dev Inserts an item at the end of the queue.
+     *
+     * Reverts with {QueueFull} if the queue is full.
      */
     function pushBack(Bytes32Deque storage deque, bytes32 value) internal {
         unchecked {
@@ -63,7 +62,7 @@ library DoubleEndedQueue {
     /**
      * @dev Removes the item at the end of the queue and returns it.
      *
-     * Reverts with `QueueEmpty` if the queue is empty.
+     * Reverts with {QueueEmpty} if the queue is empty.
      */
     function popBack(Bytes32Deque storage deque) internal returns (bytes32 value) {
         unchecked {
@@ -78,6 +77,8 @@ library DoubleEndedQueue {
 
     /**
      * @dev Inserts an item at the beginning of the queue.
+     *
+     * Reverts with {QueueFull} if the queue is full.
      */
     function pushFront(Bytes32Deque storage deque, bytes32 value) internal {
         unchecked {
@@ -133,7 +134,7 @@ library DoubleEndedQueue {
      */
     function at(Bytes32Deque storage deque, uint256 index) internal view returns (bytes32 value) {
         if (index >= length(deque)) revert QueueOutOfBounds();
-        // By construction, length is a uint128, so the check above ensures that index can be safely downcast to uint128.
+        // By construction, length is a uint128, so the check above ensures that index can be safely downcast to uint128
         unchecked {
             return deque._data[deque._begin + uint128(index)];
         }

+ 4 - 8
contracts/utils/structs/EnumerableMap.sol

@@ -48,14 +48,10 @@ import {EnumerableSet} from "./EnumerableSet.sol";
 library EnumerableMap {
     using EnumerableSet for EnumerableSet.Bytes32Set;
 
-    // To implement this library for multiple types with as little code
-    // repetition as possible, we write it in terms of a generic Map type with
-    // bytes32 keys and values.
-    // The Map implementation uses private functions, and user-facing
-    // implementations (such as Uint256ToAddressMap) are just wrappers around
-    // the underlying Map.
-    // This means that we can only create new EnumerableMaps for types that fit
-    // in bytes32.
+    // To implement this library for multiple types with as little code repetition as possible, we write it in
+    // terms of a generic Map type with bytes32 keys and values. The Map implementation uses private functions,
+    // and user-facing implementations such as `UintToAddressMap` are just wrappers around the underlying Map.
+    // This means that we can only create new EnumerableMaps for types that fit in bytes32.
 
     /**
      * @dev Query for a nonexistent map key.

+ 16 - 16
contracts/utils/structs/EnumerableSet.sol

@@ -51,9 +51,9 @@ library EnumerableSet {
     struct Set {
         // Storage of set values
         bytes32[] _values;
-        // Position of the value in the `values` array, plus 1 because index 0
-        // means a value is not in the set.
-        mapping(bytes32 value => uint256) _indexes;
+        // Position is the index of the value in the `values` array plus 1.
+        // Position 0 is used to mean a value is not in the set.
+        mapping(bytes32 value => uint256) _positions;
     }
 
     /**
@@ -67,7 +67,7 @@ library EnumerableSet {
             set._values.push(value);
             // The value is stored at length-1, but we add 1 to all indexes
             // and use 0 as a sentinel value
-            set._indexes[value] = set._values.length;
+            set._positions[value] = set._values.length;
             return true;
         } else {
             return false;
@@ -81,32 +81,32 @@ library EnumerableSet {
      * present.
      */
     function _remove(Set storage set, bytes32 value) private returns (bool) {
-        // We read and store the value's index to prevent multiple reads from the same storage slot
-        uint256 valueIndex = set._indexes[value];
+        // We cache the value's position to prevent multiple reads from the same storage slot
+        uint256 position = set._positions[value];
 
-        if (valueIndex != 0) {
+        if (position != 0) {
             // Equivalent to contains(set, value)
             // To delete an element from the _values array in O(1), we swap the element to delete with the last one in
             // the array, and then remove the last element (sometimes called as 'swap and pop').
             // This modifies the order of the array, as noted in {at}.
 
-            uint256 toDeleteIndex = valueIndex - 1;
+            uint256 valueIndex = position - 1;
             uint256 lastIndex = set._values.length - 1;
 
-            if (lastIndex != toDeleteIndex) {
+            if (valueIndex != lastIndex) {
                 bytes32 lastValue = set._values[lastIndex];
 
-                // Move the last value to the index where the value to delete is
-                set._values[toDeleteIndex] = lastValue;
-                // Update the index for the moved value
-                set._indexes[lastValue] = valueIndex; // Replace lastValue's index to valueIndex
+                // Move the lastValue to the index where the value to delete is
+                set._values[valueIndex] = lastValue;
+                // Update the tracked position of the lastValue (that was just moved)
+                set._positions[lastValue] = position;
             }
 
             // Delete the slot where the moved value was stored
             set._values.pop();
 
-            // Delete the index for the deleted slot
-            delete set._indexes[value];
+            // Delete the tracked position for the deleted slot
+            delete set._positions[value];
 
             return true;
         } else {
@@ -118,7 +118,7 @@ library EnumerableSet {
      * @dev Returns true if the value is in the set. O(1).
      */
     function _contains(Set storage set, bytes32 value) private view returns (bool) {
-        return set._indexes[value] != 0;
+        return set._positions[value] != 0;
     }
 
     /**

+ 125 - 0
contracts/utils/types/Time.sol

@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {Math} from "../math/Math.sol";
+import {SafeCast} from "../math/SafeCast.sol";
+
+/**
+ * @dev This library provides helpers for manipulating time-related objects.
+ *
+ * It uses the following types:
+ * - `uint48` for timepoints
+ * - `uint32` for durations
+ *
+ * While the library doesn't provide specific types for timepoints and duration, it does provide:
+ * - a `Delay` type to represent duration that can be programmed to change value automatically at a given point
+ * - additional helper functions
+ */
+library Time {
+    using Time for *;
+
+    /**
+     * @dev Get the block timestamp as a Timepoint.
+     */
+    function timestamp() internal view returns (uint48) {
+        return SafeCast.toUint48(block.timestamp);
+    }
+
+    /**
+     * @dev Get the block number as a Timepoint.
+     */
+    function blockNumber() internal view returns (uint48) {
+        return SafeCast.toUint48(block.number);
+    }
+
+    // ==================================================== Delay =====================================================
+    /**
+     * @dev A `Delay` is a uint32 duration that can be programmed to change value automatically at a given point in the
+     * future. The "effect" timepoint describes when the transitions happens from the "old" value to the "new" value.
+     * This allows updating the delay applied to some operation while keeping some guarantees.
+     *
+     * In particular, the {update} function guarantees that if the delay is reduced, the old delay still applies for
+     * some time. For example if the delay is currently 7 days to do an upgrade, the admin should not be able to set
+     * the delay to 0 and upgrade immediately. If the admin wants to reduce the delay, the old delay (7 days) should
+     * still apply for some time.
+     *
+     *
+     * The `Delay` type is 112 bits long, and packs the following:
+     *
+     * ```
+     *   | [uint48]: effect date (timepoint)
+     *   |           | [uint32]: value before (duration)
+     *   ↓           ↓       ↓ [uint32]: value after (duration)
+     * 0xAAAAAAAAAAAABBBBBBBBCCCCCCCC
+     * ```
+     *
+     * NOTE: The {get} and {withUpdate} functions operate using timestamps. Block number based delays are not currently
+     * supported.
+     */
+    type Delay is uint112;
+
+    /**
+     * @dev Wrap a duration into a Delay to add the one-step "update in the future" feature
+     */
+    function toDelay(uint32 duration) internal pure returns (Delay) {
+        return Delay.wrap(duration);
+    }
+
+    /**
+     * @dev Get the value at a given timepoint plus the pending value and effect timepoint if there is a scheduled
+     * change after this timepoint. If the effect timepoint is 0, then the pending value should not be considered.
+     */
+    function _getFullAt(Delay self, uint48 timepoint) private pure returns (uint32, uint32, uint48) {
+        (uint32 valueBefore, uint32 valueAfter, uint48 effect) = self.unpack();
+        return effect <= timepoint ? (valueAfter, 0, 0) : (valueBefore, valueAfter, effect);
+    }
+
+    /**
+     * @dev Get the current value plus the pending value and effect timepoint if there is a scheduled change. If the
+     * effect timepoint is 0, then the pending value should not be considered.
+     */
+    function getFull(Delay self) internal view returns (uint32, uint32, uint48) {
+        return _getFullAt(self, timestamp());
+    }
+
+    /**
+     * @dev Get the current value.
+     */
+    function get(Delay self) internal view returns (uint32) {
+        (uint32 delay, , ) = self.getFull();
+        return delay;
+    }
+
+    /**
+     * @dev Update a Delay object so that it takes a new duration after a timepoint that is automatically computed to
+     * enforce the old delay at the moment of the update. Returns the updated Delay object and the timestamp when the
+     * new delay becomes effective.
+     */
+    function withUpdate(Delay self, uint32 newValue, uint32 minSetback) internal view returns (Delay, uint48) {
+        uint32 value = self.get();
+        uint32 setback = uint32(Math.max(minSetback, value > newValue ? value - newValue : 0));
+        uint48 effect = timestamp() + setback;
+        return (pack(value, newValue, effect), effect);
+    }
+
+    /**
+     * @dev Split a delay into its components: valueBefore, valueAfter and effect (transition timepoint).
+     */
+    function unpack(Delay self) internal pure returns (uint32, uint32, uint48) {
+        uint112 raw = Delay.unwrap(self);
+
+        uint32 valueAfter = uint32(raw);
+        uint32 valueBefore = uint32(raw >> 32);
+        uint48 effect = uint48(raw >> 64);
+
+        return (valueBefore, valueAfter, effect);
+    }
+
+    /**
+     * @dev pack the components into a Delay object.
+     */
+    function pack(uint32 valueBefore, uint32 valueAfter, uint48 effect) internal pure returns (Delay) {
+        return Delay.wrap((uint112(effect) << 64) | (uint112(valueBefore) << 32) | uint112(valueAfter));
+    }
+}

+ 1 - 1
contracts/vendor/compound/ICompoundTimelock.sol

@@ -4,7 +4,7 @@
 pragma solidity ^0.8.20;
 
 /**
- * https://github.com/compound-finance/compound-protocol/blob/master/contracts/Timelock.sol[Compound's timelock] interface
+ * https://github.com/compound-finance/compound-protocol/blob/master/contracts/Timelock.sol[Compound timelock] interface
  */
 interface ICompoundTimelock {
     event NewAdmin(address indexed newAdmin);

+ 6 - 6
package-lock.json

@@ -12864,9 +12864,9 @@
       }
     },
     "node_modules/solidity-coverage/node_modules/@solidity-parser/parser": {
-      "version": "0.16.0",
-      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.0.tgz",
-      "integrity": "sha512-ESipEcHyRHg4Np4SqBCfcXwyxxna1DgFVz69bgpLV8vzl/NP1DtcKsJ4dJZXWQhY/Z4J2LeKBiOkOVZn9ct33Q==",
+      "version": "0.16.1",
+      "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.1.tgz",
+      "integrity": "sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==",
       "dev": true,
       "dependencies": {
         "antlr4ts": "^0.5.0-alpha.4"
@@ -25379,9 +25379,9 @@
       },
       "dependencies": {
         "@solidity-parser/parser": {
-          "version": "0.16.0",
-          "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.0.tgz",
-          "integrity": "sha512-ESipEcHyRHg4Np4SqBCfcXwyxxna1DgFVz69bgpLV8vzl/NP1DtcKsJ4dJZXWQhY/Z4J2LeKBiOkOVZn9ct33Q==",
+          "version": "0.16.1",
+          "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.1.tgz",
+          "integrity": "sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==",
           "dev": true,
           "requires": {
             "antlr4ts": "^0.5.0-alpha.4"

+ 16 - 9
scripts/generate/templates/Checkpoints.js

@@ -38,7 +38,8 @@ struct ${opts.checkpointTypeName} {
  *
  * Returns previous value and new value.
  * 
- * IMPORTANT: Never accept \`key\` as a user input, since an arbitrary \`type(${opts.keyTypeName}).max\` key set will disable the library.
+ * IMPORTANT: Never accept \`key\` as a user input, since an arbitrary \`type(${opts.keyTypeName}).max\` key set will disable the
+ * library.
  */
 function push(
     ${opts.historyTypeName} storage self,
@@ -49,7 +50,8 @@ function push(
 }
 
 /**
- * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if there is none.
+ * @dev Returns the value in the first (oldest) checkpoint with key greater or equal than the search key, or zero if
+ * there is none.
  */
 function lowerLookup(${opts.historyTypeName} storage self, ${opts.keyTypeName} key) internal view returns (${opts.valueTypeName}) {
     uint256 len = self.${opts.checkpointFieldName}.length;
@@ -58,7 +60,8 @@ function lowerLookup(${opts.historyTypeName} storage self, ${opts.keyTypeName} k
 }
 
 /**
- * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+ * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+ * if there is none.
  */
 function upperLookup(${opts.historyTypeName} storage self, ${opts.keyTypeName} key) internal view returns (${opts.valueTypeName}) {
     uint256 len = self.${opts.checkpointFieldName}.length;
@@ -67,9 +70,11 @@ function upperLookup(${opts.historyTypeName} storage self, ${opts.keyTypeName} k
 }
 
 /**
- * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero if there is none.
+ * @dev Returns the value in the last (most recent) checkpoint with key lower or equal than the search key, or zero
+ * if there is none.
  *
- * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high keys).
+ * NOTE: This is a variant of {upperLookup} that is optimised to find "recent" checkpoint (checkpoints with high
+ * keys).
  */
 function upperLookupRecent(${opts.historyTypeName} storage self, ${opts.keyTypeName} key) internal view returns (${opts.valueTypeName}) {
     uint256 len = self.${opts.checkpointFieldName}.length;
@@ -169,8 +174,9 @@ function _insert(
 }
 
 /**
- * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or \`high\` if there is none.
- * \`low\` and \`high\` define a section where to do the search, with inclusive \`low\` and exclusive \`high\`.
+ * @dev Return the index of the last (most recent) checkpoint with key lower or equal than the search key, or \`high\`
+ * if there is none. \`low\` and \`high\` define a section where to do the search, with inclusive \`low\` and exclusive
+ * \`high\`.
  *
  * WARNING: \`high\` should not be greater than the array's length.
  */
@@ -192,8 +198,9 @@ function _upperBinaryLookup(
 }
 
 /**
- * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or \`high\` if there is none.
- * \`low\` and \`high\` define a section where to do the search, with inclusive \`low\` and exclusive \`high\`.
+ * @dev Return the index of the first (oldest) checkpoint with key is greater or equal than the search key, or
+ * \`high\` if there is none. \`low\` and \`high\` define a section where to do the search, with inclusive \`low\` and
+ * exclusive \`high\`.
  *
  * WARNING: \`high\` should not be greater than the array's length.
  */

+ 4 - 8
scripts/generate/templates/EnumerableMap.js

@@ -57,14 +57,10 @@ import {EnumerableSet} from "./EnumerableSet.sol";
 /* eslint-enable max-len */
 
 const defaultMap = () => `\
-// To implement this library for multiple types with as little code
-// repetition as possible, we write it in terms of a generic Map type with
-// bytes32 keys and values.
-// The Map implementation uses private functions, and user-facing
-// implementations (such as Uint256ToAddressMap) are just wrappers around
-// the underlying Map.
-// This means that we can only create new EnumerableMaps for types that fit
-// in bytes32.
+// To implement this library for multiple types with as little code repetition as possible, we write it in
+// terms of a generic Map type with bytes32 keys and values. The Map implementation uses private functions,
+// and user-facing implementations such as \`UintToAddressMap\` are just wrappers around the underlying Map.
+// This means that we can only create new EnumerableMaps for types that fit in bytes32.
 
 /**
  * @dev Query for a nonexistent map key.

+ 16 - 16
scripts/generate/templates/EnumerableSet.js

@@ -61,9 +61,9 @@ const defaultSet = () => `\
 struct Set {
     // Storage of set values
     bytes32[] _values;
-    // Position of the value in the \`values\` array, plus 1 because index 0
-    // means a value is not in the set.
-    mapping(bytes32 value => uint256) _indexes;
+    // Position is the index of the value in the \`values\` array plus 1.
+    // Position 0 is used to mean a value is not in the set.
+    mapping(bytes32 value => uint256) _positions;
 }
 
 /**
@@ -77,7 +77,7 @@ function _add(Set storage set, bytes32 value) private returns (bool) {
         set._values.push(value);
         // The value is stored at length-1, but we add 1 to all indexes
         // and use 0 as a sentinel value
-        set._indexes[value] = set._values.length;
+        set._positions[value] = set._values.length;
         return true;
     } else {
         return false;
@@ -91,32 +91,32 @@ function _add(Set storage set, bytes32 value) private returns (bool) {
  * present.
  */
 function _remove(Set storage set, bytes32 value) private returns (bool) {
-    // We read and store the value's index to prevent multiple reads from the same storage slot
-    uint256 valueIndex = set._indexes[value];
+    // We cache the value's position to prevent multiple reads from the same storage slot
+    uint256 position = set._positions[value];
 
-    if (valueIndex != 0) {
+    if (position != 0) {
         // Equivalent to contains(set, value)
         // To delete an element from the _values array in O(1), we swap the element to delete with the last one in
         // the array, and then remove the last element (sometimes called as 'swap and pop').
         // This modifies the order of the array, as noted in {at}.
 
-        uint256 toDeleteIndex = valueIndex - 1;
+        uint256 valueIndex = position - 1;
         uint256 lastIndex = set._values.length - 1;
 
-        if (lastIndex != toDeleteIndex) {
+        if (valueIndex != lastIndex) {
             bytes32 lastValue = set._values[lastIndex];
 
-            // Move the last value to the index where the value to delete is
-            set._values[toDeleteIndex] = lastValue;
-            // Update the index for the moved value
-            set._indexes[lastValue] = valueIndex; // Replace lastValue's index to valueIndex
+            // Move the lastValue to the index where the value to delete is
+            set._values[valueIndex] = lastValue;
+            // Update the tracked position of the lastValue (that was just moved)
+            set._positions[lastValue] = position;
         }
 
         // Delete the slot where the moved value was stored
         set._values.pop();
 
-        // Delete the index for the deleted slot
-        delete set._indexes[value];
+        // Delete the tracked position for the deleted slot
+        delete set._positions[value];
 
         return true;
     } else {
@@ -128,7 +128,7 @@ function _remove(Set storage set, bytes32 value) private returns (bool) {
  * @dev Returns true if the value is in the set. O(1).
  */
 function _contains(Set storage set, bytes32 value) private view returns (bool) {
-    return set._indexes[value] != 0;
+    return set._positions[value] != 0;
 }
 
 /**

+ 1161 - 0
test/access/manager/AccessManager.test.js

@@ -0,0 +1,1161 @@
+const { web3 } = require('hardhat');
+const { constants, expectEvent, time } = require('@openzeppelin/test-helpers');
+const { expectRevertCustomError } = require('../../helpers/customError');
+const { selector } = require('../../helpers/methods');
+const { clockFromReceipt } = require('../../helpers/time');
+const { product } = require('../../helpers/iterate');
+const helpers = require('@nomicfoundation/hardhat-network-helpers');
+
+const AccessManager = artifacts.require('$AccessManager');
+const AccessManagedTarget = artifacts.require('$AccessManagedTarget');
+const Ownable = artifacts.require('$Ownable');
+
+const MAX_UINT64 = web3.utils.toBN((2n ** 64n - 1n).toString());
+
+const ROLES = {
+  ADMIN: web3.utils.toBN(0),
+  SOME_ADMIN: web3.utils.toBN(17),
+  SOME: web3.utils.toBN(42),
+  PUBLIC: MAX_UINT64,
+};
+Object.assign(ROLES, Object.fromEntries(Object.entries(ROLES).map(([key, value]) => [value, key])));
+
+const executeDelay = web3.utils.toBN(10);
+const grantDelay = web3.utils.toBN(10);
+const MINSETBACK = time.duration.days(5);
+
+const formatAccess = access => [access[0], access[1].toString()];
+
+contract('AccessManager', function (accounts) {
+  const [admin, manager, member, user, other] = accounts;
+
+  beforeEach(async function () {
+    this.manager = await AccessManager.new(admin);
+
+    // add member to role
+    await this.manager.$_setRoleAdmin(ROLES.SOME, ROLES.SOME_ADMIN);
+    await this.manager.$_setRoleGuardian(ROLES.SOME, ROLES.SOME_ADMIN);
+    await this.manager.$_grantRole(ROLES.SOME_ADMIN, manager, 0, 0);
+    await this.manager.$_grantRole(ROLES.SOME, member, 0, 0);
+  });
+
+  it('rejects zero address for initialAdmin', async function () {
+    await expectRevertCustomError(AccessManager.new(constants.ZERO_ADDRESS), 'AccessManagerInvalidInitialAdmin', [
+      constants.ZERO_ADDRESS,
+    ]);
+  });
+
+  it('default minsetback is 1 day', async function () {
+    expect(await this.manager.minSetback()).to.be.bignumber.equal(MINSETBACK);
+  });
+
+  it('roles are correctly initialized', async function () {
+    // role admin
+    expect(await this.manager.getRoleAdmin(ROLES.ADMIN)).to.be.bignumber.equal(ROLES.ADMIN);
+    expect(await this.manager.getRoleAdmin(ROLES.SOME_ADMIN)).to.be.bignumber.equal(ROLES.ADMIN);
+    expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN);
+    expect(await this.manager.getRoleAdmin(ROLES.PUBLIC)).to.be.bignumber.equal(ROLES.ADMIN);
+    // role guardian
+    expect(await this.manager.getRoleGuardian(ROLES.ADMIN)).to.be.bignumber.equal(ROLES.ADMIN);
+    expect(await this.manager.getRoleGuardian(ROLES.SOME_ADMIN)).to.be.bignumber.equal(ROLES.ADMIN);
+    expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN);
+    expect(await this.manager.getRoleGuardian(ROLES.PUBLIC)).to.be.bignumber.equal(ROLES.ADMIN);
+    // role members
+    expect(await this.manager.hasRole(ROLES.ADMIN, admin).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.ADMIN, manager).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME_ADMIN, admin).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME_ADMIN, manager).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME_ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME_ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME, admin).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME, manager).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+    expect(await this.manager.hasRole(ROLES.PUBLIC, admin).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.PUBLIC, manager).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.PUBLIC, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+    expect(await this.manager.hasRole(ROLES.PUBLIC, user).then(formatAccess)).to.be.deep.equal([true, '0']);
+  });
+
+  describe('Roles management', function () {
+    describe('label role', function () {
+      it('admin can emit a label event', async function () {
+        expectEvent(await this.manager.labelRole(ROLES.SOME, 'Some label', { from: admin }), 'RoleLabel', {
+          roleId: ROLES.SOME,
+          label: 'Some label',
+        });
+      });
+
+      it('admin can re-emit a label event', async function () {
+        await this.manager.labelRole(ROLES.SOME, 'Some label', { from: admin });
+
+        expectEvent(await this.manager.labelRole(ROLES.SOME, 'Updated label', { from: admin }), 'RoleLabel', {
+          roleId: ROLES.SOME,
+          label: 'Updated label',
+        });
+      });
+
+      it('emitting a label is restricted', async function () {
+        await expectRevertCustomError(
+          this.manager.labelRole(ROLES.SOME, 'Invalid label', { from: other }),
+          'AccessManagerUnauthorizedAccount',
+          [other, ROLES.ADMIN],
+        );
+      });
+    });
+
+    describe('grant role', function () {
+      describe('without a grant delay', function () {
+        it('without an execute delay', async function () {
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+          const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager });
+          const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+          expectEvent(receipt, 'RoleGranted', {
+            roleId: ROLES.SOME,
+            account: user,
+            since: timestamp,
+            delay: '0',
+            newMember: true,
+          });
+
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']);
+
+          const access = await this.manager.getAccess(ROLES.SOME, user);
+          expect(access[0]).to.be.bignumber.equal(timestamp); // inRoleSince
+          expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+          expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+          expect(access[3]).to.be.bignumber.equal('0'); // effect
+        });
+
+        it('with an execute delay', async function () {
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+          const { receipt } = await this.manager.grantRole(ROLES.SOME, user, executeDelay, { from: manager });
+          const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+          expectEvent(receipt, 'RoleGranted', {
+            roleId: ROLES.SOME,
+            account: user,
+            since: timestamp,
+            delay: executeDelay,
+            newMember: true,
+          });
+
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([
+            true,
+            executeDelay.toString(),
+          ]);
+
+          const access = await this.manager.getAccess(ROLES.SOME, user);
+          expect(access[0]).to.be.bignumber.equal(timestamp); // inRoleSince
+          expect(access[1]).to.be.bignumber.equal(executeDelay); // currentDelay
+          expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+          expect(access[3]).to.be.bignumber.equal('0'); // effect
+        });
+
+        it('to a user that is already in the role', async function () {
+          expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+          await this.manager.grantRole(ROLES.SOME, member, 0, { from: manager });
+          expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+        });
+
+        it('to a user that is scheduled for joining the role', async function () {
+          await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+          await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager });
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+        });
+
+        it('grant role is restricted', async function () {
+          await expectRevertCustomError(
+            this.manager.grantRole(ROLES.SOME, user, 0, { from: other }),
+            'AccessManagerUnauthorizedAccount',
+            [other, ROLES.SOME_ADMIN],
+          );
+        });
+      });
+
+      describe('with a grant delay', function () {
+        beforeEach(async function () {
+          await this.manager.$_setGrantDelay(ROLES.SOME, grantDelay);
+          await time.increase(MINSETBACK);
+        });
+
+        it('granted role is not active immediately', async function () {
+          const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager });
+          const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+          expectEvent(receipt, 'RoleGranted', {
+            roleId: ROLES.SOME,
+            account: user,
+            since: timestamp.add(grantDelay),
+            delay: '0',
+            newMember: true,
+          });
+
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+          const access = await this.manager.getAccess(ROLES.SOME, user);
+          expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inRoleSince
+          expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+          expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+          expect(access[3]).to.be.bignumber.equal('0'); // effect
+        });
+
+        it('granted role is active after the delay', async function () {
+          const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager });
+          const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+          expectEvent(receipt, 'RoleGranted', {
+            roleId: ROLES.SOME,
+            account: user,
+            since: timestamp.add(grantDelay),
+            delay: '0',
+            newMember: true,
+          });
+
+          await time.increase(grantDelay);
+
+          expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']);
+
+          const access = await this.manager.getAccess(ROLES.SOME, user);
+          expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inRoleSince
+          expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+          expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+          expect(access[3]).to.be.bignumber.equal('0'); // effect
+        });
+      });
+
+      it('cannot grant public role', async function () {
+        await expectRevertCustomError(
+          this.manager.$_grantRole(ROLES.PUBLIC, other, 0, executeDelay, { from: manager }),
+          'AccessManagerLockedRole',
+          [ROLES.PUBLIC],
+        );
+      });
+    });
+
+    describe('revoke role', function () {
+      it('from a user that is already in the role', async function () {
+        expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+
+        const { receipt } = await this.manager.revokeRole(ROLES.SOME, member, { from: manager });
+        expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: member });
+
+        expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const access = await this.manager.getAccess(ROLES.SOME, user);
+        expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince
+        expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+        expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(access[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('from a user that is scheduled for joining the role', async function () {
+        await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10
+
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const { receipt } = await this.manager.revokeRole(ROLES.SOME, user, { from: manager });
+        expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: user });
+
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const access = await this.manager.getAccess(ROLES.SOME, user);
+        expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince
+        expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+        expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(access[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('from a user that is not in the role', async function () {
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+        await this.manager.revokeRole(ROLES.SOME, user, { from: manager });
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+      });
+
+      it('revoke role is restricted', async function () {
+        await expectRevertCustomError(
+          this.manager.revokeRole(ROLES.SOME, member, { from: other }),
+          'AccessManagerUnauthorizedAccount',
+          [other, ROLES.SOME_ADMIN],
+        );
+      });
+    });
+
+    describe('renounce role', function () {
+      it('for a user that is already in the role', async function () {
+        expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']);
+
+        const { receipt } = await this.manager.renounceRole(ROLES.SOME, member, { from: member });
+        expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: member });
+
+        expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const access = await this.manager.getAccess(ROLES.SOME, member);
+        expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince
+        expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+        expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(access[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('for a user that is schedule for joining the role', async function () {
+        await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10
+
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const { receipt } = await this.manager.renounceRole(ROLES.SOME, user, { from: user });
+        expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: user });
+
+        expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']);
+
+        const access = await this.manager.getAccess(ROLES.SOME, user);
+        expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince
+        expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+        expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(access[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('for a user that is not in the role', async function () {
+        await this.manager.renounceRole(ROLES.SOME, user, { from: user });
+      });
+
+      it('bad user confirmation', async function () {
+        await expectRevertCustomError(
+          this.manager.renounceRole(ROLES.SOME, member, { from: user }),
+          'AccessManagerBadConfirmation',
+          [],
+        );
+      });
+    });
+
+    describe('change role admin', function () {
+      it("admin can set any role's admin", async function () {
+        expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN);
+
+        const { receipt } = await this.manager.setRoleAdmin(ROLES.SOME, ROLES.ADMIN, { from: admin });
+        expectEvent(receipt, 'RoleAdminChanged', { roleId: ROLES.SOME, admin: ROLES.ADMIN });
+
+        expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.ADMIN);
+      });
+
+      it("setting a role's admin is restricted", async function () {
+        await expectRevertCustomError(
+          this.manager.setRoleAdmin(ROLES.SOME, ROLES.SOME, { from: manager }),
+          'AccessManagerUnauthorizedAccount',
+          [manager, ROLES.ADMIN],
+        );
+      });
+    });
+
+    describe('change role guardian', function () {
+      it("admin can set any role's admin", async function () {
+        expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN);
+
+        const { receipt } = await this.manager.setRoleGuardian(ROLES.SOME, ROLES.ADMIN, { from: admin });
+        expectEvent(receipt, 'RoleGuardianChanged', { roleId: ROLES.SOME, guardian: ROLES.ADMIN });
+
+        expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.ADMIN);
+      });
+
+      it("setting a role's admin is restricted", async function () {
+        await expectRevertCustomError(
+          this.manager.setRoleGuardian(ROLES.SOME, ROLES.SOME, { from: other }),
+          'AccessManagerUnauthorizedAccount',
+          [other, ROLES.ADMIN],
+        );
+      });
+    });
+
+    describe('change execution delay', function () {
+      it('increasing the delay has immediate effect', async function () {
+        const oldDelay = web3.utils.toBN(10);
+        const newDelay = web3.utils.toBN(100);
+
+        // role is already granted (with no delay) in the initial setup. this update takes time.
+        await this.manager.$_grantRole(ROLES.SOME, member, 0, oldDelay);
+
+        const accessBefore = await this.manager.getAccess(ROLES.SOME, member);
+        expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay
+        expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect
+
+        const { receipt } = await this.manager.grantRole(ROLES.SOME, member, newDelay, {
+          from: manager,
+        });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+        expectEvent(receipt, 'RoleGranted', {
+          roleId: ROLES.SOME,
+          account: member,
+          since: timestamp,
+          delay: newDelay,
+          newMember: false,
+        });
+
+        // immediate effect
+        const accessAfter = await this.manager.getAccess(ROLES.SOME, member);
+        expect(accessAfter[1]).to.be.bignumber.equal(newDelay); // currentDelay
+        expect(accessAfter[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(accessAfter[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('decreasing the delay takes time', async function () {
+        const oldDelay = web3.utils.toBN(100);
+        const newDelay = web3.utils.toBN(10);
+
+        // role is already granted (with no delay) in the initial setup. this update takes time.
+        await this.manager.$_grantRole(ROLES.SOME, member, 0, oldDelay);
+
+        const accessBefore = await this.manager.getAccess(ROLES.SOME, member);
+        expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay
+        expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect
+
+        const { receipt } = await this.manager.grantRole(ROLES.SOME, member, newDelay, {
+          from: manager,
+        });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+        const setback = oldDelay.sub(newDelay);
+
+        expectEvent(receipt, 'RoleGranted', {
+          roleId: ROLES.SOME,
+          account: member,
+          since: timestamp.add(setback),
+          delay: newDelay,
+          newMember: false,
+        });
+
+        // no immediate effect
+        const accessAfter = await this.manager.getAccess(ROLES.SOME, member);
+        expect(accessAfter[1]).to.be.bignumber.equal(oldDelay); // currentDelay
+        expect(accessAfter[2]).to.be.bignumber.equal(newDelay); // pendingDelay
+        expect(accessAfter[3]).to.be.bignumber.equal(timestamp.add(setback)); // effect
+
+        // delayed effect
+        await time.increase(setback);
+        const accessAfterSetback = await this.manager.getAccess(ROLES.SOME, member);
+        expect(accessAfterSetback[1]).to.be.bignumber.equal(newDelay); // currentDelay
+        expect(accessAfterSetback[2]).to.be.bignumber.equal('0'); // pendingDelay
+        expect(accessAfterSetback[3]).to.be.bignumber.equal('0'); // effect
+      });
+
+      it('can set a user execution delay during the grant delay', async function () {
+        await this.manager.$_grantRole(ROLES.SOME, other, 10, 0);
+        // here: "other" is pending to get the role, but doesn't yet have it.
+
+        const { receipt } = await this.manager.grantRole(ROLES.SOME, other, executeDelay, { from: manager });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+        // increasing the execution delay from 0 to executeDelay is immediate
+        expectEvent(receipt, 'RoleGranted', {
+          roleId: ROLES.SOME,
+          account: other,
+          since: timestamp,
+          delay: executeDelay,
+          newMember: false,
+        });
+      });
+    });
+
+    describe('change grant delay', function () {
+      it('increasing the delay has immediate effect', async function () {
+        const oldDelay = web3.utils.toBN(10);
+        const newDelay = web3.utils.toBN(100);
+
+        await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay);
+        await time.increase(MINSETBACK);
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+
+        const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+        const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay));
+
+        expect(setback).to.be.bignumber.equal(MINSETBACK);
+        expectEvent(receipt, 'RoleGrantDelayChanged', {
+          roleId: ROLES.SOME,
+          delay: newDelay,
+          since: timestamp.add(setback),
+        });
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+        await time.increase(setback);
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay);
+      });
+
+      it('increasing the delay has delay effect #1', async function () {
+        const oldDelay = web3.utils.toBN(100);
+        const newDelay = web3.utils.toBN(10);
+
+        await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay);
+        await time.increase(MINSETBACK);
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+
+        const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+        const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay));
+
+        expect(setback).to.be.bignumber.equal(MINSETBACK);
+        expectEvent(receipt, 'RoleGrantDelayChanged', {
+          roleId: ROLES.SOME,
+          delay: newDelay,
+          since: timestamp.add(setback),
+        });
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+        await time.increase(setback);
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay);
+      });
+
+      it('increasing the delay has delay effect #2', async function () {
+        const oldDelay = time.duration.days(30); // more than the minsetback
+        const newDelay = web3.utils.toBN(10);
+
+        await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay);
+        await time.increase(MINSETBACK);
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+
+        const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin });
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+        const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay));
+
+        expect(setback).to.be.bignumber.gt(MINSETBACK);
+        expectEvent(receipt, 'RoleGrantDelayChanged', {
+          roleId: ROLES.SOME,
+          delay: newDelay,
+          since: timestamp.add(setback),
+        });
+
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay);
+        await time.increase(setback);
+        expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay);
+      });
+
+      it('changing the grant delay is restricted', async function () {
+        await expectRevertCustomError(
+          this.manager.setGrantDelay(ROLES.SOME, grantDelay, { from: other }),
+          'AccessManagerUnauthorizedAccount',
+          [ROLES.ADMIN, other],
+        );
+      });
+    });
+  });
+
+  describe('with AccessManaged target contract', function () {
+    beforeEach('deploy target contract', async function () {
+      this.target = await AccessManagedTarget.new(this.manager.address);
+      // helpers for indirect calls
+      this.callData = selector('fnRestricted()');
+      this.call = [this.target.address, this.callData];
+      this.opId = web3.utils.keccak256(
+        web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [user, ...this.call]),
+      );
+      this.direct = (opts = {}) => this.target.fnRestricted({ from: user, ...opts });
+      this.schedule = (opts = {}) => this.manager.schedule(...this.call, 0, { from: user, ...opts });
+      this.execute = (opts = {}) => this.manager.execute(...this.call, { from: user, ...opts });
+      this.cancel = (opts = {}) => this.manager.cancel(user, ...this.call, { from: user, ...opts });
+    });
+
+    describe('Change function permissions', function () {
+      const sigs = ['someFunction()', 'someOtherFunction(uint256)', 'oneMoreFunction(address,uint8)'].map(selector);
+
+      it('admin can set function role', async function () {
+        for (const sig of sigs) {
+          expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal(ROLES.ADMIN);
+        }
+
+        const { receipt: receipt1 } = await this.manager.setTargetFunctionRole(this.target.address, sigs, ROLES.SOME, {
+          from: admin,
+        });
+
+        for (const sig of sigs) {
+          expectEvent(receipt1, 'TargetFunctionRoleUpdated', {
+            target: this.target.address,
+            selector: sig,
+            roleId: ROLES.SOME,
+          });
+          expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal(ROLES.SOME);
+        }
+
+        const { receipt: receipt2 } = await this.manager.setTargetFunctionRole(
+          this.target.address,
+          [sigs[1]],
+          ROLES.SOME_ADMIN,
+          {
+            from: admin,
+          },
+        );
+        expectEvent(receipt2, 'TargetFunctionRoleUpdated', {
+          target: this.target.address,
+          selector: sigs[1],
+          roleId: ROLES.SOME_ADMIN,
+        });
+
+        for (const sig of sigs) {
+          expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal(
+            sig == sigs[1] ? ROLES.SOME_ADMIN : ROLES.SOME,
+          );
+        }
+      });
+
+      it('non-admin cannot set function role', async function () {
+        await expectRevertCustomError(
+          this.manager.setTargetFunctionRole(this.target.address, sigs, ROLES.SOME, { from: other }),
+          'AccessManagerUnauthorizedAccount',
+          [other, ROLES.ADMIN],
+        );
+      });
+    });
+
+    // WIP
+    describe('Calling restricted & unrestricted functions', function () {
+      for (const [callerRoles, fnRole, closed, delay] of product(
+        [[], [ROLES.SOME]],
+        [undefined, ROLES.ADMIN, ROLES.SOME, ROLES.PUBLIC],
+        [false, true],
+        [null, executeDelay],
+      )) {
+        // can we call with a delay ?
+        const indirectSuccess = (fnRole == ROLES.PUBLIC || callerRoles.includes(fnRole)) && !closed;
+
+        // can we call without a delay ?
+        const directSuccess = (fnRole == ROLES.PUBLIC || (callerRoles.includes(fnRole) && !delay)) && !closed;
+
+        const description = [
+          'Caller in roles',
+          '[' + (callerRoles ?? []).map(roleId => ROLES[roleId]).join(', ') + ']',
+          delay ? 'with a delay' : 'without a delay',
+          '+',
+          'functions open to roles',
+          '[' + (ROLES[fnRole] ?? '') + ']',
+          closed ? `(closed)` : '',
+        ].join(' ');
+
+        describe(description, function () {
+          beforeEach(async function () {
+            // setup
+            await Promise.all([
+              this.manager.$_setTargetClosed(this.target.address, closed),
+              fnRole && this.manager.$_setTargetFunctionRole(this.target.address, selector('fnRestricted()'), fnRole),
+              fnRole && this.manager.$_setTargetFunctionRole(this.target.address, selector('fnUnrestricted()'), fnRole),
+              ...callerRoles
+                .filter(roleId => roleId != ROLES.PUBLIC)
+                .map(roleId => this.manager.$_grantRole(roleId, user, 0, delay ?? 0)),
+            ]);
+
+            // post setup checks
+            expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(closed);
+
+            if (fnRole) {
+              expect(
+                await this.manager.getTargetFunctionRole(this.target.address, selector('fnRestricted()')),
+              ).to.be.bignumber.equal(fnRole);
+              expect(
+                await this.manager.getTargetFunctionRole(this.target.address, selector('fnUnrestricted()')),
+              ).to.be.bignumber.equal(fnRole);
+            }
+
+            for (const roleId of callerRoles) {
+              const access = await this.manager.getAccess(roleId, user);
+              if (roleId == ROLES.PUBLIC) {
+                expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince
+                expect(access[1]).to.be.bignumber.equal('0'); // currentDelay
+                expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+                expect(access[3]).to.be.bignumber.equal('0'); // effect
+              } else {
+                expect(access[0]).to.be.bignumber.gt('0'); // inRoleSince
+                expect(access[1]).to.be.bignumber.eq(String(delay ?? 0)); // currentDelay
+                expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay
+                expect(access[3]).to.be.bignumber.equal('0'); // effect
+              }
+            }
+          });
+
+          it('canCall', async function () {
+            const result = await this.manager.canCall(user, this.target.address, selector('fnRestricted()'));
+            expect(result[0]).to.be.equal(directSuccess);
+            expect(result[1]).to.be.bignumber.equal(!directSuccess && indirectSuccess ? delay ?? '0' : '0');
+          });
+
+          it('Calling a non restricted function never revert', async function () {
+            expectEvent(await this.target.fnUnrestricted({ from: user }), 'CalledUnrestricted', {
+              caller: user,
+            });
+          });
+
+          it(`Calling a restricted function directly should ${
+            directSuccess ? 'succeed' : 'revert'
+          }`, async function () {
+            const promise = this.direct();
+
+            if (directSuccess) {
+              expectEvent(await promise, 'CalledRestricted', { caller: user });
+            } else if (indirectSuccess) {
+              await expectRevertCustomError(promise, 'AccessManagerNotScheduled', [this.opId]);
+            } else {
+              await expectRevertCustomError(promise, 'AccessManagedUnauthorized', [user]);
+            }
+          });
+
+          it('Calling indirectly: only execute', async function () {
+            // execute without schedule
+            if (directSuccess) {
+              const nonceBefore = await this.manager.getNonce(this.opId);
+              const { receipt, tx } = await this.execute();
+
+              expectEvent.notEmitted(receipt, 'OperationExecuted', { operationId: this.opId });
+              await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address });
+
+              // nonce is not modified
+              expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore);
+            } else if (indirectSuccess) {
+              await expectRevertCustomError(this.execute(), 'AccessManagerNotScheduled', [this.opId]);
+            } else {
+              await expectRevertCustomError(this.execute(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+            }
+          });
+
+          it('Calling indirectly: schedule and execute', async function () {
+            if (directSuccess || indirectSuccess) {
+              const nonceBefore = await this.manager.getNonce(this.opId);
+              const { receipt } = await this.schedule();
+              const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+              expectEvent(receipt, 'OperationScheduled', {
+                operationId: this.opId,
+                caller: user,
+                target: this.call[0],
+                data: this.call[1],
+              });
+
+              // if can call directly, delay should be 0. Otherwise, the delay should be applied
+              expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(
+                timestamp.add(directSuccess ? web3.utils.toBN(0) : delay),
+              );
+
+              // nonce is incremented
+              expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+
+              // execute without wait
+              if (directSuccess) {
+                const { receipt, tx } = await this.execute();
+
+                await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address });
+                if (delay && fnRole !== ROLES.PUBLIC) {
+                  expectEvent(receipt, 'OperationExecuted', { operationId: this.opId });
+                  expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0');
+                }
+
+                // nonce is not modified by execute
+                expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+              } else if (indirectSuccess) {
+                await expectRevertCustomError(this.execute(), 'AccessManagerNotReady', [this.opId]);
+              } else {
+                await expectRevertCustomError(this.execute(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+              }
+            } else {
+              await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+            }
+          });
+
+          it('Calling indirectly: schedule wait and execute', async function () {
+            if (directSuccess || indirectSuccess) {
+              const nonceBefore = await this.manager.getNonce(this.opId);
+              const { receipt } = await this.schedule();
+              const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+              expectEvent(receipt, 'OperationScheduled', {
+                operationId: this.opId,
+                caller: user,
+                target: this.call[0],
+                data: this.call[1],
+              });
+
+              // if can call directly, delay should be 0. Otherwise, the delay should be applied
+              expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(
+                timestamp.add(directSuccess ? web3.utils.toBN(0) : delay),
+              );
+
+              // nonce is incremented
+              expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+
+              // wait
+              await time.increase(delay ?? 0);
+
+              // execute without wait
+              if (directSuccess || indirectSuccess) {
+                const { receipt, tx } = await this.execute();
+
+                await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address });
+                if (delay && fnRole !== ROLES.PUBLIC) {
+                  expectEvent(receipt, 'OperationExecuted', { operationId: this.opId });
+                  expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0');
+                }
+
+                // nonce is not modified by execute
+                expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+              } else {
+                await expectRevertCustomError(this.execute(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+              }
+            } else {
+              await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+            }
+          });
+
+          it('Calling directly: schedule and call', async function () {
+            if (directSuccess || indirectSuccess) {
+              const nonceBefore = await this.manager.getNonce(this.opId);
+              const { receipt } = await this.schedule();
+              const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+              expectEvent(receipt, 'OperationScheduled', {
+                operationId: this.opId,
+                caller: user,
+                target: this.call[0],
+                data: this.call[1],
+              });
+
+              // if can call directly, delay should be 0. Otherwise, the delay should be applied
+              const schedule = timestamp.add(directSuccess ? web3.utils.toBN(0) : delay);
+              expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule);
+
+              // nonce is incremented
+              expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+
+              // execute without wait
+              const promise = this.direct();
+              if (directSuccess) {
+                expectEvent(await promise, 'CalledRestricted', { caller: user });
+
+                // schedule is not reset
+                expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule);
+
+                // nonce is not modified by execute
+                expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+              } else if (indirectSuccess) {
+                await expectRevertCustomError(promise, 'AccessManagerNotReady', [this.opId]);
+              } else {
+                await expectRevertCustomError(promise, 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+              }
+            } else {
+              await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+            }
+          });
+
+          it('Calling directly: schedule wait and call', async function () {
+            if (directSuccess || indirectSuccess) {
+              const nonceBefore = await this.manager.getNonce(this.opId);
+              const { receipt } = await this.schedule();
+              const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+              expectEvent(receipt, 'OperationScheduled', {
+                operationId: this.opId,
+                caller: user,
+                target: this.call[0],
+                data: this.call[1],
+              });
+
+              // if can call directly, delay should be 0. Otherwise, the delay should be applied
+              const schedule = timestamp.add(directSuccess ? web3.utils.toBN(0) : delay);
+              expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule);
+
+              // nonce is incremented
+              expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+
+              // wait
+              await time.increase(delay ?? 0);
+
+              // execute without wait
+              const promise = await this.direct();
+              if (directSuccess) {
+                expectEvent(await promise, 'CalledRestricted', { caller: user });
+
+                // schedule is not reset
+                expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule);
+
+                // nonce is not modified by execute
+                expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+              } else if (indirectSuccess) {
+                const receipt = await promise;
+
+                expectEvent(receipt, 'CalledRestricted', { caller: user });
+                await expectEvent.inTransaction(receipt.tx, this.manager, 'OperationExecuted', {
+                  operationId: this.opId,
+                });
+
+                // schedule is reset
+                expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0');
+
+                // nonce is not modified by execute
+                expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1));
+              } else {
+                await expectRevertCustomError(this.direct(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+              }
+            } else {
+              await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]);
+            }
+          });
+
+          it('Scheduling for later than needed'); // TODO
+        });
+      }
+    });
+
+    describe('Indirect execution corner-cases', async function () {
+      beforeEach(async function () {
+        await this.manager.$_setTargetFunctionRole(this.target.address, this.callData, ROLES.SOME);
+        await this.manager.$_grantRole(ROLES.SOME, user, 0, executeDelay);
+      });
+
+      it('Checking canCall when caller is the manager depend on the _executionId', async function () {
+        const result = await this.manager.canCall(this.manager.address, this.target.address, '0x00000000');
+        expect(result[0]).to.be.false;
+        expect(result[1]).to.be.bignumber.equal('0');
+      });
+
+      it('Cannot execute earlier', async function () {
+        const { receipt } = await this.schedule();
+        const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN);
+
+        expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(timestamp.add(executeDelay));
+
+        // too early
+        await helpers.time.setNextBlockTimestamp(timestamp.add(executeDelay).subn(1));
+        await expectRevertCustomError(this.execute(), 'AccessManagerNotReady', [this.opId]);
+
+        // the revert happened one second before the execution delay expired
+        expect(await time.latest()).to.be.bignumber.equal(timestamp.add(executeDelay).subn(1));
+
+        // ok
+        await helpers.time.setNextBlockTimestamp(timestamp.add(executeDelay));
+        await this.execute();
+
+        // the success happened when the delay was reached (earliest possible)
+        expect(await time.latest()).to.be.bignumber.equal(timestamp.add(executeDelay));
+      });
+
+      it('Cannot schedule an already scheduled operation', async function () {
+        const { receipt } = await this.schedule();
+        expectEvent(receipt, 'OperationScheduled', {
+          operationId: this.opId,
+          caller: user,
+          target: this.call[0],
+          data: this.call[1],
+        });
+
+        await expectRevertCustomError(this.schedule(), 'AccessManagerAlreadyScheduled', [this.opId]);
+      });
+
+      it('Cannot cancel an operation that is not scheduled', async function () {
+        await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]);
+      });
+
+      it('Cannot cancel an operation that is already executed', async function () {
+        await this.schedule();
+        await time.increase(executeDelay);
+        await this.execute();
+
+        await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]);
+      });
+
+      it('Scheduler can cancel', async function () {
+        await this.schedule();
+
+        expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0');
+
+        expectEvent(await this.cancel({ from: manager }), 'OperationCanceled', { operationId: this.opId });
+
+        expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0');
+      });
+
+      it('Guardian can cancel', async function () {
+        await this.schedule();
+
+        expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0');
+
+        expectEvent(await this.cancel({ from: manager }), 'OperationCanceled', { operationId: this.opId });
+
+        expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0');
+      });
+
+      it('Cancel is restricted', async function () {
+        await this.schedule();
+
+        expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0');
+
+        await expectRevertCustomError(this.cancel({ from: other }), 'AccessManagerUnauthorizedCancel', [
+          other,
+          user,
+          ...this.call,
+        ]);
+
+        expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0');
+      });
+
+      it('Can re-schedule after execution', async function () {
+        await this.schedule();
+        await time.increase(executeDelay);
+        await this.execute();
+
+        // reschedule
+        const { receipt } = await this.schedule();
+        expectEvent(receipt, 'OperationScheduled', {
+          operationId: this.opId,
+          caller: user,
+          target: this.call[0],
+          data: this.call[1],
+        });
+      });
+
+      it('Can re-schedule after cancel', async function () {
+        await this.schedule();
+        await this.cancel();
+
+        // reschedule
+        const { receipt } = await this.schedule();
+        expectEvent(receipt, 'OperationScheduled', {
+          operationId: this.opId,
+          caller: user,
+          target: this.call[0],
+          data: this.call[1],
+        });
+      });
+    });
+  });
+
+  describe('with Ownable target contract', function () {
+    const roleId = web3.utils.toBN(1);
+
+    beforeEach(async function () {
+      this.ownable = await Ownable.new(this.manager.address);
+
+      // add user to role
+      await this.manager.$_grantRole(roleId, user, 0, 0);
+    });
+
+    it('initial state', async function () {
+      expect(await this.ownable.owner()).to.be.equal(this.manager.address);
+    });
+
+    describe('Contract is closed', function () {
+      beforeEach(async function () {
+        await this.manager.$_setTargetClosed(this.ownable.address, true);
+      });
+
+      it('directly call: reverts', async function () {
+        await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [user]);
+      });
+
+      it('relayed call (with role): reverts', async function () {
+        await expectRevertCustomError(
+          this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: user }),
+          'AccessManagerUnauthorizedCall',
+          [user, this.ownable.address, selector('$_checkOwner()')],
+        );
+      });
+
+      it('relayed call (without role): reverts', async function () {
+        await expectRevertCustomError(
+          this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: other }),
+          'AccessManagerUnauthorizedCall',
+          [other, this.ownable.address, selector('$_checkOwner()')],
+        );
+      });
+    });
+
+    describe('Contract is managed', function () {
+      describe('function is open to specific role', function () {
+        beforeEach(async function () {
+          await this.manager.$_setTargetFunctionRole(this.ownable.address, selector('$_checkOwner()'), roleId);
+        });
+
+        it('directly call: reverts', async function () {
+          await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [
+            user,
+          ]);
+        });
+
+        it('relayed call (with role): success', async function () {
+          await this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: user });
+        });
+
+        it('relayed call (without role): reverts', async function () {
+          await expectRevertCustomError(
+            this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: other }),
+            'AccessManagerUnauthorizedCall',
+            [other, this.ownable.address, selector('$_checkOwner()')],
+          );
+        });
+      });
+
+      describe('function is open to public role', function () {
+        beforeEach(async function () {
+          await this.manager.$_setTargetFunctionRole(this.ownable.address, selector('$_checkOwner()'), ROLES.PUBLIC);
+        });
+
+        it('directly call: reverts', async function () {
+          await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [
+            user,
+          ]);
+        });
+
+        it('relayed call (with role): success', async function () {
+          await this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: user });
+        });
+
+        it('relayed call (without role): success', async function () {
+          await this.manager.execute(this.ownable.address, selector('$_checkOwner()'), { from: other });
+        });
+      });
+    });
+  });
+
+  describe('authority update', function () {
+    beforeEach(async function () {
+      this.newManager = await AccessManager.new(admin);
+      this.target = await AccessManagedTarget.new(this.manager.address);
+    });
+
+    it('admin can change authority', async function () {
+      expect(await this.target.authority()).to.be.equal(this.manager.address);
+
+      const { tx } = await this.manager.updateAuthority(this.target.address, this.newManager.address, { from: admin });
+      await expectEvent.inTransaction(tx, this.target, 'AuthorityUpdated', { authority: this.newManager.address });
+
+      expect(await this.target.authority()).to.be.equal(this.newManager.address);
+    });
+
+    it('cannot set an address without code as the authority', async function () {
+      await expectRevertCustomError(
+        this.manager.updateAuthority(this.target.address, user, { from: admin }),
+        'AccessManagedInvalidAuthority',
+        [user],
+      );
+    });
+
+    it('updateAuthority is restricted on manager', async function () {
+      await expectRevertCustomError(
+        this.manager.updateAuthority(this.target.address, this.newManager.address, { from: other }),
+        'AccessManagerUnauthorizedAccount',
+        [other, ROLES.ADMIN],
+      );
+    });
+
+    it('setAuthority is restricted on AccessManaged', async function () {
+      await expectRevertCustomError(
+        this.target.setAuthority(this.newManager.address, { from: admin }),
+        'AccessManagedUnauthorized',
+        [admin],
+      );
+    });
+  });
+
+  // TODO:
+  // - check opening/closing a contract
+  // - check updating the contract delay
+  // - check the delay applies to admin function
+  describe.skip('contract modes', function () {});
+});

+ 6 - 0
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 () {

+ 407 - 0
test/governance/extensions/GovernorTimelockAccess.test.js

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

+ 12 - 2
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');
@@ -29,6 +30,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 +43,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 +94,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 +105,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();
 

+ 14 - 4
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,

+ 16 - 0
test/helpers/iterate.js

@@ -0,0 +1,16 @@
+// Map values in an object
+const mapValues = (obj, fn) => Object.fromEntries(Object.entries(obj).map(([k, v]) => [k, fn(v)]));
+
+// Array of number or bigint
+const max = (...values) => values.slice(1).reduce((x, y) => (x > y ? x : y), values[0]);
+const min = (...values) => values.slice(1).reduce((x, y) => (x < y ? x : y), values[0]);
+
+// Cartesian product of a list of arrays
+const product = (...arrays) => arrays.reduce((a, b) => a.flatMap(ai => b.map(bi => [...ai, bi])), [[]]);
+
+module.exports = {
+  mapValues,
+  max,
+  min,
+  product,
+};

+ 0 - 7
test/helpers/map-values.js

@@ -1,7 +0,0 @@
-function mapValues(obj, fn) {
-  return Object.fromEntries(Object.entries(obj).map(([k, v]) => [k, fn(v)]));
-}
-
-module.exports = {
-  mapValues,
-};

+ 5 - 0
test/helpers/methods.js

@@ -0,0 +1,5 @@
+const { soliditySha3 } = require('web3-utils');
+
+module.exports = {
+  selector: signature => soliditySha3(signature).substring(0, 10),
+};

+ 10 - 10
test/proxy/utils/Initializable.test.js

@@ -42,13 +42,13 @@ contract('Initializable', function () {
       });
 
       it('initializer does not run again', async function () {
-        await expectRevertCustomError(this.contract.initialize(), 'AlreadyInitialized', []);
+        await expectRevertCustomError(this.contract.initialize(), 'InvalidInitialization', []);
       });
     });
 
     describe('nested under an initializer', function () {
       it('initializer modifier reverts', async function () {
-        await expectRevertCustomError(this.contract.initializerNested(), 'AlreadyInitialized', []);
+        await expectRevertCustomError(this.contract.initializerNested(), 'InvalidInitialization', []);
       });
 
       it('onlyInitializing modifier succeeds', async function () {
@@ -100,9 +100,9 @@ contract('Initializable', function () {
 
     it('cannot nest reinitializers', async function () {
       expect(await this.contract.counter()).to.be.bignumber.equal('0');
-      await expectRevertCustomError(this.contract.nestedReinitialize(2, 2), 'AlreadyInitialized', []);
-      await expectRevertCustomError(this.contract.nestedReinitialize(2, 3), 'AlreadyInitialized', []);
-      await expectRevertCustomError(this.contract.nestedReinitialize(3, 2), 'AlreadyInitialized', []);
+      await expectRevertCustomError(this.contract.nestedReinitialize(2, 2), 'InvalidInitialization', []);
+      await expectRevertCustomError(this.contract.nestedReinitialize(2, 3), 'InvalidInitialization', []);
+      await expectRevertCustomError(this.contract.nestedReinitialize(3, 2), 'InvalidInitialization', []);
     });
 
     it('can chain reinitializers', async function () {
@@ -121,18 +121,18 @@ contract('Initializable', function () {
     describe('contract locking', function () {
       it('prevents initialization', async function () {
         await this.contract.disableInitializers();
-        await expectRevertCustomError(this.contract.initialize(), 'AlreadyInitialized', []);
+        await expectRevertCustomError(this.contract.initialize(), 'InvalidInitialization', []);
       });
 
       it('prevents re-initialization', async function () {
         await this.contract.disableInitializers();
-        await expectRevertCustomError(this.contract.reinitialize(255), 'AlreadyInitialized', []);
+        await expectRevertCustomError(this.contract.reinitialize(255), 'InvalidInitialization', []);
       });
 
       it('can lock contract after initialization', async function () {
         await this.contract.initialize();
         await this.contract.disableInitializers();
-        await expectRevertCustomError(this.contract.reinitialize(255), 'AlreadyInitialized', []);
+        await expectRevertCustomError(this.contract.reinitialize(255), 'InvalidInitialization', []);
       });
     });
   });
@@ -207,8 +207,8 @@ contract('Initializable', function () {
 
   describe('disabling initialization', function () {
     it('old and new patterns in bad sequence', async function () {
-      await expectRevertCustomError(DisableBad1.new(), 'AlreadyInitialized', []);
-      await expectRevertCustomError(DisableBad2.new(), 'AlreadyInitialized', []);
+      await expectRevertCustomError(DisableBad1.new(), 'InvalidInitialization', []);
+      await expectRevertCustomError(DisableBad2.new(), 'InvalidInitialization', []);
     });
 
     it('old and new patterns in good sequence', async function () {

+ 17 - 11
test/token/ERC721/extensions/ERC721Royalty.test.js

@@ -1,15 +1,15 @@
-const { BN, constants } = require('@openzeppelin/test-helpers');
+require('@openzeppelin/test-helpers');
 
 const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior');
 
 const ERC721Royalty = artifacts.require('$ERC721Royalty');
 
 contract('ERC721Royalty', function (accounts) {
-  const [account1, account2] = accounts;
-  const tokenId1 = new BN('1');
-  const tokenId2 = new BN('2');
-  const royalty = new BN('200');
-  const salePrice = new BN('1000');
+  const [account1, account2, recipient] = accounts;
+  const tokenId1 = web3.utils.toBN('1');
+  const tokenId2 = web3.utils.toBN('2');
+  const royalty = web3.utils.toBN('200');
+  const salePrice = web3.utils.toBN('1000');
 
   beforeEach(async function () {
     this.token = await ERC721Royalty.new('My Token', 'TKN');
@@ -25,15 +25,21 @@ contract('ERC721Royalty', function (accounts) {
 
   describe('token specific functions', function () {
     beforeEach(async function () {
-      await this.token.$_setTokenRoyalty(tokenId1, account1, royalty);
+      await this.token.$_setTokenRoyalty(tokenId1, recipient, royalty);
     });
 
-    it('removes royalty information after burn', async function () {
+    it('royalty information are kept during burn and re-mint', async function () {
       await this.token.$_burn(tokenId1);
-      const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice);
 
-      expect(tokenInfo[0]).to.be.equal(constants.ZERO_ADDRESS);
-      expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0'));
+      const tokenInfoA = await this.token.royaltyInfo(tokenId1, salePrice);
+      expect(tokenInfoA[0]).to.be.equal(recipient);
+      expect(tokenInfoA[1]).to.be.bignumber.equal(salePrice.mul(royalty).divn(1e4));
+
+      await this.token.$_mint(account2, tokenId1);
+
+      const tokenInfoB = await this.token.royaltyInfo(tokenId1, salePrice);
+      expect(tokenInfoB[0]).to.be.equal(recipient);
+      expect(tokenInfoB[1]).to.be.bignumber.equal(salePrice.mul(royalty).divn(1e4));
     });
   });
 

+ 20 - 6
test/token/ERC721/extensions/ERC721URIStorage.test.js

@@ -50,10 +50,14 @@ contract('ERC721URIStorage', function (accounts) {
       });
     });
 
-    it('reverts when setting for non existent token id', async function () {
-      await expectRevertCustomError(this.token.$_setTokenURI(nonExistentTokenId, sampleUri), 'ERC721NonexistentToken', [
-        nonExistentTokenId,
-      ]);
+    it('setting the uri for non existent token id is allowed', async function () {
+      expectEvent(await this.token.$_setTokenURI(nonExistentTokenId, sampleUri), 'MetadataUpdate', {
+        _tokenId: nonExistentTokenId,
+      });
+
+      // value will be accessible after mint
+      await this.token.$_mint(owner, nonExistentTokenId);
+      expect(await this.token.tokenURI(nonExistentTokenId)).to.be.equal(sampleUri);
     });
 
     it('base URI can be set', async function () {
@@ -84,7 +88,7 @@ contract('ERC721URIStorage', function (accounts) {
     });
 
     it('tokens without URI can be burnt ', async function () {
-      await this.token.$_burn(firstTokenId, { from: owner });
+      await this.token.$_burn(firstTokenId);
 
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
     });
@@ -92,9 +96,19 @@ contract('ERC721URIStorage', function (accounts) {
     it('tokens with URI can be burnt ', async function () {
       await this.token.$_setTokenURI(firstTokenId, sampleUri);
 
-      await this.token.$_burn(firstTokenId, { from: owner });
+      await this.token.$_burn(firstTokenId);
+
+      await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
+    });
+
+    it('tokens URI is kept if token is burnt and reminted ', async function () {
+      await this.token.$_setTokenURI(firstTokenId, sampleUri);
 
+      await this.token.$_burn(firstTokenId);
       await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]);
+
+      await this.token.$_mint(owner, firstTokenId);
+      expect(await this.token.tokenURI(firstTokenId)).to.be.equal(sampleUri);
     });
   });
 });

+ 1 - 2
test/utils/Nonces.test.js

@@ -42,8 +42,7 @@ contract('Nonces', function (accounts) {
       const currentNonce = await this.nonces.nonces(sender);
       expect(currentNonce).to.be.bignumber.equal('0');
 
-      const { receipt } = await this.nonces.$_useCheckedNonce(sender, currentNonce);
-      expectEvent(receipt, 'return$_useCheckedNonce', [currentNonce]);
+      await this.nonces.$_useCheckedNonce(sender, currentNonce);
 
       expect(await this.nonces.nonces(sender)).to.be.bignumber.equal('1');
     });

+ 1 - 1
test/utils/cryptography/EIP712.test.js

@@ -3,7 +3,7 @@ const Wallet = require('ethereumjs-wallet').default;
 
 const { getDomain, domainType, domainSeparator, hashTypedData } = require('../../helpers/eip712');
 const { getChainId } = require('../../helpers/chainid');
-const { mapValues } = require('../../helpers/map-values');
+const { mapValues } = require('../../helpers/iterate');
 
 const EIP712Verifier = artifacts.require('$EIP712Verifier');
 const Clones = artifacts.require('$Clones');

+ 1 - 0
test/utils/introspection/SupportsInterface.behavior.js

@@ -61,6 +61,7 @@ const INTERFACES = {
     'proposalDeadline(uint256)',
     'proposalProposer(uint256)',
     'proposalEta(uint256)',
+    'proposalNeedsQueuing(uint256)',
     'votingDelay()',
     'votingPeriod()',
     'quorum(uint256)',

+ 1 - 1
test/utils/structs/EnumerableMap.test.js

@@ -1,5 +1,5 @@
 const { BN, constants } = require('@openzeppelin/test-helpers');
-const { mapValues } = require('../../helpers/map-values');
+const { mapValues } = require('../../helpers/iterate');
 
 const EnumerableMap = artifacts.require('$EnumerableMap');
 

+ 1 - 1
test/utils/structs/EnumerableSet.test.js

@@ -1,5 +1,5 @@
 const EnumerableSet = artifacts.require('$EnumerableSet');
-const { mapValues } = require('../../helpers/map-values');
+const { mapValues } = require('../../helpers/iterate');
 
 const { shouldBehaveLikeSet } = require('./EnumerableSet.behavior');
 

+ 140 - 0
test/utils/types/Time.test.js

@@ -0,0 +1,140 @@
+require('@openzeppelin/test-helpers');
+
+const { expect } = require('chai');
+const { clock } = require('../../helpers/time');
+const { product, max } = require('../../helpers/iterate');
+
+const Time = artifacts.require('$Time');
+
+const MAX_UINT32 = 1n << (32n - 1n);
+const MAX_UINT48 = 1n << (48n - 1n);
+const SOME_VALUES = [0n, 1n, 2n, 15n, 16n, 17n, 42n];
+
+const asUint = (value, size) => {
+  if (typeof value != 'bigint') {
+    value = BigInt(value);
+  }
+  // chai does not support bigint :/
+  if (value < 0 || value >= 1n << BigInt(size)) {
+    throw new Error(`value is not a valid uint${size}`);
+  }
+  return value;
+};
+
+const unpackDelay = delay => ({
+  valueBefore: (asUint(delay, 112) >> 32n) % (1n << 32n),
+  valueAfter: (asUint(delay, 112) >> 0n) % (1n << 32n),
+  effect: (asUint(delay, 112) >> 64n) % (1n << 48n),
+});
+
+const packDelay = ({ valueBefore, valueAfter = 0n, effect = 0n }) =>
+  (asUint(valueAfter, 32) << 0n) + (asUint(valueBefore, 32) << 32n) + (asUint(effect, 48) << 64n);
+
+const effectSamplesForTimepoint = timepoint => [
+  0n,
+  timepoint,
+  ...product([-1n, 1n], [1n, 2n, 17n, 42n])
+    .map(([sign, shift]) => timepoint + sign * shift)
+    .filter(effect => effect > 0n && effect <= MAX_UINT48),
+  MAX_UINT48,
+];
+
+contract('Time', function () {
+  beforeEach(async function () {
+    this.mock = await Time.new();
+  });
+
+  describe('clocks', function () {
+    it('timestamp', async function () {
+      expect(await this.mock.$timestamp()).to.be.bignumber.equal(web3.utils.toBN(await clock.timestamp()));
+    });
+
+    it('block number', async function () {
+      expect(await this.mock.$blockNumber()).to.be.bignumber.equal(web3.utils.toBN(await clock.blocknumber()));
+    });
+  });
+
+  describe('Delay', function () {
+    describe('packing and unpacking', function () {
+      const valueBefore = 17n;
+      const valueAfter = 42n;
+      const effect = 69n;
+      const delay = 1272825341158973505578n;
+
+      it('pack', async function () {
+        const packed = await this.mock.$pack(valueBefore, valueAfter, effect);
+        expect(packed).to.be.bignumber.equal(delay.toString());
+
+        const packed2 = packDelay({ valueBefore, valueAfter, effect });
+        expect(packed2).to.be.equal(delay);
+      });
+
+      it('unpack', async function () {
+        const unpacked = await this.mock.$unpack(delay);
+        expect(unpacked[0]).to.be.bignumber.equal(valueBefore.toString());
+        expect(unpacked[1]).to.be.bignumber.equal(valueAfter.toString());
+        expect(unpacked[2]).to.be.bignumber.equal(effect.toString());
+
+        const unpacked2 = unpackDelay(delay);
+        expect(unpacked2).to.be.deep.equal({ valueBefore, valueAfter, effect });
+      });
+    });
+
+    it('toDelay', async function () {
+      for (const value of [...SOME_VALUES, MAX_UINT32]) {
+        const delay = await this.mock.$toDelay(value).then(unpackDelay);
+        expect(delay).to.be.deep.equal({ valueBefore: 0n, valueAfter: value, effect: 0n });
+      }
+    });
+
+    it('get & getFull', async function () {
+      const timepoint = await clock.timestamp().then(BigInt);
+      const valueBefore = 24194n;
+      const valueAfter = 4214143n;
+
+      for (const effect of effectSamplesForTimepoint(timepoint)) {
+        const isPast = effect <= timepoint;
+
+        const delay = packDelay({ valueBefore, valueAfter, effect });
+
+        expect(await this.mock.$get(delay)).to.be.bignumber.equal(String(isPast ? valueAfter : valueBefore));
+
+        const result = await this.mock.$getFull(delay);
+        expect(result[0]).to.be.bignumber.equal(String(isPast ? valueAfter : valueBefore));
+        expect(result[1]).to.be.bignumber.equal(String(isPast ? 0n : valueAfter));
+        expect(result[2]).to.be.bignumber.equal(String(isPast ? 0n : effect));
+      }
+    });
+
+    it('withUpdate', async function () {
+      const timepoint = await clock.timestamp().then(BigInt);
+      const valueBefore = 24194n;
+      const valueAfter = 4214143n;
+      const newvalueAfter = 94716n;
+
+      for (const effect of effectSamplesForTimepoint(timepoint))
+        for (const minSetback of [...SOME_VALUES, MAX_UINT32]) {
+          const isPast = effect <= timepoint;
+          const expectedvalueBefore = isPast ? valueAfter : valueBefore;
+          const expectedSetback = max(minSetback, expectedvalueBefore - newvalueAfter, 0n);
+
+          const result = await this.mock.$withUpdate(
+            packDelay({ valueBefore, valueAfter, effect }),
+            newvalueAfter,
+            minSetback,
+          );
+
+          expect(result[0]).to.be.bignumber.equal(
+            String(
+              packDelay({
+                valueBefore: expectedvalueBefore,
+                valueAfter: newvalueAfter,
+                effect: timepoint + expectedSetback,
+              }),
+            ),
+          );
+          expect(result[1]).to.be.bignumber.equal(String(timepoint + expectedSetback));
+        }
+    });
+  });
+});