From 8b57738ccadd25032acf8ee60be54b6e95ae5a99 Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 23 Mar 2022 11:28:00 -0300 Subject: [PATCH] Authorizer: Rename and improve wording (#1167) * authorizer: rename to timelock authorizer * authorizer: improve internal wording * authorizer: rename permission constants * authorizer: fix tests * authorizer: apply code review suggestions * fix linter --- contracts/Authorizer.sol | 358 ---------------- contracts/TimelockAuthorizer.sol | 387 ++++++++++++++++++ test/AssetManagement.test.ts | 2 +- test/ExitPool.test.ts | 2 +- test/FlashLoan.test.ts | 2 +- test/InternalBalance.test.ts | 2 +- test/JoinPool.test.ts | 2 +- test/PoolRegistry.test.ts | 2 +- test/SwapValidation.test.ts | 2 +- test/Swaps.test.ts | 2 +- ...zer.test.ts => TimelockAuthorizer.test.ts} | 222 +++++----- test/VaultAuthorization.test.ts | 4 +- 12 files changed, 511 insertions(+), 476 deletions(-) delete mode 100644 contracts/Authorizer.sol create mode 100644 contracts/TimelockAuthorizer.sol rename test/{Authorizer.test.ts => TimelockAuthorizer.test.ts} (85%) diff --git a/contracts/Authorizer.sol b/contracts/Authorizer.sol deleted file mode 100644 index b59feb9619..0000000000 --- a/contracts/Authorizer.sol +++ /dev/null @@ -1,358 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -pragma solidity ^0.7.0; - -import "@balancer-labs/v2-solidity-utils/contracts/openzeppelin/Address.sol"; -import "@balancer-labs/v2-solidity-utils/contracts/helpers/InputHelpers.sol"; -import "@balancer-labs/v2-solidity-utils/contracts/helpers/BalancerErrors.sol"; -import "@balancer-labs/v2-solidity-utils/contracts/helpers/IAuthentication.sol"; - -import "./interfaces/IAuthorizer.sol"; -import "./interfaces/IVault.sol"; - -/** - * @dev Basic Authorizer implementation, based on OpenZeppelin's Access Control. - * - * Users are allowed to perform actions if they have the role with the same identifier. In this sense, roles are not - * being truly used as such, since they each map to a single action identifier. - * - * This temporary implementation is expected to be replaced soon after launch by a more sophisticated one, able to - * manage permissions across multiple contracts and to natively handle timelocks. - */ -contract Authorizer is IAuthorizer { - using Address for address; - - uint256 public constant MAX_DELAY = 2 * (365 days); - address public constant EVERYWHERE = address(-1); - - bytes32 public constant GRANT_PERMISSION = keccak256("GRANT_PERMISSION"); - bytes32 public constant REVOKE_PERMISSION = keccak256("REVOKE_PERMISSION"); - bytes32 public constant EXECUTE_PERMISSION = keccak256("EXECUTE_PERMISSION"); - bytes32 public constant SET_DELAY_PERMISSION = keccak256("SET_DELAY_PERMISSION"); - - struct ScheduledAction { - address where; - bytes data; - bool executed; - bool cancelled; - bool protected; - uint256 executableAt; - } - - IAuthentication private immutable _vault; - - ScheduledAction[] public scheduledActions; - mapping(bytes32 => bool) public permissionGranted; - mapping(bytes32 => uint256) public delays; - - /** - * @dev Emitted when a new action with ID `id` is scheduled - */ - event ActionScheduled(bytes32 indexed action, uint256 indexed id); - - /** - * @dev Emitted when an action with ID `id` is executed - */ - event ActionExecuted(uint256 indexed id); - - /** - * @dev Emitted when an action with ID `id` is cancelled - */ - event ActionCancelled(uint256 indexed id); - - /** - * @dev Emitted when a new `delay` is set in order to perform `action` - */ - event ActionDelaySet(bytes32 indexed action, uint256 delay); - - /** - * @dev Emitted when `account` is granted permission to perform `action` in `where`. - */ - event PermissionGranted(bytes32 indexed action, address indexed account, address indexed where); - - /** - * @dev Emitted when an `account`'s permission to perform `action` is revoked from `where`. - */ - event PermissionRevoked(bytes32 indexed action, address indexed account, address indexed where); - - constructor(address admin, IAuthentication vault) { - _vault = vault; - _grantPermission(GRANT_PERMISSION, admin, EVERYWHERE); - _grantPermission(REVOKE_PERMISSION, admin, EVERYWHERE); - } - - function getVault() external view returns (address) { - return address(_vault); - } - - /** - * @dev Tells the permission ID for action `action`, account `account` and target `where` - */ - function permissionId( - bytes32 action, - address account, - address where - ) public pure returns (bytes32) { - return keccak256(abi.encodePacked(action, account, where)); - } - - /** - * @dev Tells whether `account` has explicit permission to perform `action` in `where` - */ - function hasPermission( - bytes32 action, - address account, - address where - ) public view returns (bool) { - return - permissionGranted[permissionId(action, account, where)] || - permissionGranted[permissionId(action, account, EVERYWHERE)]; - } - - /** - * @dev Tells whether `account` can perform `action` in `where` - */ - function canPerform( - bytes32 action, - address account, - address where - ) public view override returns (bool) { - return (delays[action] > 0) ? account == address(this) : hasPermission(action, account, where); - } - - /** - * @dev Sets a new delay for `action` - */ - function setDelay(bytes32 action, uint256 delay) external { - _require(msg.sender == address(this), Errors.SENDER_NOT_ALLOWED); - - bytes32 setAuthorizerId = _vault.getActionId(IVault.setAuthorizer.selector); - require(action == setAuthorizerId || delay <= delays[setAuthorizerId], "DELAY_EXCEEDS_SET_AUTHORIZER"); - - delays[action] = delay; - emit ActionDelaySet(action, delay); - } - - /** - * @dev Schedules a delay change of `newDelay` for `action` - */ - function scheduleDelayChange( - bytes32 action, - uint256 newDelay, - address[] memory executors - ) external returns (uint256 id) { - require(newDelay <= MAX_DELAY, "DELAY_TOO_LARGE"); - bytes32 setDelayAction = keccak256(abi.encodePacked(SET_DELAY_PERMISSION, action)); - _require(hasPermission(setDelayAction, msg.sender, address(this)), Errors.SENDER_NOT_ALLOWED); - - uint256 actionDelay = delays[action]; - bytes memory data = abi.encodeWithSelector(this.setDelay.selector, action, newDelay); - return _scheduleWithDelay(setDelayAction, address(this), data, actionDelay, executors); - } - - /** - * @dev Schedules a new action - */ - function schedule( - address where, - bytes memory data, - address[] memory executors - ) external returns (uint256 id) { - require(where != address(this), "CANNOT_SCHEDULE_AUTHORIZER_ACTIONS"); - bytes32 action = IAuthentication(where).getActionId(_decodeSelector(data)); - _require(hasPermission(action, msg.sender, where), Errors.SENDER_NOT_ALLOWED); - return _schedule(action, where, data, executors); - } - - /** - * @dev Executes action `id` - */ - function execute(uint256 id) external returns (bytes memory result) { - require(id < scheduledActions.length, "ACTION_DOES_NOT_EXIST"); - ScheduledAction storage scheduledAction = scheduledActions[id]; - require(!scheduledAction.executed, "ACTION_ALREADY_EXECUTED"); - require(!scheduledAction.cancelled, "ACTION_ALREADY_CANCELLED"); - - // solhint-disable-next-line not-rely-on-time - require(block.timestamp >= scheduledAction.executableAt, "ACTION_NOT_EXECUTABLE"); - bool isAllowed = !scheduledAction.protected || hasPermission(_executeActionId(id), msg.sender, address(this)); - _require(isAllowed, Errors.SENDER_NOT_ALLOWED); - - scheduledAction.executed = true; - result = scheduledAction.where.functionCall(scheduledAction.data); - emit ActionExecuted(id); - } - - /** - * @dev Cancels action `id` - */ - function cancel(uint256 id) external { - require(id < scheduledActions.length, "ACTION_DOES_NOT_EXIST"); - ScheduledAction storage scheduledAction = scheduledActions[id]; - - require(!scheduledAction.executed, "ACTION_ALREADY_EXECUTED"); - require(!scheduledAction.cancelled, "ACTION_ALREADY_CANCELLED"); - - bytes32 action = IAuthentication(scheduledAction.where).getActionId(_decodeSelector(scheduledAction.data)); - _require(hasPermission(action, msg.sender, scheduledAction.where), Errors.SENDER_NOT_ALLOWED); - - scheduledAction.cancelled = true; - emit ActionCancelled(id); - } - - /** - * @dev Grants multiple permissions to a single account - */ - function grantPermissions( - bytes32[] memory actions, - address account, - address[] memory where - ) external { - InputHelpers.ensureInputLengthMatch(actions.length, where.length); - for (uint256 i = 0; i < actions.length; i++) { - _require(canPerform(GRANT_PERMISSION, msg.sender, where[i]), Errors.SENDER_NOT_ALLOWED); - _grantPermission(actions[i], account, where[i]); - } - } - - /** - * @dev Schedules a grant permission to a single account - */ - function scheduleGrantPermission( - bytes32 action, - address account, - address where, - address[] memory executors - ) external returns (uint256 id) { - _require(hasPermission(GRANT_PERMISSION, msg.sender, where), Errors.SENDER_NOT_ALLOWED); - bytes memory data = abi.encodeWithSelector(this.grantPermissions.selector, _arr(action), account, _arr(where)); - return _schedule(GRANT_PERMISSION, address(this), data, executors); - } - - /** - * @dev Revokes multiple permissions from a single account - */ - function revokePermissions( - bytes32[] memory actions, - address account, - address[] memory where - ) external { - InputHelpers.ensureInputLengthMatch(actions.length, where.length); - for (uint256 i = 0; i < actions.length; i++) { - _require(canPerform(REVOKE_PERMISSION, msg.sender, where[i]), Errors.SENDER_NOT_ALLOWED); - _revokePermission(actions[i], account, where[i]); - } - } - - /** - * @dev Schedules a revoke permission for a single account - */ - function scheduleRevokePermission( - bytes32 action, - address account, - address where, - address[] memory executors - ) external returns (uint256 id) { - _require(hasPermission(REVOKE_PERMISSION, msg.sender, where), Errors.SENDER_NOT_ALLOWED); - bytes memory data = abi.encodeWithSelector(this.revokePermissions.selector, _arr(action), account, _arr(where)); - return _schedule(REVOKE_PERMISSION, address(this), data, executors); - } - - /** - * @dev Renounces from multiple permissions - */ - function renouncePermissions(bytes32[] memory actions, address[] memory where) external { - InputHelpers.ensureInputLengthMatch(actions.length, where.length); - for (uint256 i = 0; i < actions.length; i++) { - _revokePermission(actions[i], msg.sender, where[i]); - } - } - - function _grantPermission( - bytes32 action, - address account, - address where - ) private { - bytes32 permission = permissionId(action, account, where); - if (!permissionGranted[permission]) { - permissionGranted[permission] = true; - emit PermissionGranted(action, account, where); - } - } - - function _revokePermission( - bytes32 action, - address account, - address where - ) private { - bytes32 permission = permissionId(action, account, where); - if (permissionGranted[permission]) { - permissionGranted[permission] = false; - emit PermissionRevoked(action, account, where); - } - } - - function _schedule( - bytes32 action, - address where, - bytes memory data, - address[] memory executors - ) private returns (uint256 id) { - uint256 delay = delays[action]; - require(delay > 0, "CANNOT_SCHEDULE_ACTION"); - return _scheduleWithDelay(action, where, data, delay, executors); - } - - function _scheduleWithDelay( - bytes32 action, - address where, - bytes memory data, - uint256 delay, - address[] memory executors - ) private returns (uint256 id) { - id = scheduledActions.length; - emit ActionScheduled(action, id); - - // solhint-disable-next-line not-rely-on-time - uint256 executableAt = block.timestamp + delay; - bool protected = executors.length > 0; - scheduledActions.push(ScheduledAction(where, data, false, false, protected, executableAt)); - - bytes32 executeActionId = _executeActionId(id); - for (uint256 i = 0; i < executors.length; i++) { - _grantPermission(executeActionId, executors[i], address(this)); - } - } - - function _executeActionId(uint256 id) internal pure returns (bytes32) { - return keccak256(abi.encodePacked(EXECUTE_PERMISSION, id)); - } - - function _decodeSelector(bytes memory data) internal pure returns (bytes4) { - // The bytes4 type is left-aligned and padded with zeros: we make use of that property to build the selector - if (data.length < 4) return bytes4(0); - return bytes4(data[0]) | (bytes4(data[1]) >> 8) | (bytes4(data[2]) >> 16) | (bytes4(data[3]) >> 24); - } - - function _arr(bytes32 item) private pure returns (bytes32[] memory result) { - result = new bytes32[](1); - result[0] = item; - } - - function _arr(address item) private pure returns (address[] memory result) { - result = new address[](1); - result[0] = item; - } -} diff --git a/contracts/TimelockAuthorizer.sol b/contracts/TimelockAuthorizer.sol new file mode 100644 index 0000000000..ccbfd1f218 --- /dev/null +++ b/contracts/TimelockAuthorizer.sol @@ -0,0 +1,387 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +pragma solidity ^0.7.0; + +import "@balancer-labs/v2-solidity-utils/contracts/openzeppelin/Address.sol"; +import "@balancer-labs/v2-solidity-utils/contracts/helpers/InputHelpers.sol"; +import "@balancer-labs/v2-solidity-utils/contracts/helpers/BalancerErrors.sol"; +import "@balancer-labs/v2-solidity-utils/contracts/helpers/IAuthentication.sol"; + +import "./interfaces/IAuthorizer.sol"; +import "./interfaces/IVault.sol"; + +/** + * @dev Basic Authorizer implementation using timelocks. + * + * Users are allowed to perform actions if they have the permission to do so. + * + * This Authorizer implementation allows defining a delay per action identifier. If a delay is set for an action, users + * are now allowed to schedule an execution that will be triggered in the future by the Authorizer instead of executing + * it themselves directly. + * + * Glossary: + * - Action: Op that can be performed to a target contract. These are identified by a unique bytes32 `actionId` defined + * by each target contract following `IAuthentication#getActionId`. + * - Scheduled execution: The Authorizer can define different delays per `actionId` in order to determine that a + * specific time window must pass before these can be executed. When a delay is set for an `actionId`, executions + * must be scheduled. These executions are identified with an unsigned integer called `scheduledExecutionId`. + * - Permission: Unique identifier to refer to a user (who) that is allowed to perform an action (what) in a specific + * target contract (where). This identifier is called `permissionId` and is computed as + * `keccak256(actionId, account, where)`. + */ +contract TimelockAuthorizer is IAuthorizer, IAuthentication { + using Address for address; + + uint256 public constant MAX_DELAY = 2 * (365 days); + address public constant EVERYWHERE = address(-1); + + struct ScheduledExecution { + address where; + bytes data; + bool executed; + bool cancelled; + bool protected; + uint256 executableAt; + } + + // solhint-disable var-name-mixedcase + bytes32 public immutable GRANT_ACTION_ID; + bytes32 public immutable REVOKE_ACTION_ID; + bytes32 public immutable SCHEDULE_DELAY_ACTION_ID; + + IAuthentication private immutable _vault; + ScheduledExecution[] public scheduledExecutions; + mapping(bytes32 => bool) public isPermissionGranted; + mapping(bytes32 => uint256) public delaysPerActionId; + + /** + * @dev Emitted when a new action `actionId` is scheduled + */ + event ExecutionScheduled(bytes32 indexed actionId, uint256 indexed scheduledExecutionId); + + /** + * @dev Emitted when an action `actionId` is executed + */ + event ActionExecuted(uint256 indexed scheduledExecutionId); + + /** + * @dev Emitted when an action `actionId` is cancelled + */ + event ActionCancelled(uint256 indexed scheduledExecutionId); + + /** + * @dev Emitted when a new `delay` is set in order to perform action `actionId` + */ + event ActionDelaySet(bytes32 indexed actionId, uint256 delay); + + /** + * @dev Emitted when `account` is granted permission to perform action `actionId` in `where`. + */ + event PermissionGranted(bytes32 indexed actionId, address indexed account, address indexed where); + + /** + * @dev Emitted when an `account`'s permission to perform action `actionId` is revoked from `where`. + */ + event PermissionRevoked(bytes32 indexed actionId, address indexed account, address indexed where); + + constructor(address admin, IAuthentication vault) { + _vault = vault; + GRANT_ACTION_ID = getActionId(TimelockAuthorizer.grantPermissions.selector); + REVOKE_ACTION_ID = getActionId(TimelockAuthorizer.revokePermissions.selector); + SCHEDULE_DELAY_ACTION_ID = getActionId(TimelockAuthorizer.scheduleDelayChange.selector); + + _grantPermission(getActionId(TimelockAuthorizer.grantPermissions.selector), admin, EVERYWHERE); + _grantPermission(getActionId(TimelockAuthorizer.revokePermissions.selector), admin, EVERYWHERE); + } + + /** + * @dev Tells the action ID for a certain function selector + */ + function getActionId(bytes4 selector) public view override returns (bytes32) { + return keccak256(abi.encodePacked(bytes32(uint256(address(this))), selector)); + } + + function getVault() external view returns (address) { + return address(_vault); + } + + /** + * @dev Tells the permission ID for action `actionId`, account `account` and target `where` + */ + function permissionId( + bytes32 actionId, + address account, + address where + ) public pure returns (bytes32) { + return keccak256(abi.encodePacked(actionId, account, where)); + } + + /** + * @dev Tells whether `account` has explicit permission to perform action `actionId` in `where` + */ + function hasPermission( + bytes32 actionId, + address account, + address where + ) public view returns (bool) { + return + isPermissionGranted[permissionId(actionId, account, where)] || + isPermissionGranted[permissionId(actionId, account, EVERYWHERE)]; + } + + /** + * @dev Tells whether `account` can perform action `actionId` in `where` + */ + function canPerform( + bytes32 actionId, + address account, + address where + ) public view override returns (bool) { + return (delaysPerActionId[actionId] > 0) ? account == address(this) : hasPermission(actionId, account, where); + } + + /** + * @dev Sets a new delay for action `actionId` + */ + function setDelay(bytes32 actionId, uint256 delay) external { + _require(msg.sender == address(this), Errors.SENDER_NOT_ALLOWED); + + bytes32 setAuthorizerActionId = _vault.getActionId(IVault.setAuthorizer.selector); + bool isAllowed = actionId == setAuthorizerActionId || delay <= delaysPerActionId[setAuthorizerActionId]; + require(isAllowed, "DELAY_EXCEEDS_SET_AUTHORIZER"); + + delaysPerActionId[actionId] = delay; + emit ActionDelaySet(actionId, delay); + } + + /** + * @dev Schedules a delay change of `newDelay` for action `actionId` + */ + function scheduleDelayChange( + bytes32 actionId, + uint256 newDelay, + address[] memory executors + ) external returns (uint256 scheduledExecutionId) { + require(newDelay <= MAX_DELAY, "DELAY_TOO_LARGE"); + bytes32 scheduleDelayActionId = keccak256(abi.encodePacked(SCHEDULE_DELAY_ACTION_ID, actionId)); + _require(hasPermission(scheduleDelayActionId, msg.sender, address(this)), Errors.SENDER_NOT_ALLOWED); + + // The delay change is scheduled to execute after the current delay for the action has elapsed. This is + // critical, as otherwise it'd be possible to execute an action with a delay shorter than its current one + // by first changing it to a smaller (or zero) value. + + uint256 actionDelay = delaysPerActionId[actionId]; + bytes memory data = abi.encodeWithSelector(this.setDelay.selector, actionId, newDelay); + return _scheduleWithDelay(scheduleDelayActionId, address(this), data, actionDelay, executors); + } + + /** + * @dev Schedules a new action + */ + function schedule( + address where, + bytes memory data, + address[] memory executors + ) external returns (uint256 scheduledExecutionId) { + require(where != address(this), "CANNOT_SCHEDULE_AUTHORIZER_ACTIONS"); + bytes32 actionId = IAuthentication(where).getActionId(_decodeSelector(data)); + _require(hasPermission(actionId, msg.sender, where), Errors.SENDER_NOT_ALLOWED); + return _schedule(actionId, where, data, executors); + } + + /** + * @dev Executes a scheduled action `scheduledExecutionId` + */ + function execute(uint256 scheduledExecutionId) external returns (bytes memory result) { + require(scheduledExecutionId < scheduledExecutions.length, "ACTION_DOES_NOT_EXIST"); + ScheduledExecution storage scheduledExecution = scheduledExecutions[scheduledExecutionId]; + require(!scheduledExecution.executed, "ACTION_ALREADY_EXECUTED"); + require(!scheduledExecution.cancelled, "ACTION_ALREADY_CANCELLED"); + + // solhint-disable-next-line not-rely-on-time + require(block.timestamp >= scheduledExecution.executableAt, "ACTION_NOT_EXECUTABLE"); + bytes32 executeActionId = _getExecuteActionId(scheduledExecutionId); + bool isAllowed = !scheduledExecution.protected || hasPermission(executeActionId, msg.sender, address(this)); + _require(isAllowed, Errors.SENDER_NOT_ALLOWED); + + scheduledExecution.executed = true; + result = scheduledExecution.where.functionCall(scheduledExecution.data); + emit ActionExecuted(scheduledExecutionId); + } + + /** + * @dev Cancels a scheduled action `scheduledExecutionId` + */ + function cancel(uint256 scheduledExecutionId) external { + require(scheduledExecutionId < scheduledExecutions.length, "ACTION_DOES_NOT_EXIST"); + ScheduledExecution storage scheduledExecution = scheduledExecutions[scheduledExecutionId]; + + require(!scheduledExecution.executed, "ACTION_ALREADY_EXECUTED"); + require(!scheduledExecution.cancelled, "ACTION_ALREADY_CANCELLED"); + + // The permission to cancel a scheduled action is the same one used to schedule it + IAuthentication target = IAuthentication(scheduledExecution.where); + bytes32 actionId = target.getActionId(_decodeSelector(scheduledExecution.data)); + _require(hasPermission(actionId, msg.sender, scheduledExecution.where), Errors.SENDER_NOT_ALLOWED); + + scheduledExecution.cancelled = true; + emit ActionCancelled(scheduledExecutionId); + } + + /** + * @dev Grants multiple permissions to a single account + */ + function grantPermissions( + bytes32[] memory actionIds, + address account, + address[] memory where + ) external { + InputHelpers.ensureInputLengthMatch(actionIds.length, where.length); + for (uint256 i = 0; i < actionIds.length; i++) { + _require(canPerform(GRANT_ACTION_ID, msg.sender, where[i]), Errors.SENDER_NOT_ALLOWED); + _grantPermission(actionIds[i], account, where[i]); + } + } + + /** + * @dev Schedules a grant permission to a single account + */ + function scheduleGrantPermission( + bytes32 actionId, + address account, + address where, + address[] memory executors + ) external returns (uint256 scheduledExecutionId) { + _require(hasPermission(GRANT_ACTION_ID, msg.sender, where), Errors.SENDER_NOT_ALLOWED); + bytes memory data = abi.encodeWithSelector(this.grantPermissions.selector, _ar(actionId), account, _ar(where)); + return _schedule(GRANT_ACTION_ID, address(this), data, executors); + } + + /** + * @dev Revokes multiple permissions from a single account + */ + function revokePermissions( + bytes32[] memory actionIds, + address account, + address[] memory where + ) external { + InputHelpers.ensureInputLengthMatch(actionIds.length, where.length); + for (uint256 i = 0; i < actionIds.length; i++) { + _require(canPerform(REVOKE_ACTION_ID, msg.sender, where[i]), Errors.SENDER_NOT_ALLOWED); + _revokePermission(actionIds[i], account, where[i]); + } + } + + /** + * @dev Schedules a revoke permission for a single account + */ + function scheduleRevokePermission( + bytes32 actionId, + address account, + address where, + address[] memory executors + ) external returns (uint256 scheduledExecutionId) { + _require(hasPermission(REVOKE_ACTION_ID, msg.sender, where), Errors.SENDER_NOT_ALLOWED); + bytes memory data = abi.encodeWithSelector(this.revokePermissions.selector, _ar(actionId), account, _ar(where)); + return _schedule(REVOKE_ACTION_ID, address(this), data, executors); + } + + /** + * @dev Renounces from multiple permissions + */ + function renouncePermissions(bytes32[] memory actionIds, address[] memory where) external { + InputHelpers.ensureInputLengthMatch(actionIds.length, where.length); + for (uint256 i = 0; i < actionIds.length; i++) { + _revokePermission(actionIds[i], msg.sender, where[i]); + } + } + + function _grantPermission( + bytes32 actionId, + address account, + address where + ) private { + bytes32 permission = permissionId(actionId, account, where); + if (!isPermissionGranted[permission]) { + isPermissionGranted[permission] = true; + emit PermissionGranted(actionId, account, where); + } + } + + function _revokePermission( + bytes32 actionId, + address account, + address where + ) private { + bytes32 permission = permissionId(actionId, account, where); + if (isPermissionGranted[permission]) { + isPermissionGranted[permission] = false; + emit PermissionRevoked(actionId, account, where); + } + } + + function _schedule( + bytes32 actionId, + address where, + bytes memory data, + address[] memory executors + ) private returns (uint256 scheduledExecutionId) { + uint256 delay = delaysPerActionId[actionId]; + require(delay > 0, "CANNOT_SCHEDULE_ACTION"); + return _scheduleWithDelay(actionId, where, data, delay, executors); + } + + function _scheduleWithDelay( + bytes32 actionId, + address where, + bytes memory data, + uint256 delay, + address[] memory executors + ) private returns (uint256 scheduledExecutionId) { + scheduledExecutionId = scheduledExecutions.length; + emit ExecutionScheduled(actionId, scheduledExecutionId); + + // solhint-disable-next-line not-rely-on-time + uint256 executableAt = block.timestamp + delay; + bool protected = executors.length > 0; + scheduledExecutions.push(ScheduledExecution(where, data, false, false, protected, executableAt)); + + bytes32 executeActionId = _getExecuteActionId(scheduledExecutionId); + for (uint256 i = 0; i < executors.length; i++) { + _grantPermission(executeActionId, executors[i], address(this)); + } + } + + function _getExecuteActionId(uint256 scheduledExecutionId) internal view returns (bytes32) { + return + keccak256(abi.encodePacked(bytes32(uint256(address(this))), this.execute.selector, scheduledExecutionId)); + } + + function _decodeSelector(bytes memory data) internal pure returns (bytes4) { + // The bytes4 type is left-aligned and padded with zeros: we make use of that property to build the selector + if (data.length < 4) return bytes4(0); + return bytes4(data[0]) | (bytes4(data[1]) >> 8) | (bytes4(data[2]) >> 16) | (bytes4(data[3]) >> 24); + } + + function _ar(bytes32 item) private pure returns (bytes32[] memory result) { + result = new bytes32[](1); + result[0] = item; + } + + function _ar(address item) private pure returns (address[] memory result) { + result = new address[](1); + result[0] = item; + } +} diff --git a/test/AssetManagement.test.ts b/test/AssetManagement.test.ts index e76b0ba995..e417d96420 100644 --- a/test/AssetManagement.test.ts +++ b/test/AssetManagement.test.ts @@ -31,7 +31,7 @@ describe('Asset Management', function () { }); sharedBeforeEach('deploy vault', async () => { - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, ZERO_ADDRESS, MONTH, MONTH] }); }); diff --git a/test/ExitPool.test.ts b/test/ExitPool.test.ts index d42401db00..abd2558df2 100644 --- a/test/ExitPool.test.ts +++ b/test/ExitPool.test.ts @@ -33,7 +33,7 @@ describe('Exit Pool', () => { sharedBeforeEach('deploy vault & tokens', async () => { const WETH = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, WETH.address, MONTH, MONTH] }); vault = vault.connect(lp); feesCollector = await deployedAt('ProtocolFeesCollector', await vault.getProtocolFeesCollector()); diff --git a/test/FlashLoan.test.ts b/test/FlashLoan.test.ts index 5a1c55e65b..004ad90788 100644 --- a/test/FlashLoan.test.ts +++ b/test/FlashLoan.test.ts @@ -25,7 +25,7 @@ describe('Flash Loans', () => { sharedBeforeEach('deploy vault & tokens', async () => { const WETH = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, WETH.address, 0, 0] }); recipient = await deploy('MockFlashLoanRecipient', { from: other, args: [vault.address] }); feesCollector = await deployedAt('ProtocolFeesCollector', await vault.getProtocolFeesCollector()); diff --git a/test/InternalBalance.test.ts b/test/InternalBalance.test.ts index c283e16ec7..bdd1ca823b 100644 --- a/test/InternalBalance.test.ts +++ b/test/InternalBalance.test.ts @@ -38,7 +38,7 @@ describe('Internal Balance', () => { tokens = await TokenList.create(['DAI', 'MKR'], { sorted: true }); weth = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, weth.address, MONTH, MONTH] }); }); diff --git a/test/JoinPool.test.ts b/test/JoinPool.test.ts index e4fcdb1baa..fd01b49d72 100644 --- a/test/JoinPool.test.ts +++ b/test/JoinPool.test.ts @@ -30,7 +30,7 @@ describe('Join Pool', () => { sharedBeforeEach('deploy vault & tokens', async () => { const WETH = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, WETH.address, MONTH, MONTH] }); feesCollector = await deployedAt('ProtocolFeesCollector', await vault.getProtocolFeesCollector()); diff --git a/test/PoolRegistry.test.ts b/test/PoolRegistry.test.ts index 92bbfa734a..322e2302c3 100644 --- a/test/PoolRegistry.test.ts +++ b/test/PoolRegistry.test.ts @@ -26,7 +26,7 @@ describe('PoolRegistry', () => { sharedBeforeEach('deploy vault & tokens', async () => { const weth = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, weth.address, 0, 0] }); allTokens = await TokenList.create(['DAI', 'MKR', 'SNX'], { sorted: true }); diff --git a/test/SwapValidation.test.ts b/test/SwapValidation.test.ts index 97da36dd6c..0a99da1f3e 100644 --- a/test/SwapValidation.test.ts +++ b/test/SwapValidation.test.ts @@ -30,7 +30,7 @@ describe('Swap Validation', () => { sharedBeforeEach('setup', async () => { const WETH = await TokensDeployer.deployToken({ symbol: 'WETH' }); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, WETH.address, MONTH, MONTH] }); tokens = await TokenList.create(['DAI', 'MKR', 'SNX', 'BAT'], { sorted: true }); diff --git a/test/Swaps.test.ts b/test/Swaps.test.ts index e2e98e80ed..58a1d77ce1 100644 --- a/test/Swaps.test.ts +++ b/test/Swaps.test.ts @@ -62,7 +62,7 @@ describe('Swaps', () => { sharedBeforeEach('deploy vault and tokens', async () => { tokens = await TokenList.create(['DAI', 'MKR', 'SNX', 'WETH']); - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, tokens.WETH.address, 0, 0] }); await tokens.mint({ to: [lp, trader], amount: bn(200e18) }); diff --git a/test/Authorizer.test.ts b/test/TimelockAuthorizer.test.ts similarity index 85% rename from test/Authorizer.test.ts rename to test/TimelockAuthorizer.test.ts index bb8119ab1e..47ab443e01 100644 --- a/test/Authorizer.test.ts +++ b/test/TimelockAuthorizer.test.ts @@ -4,15 +4,15 @@ import { Contract } from 'ethers'; import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/dist/src/signer-with-address'; import * as expectEvent from '@balancer-labs/v2-helpers/src/test/expectEvent'; -import Authorizer from '@balancer-labs/v2-helpers/src/models/authorizer/Authorizer'; +import TimelockAuthorizer from '@balancer-labs/v2-helpers/src/models/authorizer/TimelockAuthorizer'; import { deploy } from '@balancer-labs/v2-helpers/src/contract'; import { actionId } from '@balancer-labs/v2-helpers/src/models/misc/actions'; import { BigNumberish } from '@balancer-labs/v2-helpers/src/numbers'; import { ZERO_ADDRESS } from '@balancer-labs/v2-helpers/src/constants'; import { advanceTime, currentTimestamp, DAY } from '@balancer-labs/v2-helpers/src/time'; -describe('Authorizer', () => { - let authorizer: Authorizer, vault: Contract; +describe('TimelockAuthorizer', () => { + let authorizer: TimelockAuthorizer, vault: Contract; let admin: SignerWithAddress, grantee: SignerWithAddress, from: SignerWithAddress; before('setup signers', async () => { @@ -23,15 +23,15 @@ describe('Authorizer', () => { const ACTION_2 = '0x0000000000000000000000000000000000000000000000000000000000000002'; const ACTIONS = [ACTION_1, ACTION_2]; - const EVERYWHERE = Authorizer.EVERYWHERE; + const EVERYWHERE = TimelockAuthorizer.EVERYWHERE; const WHERE = [ethers.Wallet.createRandom().address, ethers.Wallet.createRandom().address]; const NOT_WHERE = ethers.Wallet.createRandom().address; sharedBeforeEach('deploy authorizer', async () => { - const oldAuthorizer = await Authorizer.create({ admin }); + const oldAuthorizer = await TimelockAuthorizer.create({ admin }); vault = await deploy('Vault', { args: [oldAuthorizer.address, ZERO_ADDRESS, 0, 0] }); - authorizer = await Authorizer.create({ admin, vault }); + authorizer = await TimelockAuthorizer.create({ admin, vault }); const setAuthorizerAction = await actionId(vault, 'setAuthorizer'); await oldAuthorizer.grantPermissions(setAuthorizerAction, admin, vault, { from: admin }); @@ -39,90 +39,87 @@ describe('Authorizer', () => { }); describe('admin', () => { - let GRANT_PERMISSION: string, REVOKE_PERMISSION: string; + let GRANT_ACTION_ID: string, REVOKE_ACTION_ID: string; sharedBeforeEach('set constants', async () => { - GRANT_PERMISSION = await authorizer.GRANT_PERMISSION(); - REVOKE_PERMISSION = await authorizer.REVOKE_PERMISSION(); + GRANT_ACTION_ID = await authorizer.GRANT_ACTION_ID(); + REVOKE_ACTION_ID = await authorizer.REVOKE_ACTION_ID(); }); it('defines its permissions correctly', async () => { - expect(GRANT_PERMISSION).to.be.equal(ethers.utils.solidityKeccak256(['string'], ['GRANT_PERMISSION'])); - expect(REVOKE_PERMISSION).to.be.equal(ethers.utils.solidityKeccak256(['string'], ['REVOKE_PERMISSION'])); - const expectedGrantId = ethers.utils.solidityKeccak256( ['bytes32', 'address', 'address'], - [GRANT_PERMISSION, admin.address, EVERYWHERE] + [GRANT_ACTION_ID, admin.address, EVERYWHERE] ); - expect(await authorizer.permissionId(GRANT_PERMISSION, admin, EVERYWHERE)).to.be.equal(expectedGrantId); + expect(await authorizer.permissionId(GRANT_ACTION_ID, admin, EVERYWHERE)).to.be.equal(expectedGrantId); const expectedRevokeId = ethers.utils.solidityKeccak256( ['bytes32', 'address', 'address'], - [REVOKE_PERMISSION, admin.address, EVERYWHERE] + [REVOKE_ACTION_ID, admin.address, EVERYWHERE] ); - expect(await authorizer.permissionId(REVOKE_PERMISSION, admin, EVERYWHERE)).to.be.equal(expectedRevokeId); + expect(await authorizer.permissionId(REVOKE_ACTION_ID, admin, EVERYWHERE)).to.be.equal(expectedRevokeId); }); it('can grant permissions everywhere', async () => { - expect(await authorizer.canPerform(GRANT_PERMISSION, admin, WHERE)).to.be.true; - expect(await authorizer.canPerform(GRANT_PERMISSION, admin, EVERYWHERE)).to.be.true; + expect(await authorizer.canPerform(GRANT_ACTION_ID, admin, WHERE)).to.be.true; + expect(await authorizer.canPerform(GRANT_ACTION_ID, admin, EVERYWHERE)).to.be.true; }); it('can revoke permissions everywhere', async () => { - expect(await authorizer.canPerform(REVOKE_PERMISSION, admin, WHERE)).to.be.true; - expect(await authorizer.canPerform(REVOKE_PERMISSION, admin, EVERYWHERE)).to.be.true; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, admin, WHERE)).to.be.true; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, admin, EVERYWHERE)).to.be.true; }); it('can grant permission to other address to grant permissions for a custom contract', async () => { - await authorizer.grantPermissions(GRANT_PERMISSION, grantee, WHERE[0], { from: admin }); + await authorizer.grantPermissions(GRANT_ACTION_ID, grantee, WHERE[0], { from: admin }); - expect(await authorizer.canPerform(GRANT_PERMISSION, grantee, WHERE[0])).to.be.true; - expect(await authorizer.canPerform(GRANT_PERMISSION, grantee, EVERYWHERE)).to.be.false; + expect(await authorizer.canPerform(GRANT_ACTION_ID, grantee, WHERE[0])).to.be.true; + expect(await authorizer.canPerform(GRANT_ACTION_ID, grantee, EVERYWHERE)).to.be.false; }); it('can grant permission to other address to grant permissions everywhere', async () => { - await authorizer.grantPermissionsGlobally(GRANT_PERMISSION, grantee, { from: admin }); + await authorizer.grantPermissionsGlobally(GRANT_ACTION_ID, grantee, { from: admin }); - expect(await authorizer.canPerform(GRANT_PERMISSION, grantee, WHERE)).to.be.true; - expect(await authorizer.canPerform(GRANT_PERMISSION, grantee, EVERYWHERE)).to.be.true; + expect(await authorizer.canPerform(GRANT_ACTION_ID, grantee, WHERE)).to.be.true; + expect(await authorizer.canPerform(GRANT_ACTION_ID, grantee, EVERYWHERE)).to.be.true; }); it('can grant permission to other address to revoke permissions for a custom contract', async () => { - await authorizer.grantPermissions(REVOKE_PERMISSION, grantee, WHERE[0], { from: admin }); + await authorizer.grantPermissions(REVOKE_ACTION_ID, grantee, WHERE[0], { from: admin }); - expect(await authorizer.canPerform(REVOKE_PERMISSION, grantee, WHERE[0])).to.be.true; - expect(await authorizer.canPerform(REVOKE_PERMISSION, grantee, EVERYWHERE)).to.be.false; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, grantee, WHERE[0])).to.be.true; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, grantee, EVERYWHERE)).to.be.false; }); it('can grant permission to other address to revoke permissions everywhere', async () => { - await authorizer.grantPermissionsGlobally(REVOKE_PERMISSION, grantee, { from: admin }); + await authorizer.grantPermissionsGlobally(REVOKE_ACTION_ID, grantee, { from: admin }); - expect(await authorizer.canPerform(REVOKE_PERMISSION, grantee, WHERE)).to.be.true; - expect(await authorizer.canPerform(REVOKE_PERMISSION, grantee, EVERYWHERE)).to.be.true; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, grantee, WHERE)).to.be.true; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, grantee, EVERYWHERE)).to.be.true; }); it('can have their global permissions revoked by an authorized address for any contract', async () => { - await authorizer.grantPermissions(REVOKE_PERMISSION, grantee, EVERYWHERE, { from: admin }); + await authorizer.grantPermissions(REVOKE_ACTION_ID, grantee, EVERYWHERE, { from: admin }); - await authorizer.revokePermissions(GRANT_PERMISSION, admin, EVERYWHERE, { from: grantee }); - expect(await authorizer.canPerform(GRANT_PERMISSION, admin, WHERE)).to.be.false; - expect(await authorizer.canPerform(GRANT_PERMISSION, admin, EVERYWHERE)).to.be.false; + await authorizer.revokePermissions(GRANT_ACTION_ID, admin, EVERYWHERE, { from: grantee }); + expect(await authorizer.canPerform(GRANT_ACTION_ID, admin, WHERE)).to.be.false; + expect(await authorizer.canPerform(GRANT_ACTION_ID, admin, EVERYWHERE)).to.be.false; - await authorizer.revokePermissions(REVOKE_PERMISSION, admin, EVERYWHERE, { from: grantee }); - expect(await authorizer.canPerform(REVOKE_PERMISSION, admin, WHERE)).to.be.false; - expect(await authorizer.canPerform(REVOKE_PERMISSION, admin, EVERYWHERE)).to.be.false; + await authorizer.revokePermissions(REVOKE_ACTION_ID, admin, EVERYWHERE, { from: grantee }); + expect(await authorizer.canPerform(REVOKE_ACTION_ID, admin, WHERE)).to.be.false; + expect(await authorizer.canPerform(REVOKE_ACTION_ID, admin, EVERYWHERE)).to.be.false; }); it('cannot have their global permissions revoked by an authorized address for a specific contract', async () => { - await authorizer.grantPermissions(REVOKE_PERMISSION, grantee, WHERE[0], { from: admin }); - await authorizer.grantPermissions(REVOKE_PERMISSION, grantee, WHERE[1], { from: admin }); + await authorizer.grantPermissions(REVOKE_ACTION_ID, grantee, WHERE[0], { from: admin }); + await authorizer.grantPermissions(REVOKE_ACTION_ID, grantee, WHERE[1], { from: admin }); await expect( - authorizer.revokePermissions(GRANT_PERMISSION, admin, EVERYWHERE, { from: grantee }) + authorizer.revokePermissions(GRANT_ACTION_ID, admin, EVERYWHERE, { from: grantee }) ).to.be.revertedWith('SENDER_NOT_ALLOWED'); await expect( - authorizer.revokePermissions(REVOKE_PERMISSION, admin, EVERYWHERE, { from: grantee }) + authorizer.revokePermissions(REVOKE_ACTION_ID, admin, EVERYWHERE, { from: grantee }) ).to.be.revertedWith('SENDER_NOT_ALLOWED'); }); }); @@ -159,7 +156,7 @@ describe('Authorizer', () => { ACTIONS.forEach((action, i) => { expectEvent.inReceipt(receipt, 'PermissionGranted', { - action, + actionId: action, account: grantee.address, where: WHERE[i], }); @@ -169,12 +166,16 @@ describe('Authorizer', () => { context('when there is a delay set to grant permissions', () => { const delay = DAY; - const GRANT_PERMISSION = ethers.utils.solidityKeccak256(['string'], ['GRANT_PERMISSION']); + let GRANT_ACTION_ID: string; + + sharedBeforeEach('set constants', async () => { + GRANT_ACTION_ID = await authorizer.GRANT_ACTION_ID(); + }); sharedBeforeEach('set delay', async () => { const setAuthorizerAction = await actionId(vault, 'setAuthorizer'); await authorizer.setDelay(setAuthorizerAction, delay * 2, { from: admin }); - await authorizer.setDelay(GRANT_PERMISSION, delay, { from: admin }); + await authorizer.setDelay(GRANT_ACTION_ID, delay, { from: admin }); }); it('reverts', async () => { @@ -254,7 +255,7 @@ describe('Authorizer', () => { ACTIONS.forEach((action, i) => { expectEvent.inReceipt(receipt, 'PermissionGranted', { - action, + actionId: action, account: grantee.address, where: WHERE[i], }); @@ -301,9 +302,9 @@ describe('Authorizer', () => { for (const action of ACTIONS) { expectEvent.inReceipt(receipt, 'PermissionGranted', { - action, + actionId: action, account: grantee.address, - where: Authorizer.EVERYWHERE, + where: TimelockAuthorizer.EVERYWHERE, }); } }); @@ -332,9 +333,9 @@ describe('Authorizer', () => { for (const action of ACTIONS) { expectEvent.inReceipt(receipt, 'PermissionGranted', { - action, + actionId: action, account: grantee.address, - where: Authorizer.EVERYWHERE, + where: TimelockAuthorizer.EVERYWHERE, }); } }); @@ -425,7 +426,7 @@ describe('Authorizer', () => { ACTIONS.forEach((action, i) => { expectEvent.inReceipt(receipt, 'PermissionRevoked', { - action, + actionId: action, account: grantee.address, where: WHERE[i], }); @@ -435,12 +436,16 @@ describe('Authorizer', () => { context('when there is a delay set to grant permissions', () => { const delay = DAY; - const REVOKE_PERMISSION = ethers.utils.solidityKeccak256(['string'], ['REVOKE_PERMISSION']); + let REVOKE_ACTION_ID: string; + + sharedBeforeEach('set constants', async () => { + REVOKE_ACTION_ID = await authorizer.REVOKE_ACTION_ID(); + }); sharedBeforeEach('set delay', async () => { const setAuthorizerAction = await actionId(vault, 'setAuthorizer'); await authorizer.setDelay(setAuthorizerAction, delay * 2, { from: admin }); - await authorizer.setDelay(REVOKE_PERMISSION, delay, { from: admin }); + await authorizer.setDelay(REVOKE_ACTION_ID, delay, { from: admin }); }); it('reverts', async () => { @@ -571,9 +576,9 @@ describe('Authorizer', () => { for (const action of ACTIONS) { expectEvent.inReceipt(receipt, 'PermissionRevoked', { - action, + actionId: action, account: grantee.address, - where: Authorizer.EVERYWHERE, + where: TimelockAuthorizer.EVERYWHERE, }); } }); @@ -713,10 +718,11 @@ describe('Authorizer', () => { describe('setDelay', () => { const action = ACTION_1; - const SET_DELAY_PERMISSION = ethers.utils.solidityKeccak256(['string'], ['SET_DELAY_PERMISSION']); const grantPermission = async (actionId: string) => { - const setDelayAction = ethers.utils.solidityKeccak256(['bytes32', 'bytes32'], [SET_DELAY_PERMISSION, actionId]); + const SCHEDULE_DELAY_ACTION_ID = await authorizer.SCHEDULE_DELAY_ACTION_ID(); + const args = [SCHEDULE_DELAY_ACTION_ID, actionId]; + const setDelayAction = ethers.utils.solidityKeccak256(['bytes32', 'bytes32'], args); await authorizer.grantPermissions(setDelayAction, admin, authorizer, { from: admin }); }; @@ -745,12 +751,12 @@ describe('Authorizer', () => { it('schedules a delay change', async () => { const id = await authorizer.scheduleDelayChange(action, delay, [], { from: admin }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.false; - expect(scheduledAction.data).to.be.equal(expectedData); - expect(scheduledAction.where).to.be.equal(authorizer.address); - expect(scheduledAction.protected).to.be.false; - expect(scheduledAction.executableAt).to.be.at.most(await currentTimestamp()); + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.false; + expect(scheduledExecution.data).to.be.equal(expectedData); + expect(scheduledExecution.where).to.be.equal(authorizer.address); + expect(scheduledExecution.protected).to.be.false; + expect(scheduledExecution.executableAt).to.be.at.most(await currentTimestamp()); }); it('can be executed immediately', async () => { @@ -764,7 +770,7 @@ describe('Authorizer', () => { const id = await authorizer.scheduleDelayChange(action, delay, [], { from: admin }); const receipt = await authorizer.execute(id); - expectEvent.inReceipt(await receipt.wait(), 'ActionDelaySet', { action, delay }); + expectEvent.inReceipt(await receipt.wait(), 'ActionDelaySet', { actionId: action, delay }); }); }); @@ -779,12 +785,12 @@ describe('Authorizer', () => { it('schedules a delay change', async () => { const id = await authorizer.scheduleDelayChange(action, delay, [], { from: admin }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.false; - expect(scheduledAction.data).to.be.equal(expectedData); - expect(scheduledAction.where).to.be.equal(authorizer.address); - expect(scheduledAction.protected).to.be.false; - expect(scheduledAction.executableAt).to.be.at.most((await currentTimestamp()).add(previousDelay)); + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.false; + expect(scheduledExecution.data).to.be.equal(expectedData); + expect(scheduledExecution.where).to.be.equal(authorizer.address); + expect(scheduledExecution.protected).to.be.false; + expect(scheduledExecution.executableAt).to.be.at.most((await currentTimestamp()).add(previousDelay)); }); it('cannot be executed immediately', async () => { @@ -802,7 +808,7 @@ describe('Authorizer', () => { await advanceTime(previousDelay); const receipt = await authorizer.execute(id); - expectEvent.inReceipt(await receipt.wait(), 'ActionDelaySet', { action, delay }); + expectEvent.inReceipt(await receipt.wait(), 'ActionDelaySet', { actionId: action, delay }); }); }); }); @@ -850,10 +856,10 @@ describe('Authorizer', () => { describe('schedule', () => { let where: Contract, action: string, data: string, executors: SignerWithAddress[]; - let anotherVault: Contract, newAuthorizer: Authorizer; + let anotherVault: Contract, newAuthorizer: TimelockAuthorizer; sharedBeforeEach('deploy sample instances', async () => { - newAuthorizer = await Authorizer.create({ admin }); + newAuthorizer = await TimelockAuthorizer.create({ admin }); anotherVault = await deploy('Vault', { args: [authorizer.address, ZERO_ADDRESS, 0, 0] }); }); @@ -890,15 +896,15 @@ describe('Authorizer', () => { executors = []; }); - it('schedules a non-protected action', async () => { + it('schedules a non-protected execution', async () => { const id = await schedule(); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.false; - expect(scheduledAction.data).to.be.equal(data); - expect(scheduledAction.where).to.be.equal(where.address); - expect(scheduledAction.protected).to.be.false; - expect(scheduledAction.executableAt).to.be.at.most((await currentTimestamp()).add(delay)); + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.false; + expect(scheduledExecution.data).to.be.equal(data); + expect(scheduledExecution.where).to.be.equal(where.address); + expect(scheduledExecution.protected).to.be.false; + expect(scheduledExecution.executableAt).to.be.at.most((await currentTimestamp()).add(delay)); }); it('cannot execute the action immediately', async () => { @@ -911,10 +917,10 @@ describe('Authorizer', () => { await advanceTime(delay); const receipt = await authorizer.execute(id); - expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { id }); + expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { scheduledExecutionId: id }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.true; + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.true; expect(await vault.getAuthorizer()).to.be.equal(newAuthorizer.address); }); @@ -933,15 +939,15 @@ describe('Authorizer', () => { executors = [admin]; }); - it('schedules the requested action', async () => { + it('schedules the requested execution', async () => { const id = await schedule(); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.false; - expect(scheduledAction.data).to.be.equal(data); - expect(scheduledAction.where).to.be.equal(where.address); - expect(scheduledAction.protected).to.be.true; - expect(scheduledAction.executableAt).to.be.at.most((await currentTimestamp()).add(delay)); + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.false; + expect(scheduledExecution.data).to.be.equal(data); + expect(scheduledExecution.where).to.be.equal(where.address); + expect(scheduledExecution.protected).to.be.true; + expect(scheduledExecution.executableAt).to.be.at.most((await currentTimestamp()).add(delay)); }); it('cannot execute the action immediately', async () => { @@ -958,10 +964,10 @@ describe('Authorizer', () => { await expect(authorizer.execute(id, { from: grantee })).to.be.revertedWith('SENDER_NOT_ALLOWED'); const receipt = await authorizer.execute(id, { from: executors[0] }); - expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { id }); + expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { scheduledExecutionId: id }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.true; + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.true; expect(await vault.getAuthorizer()).to.be.equal(newAuthorizer.address); }); @@ -1028,10 +1034,10 @@ describe('Authorizer', () => { describe('execute', () => { const delay = DAY; - let executors: SignerWithAddress[], newAuthorizer: Authorizer; + let executors: SignerWithAddress[], newAuthorizer: TimelockAuthorizer; sharedBeforeEach('deploy sample instances', async () => { - newAuthorizer = await Authorizer.create({ admin }); + newAuthorizer = await TimelockAuthorizer.create({ admin }); }); sharedBeforeEach('grant set authorizer permission with delay', async () => { @@ -1059,7 +1065,7 @@ describe('Authorizer', () => { }); context('when the action was not cancelled', () => { - sharedBeforeEach('schedule action', async () => { + sharedBeforeEach('schedule execution', async () => { id = await schedule(); }); @@ -1071,8 +1077,8 @@ describe('Authorizer', () => { it('executes the action', async () => { await authorizer.execute(id, { from }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.true; + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.true; expect(await vault.getAuthorizer()).to.be.equal(newAuthorizer.address); }); @@ -1080,7 +1086,7 @@ describe('Authorizer', () => { it('emits an event', async () => { const receipt = await authorizer.execute(id, { from }); - expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { id }); + expectEvent.inReceipt(await receipt.wait(), 'ActionExecuted', { scheduledExecutionId: id }); }); it('cannot be executed twice', async () => { @@ -1134,8 +1140,8 @@ describe('Authorizer', () => { await authorizer.execute(id); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.executed).to.be.true; + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.executed).to.be.true; expect(await vault.getAuthorizer()).to.be.equal(newAuthorizer.address); }); @@ -1151,10 +1157,10 @@ describe('Authorizer', () => { describe('cancel', () => { const delay = DAY; - let executors: SignerWithAddress[], newAuthorizer: Authorizer; + let executors: SignerWithAddress[], newAuthorizer: TimelockAuthorizer; sharedBeforeEach('deploy sample instances', async () => { - newAuthorizer = await Authorizer.create({ admin }); + newAuthorizer = await TimelockAuthorizer.create({ admin }); }); sharedBeforeEach('grant set authorizer permission with delay', async () => { @@ -1177,21 +1183,21 @@ describe('Authorizer', () => { }); context('when the action was not executed', () => { - sharedBeforeEach('schedule action', async () => { + sharedBeforeEach('schedule execution', async () => { id = await schedule(); }); it('cancels the action', async () => { await authorizer.cancel(id, { from }); - const scheduledAction = await authorizer.scheduledActions(id); - expect(scheduledAction.cancelled).to.be.true; + const scheduledExecution = await authorizer.scheduledExecutions(id); + expect(scheduledExecution.cancelled).to.be.true; }); it('emits an event', async () => { const receipt = await authorizer.cancel(id, { from }); - expectEvent.inReceipt(await receipt.wait(), 'ActionCancelled', { id }); + expectEvent.inReceipt(await receipt.wait(), 'ActionCancelled', { scheduledExecutionId: id }); }); it('cannot be cancelled twice', async () => { diff --git a/test/VaultAuthorization.test.ts b/test/VaultAuthorization.test.ts index 6674b5ddd9..7f6fc6c462 100644 --- a/test/VaultAuthorization.test.ts +++ b/test/VaultAuthorization.test.ts @@ -22,7 +22,7 @@ describe('VaultAuthorization', function () { }); sharedBeforeEach('deploy authorizer', async () => { - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); }); async function deployVault(authorizer: string): Promise { @@ -243,7 +243,7 @@ describe('VaultAuthorization', function () { const BUFFER_PERIOD_DURATION = MONTH; sharedBeforeEach(async () => { - authorizer = await deploy('Authorizer', { args: [admin.address, ZERO_ADDRESS] }); + authorizer = await deploy('TimelockAuthorizer', { args: [admin.address, ZERO_ADDRESS] }); vault = await deploy('Vault', { args: [authorizer.address, ZERO_ADDRESS, PAUSE_WINDOW_DURATION, BUFFER_PERIOD_DURATION], });