diff --git a/.changeset/violet-melons-press.md b/.changeset/violet-melons-press.md new file mode 100644 index 00000000000..18fd70b5851 --- /dev/null +++ b/.changeset/violet-melons-press.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +`GovernorTimelockAccess`: Added a module to connect a governor with an instance of `AccessManager`, allowing the governor to make calls that are delay-restricted by the manager using the normal `queue` workflow. diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol index 929a6736bca..01ac7664e00 100644 --- a/contracts/access/manager/AccessManager.sol +++ b/contracts/access/manager/AccessManager.sol @@ -89,7 +89,14 @@ contract AccessManager is Context, Multicall, IAccessManager { mapping(address target => AccessMode mode) private _contractMode; mapping(uint64 classId => Class) private _classes; mapping(uint64 groupId => Group) private _groups; - mapping(bytes32 operationId => uint48 schedule) private _schedules; + + struct Schedule { + uint48 timepoint; + uint32 nonce; + } + + mapping(bytes32 operationId => Schedule) private _schedules; + mapping(bytes4 selector => Time.Delay delay) private _adminDelays; // This should be transcient storage when supported by the EVM. @@ -568,18 +575,34 @@ 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 @@ -587,37 +610,48 @@ contract AccessManager is Context, Multicall, IAccessManager { uint48 minWhen = Time.timestamp() + setback; + if (when == 0) { + when = minWhen; + } + // If caller is not authorised, revert - if (!allowed && (setback == 0 || when.isSetAndPast(minWhen - 1))) { + if (!allowed && (setback == 0 || when < minWhen)) { revert AccessManagerUnauthorizedCall(caller, target, bytes4(data[0:4])); } // If caller is authorised, schedule operation - bytes32 operationId = _hashOperation(caller, target, data); + operationId = _hashOperation(caller, target, data); // Cannot reschedule unless the operation has expired - uint48 prevTimepoint = _schedules[operationId]; + uint48 prevTimepoint = _schedules[operationId].timepoint; if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) { revert AccessManagerAlreadyScheduled(operationId); } - uint48 timepoint = when == 0 ? minWhen : when; - _schedules[operationId] = timepoint; - emit OperationScheduled(operationId, timepoint, caller, target, data); + 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,11 +716,14 @@ 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: * @@ -688,12 +731,12 @@ contract AccessManager is Context, Multicall, IAccessManager { * * Emits a {OperationCanceled} event. */ - function cancel(address caller, address target, bytes calldata data) public virtual { + function cancel(address caller, address target, bytes calldata data) public virtual returns (uint32) { address msgsender = _msgSender(); bytes4 selector = bytes4(data[0:4]); bytes32 operationId = _hashOperation(caller, target, data); - if (_schedules[operationId] == 0) { + if (_schedules[operationId].timepoint == 0) { revert AccessManagerNotScheduled(operationId); } else if (caller != msgsender) { // calls can only be canceled by the account that scheduled them, a global admin, or by a guardian of the required group. @@ -705,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; } /** diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol index 312b4da740a..f4a7da362ff 100644 --- a/contracts/access/manager/IAccessManager.sol +++ b/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; diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 9d9e2eb5907..e661b568ec7 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -98,14 +98,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 /** * @dev See {IGovernor-name}. */ - function name() public view virtual override returns (string memory) { + function name() public view virtual returns (string memory) { return _name; } /** * @dev See {IGovernor-version}. */ - function version() public view virtual override returns (string memory) { + function version() public view virtual returns (string memory) { return "1"; } @@ -127,14 +127,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash - ) public pure virtual override returns (uint256) { + ) public pure virtual returns (uint256) { return uint256(keccak256(abi.encode(targets, values, calldatas, descriptionHash))); } /** * @dev See {IGovernor-state}. */ - function state(uint256 proposalId) public view virtual override returns (ProposalState) { + function state(uint256 proposalId) public view virtual returns (ProposalState) { // ProposalCore is just one slot. We can load it from storage to stack with a single sload ProposalCore storage proposal = _proposals[proposalId]; bool proposalExecuted = proposal.executed; @@ -176,38 +176,45 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 /** * @dev See {IGovernor-proposalThreshold}. */ - function proposalThreshold() public view virtual override returns (uint256) { + function proposalThreshold() public view virtual returns (uint256) { return 0; } /** * @dev See {IGovernor-proposalSnapshot}. */ - function proposalSnapshot(uint256 proposalId) public view virtual override returns (uint256) { + function proposalSnapshot(uint256 proposalId) public view virtual returns (uint256) { return _proposals[proposalId].voteStart; } /** * @dev See {IGovernor-proposalDeadline}. */ - function proposalDeadline(uint256 proposalId) public view virtual override returns (uint256) { + function proposalDeadline(uint256 proposalId) public view virtual returns (uint256) { return _proposals[proposalId].voteStart + _proposals[proposalId].voteDuration; } /** * @dev See {IGovernor-proposalProposer}. */ - function proposalProposer(uint256 proposalId) public view virtual override returns (address) { + function proposalProposer(uint256 proposalId) public view virtual returns (address) { return _proposals[proposalId].proposer; } /** * @dev See {IGovernor-proposalEta}. */ - function proposalEta(uint256 proposalId) public view virtual override returns (uint256) { + function proposalEta(uint256 proposalId) public view virtual returns (uint256) { return _proposals[proposalId].eta; } + /** + * @dev See {IGovernor-proposalNeedsQueuing}. + */ + function proposalNeedsQueuing(uint256) public view virtual returns (bool) { + return false; + } + /** * @dev Reverts if the `msg.sender` is not the executor. In case the executor is not this contract * itself, the function reverts if `msg.data` is not whitelisted as a result of an {execute} @@ -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,7 +347,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash - ) public virtual override returns (uint256) { + ) public virtual returns (uint256) { uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Succeeded)); @@ -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( diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index 3b31bc502f8..a3f6fec5228 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -245,6 +245,12 @@ interface IGovernor is IERC165, IERC6372 { */ function proposalEta(uint256 proposalId) external view returns (uint256); + /** + * @notice module:core + * @dev Whether a proposal needs to be queued before execution. + */ + function proposalNeedsQueuing(uint256 proposalId) external view returns (bool); + /** * @notice module:user-config * @dev Delay, between the proposal is created and the vote starts. The unit this duration is expressed in depends diff --git a/contracts/governance/extensions/GovernorTimelockAccess.sol b/contracts/governance/extensions/GovernorTimelockAccess.sol new file mode 100644 index 00000000000..6ebb3f1d6cb --- /dev/null +++ b/contracts/governance/extensions/GovernorTimelockAccess.sol @@ -0,0 +1,282 @@ +// 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-relay} function. + * + * Note that some operations may be cancelable in the {AccessManager} by the admin or a set of guardians, depending on + * the restricted operation being invoked. Since proposals are atomic, the cancellation by a guardian of a single + * operation in a proposal will cause all of it to become unable to execute. + */ +abstract contract GovernorTimelockAccess is Governor { + // An execution plan is produced at the moment a proposal is created, in order to fix at that point the exact + // execution semantics of the proposal, namely whether a call will go through {AccessManager-relay}. + struct ExecutionPlan { + uint16 length; + uint32 delay; + // We use mappings instead of arrays because it allows us to pack values in storage more tightly without storing + // the length redundantly. + // We pack 8 operations' data in each bucket. Each uint32 value is set to 1 upon proposal creation if it has to + // be scheduled and relayed through the manager. Upon queuing, the value is set to nonce + 1, where the nonce is + // that which we get back from the manager when scheduling the operation. + mapping(uint256 operationBucket => uint32[8]) managerData; + } + + 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); + + event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds); + + /** + * @dev Initialize the governor with an {AccessManager} and initial base delay. + */ + constructor(address manager, uint32 initialBaseDelay) { + _manager = IAccessManager(manager); + _setBaseDelaySeconds(initialBaseDelay); + } + + /** + * @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 Public accessor to check the execution plan, including the number of seconds that the proposal will be + * delayed since queuing, and an array indicating which of the proposal actions will be executed indirectly through + * the associated {AccessManager}. + */ + function proposalExecutionPlan(uint256 proposalId) public view returns (uint32, bool[] memory) { + ExecutionPlan storage plan = _executionPlan[proposalId]; + + uint32 delay = plan.delay; + uint32 length = plan.length; + bool[] memory indirect = new bool[](length); + for (uint256 i = 0; i < length; ++i) { + (indirect[i], ) = _getManagerData(plan, i); + } + + return (delay, indirect); + } + + /** + * @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) { + uint32 delay = _detectExecutionRequirements(targets[i], bytes4(calldatas[i])); + if (delay > 0) { + _setManagerData(plan, i, 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 eta = Time.timestamp() + plan.delay; + + for (uint256 i = 0; i < targets.length; ++i) { + (bool delayed, ) = _getManagerData(plan, i); + if (delayed) { + (, uint32 nonce) = _manager.schedule(targets[i], calldatas[i], eta); + _setManagerData(plan, i, nonce); + } + } + + return eta; + } + + /** + * @dev Mechanism to execute a proposal, potentially going through {AccessManager-relay} for delayed operations. + */ + function _executeOperations( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 /* descriptionHash */ + ) internal virtual override { + uint48 eta = SafeCast.toUint48(proposalEta(proposalId)); + if (block.timestamp < eta) { + revert GovernorUnmetDelay(proposalId, eta); + } + + ExecutionPlan storage plan = _executionPlan[proposalId]; + + for (uint256 i = 0; i < targets.length; ++i) { + (bool delayed, uint32 nonce) = _getManagerData(plan, i); + if (delayed) { + uint32 relayedNonce = _manager.relay{value: values[i]}(targets[i], calldatas[i]); + if (relayedNonce != nonce) { + revert GovernorMismatchedNonce(proposalId, nonce, relayedNonce); + } + } 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 eta = SafeCast.toUint48(proposalEta(proposalId)); + + ExecutionPlan storage plan = _executionPlan[proposalId]; + + // If the proposal has been scheduled it will have an ETA and we have to externally cancel + if (eta != 0) { + for (uint256 i = 0; i < targets.length; ++i) { + (bool delayed, uint32 nonce) = _getManagerData(plan, i); + if (delayed) { + // Attempt to cancel considering the operation could have been cancelled and rescheduled already + uint32 canceledNonce = _manager.cancel(address(this), targets[i], calldatas[i]); + if (canceledNonce != nonce) { + revert GovernorMismatchedNonce(proposalId, nonce, canceledNonce); + } + } + } + } + + return proposalId; + } + + /** + * @dev Check if the execution of a call needs to be performed through an AccessManager and what delay should be + * applied to this call. + * + * Returns { manager: address(0), delay: 0 } if: + * - target does not have code + * - target does not implement IAccessManaged + * - calling canCall on the target's manager returns a 0 delay + * - calling canCall on the target's manager reverts + * Otherwise (calling canCall on the target's manager returns a non 0 delay), return the address of the + * AccessManager to use, and the delay for this call. + */ + function _detectExecutionRequirements(address target, bytes4 selector) private view returns (uint32 delay) { + (, delay) = AuthorityUtils.canCallWithDelay(address(_manager), address(this), target, selector); + } + + /** + * @dev Returns whether the operation at an index is delayed by the manager, and its scheduling nonce once queued. + */ + function _getManagerData(ExecutionPlan storage plan, uint256 index) private view returns (bool, uint32) { + (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index); + uint32 nonce = plan.managerData[bucket][subindex]; + unchecked { + return nonce > 0 ? (true, nonce - 1) : (false, 0); + } + } + + /** + * @dev Marks an operation at an index as delayed by the manager, and sets its scheduling nonce. + */ + function _setManagerData(ExecutionPlan storage plan, uint256 index, uint32 nonce) private { + (uint256 bucket, uint256 subindex) = _getManagerDataIndices(index); + plan.managerData[bucket][subindex] = nonce + 1; + } + + /** + * @dev Returns bucket and subindex for reading manager data from the packed array mapping. + */ + function _getManagerDataIndices(uint256 index) private pure returns (uint256 bucket, uint256 subindex) { + bucket = index >> 3; // index / 8 + subindex = index & 7; // index % 8 + } +} diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol index fe554dad5d2..a3d47bea122 100644 --- a/contracts/governance/extensions/GovernorTimelockCompound.sol +++ b/contracts/governance/extensions/GovernorTimelockCompound.sol @@ -54,6 +54,13 @@ abstract contract GovernorTimelockCompound is Governor { return address(_timelock); } + /** + * @dev See {IGovernor-proposalNeedsQueuing}. + */ + function proposalNeedsQueuing(uint256) public view virtual override returns (bool) { + return true; + } + /** * @dev Function to queue a proposal to the timelock. */ diff --git a/contracts/governance/extensions/GovernorTimelockControl.sol b/contracts/governance/extensions/GovernorTimelockControl.sol index c468f328b31..5f1e58ded6e 100644 --- a/contracts/governance/extensions/GovernorTimelockControl.sol +++ b/contracts/governance/extensions/GovernorTimelockControl.sol @@ -67,6 +67,13 @@ abstract contract GovernorTimelockControl is Governor { return address(_timelock); } + /** + * @dev See {IGovernor-proposalNeedsQueuing}. + */ + function proposalNeedsQueuing(uint256) public view virtual override returns (bool) { + return true; + } + /** * @dev Function to queue a proposal to the timelock. */ diff --git a/contracts/mocks/docs/governance/MyGovernor.sol b/contracts/mocks/docs/governance/MyGovernor.sol index 7cd5f3c2c74..d898fc71563 100644 --- a/contracts/mocks/docs/governance/MyGovernor.sol +++ b/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, diff --git a/contracts/mocks/governance/GovernorStorageMock.sol b/contracts/mocks/governance/GovernorStorageMock.sol index 3d08a003170..88c6bf906bf 100644 --- a/contracts/mocks/governance/GovernorStorageMock.sol +++ b/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, diff --git a/contracts/mocks/governance/GovernorTimelockAccessMock.sol b/contracts/mocks/governance/GovernorTimelockAccessMock.sol new file mode 100644 index 00000000000..3d1bbeeef97 --- /dev/null +++ b/contracts/mocks/governance/GovernorTimelockAccessMock.sol @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IGovernor, Governor} from "../../governance/Governor.sol"; +import {GovernorTimelockAccess} from "../../governance/extensions/GovernorTimelockAccess.sol"; +import {GovernorSettings} from "../../governance/extensions/GovernorSettings.sol"; +import {GovernorCountingSimple} from "../../governance/extensions/GovernorCountingSimple.sol"; +import {GovernorVotesQuorumFraction} from "../../governance/extensions/GovernorVotesQuorumFraction.sol"; + +abstract contract GovernorTimelockAccessMock is + GovernorSettings, + GovernorTimelockAccess, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ + function nonGovernanceFunction() external {} + + function quorum(uint256 blockNumber) public view override(Governor, GovernorVotesQuorumFraction) returns (uint256) { + return super.quorum(blockNumber); + } + + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } + + function proposalNeedsQueuing( + uint256 proposalId + ) public view virtual override(Governor, GovernorTimelockAccess) returns (bool) { + return super.proposalNeedsQueuing(proposalId); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public override(Governor, GovernorTimelockAccess) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + + function _queueOperations( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockAccess) returns (uint48) { + return super._queueOperations(proposalId, targets, values, calldatas, descriptionHash); + } + + function _executeOperations( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockAccess) { + super._executeOperations(proposalId, targets, values, calldatas, descriptionHash); + } + + function _cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockAccess) returns (uint256) { + return super._cancel(targets, values, calldatas, descriptionHash); + } +} diff --git a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol index 124b0346fbf..03ef62510f5 100644 --- a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol +++ b/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, diff --git a/contracts/mocks/governance/GovernorTimelockControlMock.sol b/contracts/mocks/governance/GovernorTimelockControlMock.sol index 125aa37df11..edaccc0b769 100644 --- a/contracts/mocks/governance/GovernorTimelockControlMock.sol +++ b/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, diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 9dfa53e7bbd..45cceba9a89 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -100,6 +100,9 @@ contract('Governor', function (accounts) { expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(value); expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal('0'); + expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0'); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(false); + // Run proposal const txPropose = await this.helper.propose({ from: proposer }); @@ -164,6 +167,9 @@ contract('Governor', function (accounts) { expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(true); expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal('0'); expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal(value); + + expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0'); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(false); }); it('send ethers', async function () { diff --git a/test/governance/extensions/GovernorTimelockAccess.test.js b/test/governance/extensions/GovernorTimelockAccess.test.js new file mode 100644 index 00000000000..776ec9390e3 --- /dev/null +++ b/test/governance/extensions/GovernorTimelockAccess.test.js @@ -0,0 +1,246 @@ +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 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 classId = '1'; + const groupId = '1'; + + await this.manager.setContractClass(this.receiver.address, classId, { from: admin }); + await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin }); + await this.manager.grantGroup(groupId, this.mock.address, delay, { from: admin }); + + this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr'); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + const txQueue = await this.helper.queue(); + await this.helper.waitForEta(); + const txExecute = await this.helper.execute(); + + expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id }); + await expectEvent.inTransaction(txQueue.tx, this.manager, 'OperationScheduled', { + operationId: this.restricted.operationId, + nonce: '1', + schedule: web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(delay), + caller: this.mock.address, + target: this.restricted.operation.target, + data: this.restricted.operation.data, + }); + + expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id }); + await expectEvent.inTransaction(txExecute.tx, this.manager, 'OperationExecuted', { + operationId: this.restricted.operationId, + nonce: '1', + }); + await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledRestricted'); + }); + + it('bundle of varied operations', async function () { + const managerDelay = 1000; + const classId = '1'; + const groupId = '1'; + + const baseDelay = managerDelay * 2; + + await this.mock.$_setBaseDelaySeconds(baseDelay); + + await this.manager.setContractClass(this.receiver.address, classId, { from: admin }); + await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin }); + await this.manager.grantGroup(groupId, this.mock.address, managerDelay, { from: admin }); + + this.proposal = await this.helper.setProposal( + [this.restricted.operation, this.unrestricted.operation], + 'descr', + ); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + const txQueue = await this.helper.queue(); + await this.helper.waitForEta(); + const txExecute = await this.helper.execute(); + + expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id }); + await expectEvent.inTransaction(txQueue.tx, this.manager, 'OperationScheduled', { + operationId: this.restricted.operationId, + nonce: '1', + schedule: web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(baseDelay), + caller: this.mock.address, + target: this.restricted.operation.target, + data: this.restricted.operation.data, + }); + + expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id }); + await expectEvent.inTransaction(txExecute.tx, this.manager, 'OperationExecuted', { + operationId: this.restricted.operationId, + nonce: '1', + }); + await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledRestricted'); + await expectEvent.inTransaction(txExecute.tx, this.receiver, 'CalledUnrestricted'); + }); + + it('cancellation after queue (internal)', async function () { + const delay = 1000; + const classId = '1'; + const groupId = '1'; + + await this.manager.setContractClass(this.receiver.address, classId, { from: admin }); + await this.manager.setClassFunctionGroup(classId, [this.restricted.selector], groupId, { from: admin }); + await this.manager.grantGroup(groupId, this.mock.address, delay, { from: admin }); + + this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr'); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + 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]), + ]); + }); + }); + } +}); diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index 0f7fd635255..e9d6f83736a 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -5,6 +5,7 @@ const Enums = require('../../helpers/enums'); const { GovernorHelper, proposalStatesToBitMap } = require('../../helpers/governance'); const { expectRevertCustomError } = require('../../helpers/customError'); const { computeCreateAddress } = require('../../helpers/create'); +const { clockFromReceipt } = require('../../helpers/time'); const Timelock = artifacts.require('CompTimelock'); const Governor = artifacts.require('$GovernorTimelockCompoundMock'); @@ -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(); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index fe40d30e9e1..ec03d614475 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -4,6 +4,7 @@ const { expect } = require('chai'); const Enums = require('../../helpers/enums'); const { GovernorHelper, proposalStatesToBitMap, timelockSalt } = require('../../helpers/governance'); const { expectRevertCustomError } = require('../../helpers/customError'); +const { clockFromReceipt } = require('../../helpers/time'); const Timelock = artifacts.require('TimelockController'); const Governor = artifacts.require('$GovernorTimelockControlMock'); @@ -33,13 +34,15 @@ contract('GovernorTimelockControl', function (accounts) { const votingPeriod = web3.utils.toBN(16); const value = web3.utils.toWei('1'); + const delay = 3600; + for (const { mode, Token } of TOKENS) { describe(`using ${Token._json.contractName}`, function () { beforeEach(async function () { const [deployer] = await web3.eth.getAccounts(); this.token = await Token.new(tokenName, tokenSymbol, tokenName, version); - this.timelock = await Timelock.new(3600, [], [], deployer); + this.timelock = await Timelock.new(delay, [], [], deployer); this.mock = await Governor.new( name, votingDelay, @@ -107,6 +110,9 @@ contract('GovernorTimelockControl', function (accounts) { }); it('nominal', async function () { + expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0'); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true); + await this.helper.propose(); await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); @@ -116,6 +122,11 @@ contract('GovernorTimelockControl', function (accounts) { await this.helper.waitForDeadline(); const txQueue = await this.helper.queue(); await this.helper.waitForEta(); + + const eta = web3.utils.toBN(await clockFromReceipt.timestamp(txQueue.receipt)).addn(delay); + expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal(eta); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(true); + const txExecute = await this.helper.execute(); expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id }); @@ -352,11 +363,10 @@ contract('GovernorTimelockControl', function (accounts) { const data = this.mock.contract.methods.relay(constants.ZERO_ADDRESS, 0, '0x').encodeABI(); const predecessor = constants.ZERO_BYTES32; const salt = constants.ZERO_BYTES32; - const delay = 3600; await this.timelock.schedule(target, value, data, predecessor, salt, delay, { from: owner }); - await time.increase(3600); + await time.increase(delay); await expectRevertCustomError( this.timelock.execute(target, value, data, predecessor, salt, { from: owner }), @@ -369,7 +379,7 @@ contract('GovernorTimelockControl', function (accounts) { describe('updateTimelock', function () { beforeEach(async function () { this.newTimelock = await Timelock.new( - 3600, + delay, [this.mock.address], [this.mock.address], constants.ZERO_ADDRESS, diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index d254082d2da..49a30e75576 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -61,6 +61,7 @@ const INTERFACES = { 'proposalDeadline(uint256)', 'proposalProposer(uint256)', 'proposalEta(uint256)', + 'proposalNeedsQueuing(uint256)', 'votingDelay()', 'votingPeriod()', 'quorum(uint256)',