Add Governor module connecting with AccessManager

.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.

contracts/access/manager/AccessManager.sol

@@ -89,7 +89,14 @@ contract AccessManager is Context, Multicall, IAccessManager {
  mapping(address target => AccessMode mode) private _contractMode;
  mapping(uint64 classId => Class) private _classes;
  mapping(uint64 groupId => Group) private _groups;
- mapping(bytes32 operationId => uint48 schedule) private _schedules;
+
+ struct Schedule {
+ uint48 timepoint;
+ uint32 nonce;
+ }
+
+ mapping(bytes32 operationId => Schedule) private _schedules;
+
  mapping(bytes4 selector => Time.Delay delay) private _adminDelays;
 
  // This should be transcient storage when supported by the EVM.
@@ -568,56 +575,83 @@ contract AccessManager is Context, Multicall, IAccessManager {
  * operation is not yet scheduled, has expired, was executed, or was canceled.
  */
  function getSchedule(bytes32 id) public view virtual returns (uint48) {
- uint48 timepoint = _schedules[id];
+ uint48 timepoint = _schedules[id].timepoint;
  return _isExpired(timepoint) ? 0 : timepoint;
  }
 
+ /**
+ * @dev Return the nonce for the latest scheduled operation with a given id. Returns 0 if the operation has never
+ * been scheduled.
+ */
+ function getNonce(bytes32 id) public view virtual returns (uint32) {
+ return _schedules[id].nonce;
+ }
+
  /**
  * @dev Schedule a delayed operation for future execution, and return the operation identifier. It is possible to
  * choose the timestamp at which the operation becomes executable as long as it satisfies the execution delays
  * 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 {relay} and {cancel}.
+ *
  * Emits a {OperationScheduled} event.
  */
- function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32) {
+ function schedule(
+ address target,
+ bytes calldata data,
+ uint48 when
+ ) public virtual returns (bytes32 operationId, uint32 nonce) {
  address caller = _msgSender();
 
  // Fetch restriction to that apply to the caller on the targeted function
  (bool allowed, uint32 setback) = _canCallExtended(caller, target, data);
 
  uint48 minWhen = Time.timestamp() + setback;
 
+ if (when == 0) {
+ when = minWhen;
+ }
+
  // If caller is not authorised, revert
- if (!allowed && (setback == 0 || when.isSetAndPast(minWhen - 1))) {
+ if (!allowed && (setback == 0 || when < minWhen)) {
  revert AccessManagerUnauthorizedCall(caller, target, bytes4(data[0:4]));
  }
 
  // If caller is authorised, schedule operation
- bytes32 operationId = _hashOperation(caller, target, data);
+ operationId = _hashOperation(caller, target, data);
 
  // Cannot reschedule unless the operation has expired
- uint48 prevTimepoint = _schedules[operationId];
+ uint48 prevTimepoint = _schedules[operationId].timepoint;
  if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) {
  revert AccessManagerAlreadyScheduled(operationId);
  }
 
- uint48 timepoint = when == 0 ? minWhen : when;
- _schedules[operationId] = timepoint;
- emit OperationScheduled(operationId, timepoint, caller, target, data);
+ 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);
 
- return operationId;
+ // Using named return values because otherwise we get stack too deep
  }
 
  /**
  * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the
  * execution delay is 0.
  *
+ * Returns the nonce that identifies the previously scheduled operation that is relayed, or 0 if the
+ * operation wasn't previously scheduled (if the caller doesn't have an execution delay).
+ *
  * Emits an {OperationExecuted} event only if the call was scheduled and delayed.
  */
  // Reentrancy is not an issue because permissions are checked on msg.sender. Additionally,
  // _consumeScheduledOp guarantees a scheduled operation is only executed once.
  // slither-disable-next-line reentrancy-no-eth
- function relay(address target, bytes calldata data) public payable virtual {
+ function relay(address target, bytes calldata data) public payable virtual returns (uint32) {
  address caller = _msgSender();
 
  // Fetch restriction to that apply to the caller on the targeted function
@@ -630,9 +664,10 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
  // If caller is authorised, check operation was scheduled early enough
  bytes32 operationId = _hashOperation(caller, target, data);
+ uint32 nonce;
 
  if (setback != 0) {
- _consumeScheduledOp(operationId);
+ nonce = _consumeScheduledOp(operationId);
  }
 
  // Mark the target and selector as authorised
@@ -644,6 +679,8 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
  // Reset relay identifier
  _relayIdentifier = relayIdentifierBefore;
+
+ return nonce;
  }
 
  /**
@@ -663,9 +700,12 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
  /**
  * @dev Internal variant of {consumeScheduledOp} that operates on bytes32 operationId.
+ *
+ * Returns the nonce of the scheduled operation that is consumed.
  */
- function _consumeScheduledOp(bytes32 operationId) internal virtual {
- uint48 timepoint = _schedules[operationId];
+ function _consumeScheduledOp(bytes32 operationId) internal virtual returns (uint32) {
+ uint48 timepoint = _schedules[operationId].timepoint;
+ uint32 nonce = _schedules[operationId].nonce;
 
  if (timepoint == 0) {
  revert AccessManagerNotScheduled(operationId);
@@ -676,24 +716,27 @@ contract AccessManager is Context, Multicall, IAccessManager {
  }
 
  delete _schedules[operationId];
- emit OperationExecuted(operationId, timepoint);
+ emit OperationExecuted(operationId, nonce);
+
+ return nonce;
  }
 
  /**
- * @dev Cancel a scheduled (delayed) operation.
+ * @dev Cancel a scheduled (delayed) operation. Returns the nonce that identifies the previously scheduled
+ * operation that is cancelled.
  *
  * Requirements:
  *
  * - the caller must be the proposer, or a guardian of the targeted function
  *
  * Emits a {OperationCanceled} event.
  */
- function cancel(address caller, address target, bytes calldata data) public virtual {
+ function cancel(address caller, address target, bytes calldata data) public virtual returns (uint32) {
  address msgsender = _msgSender();
  bytes4 selector = bytes4(data[0:4]);
 
  bytes32 operationId = _hashOperation(caller, target, data);
- if (_schedules[operationId] == 0) {
+ if (_schedules[operationId].timepoint == 0) {
  revert AccessManagerNotScheduled(operationId);
  } else if (caller != msgsender) {
  // calls can only be canceled by the account that scheduled them, a global admin, or by a guardian of the required group.
@@ -705,9 +748,11 @@ contract AccessManager is Context, Multicall, IAccessManager {
  }
  }
 
- uint48 timepoint = _schedules[operationId];
- delete _schedules[operationId];
- emit OperationCanceled(operationId, timepoint);
+ delete _schedules[operationId].timepoint;
+ uint32 nonce = _schedules[operationId].nonce;
+ emit OperationCanceled(operationId, nonce);
+
+ return nonce;
  }
 
  /**

contracts/access/manager/IAccessManager.sol

@@ -9,17 +9,24 @@ interface IAccessManager {
  /**
  * @dev A delayed operation was scheduled.
  */
- event OperationScheduled(bytes32 indexed operationId, uint48 schedule, address caller, address target, bytes data);
+ event OperationScheduled(
+ bytes32 indexed operationId,
+ uint32 indexed nonce,
+ uint48 schedule,
+ address caller,
+ address target,
+ bytes data
+ );
 
  /**
  * @dev A scheduled operation was executed.
  */
- event OperationExecuted(bytes32 indexed operationId, uint48 schedule);
+ event OperationExecuted(bytes32 indexed operationId, uint32 indexed nonce);
 
  /**
  * @dev A scheduled operation was canceled.
  */
- event OperationCanceled(bytes32 indexed operationId, uint48 schedule);
+ event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce);
 
  event GroupLabel(uint64 indexed groupId, string label);
  event GroupGranted(uint64 indexed groupId, address indexed account, uint32 delay, uint48 since);
@@ -94,11 +101,13 @@ interface IAccessManager {
 
  function getSchedule(bytes32 id) external returns (uint48);
 
- function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32);
+ function getNonce(bytes32 id) external returns (uint32);
+
+ function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32, uint32);
 
- function relay(address target, bytes calldata data) external payable;
+ function relay(address target, bytes calldata data) external payable returns (uint32);
 
- function cancel(address caller, address target, bytes calldata data) external;
+ function cancel(address caller, address target, bytes calldata data) external returns (uint32);
 
  function consumeScheduledOp(address caller, bytes calldata data) external;