فهرست منبع

Document AccessManager functions and events in IAccessManager (#4660)

Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
Hadrien Croubois 2 سال پیش
والد
کامیت
e78628bfcf

+ 24 - 25
certora/diff/access_manager_AccessManager.sol.patch

@@ -1,5 +1,5 @@
---- access/manager/AccessManager.sol	2023-10-04 11:20:52.802378968 +0200
-+++ access/manager/AccessManager.sol	2023-10-04 14:49:43.126279234 +0200
+--- access/manager/AccessManager.sol	2023-10-05 12:17:09.694051809 -0300
++++ access/manager/AccessManager.sol	2023-10-05 12:26:18.498688718 -0300
 @@ -6,7 +6,6 @@
  import {IAccessManaged} from "./IAccessManaged.sol";
  import {Address} from "../../utils/Address.sol";
@@ -8,7 +8,7 @@
  import {Math} from "../../utils/math/Math.sol";
  import {Time} from "../../utils/types/Time.sol";
  
-@@ -48,7 +47,8 @@
+@@ -57,7 +56,8 @@
   * mindful of the danger associated with functions such as {{Ownable-renounceOwnership}} or
   * {{AccessControl-renounceRole}}.
   */
@@ -18,17 +18,17 @@
      using Time for *;
  
      // Structure that stores the details for a target contract.
-@@ -93,7 +93,7 @@
-     mapping(bytes32 operationId => Schedule) private _schedules;
+@@ -105,7 +105,7 @@
  
+     // Used to identify operations that are currently being executed via {execute}.
      // This should be transient storage when supported by the EVM.
 -    bytes32 private _executionId;
 +    bytes32 internal _executionId; // private → internal for FV
  
      /**
       * @dev Check that the caller is authorized to perform the operation, following the restrictions encoded in
-@@ -185,6 +185,11 @@
-         return _targets[target].adminDelay.get();
+@@ -253,6 +253,11 @@
+         _setGrantDelay(roleId, newDelay);
      }
  
 +    // Exposed for FV
@@ -37,10 +37,10 @@
 +    }
 +
      /**
-      * @dev Get the id of the role that acts as an admin for given role.
+      * @dev Internal version of {grantRole} without access control. Returns true if the role was newly granted.
       *
-@@ -213,6 +218,11 @@
-         return _roles[roleId].grantDelay.get();
+@@ -287,6 +292,11 @@
+         return newMember;
      }
  
 +    // Exposed for FV
@@ -49,18 +49,9 @@
 +    }
 +
      /**
-      * @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
-@@ -749,7 +759,7 @@
-     /**
-      * @dev Hashing function for execute protection
-      */
--    function _hashExecutionId(address target, bytes4 selector) private pure returns (bytes32) {
-+    function _hashExecutionId(address target, bytes4 selector) internal pure returns (bytes32) { // private → internal for FV
-         return keccak256(abi.encode(target, selector));
-     }
- 
-@@ -769,7 +779,7 @@
+      * @dev Internal version of {revokeRole} without access control. This logic is also used by {renounceRole}.
+      * Returns true if the role was previously granted.
+@@ -586,7 +596,7 @@
      /**
       * @dev Check if the current call is authorized according to admin logic.
       */
@@ -69,7 +60,7 @@
          address caller = _msgSender();
          (bool immediate, uint32 delay) = _canCallSelf(caller, _msgData());
          if (!immediate) {
-@@ -792,7 +802,7 @@
+@@ -609,7 +619,7 @@
       */
      function _getAdminRestrictions(
          bytes calldata data
@@ -78,7 +69,7 @@
          if (data.length < 4) {
              return (false, 0, 0);
          }
-@@ -847,7 +857,7 @@
+@@ -662,7 +672,7 @@
          address caller,
          address target,
          bytes calldata data
@@ -87,7 +78,7 @@
          if (target == address(this)) {
              return _canCallSelf(caller, data);
          } else {
-@@ -901,7 +911,7 @@
+@@ -716,14 +726,14 @@
      /**
       * @dev Extracts the selector from calldata. Panics if data is not at least 4 bytes
       */
@@ -95,4 +86,12 @@
 +    function _checkSelector(bytes calldata data) internal pure returns (bytes4) { // private → internal for FV
          return bytes4(data[0:4]);
      }
+ 
+     /**
+      * @dev Hashing function for execute protection
+      */
+-    function _hashExecutionId(address target, bytes4 selector) private pure returns (bytes32) {
++    function _hashExecutionId(address target, bytes4 selector) internal pure returns (bytes32) { // private → internal for FV
+         return keccak256(abi.encode(target, selector));
+     }
  }

+ 5 - 1
contracts/access/README.adoc

@@ -32,8 +32,12 @@ This directory provides ways to restrict who can access the functions of a contr
 
 {{IAuthority}}
 
+{{IAccessManager}}
+
 {{AccessManager}}
 
+{{IAccessManaged}}
+
 {{AccessManaged}}
 
-{{AccessManagerAdapter}}
+{{AuthorityUtils}}

+ 3 - 11
contracts/access/manager/AccessManaged.sol

@@ -57,16 +57,12 @@ abstract contract AccessManaged is Context, IAccessManaged {
         _;
     }
 
-    /**
-     * @dev Returns the current authority.
-     */
+    /// @inheritdoc IAccessManaged
     function authority() public view virtual returns (address) {
         return _authority;
     }
 
-    /**
-     * @dev Transfers control to a new authority. The caller must be the current authority.
-     */
+    /// @inheritdoc IAccessManaged
     function setAuthority(address newAuthority) public virtual {
         address caller = _msgSender();
         if (caller != authority()) {
@@ -78,11 +74,7 @@ abstract contract AccessManaged is Context, IAccessManaged {
         _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.
-     */
+    /// @inheritdoc IAccessManaged
     function isConsumingScheduledOp() public view returns (bytes4) {
         return _consumingSchedule ? this.isConsumingScheduledOp.selector : bytes4(0);
     }

+ 59 - 267
contracts/access/manager/AccessManager.sol

@@ -126,26 +126,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     // =================================================== 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 if the call can be performed without any delay. If the call is subject to a
-     * previously set delay (not zero), then the function should return false and the caller should schedule the operation
-     * for future execution.
-     *
-     * If `immediate` is true, the delay can be disregarded and the operation can be immediately executed, otherwise
-     * the operation can be executed if and only if delay is greater than 0.
-     *
-     * 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.
-     *
-     * NOTE: This function does not report the permissions of this manager itself. These are defined by the
-     * {_canCallSelf} function instead.
-     */
+    /// @inheritdoc IAccessManager
     function canCall(
         address caller,
         address target,
@@ -164,86 +145,47 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
     }
 
-    /**
-     * @dev Expiration delay for scheduled proposals. Defaults to 1 week.
-     *
-     * IMPORTANT: Avoid overriding the expiration with 0. Otherwise every contract proposal will be expired immediately,
-     * disabling any scheduling usage.
-     */
+    /// @inheritdoc IAccessManager
     function expiration() public view virtual returns (uint32) {
         return 1 weeks;
     }
 
-    /**
-     * @dev Minimum setback for all delay updates, with the exception of execution delays. It
-     * can be increased without setback (and in the event of an accidental increase can be reset
-     * via {revokeRole}). Defaults to 5 days.
-     */
+    /// @inheritdoc IAccessManager
     function minSetback() public view virtual returns (uint32) {
         return 5 days;
     }
 
-    /**
-     * @dev Get whether the contract is closed disabling any access. Otherwise role permissions are applied.
-     */
+    /// @inheritdoc IAccessManager
     function isTargetClosed(address target) public view virtual returns (bool) {
         return _targets[target].closed;
     }
 
-    /**
-     * @dev Get the role required to call a function.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     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 the 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.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     function getRoleGuardian(uint64 roleId) public view virtual returns (uint64) {
         return _roles[roleId].guardian;
     }
 
-    /**
-     * @dev Get the role current grant delay.
-     *
-     * Its value may change at any point without an event emitted following a call to {setGrantDelay}.
-     * Changes to this value, including effect timepoint are notified in advance by the {RoleGrantDelayChanged} event.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     function getAccess(
         uint64 roleId,
         address account
@@ -256,10 +198,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         return (since, currentDelay, pendingDelay, effect);
     }
 
-    /**
-     * @dev Check if a given account currently has the permission level corresponding to a given role. Note that this
-     * permission might be associated with an execution delay. {getAccess} can provide more details.
-     */
+    /// @inheritdoc IAccessManager
     function hasRole(
         uint64 roleId,
         address account
@@ -273,15 +212,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     // =============================================== ROLE MANAGEMENT ===============================================
-    /**
-     * @dev Give a label to a role, for improved role discoverabily by UIs.
-     *
-     * Requirements:
-     *
-     * - the caller must be a global admin
-     *
-     * Emits a {RoleLabel} event.
-     */
+    /// @inheritdoc IAccessManager
     function labelRole(uint64 roleId, string calldata label) public virtual onlyAuthorized {
         if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) {
             revert AccessManagerLockedRole(roleId);
@@ -289,55 +220,17 @@ contract AccessManager is Context, Multicall, IAccessManager {
         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 of 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})
-     * - granted role must not be the `PUBLIC_ROLE`
-     *
-     * Emits a {RoleGranted} event.
-     */
+    /// @inheritdoc IAccessManager
     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})
-     * - revoked role must not be the `PUBLIC_ROLE`
-     *
-     * Emits a {RoleRevoked} event if the account had the role.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     function renounceRole(uint64 roleId, address callerConfirmation) public virtual {
         if (callerConfirmation != _msgSender()) {
             revert AccessManagerBadConfirmation();
@@ -345,41 +238,17 @@ contract AccessManager is Context, Multicall, IAccessManager {
         _revokeRole(roleId, callerConfirmation);
     }
 
-    /**
-     * @dev Change admin role for a given role.
-     *
-     * Requirements:
-     *
-     * - the caller must be a global admin
-     *
-     * Emits a {RoleAdminChanged} event
-     */
+    /// @inheritdoc IAccessManager
     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
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     function setGrantDelay(uint64 roleId, uint32 newDelay) public virtual onlyAuthorized {
         _setGrantDelay(roleId, newDelay);
     }
@@ -492,15 +361,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     // ============================================= 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.
-     */
+    /// @inheritdoc IAccessManager
     function setTargetFunctionRole(
         address target,
         bytes4[] calldata selectors,
@@ -521,15 +382,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         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.
-     */
+    /// @inheritdoc IAccessManager
     function setTargetAdminDelay(address target, uint32 newDelay) public virtual onlyAuthorized {
         _setTargetAdminDelay(target, newDelay);
     }
@@ -547,15 +400,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     // =============================================== MODE MANAGEMENT ================================================
-    /**
-     * @dev Set the closed flag for a contract.
-     *
-     * Requirements:
-     *
-     * - the caller must be a global admin
-     *
-     * Emits a {TargetClosed} event.
-     */
+    /// @inheritdoc IAccessManager
     function setTargetClosed(address target, bool closed) public virtual onlyAuthorized {
         _setTargetClosed(target, closed);
     }
@@ -574,38 +419,18 @@ contract AccessManager is Context, Multicall, IAccessManager {
     }
 
     // ============================================== 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.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     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.
-     */
+    /// @inheritdoc IAccessManager
     function schedule(
         address target,
         bytes calldata data,
@@ -653,15 +478,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
         }
     }
 
-    /**
-     * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the
-     * execution delay is 0.
-     *
-     * Returns the nonce that identifies the previously scheduled operation that is 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.
-     */
+    /// @inheritdoc IAccessManager
     // Reentrancy is not an issue because permissions are checked on msg.sender. Additionally,
     // _consumeScheduledOp guarantees a scheduled operation is only executed once.
     // slither-disable-next-line reentrancy-no-eth
@@ -698,15 +515,31 @@ contract AccessManager is Context, Multicall, IAccessManager {
         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.
-     */
+    /// @inheritdoc IAccessManager
+    function cancel(address caller, address target, bytes calldata data) public virtual returns (uint32) {
+        address msgsender = _msgSender();
+        bytes4 selector = _checkSelector(data);
+
+        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;
+    }
+
+    /// @inheritdoc IAccessManager
     function consumeScheduledOp(address caller, bytes calldata data) public virtual {
         address target = _msgSender();
         if (IAccessManaged(target).isConsumingScheduledOp() != IAccessManaged.isConsumingScheduledOp.selector) {
@@ -738,61 +571,13 @@ contract AccessManager is Context, Multicall, IAccessManager {
         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 = _checkSelector(data);
-
-        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
-     */
+    /// @inheritdoc IAccessManager
     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
-     */
+    /// @inheritdoc IAccessManager
     function updateAuthority(address target, address newAuthority) public virtual onlyAuthorized {
         IAccessManaged(target).setAuthority(newAuthority);
     }
@@ -934,4 +719,11 @@ contract AccessManager is Context, Multicall, IAccessManager {
     function _checkSelector(bytes calldata data) private pure returns (bytes4) {
         return bytes4(data[0:4]);
     }
+
+    /**
+     * @dev Hashing function for execute protection
+     */
+    function _hashExecutionId(address target, bytes4 selector) private pure returns (bytes32) {
+        return keccak256(abi.encode(target, selector));
+    }
 }

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

@@ -3,15 +3,29 @@
 pragma solidity ^0.8.20;
 
 interface IAccessManaged {
+    /**
+     * @dev Authority that manages this contract was updated.
+     */
     event AuthorityUpdated(address authority);
 
     error AccessManagedUnauthorized(address caller);
     error AccessManagedRequiredDelay(address caller, uint32 delay);
     error AccessManagedInvalidAuthority(address authority);
 
+    /**
+     * @dev Returns the current authority.
+     */
     function authority() external view returns (address);
 
+    /**
+     * @dev Transfers control to a new authority. The caller must be the current authority.
+     */
     function setAuthority(address) external;
 
+    /**
+     * @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() external view returns (bytes4);
 }

+ 273 - 2
contracts/access/manager/IAccessManager.sol

@@ -28,7 +28,11 @@ interface IAccessManager {
      */
     event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce);
 
+    /**
+     * @dev Informational labelling for a roleId.
+     */
     event RoleLabel(uint64 indexed roleId, string label);
+
     /**
      * @dev Emitted when `account` is granted `roleId`.
      *
@@ -37,12 +41,40 @@ interface IAccessManager {
      * otherwise it indicates the execution delay for this account and roleId is updated.
      */
     event RoleGranted(uint64 indexed roleId, address indexed account, uint32 delay, uint48 since, bool newMember);
+
+    /**
+     * @dev Emitted when `account` membership or `roleId` is revoked. Unlike granting, revoking is instantaneous.
+     */
     event RoleRevoked(uint64 indexed roleId, address indexed account);
+
+    /**
+     * @dev Role acting as admin over a given `roleId` is updated.
+     */
     event RoleAdminChanged(uint64 indexed roleId, uint64 indexed admin);
+
+    /**
+     * @dev Role acting as guardian over a given `roleId` is updated.
+     */
     event RoleGuardianChanged(uint64 indexed roleId, uint64 indexed guardian);
+
+    /**
+     * @dev Grant delay for a given `roleId` will be updated to `delay` when `since` is reached.
+     */
     event RoleGrantDelayChanged(uint64 indexed roleId, uint32 delay, uint48 since);
+
+    /**
+     * @dev Target mode is updated (true = closed, false = open).
+     */
     event TargetClosed(address indexed target, bool closed);
+
+    /**
+     * @dev Role required to invoke `selector` on `target` is updated to `roleId`.
+     */
     event TargetFunctionRoleUpdated(address indexed target, bytes4 selector, uint64 indexed roleId);
+
+    /**
+     * @dev Admin delay for a given `target` will be updated to `delay` when `since` is reached.
+     */
     event TargetAdminDelayUpdated(address indexed target, uint32 delay, uint48 since);
 
     error AccessManagerAlreadyScheduled(bytes32 operationId);
@@ -58,63 +90,302 @@ interface IAccessManager {
     error AccessManagerUnauthorizedCancel(address msgsender, address caller, address target, bytes4 selector);
     error AccessManagerInvalidInitialAdmin(address initialAdmin);
 
+    /**
+     * @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 if the call can be performed without any delay. If the call is subject to a
+     * previously set delay (not zero), then the function should return false and the caller should schedule the operation
+     * for future execution.
+     *
+     * If `immediate` is true, the delay can be disregarded and the operation can be immediately executed, otherwise
+     * the operation can be executed if and only if delay is greater than 0.
+     *
+     * 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.
+     *
+     * NOTE: This function does not report the permissions of this manager itself. These are defined by the
+     * {_canCallSelf} function instead.
+     */
     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);
-
+    /**
+     * @dev Expiration delay for scheduled proposals. Defaults to 1 week.
+     *
+     * IMPORTANT: Avoid overriding the expiration with 0. Otherwise every contract proposal will be expired immediately,
+     * disabling any scheduling usage.
+     */
     function expiration() external view returns (uint32);
 
+    /**
+     * @dev Minimum setback for all delay updates, with the exception of execution delays. It
+     * can be increased without setback (and reset via {revokeRole} in the case event of an
+     * accidental increase). Defaults to 5 days.
+     */
+    function minSetback() external view returns (uint32);
+
+    /**
+     * @dev Get whether the contract is closed disabling any access. Otherwise role permissions are applied.
+     */
     function isTargetClosed(address target) external view returns (bool);
 
+    /**
+     * @dev Get the role required to call a function.
+     */
     function getTargetFunctionRole(address target, bytes4 selector) external view returns (uint64);
 
+    /**
+     * @dev Get the admin delay for a target contract. Changes to contract configuration are subject to this delay.
+     */
     function getTargetAdminDelay(address target) external view returns (uint32);
 
+    /**
+     * @dev Get the id of the role that acts as an admin for the 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) external view returns (uint64);
 
+    /**
+     * @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) external view returns (uint64);
 
+    /**
+     * @dev Get the role current grant delay.
+     *
+     * Its value may change at any point without an event emitted following a call to {setGrantDelay}.
+     * Changes to this value, including effect timepoint are notified in advance by the {RoleGrantDelayChanged} event.
+     */
     function getRoleGrantDelay(uint64 roleId) external view returns (uint32);
 
+    /**
+     * @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) external view returns (uint48, uint32, uint32, uint48);
 
+    /**
+     * @dev Check if a given account currently has the permission level corresponding to a given role. Note that this
+     * permission might be associated with an execution delay. {getAccess} can provide more details.
+     */
     function hasRole(uint64 roleId, address account) external view returns (bool, uint32);
 
+    /**
+     * @dev Give a label to a role, for improved role discoverability by UIs.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     *
+     * Emits a {RoleLabel} event.
+     */
     function labelRole(uint64 roleId, string calldata label) external;
 
+    /**
+     * @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 of 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})
+     * - granted role must not be the `PUBLIC_ROLE`
+     *
+     * Emits a {RoleGranted} event.
+     */
     function grantRole(uint64 roleId, address account, uint32 executionDelay) external;
 
+    /**
+     * @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})
+     * - revoked role must not be the `PUBLIC_ROLE`
+     *
+     * Emits a {RoleRevoked} event if the account had the role.
+     */
     function revokeRole(uint64 roleId, address account) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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.
+     */
     function setTargetAdminDelay(address target, uint32 newDelay) external;
 
+    /**
+     * @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) external;
 
+    /**
+     * @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) external view returns (uint48);
 
+    /**
+     * @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) external view returns (uint32);
 
+    /**
+     * @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) external returns (bytes32, uint32);
 
+    /**
+     * @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.
+     */
     function execute(address target, bytes calldata data) external payable returns (uint32);
 
+    /**
+     * @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) external returns (uint32);
 
+    /**
+     * @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) external;
 
+    /**
+     * @dev Hashing function for delayed operations.
+     */
+    function hashOperation(address caller, address target, bytes calldata data) external view returns (bytes32);
+
+    /**
+     * @dev Changes the authority of a target managed by this manager instance.
+     *
+     * Requirements:
+     *
+     * - the caller must be a global admin
+     */
     function updateAuthority(address target, address newAuthority) external;
 }