From 9bb8008c2334e669aeea576620a53fe0cba763c3 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 7 Aug 2023 06:57:10 +0200 Subject: [PATCH] Access Manager (#4416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García Co-authored-by: Francisco Giordano --- contracts/access/manager/AccessManaged.sol | 93 +- contracts/access/manager/AccessManager.sol | 981 ++++++++--- .../access/manager/AccessManagerAdapter.sol | 54 - contracts/access/manager/AuthorityUtils.sol | 31 + contracts/access/manager/IAccessManaged.sol | 17 + contracts/access/manager/IAccessManager.sol | 110 ++ contracts/access/manager/IAuthority.sol | 2 +- contracts/mocks/AccessManagedTarget.sol | 18 + contracts/mocks/AccessManagerMocks.sol | 34 - contracts/utils/types/Time.sol | 139 ++ package-lock.json | 14 +- test/access/manager/AccessManaged.test.js | 55 - test/access/manager/AccessManager.test.js | 1508 ++++++++++++----- test/helpers/enums.js | 1 - test/helpers/methods.js | 5 + 15 files changed, 2184 insertions(+), 878 deletions(-) delete mode 100644 contracts/access/manager/AccessManagerAdapter.sol create mode 100644 contracts/access/manager/AuthorityUtils.sol create mode 100644 contracts/access/manager/IAccessManaged.sol create mode 100644 contracts/access/manager/IAccessManager.sol create mode 100644 contracts/mocks/AccessManagedTarget.sol delete mode 100644 contracts/mocks/AccessManagerMocks.sol create mode 100644 contracts/utils/types/Time.sol delete mode 100644 test/access/manager/AccessManaged.test.js create mode 100644 test/helpers/methods.js diff --git a/contracts/access/manager/AccessManaged.sol b/contracts/access/manager/AccessManaged.sol index 08da07c58..087aee4c4 100644 --- a/contracts/access/manager/AccessManaged.sol +++ b/contracts/access/manager/AccessManaged.sol @@ -1,9 +1,12 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity ^0.8.20; -import "../../utils/Context.sol"; -import "./IAuthority.sol"; +import {IAuthority} from "./IAuthority.sol"; +import {AuthorityUtils} from "./AuthorityUtils.sol"; +import {IAccessManager} from "./IAccessManager.sol"; +import {IAccessManaged} from "./IAccessManaged.sol"; +import {Context} from "../../utils/Context.sol"; /** * @dev This contract module makes available a {restricted} modifier. Functions decorated with this modifier will be @@ -13,10 +16,17 @@ import "./IAuthority.sol"; * IMPORTANT: The `restricted` modifier should never be used on `internal` functions, judiciously used in `public` * functions, and ideally only used in `external` functions. See {restricted}. */ -contract AccessManaged is Context { - event AuthorityUpdated(address indexed sender, IAuthority indexed newAuthority); +abstract contract AccessManaged is Context, IAccessManaged { + address private _authority; - IAuthority private _authority; + bool private _consumingSchedule; + + /** + * @dev Initializes the contract connected to an initial authority. + */ + constructor(address initialAuthority) { + _setAuthority(initialAuthority); + } /** * @dev Restricts access to a function as defined by the connected Authority for this contract and the @@ -24,9 +34,9 @@ contract AccessManaged is Context { * * [IMPORTANT] * ==== - * In general, this modifier should only be used on `external` functions. It is okay to use it on `public` functions - * that are used as external entry points and are not called internally. Unless you know what you're doing, it - * should never be used on `internal` functions. Failure to follow these rules can have critical security + * In general, this modifier should only be used on `external` functions. It is okay to use it on `public` + * functions that are used as external entry points and are not called internally. Unless you know what you're + * doing, it should never be used on `internal` functions. Failure to follow these rules can have critical security * implications! This is because the permissions are determined by the function that entered the contract, i.e. the * function at the bottom of the call stack, and not the function where the modifier is visible in the source code. * ==== @@ -35,53 +45,76 @@ contract AccessManaged is Context { * ==== * Selector collisions are mitigated by scoping permissions per contract, but some edge cases must be considered: * - * * If the https://docs.soliditylang.org/en/latest/contracts.html#receive-ether-function[`receive()`] function is restricted, - * any other function with a `0x00000000` selector will share permissions with `receive()`. - * * Similarly, if there's no `receive()` function but a `fallback()` instead, the fallback might be called with empty `calldata`, - * sharing the `0x00000000` selector permissions as well. - * * For any other selector, if the restricted function is set on an upgradeable contract, an upgrade may remove the restricted - * function and replace it with a new method whose selector replaces the last one, keeping the previous permissions. + * * If the https://docs.soliditylang.org/en/v0.8.20/contracts.html#receive-ether-function[`receive()`] function + * is restricted, any other function with a `0x00000000` selector will share permissions with `receive()`. + * * Similarly, if there's no `receive()` function but a `fallback()` instead, the fallback might be called with + * empty `calldata`, sharing the `0x00000000` selector permissions as well. + * * For any other selector, if the restricted function is set on an upgradeable contract, an upgrade may remove + * the restricted function and replace it with a new method whose selector replaces the last one, keeping the + * previous permissions. * ==== */ modifier restricted() { - _checkCanCall(_msgSender(), msg.sig); + _checkCanCall(_msgSender(), _msgData()); _; } - /** - * @dev Initializes the contract connected to an initial authority. - */ - constructor(IAuthority initialAuthority) { - _setAuthority(initialAuthority); - } - /** * @dev Returns the current authority. */ - function authority() public view virtual returns (IAuthority) { + function authority() public view virtual returns (address) { return _authority; } /** * @dev Transfers control to a new authority. The caller must be the current authority. */ - function setAuthority(IAuthority newAuthority) public virtual { - require(_msgSender() == address(_authority), "AccessManaged: not current authority"); + function setAuthority(address newAuthority) public virtual { + address caller = _msgSender(); + if (caller != authority()) { + revert AccessManagedUnauthorized(caller); + } + if (newAuthority.code.length == 0) { + revert AccessManagedInvalidAuthority(newAuthority); + } _setAuthority(newAuthority); } + /** + * @dev Returns true only in the context of a delayed restricted call, at the moment that the scheduled operation is + * being consumed. Prevents denial of service for delayed restricted calls in the case that the contract performs + * attacker controlled calls. + */ + function isConsumingScheduledOp() public view returns (bool) { + return _consumingSchedule; + } + /** * @dev Transfers control to a new authority. Internal function with no access restriction. */ - function _setAuthority(IAuthority newAuthority) internal virtual { + function _setAuthority(address newAuthority) internal virtual { _authority = newAuthority; - emit AuthorityUpdated(_msgSender(), newAuthority); + emit AuthorityUpdated(newAuthority); } /** * @dev Reverts if the caller is not allowed to call the function identified by a selector. */ - function _checkCanCall(address caller, bytes4 selector) internal view virtual { - require(_authority.canCall(caller, address(this), selector), "AccessManaged: authority rejected"); + function _checkCanCall(address caller, bytes calldata data) internal virtual { + (bool allowed, uint32 delay) = AuthorityUtils.canCallWithDelay( + authority(), + caller, + address(this), + bytes4(data) + ); + if (!allowed) { + if (delay > 0) { + _consumingSchedule = true; + IAccessManager(authority()).consumeScheduledOp(caller, data); + _consumingSchedule = false; + } else { + revert AccessManagedUnauthorized(caller); + } + } } } diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol index 2efa667f2..d3b65b0e2 100644 --- a/contracts/access/manager/AccessManager.sol +++ b/contracts/access/manager/AccessManager.sol @@ -1,52 +1,13 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.13; +pragma solidity ^0.8.20; -import "../AccessControlDefaultAdminRules.sol"; -import "./IAuthority.sol"; -import "./AccessManaged.sol"; - -interface IAccessManager is IAuthority, IAccessControlDefaultAdminRules { - enum AccessMode { - Custom, - Closed, - Open - } - - event GroupUpdated(uint8 indexed group, string name); - - event GroupAllowed(address indexed target, bytes4 indexed selector, uint8 indexed group, bool allowed); - - event AccessModeUpdated(address indexed target, AccessMode previousMode, AccessMode indexed mode); - - function createGroup(uint8 group, string calldata name) external; - - function updateGroupName(uint8 group, string calldata name) external; - - function hasGroup(uint8 group) external view returns (bool); - - function getUserGroups(address user) external view returns (bytes32 groups); - - function grantGroup(uint8 group, address user) external; - - function revokeGroup(uint8 group, address user) external; - - function renounceGroup(uint8 group, address user) external; - - function getFunctionAllowedGroups(address target, bytes4 selector) external view returns (bytes32 groups); - - function setFunctionAllowedGroup(address target, bytes4[] calldata selectors, uint8 group, bool allowed) external; - - function getContractMode(address target) external view returns (AccessMode); - - function setContractModeCustom(address target) external; - - function setContractModeOpen(address target) external; - - function setContractModeClosed(address target) external; - - function transferContractAuthority(address target, address newAuthority) external; -} +import {IAccessManager} from "./IAccessManager.sol"; +import {IAccessManaged} from "./IAccessManaged.sol"; +import {Address} from "../../utils/Address.sol"; +import {Context} from "../../utils/Context.sol"; +import {Multicall} from "../../utils/Multicall.sol"; +import {Time} from "../../utils/types/Time.sol"; /** * @dev AccessManager is a central contract to store the permissions of a system. @@ -55,9 +16,8 @@ interface IAccessManager is IAuthority, IAccessControlDefaultAdminRules { * exact details of how access is restricted for each of those functions is configurable by the admins of the instance. * These restrictions are expressed in terms of "groups". * - * An AccessManager instance will define a set of groups. Each of them must be created before they can be granted, with - * a maximum of 255 created groups. Users can be added into any number of these groups. Each of them defines an - * AccessControl role, and may confer access to some of the restricted functions in the system, as configured by admins + * An AccessManager instance will define a set of groups. Accounts can be added into any number of these groups. Each of + * them defines a role, and may confer access to some of the restricted functions in the system, as configured by admins * through the use of {setFunctionAllowedGroup}. * * Note that a function in a target contract may become permissioned in this way only when: 1) said contract is @@ -66,277 +26,784 @@ interface IAccessManager is IAuthority, IAccessControlDefaultAdminRules { * * There is a special group defined by default named "public" which all accounts automatically have. * - * Contracts can also be configured in two special modes: 1) the "open" mode, where all functions are allowed to the - * "public" group, and 2) the "closed" mode, where no function is allowed to any group. + * Contracts where functions are mapped to groups are said to be in a "custom" mode, but contracts can also be + * configured in two special modes: 1) the "open" mode, where all functions are allowed to the "public" group, and 2) + * the "closed" mode, where no function is allowed to any group. * * Since all the permissions of the managed system can be modified by the admins of this instance, it is expected that - * it will be highly secured (e.g., a multisig or a well-configured DAO). Additionally, {AccessControlDefaultAdminRules} - * is included to enforce security rules on this account. + * they will be highly secured (e.g., a multisig or a well-configured DAO). * - * NOTE: Some of the functions in this contract, such as {getUserGroups}, return a `bytes32` bitmap to succinctly - * represent a set of groups. In a bitmap, bit `n` (counting from the least significant bit) will be 1 if and only if - * the group with number `n` is in the set. For example, the hex value `0x05` represents the set of the two groups - * numbered 0 and 2 from its binary equivalence `0b101` + * NOTE: This contract implements a form of the {IAuthority} interface, but {canCall} has additional return data so it + * doesn't inherit `IAuthority`. It is however compatible with the `IAuthority` interface since the first 32 bytes of + * the return data are a boolean as expected by that interface. + * + * NOTE: Systems that implement other access control mechanisms (for example using {Ownable}) can be paired with an + * {AccessManager} by transferring permissions (ownership in the case of {Ownable}) directly to the {AccessManager}. + * Users will be able to interact with these contracts through the {relay} function, following the access rules + * registered in the {AccessManager}. Keep in mind that in that context, the msg.sender seen by restricted functions + * will be {AccessManager} itself. + * + * WARNING: When granting permissions over an {Ownable} or {AccessControl} contract to an {AccessManager}, be very + * mindful of the danger associated with functions such as {{Ownable-renounceOwnership}} or + * {{AccessControl-renounceRole}}. */ -contract AccessManager is IAccessManager, AccessControlDefaultAdminRules { - bytes32 private _createdGroups; +contract AccessManager is Context, Multicall, IAccessManager { + using Time for *; - // user -> groups - mapping(address => bytes32) private _userGroups; + struct AccessMode { + uint64 familyId; + bool closed; + } - // target -> selector -> groups - mapping(address => mapping(bytes4 => bytes32)) private _allowedGroups; + // Structure that stores the details for a group/account pair. This structures fit into a single slot. + struct Access { + // Timepoint at which the user gets the permission. If this is either 0, or in the future, the group permission + // are not available. Should be checked using {Time-isSetAndPast} + uint48 since; + // delay for execution. Only applies to restricted() / relay() calls. This does not restrict access to + // functions that use the `onlyGroup` modifier. + Time.Delay delay; + } - // target -> mode - mapping(address => AccessMode) private _contractMode; + // Structure that stores the details of a group, including: + // - the members of the group + // - the admin group (that can grant or revoke permissions) + // - the guardian group (that can cancel operations targeting functions that need this group + // - the grand delay + struct Group { + mapping(address user => Access access) members; + uint64 admin; + uint64 guardian; + Time.Delay delay; // delay for granting + } - uint8 private constant _GROUP_PUBLIC = type(uint8).max; + struct Family { + mapping(bytes4 selector => uint64 groupId) allowedGroups; + Time.Delay adminDelay; + } + + uint64 public constant ADMIN_GROUP = type(uint64).min; // 0 + uint64 public constant PUBLIC_GROUP = type(uint64).max; // 2**64-1 + + mapping(address target => AccessMode mode) private _contractMode; + mapping(uint64 familyId => Family) private _families; + mapping(uint64 groupId => Group) private _groups; + mapping(bytes32 operationId => uint48 schedule) private _schedules; + mapping(bytes4 selector => Time.Delay delay) private _adminDelays; + + // This should be transcient storage when supported by the EVM. + bytes32 private _relayIdentifier; /** - * @dev Initializes an AccessManager with initial default admin and transfer delay. + * @dev Check that the caller has a given permission level (`groupId`). Note that this does NOT consider execution + * delays that may be associated to that group. */ - constructor( - uint48 initialDefaultAdminDelay, - address initialDefaultAdmin - ) AccessControlDefaultAdminRules(initialDefaultAdminDelay, initialDefaultAdmin) { - _createGroup(_GROUP_PUBLIC, "public"); + modifier onlyGroup(uint64 groupId) { + _checkGroup(groupId); + _; } /** - * @dev Returns true if the caller can invoke on a target the function identified by a function selector. - * Entrypoint for {AccessManaged} contracts. + * @dev Check that the caller is an admin and that the top-level function currently executing has been scheduled + * sufficiently ahead of time, if necessary according to admin delays. */ - function canCall(address caller, address target, bytes4 selector) public view virtual returns (bool) { - bytes32 allowedGroups = getFunctionAllowedGroups(target, selector); - bytes32 callerGroups = getUserGroups(caller); - return callerGroups & allowedGroups != 0; + modifier withFamilyDelay(uint64 familyId) { + _checkFamilyDelay(familyId); + _; } + constructor(address initialAdmin) { + // admin is active immediately and without any execution delay. + _grantGroup(ADMIN_GROUP, initialAdmin, 0, 0); + } + + // =================================================== GETTERS ==================================================== /** - * @dev Creates a new group with a group number that can be chosen arbitrarily but must be unused, and gives it a - * human-readable name. The caller must be the default admin. + * @dev Check if an address (`caller`) is authorised to call a given function on a given contract directly (with + * no restriction). Additionally, it returns the delay needed to perform the call indirectly through the {schedule} + * & {relay} workflow. * - * Group numbers are not auto-incremented in order to avoid race conditions, but administrators can safely use - * sequential numbers. + * This function is usually called by the targeted contract to control immediate execution of restricted functions. + * Therefore we only return true is the call can be performed without any delay. If the call is subject to a delay, + * then the function should return false, and the caller should schedule the operation for future execution. * - * Emits {GroupUpdated}. - */ - function createGroup(uint8 group, string memory name) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - _createGroup(group, name); - } - - /** - * @dev Updates an existing group's name. The caller must be the default admin. - */ - function updateGroupName(uint8 group, string memory name) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - require(group != _GROUP_PUBLIC, "AccessManager: built-in group"); - require(hasGroup(group), "AccessManager: unknown group"); - emit GroupUpdated(group, name); - } - - /** - * @dev Returns true if the group has already been created via {createGroup}. - */ - function hasGroup(uint8 group) public view virtual returns (bool) { - return _getGroup(_createdGroups, group); - } - - /** - * @dev Returns a bitmap of the groups the user has. See note on bitmaps above. - */ - function getUserGroups(address user) public view virtual returns (bytes32) { - return _userGroups[user] | _groupMask(_GROUP_PUBLIC); - } - - /** - * @dev Grants a user a group. + * We may be able to hash the operation, and check if the call was scheduled, but we would not be able to cleanup + * the schedule, leaving the possibility of multiple executions. Maybe this function should not be view? * - * Emits {RoleGranted} with the role id of the group, if wasn't already held by the user. + * NOTE: The IAuthority interface does not include the `uint32` delay. This is an extension of that interface that + * is backward compatible. Some contract may thus ignore the second return argument. In that case they will fail + * to identify the indirect workflow, and will consider call that require a delay to be forbidden. */ - function grantGroup(uint8 group, address user) public virtual { - grantRole(_encodeGroupRole(group), user); // will check msg.sender - } - - /** - * @dev Removes a group from a user. - * - * Emits {RoleRevoked} with the role id of the group, if previously held by the user. - */ - function revokeGroup(uint8 group, address user) public virtual { - revokeRole(_encodeGroupRole(group), user); // will check msg.sender - } - - /** - * @dev Allows a user to renounce a group. - * - * Emits {RoleRevoked} with the role id of the group, if previously held by the user. - */ - function renounceGroup(uint8 group, address user) public virtual { - renounceRole(_encodeGroupRole(group), user); // will check msg.sender - } - - /** - * @dev Returns a bitmap of the groups that are allowed to call a function of a target contract. If the target - * contract is in open or closed mode it will be reflected in the return value. - */ - function getFunctionAllowedGroups(address target, bytes4 selector) public view virtual returns (bytes32) { - AccessMode mode = getContractMode(target); - if (mode == AccessMode.Open) { - return _groupMask(_GROUP_PUBLIC); - } else if (mode == AccessMode.Closed) { - return 0; + function canCall(address caller, address target, bytes4 selector) public view virtual returns (bool, uint32) { + (uint64 familyId, bool closed) = getContractFamily(target); + if (closed) { + return (false, 0); + } else if (caller == address(this)) { + // Caller is AccessManager => call was relayed. In that case the relay already checked permissions. We + // verify that the call "identifier", which is set during the relay call, is correct. + return (_relayIdentifier == _hashRelayIdentifier(target, selector), 0); } else { - return _allowedGroups[target][selector]; + uint64 groupId = getFamilyFunctionGroup(familyId, selector); + (bool inGroup, uint32 currentDelay) = hasGroup(groupId, caller); + return (inGroup && currentDelay == 0, currentDelay); } } /** - * @dev Changes whether a group is allowed to call a function of a contract, according to the `allowed` argument. - * The caller must be the default admin. + * @dev Expiration delay for scheduled proposals. Defaults to 1 week. */ - function setFunctionAllowedGroup( - address target, + function expiration() public view virtual returns (uint32) { + return 1 weeks; + } + + /** + * @dev Minimum setback for delay updates. Defaults to 1 day. + */ + function minSetback() public view virtual returns (uint32) { + return 0; // TODO: set to 1 day + } + + /** + * @dev Get the mode under which a contract is operating. + */ + function getContractFamily(address target) public view virtual returns (uint64, bool) { + AccessMode storage mode = _contractMode[target]; + return (mode.familyId, mode.closed); + } + + /** + * @dev Get the permission level (group) required to call a function. This only applies for contract that are + * operating under the `Custom` mode. + */ + function getFamilyFunctionGroup(uint64 familyId, bytes4 selector) public view virtual returns (uint64) { + return _families[familyId].allowedGroups[selector]; + } + + function getFamilyAdminDelay(uint64 familyId) public view virtual returns (uint32) { + return _families[familyId].adminDelay.get(); + } + + /** + * @dev Get the id of the group that acts as an admin for given group. + * + * The admin permission is required to grant the group, revoke the group and update the execution delay to execute + * an operation that is restricted to this group. + */ + function getGroupAdmin(uint64 groupId) public view virtual returns (uint64) { + return _groups[groupId].admin; + } + + /** + * @dev Get the group that acts as a guardian for a given group. + * + * The guardian permission allows canceling operations that have been scheduled under the group. + */ + function getGroupGuardian(uint64 groupId) public view virtual returns (uint64) { + return _groups[groupId].guardian; + } + + /** + * @dev Get the group current grant delay, that value may change at any point, without an event emitted, following + * a call to {setGrantDelay}. Changes to this value, including effect timepoint are notified by the + * {GroupGrantDelayChanged} event. + */ + function getGroupGrantDelay(uint64 groupId) public view virtual returns (uint32) { + return _groups[groupId].delay.get(); + } + + /** + * @dev Get the access details for a given account in a given group. These details include the timepoint at which + * membership becomes active, and the delay applied to all operation by this user that require this permission + * level. + * + * Returns: + * [0] Timestamp at which the account membership becomes valid. 0 means role is not granted. + * [1] Current execution delay for the account. + * [2] Pending execution delay for the account. + * [3] Timestamp at which the pending execution delay will become active. 0 means no delay update is scheduled. + */ + function getAccess(uint64 groupId, address account) public view virtual returns (uint48, uint32, uint32, uint48) { + Access storage access = _groups[groupId].members[account]; + + uint48 since = access.since; + (uint32 currentDelay, uint32 pendingDelay, uint48 effect) = access.delay.getFull(); + + return (since, currentDelay, pendingDelay, effect); + } + + /** + * @dev Check if a given account currently had the permission level corresponding to a given group. Note that this + * permission might be associated with a delay. {getAccess} can provide more details. + */ + function hasGroup(uint64 groupId, address account) public view virtual returns (bool, uint32) { + if (groupId == PUBLIC_GROUP) { + return (true, 0); + } else { + (uint48 inGroupSince, uint32 currentDelay, , ) = getAccess(groupId, account); + return (inGroupSince.isSetAndPast(Time.timestamp()), currentDelay); + } + } + + // =============================================== GROUP MANAGEMENT =============================================== + /** + * @dev Give a label to a group, for improved group discoverabily by UIs. + * + * Emits a {GroupLabel} event. + */ + function labelGroup(uint64 groupId, string calldata label) public virtual onlyGroup(ADMIN_GROUP) { + if (groupId == ADMIN_GROUP || groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } + emit GroupLabel(groupId, label); + } + + /** + * @dev Add `account` to `groupId`. This gives him the authorization to call any function that is restricted to + * this group. An optional execution delay (in seconds) can be set. If that delay is non 0, the user is required + * to schedule any operation that is restricted to members this group. The user will only be able to execute the + * operation after the delay expires. During this delay, admin and guardians can cancel the operation (see + * {cancel}). + * + * Requirements: + * + * - the caller must be in the group's admins + * + * Emits a {GroupGranted} event + */ + function grantGroup( + uint64 groupId, + address account, + uint32 executionDelay + ) public virtual onlyGroup(getGroupAdmin(groupId)) { + _grantGroup(groupId, account, getGroupGrantDelay(groupId), executionDelay); + } + + /** + * @dev Remove an account for a group, with immediate effect. + * + * Requirements: + * + * - the caller must be in the group's admins + * + * Emits a {GroupRevoked} event + */ + function revokeGroup(uint64 groupId, address account) public virtual onlyGroup(getGroupAdmin(groupId)) { + _revokeGroup(groupId, account); + } + + /** + * @dev Renounce group permissions for the calling account, with immediate effect. + * + * Requirements: + * + * - the caller must be `callerConfirmation`. + * + * Emits a {GroupRevoked} event + */ + function renounceGroup(uint64 groupId, address callerConfirmation) public virtual { + if (callerConfirmation != _msgSender()) { + revert AccessManagerBadConfirmation(); + } + _revokeGroup(groupId, callerConfirmation); + } + + /** + * @dev Set the execution delay for a given account in a given group. This update is not immediate and follows the + * delay rules. For example, If a user currently has a delay of 3 hours, and this is called to reduce that delay to + * 1 hour, the new delay will take some time to take effect, enforcing that any operation executed in the 3 hours + * that follows this update was indeed scheduled before this update. + * + * Requirements: + * + * - the caller must be in the group's admins + * + * Emits a {GroupExecutionDelayUpdated} event + */ + function setExecuteDelay( + uint64 groupId, + address account, + uint32 newDelay + ) public virtual onlyGroup(getGroupAdmin(groupId)) { + _setExecuteDelay(groupId, account, newDelay); + } + + /** + * @dev Change admin group for a given group. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {GroupAdminChanged} event + */ + function setGroupAdmin(uint64 groupId, uint64 admin) public virtual onlyGroup(ADMIN_GROUP) { + _setGroupAdmin(groupId, admin); + } + + /** + * @dev Change guardian group for a given group. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {GroupGuardianChanged} event + */ + function setGroupGuardian(uint64 groupId, uint64 guardian) public virtual onlyGroup(ADMIN_GROUP) { + _setGroupGuardian(groupId, guardian); + } + + /** + * @dev Update the delay for granting a `groupId`. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {GroupGrantDelayChanged} event + */ + function setGrantDelay(uint64 groupId, uint32 newDelay) public virtual onlyGroup(ADMIN_GROUP) { + _setGrantDelay(groupId, newDelay); + } + + /** + * @dev Internal version of {grantGroup} without access control. + * + * Emits a {GroupGranted} event + */ + function _grantGroup(uint64 groupId, address account, uint32 grantDelay, uint32 executionDelay) internal virtual { + if (groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } else if (_groups[groupId].members[account].since != 0) { + revert AccessManagerAccountAlreadyInGroup(groupId, account); + } + + uint48 since = Time.timestamp() + grantDelay; + _groups[groupId].members[account] = Access({since: since, delay: executionDelay.toDelay()}); + + emit GroupGranted(groupId, account, since, executionDelay); + } + + /** + * @dev Internal version of {revokeGroup} without access control. This logic is also used by {renounceGroup}. + * + * Emits a {GroupRevoked} event + */ + function _revokeGroup(uint64 groupId, address account) internal virtual { + if (groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } else if (_groups[groupId].members[account].since == 0) { + revert AccessManagerAccountNotInGroup(groupId, account); + } + + delete _groups[groupId].members[account]; + + emit GroupRevoked(groupId, account); + } + + /** + * @dev Internal version of {setExecuteDelay} without access control. + * + * Emits a {GroupExecutionDelayUpdated} event. + */ + function _setExecuteDelay(uint64 groupId, address account, uint32 newDuration) internal virtual { + if (groupId == PUBLIC_GROUP || groupId == ADMIN_GROUP) { + revert AccessManagerLockedGroup(groupId); + } else if (_groups[groupId].members[account].since == 0) { + revert AccessManagerAccountNotInGroup(groupId, account); + } + + Time.Delay updated = _groups[groupId].members[account].delay.withUpdate(newDuration, minSetback()); + _groups[groupId].members[account].delay = updated; + + (, , uint48 effect) = updated.unpack(); + emit GroupExecutionDelayUpdated(groupId, account, newDuration, effect); + } + + /** + * @dev Internal version of {setGroupAdmin} without access control. + * + * Emits a {GroupAdminChanged} event + */ + function _setGroupAdmin(uint64 groupId, uint64 admin) internal virtual { + if (groupId == ADMIN_GROUP || groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } + + _groups[groupId].admin = admin; + + emit GroupAdminChanged(groupId, admin); + } + + /** + * @dev Internal version of {setGroupGuardian} without access control. + * + * Emits a {GroupGuardianChanged} event + */ + function _setGroupGuardian(uint64 groupId, uint64 guardian) internal virtual { + if (groupId == ADMIN_GROUP || groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } + + _groups[groupId].guardian = guardian; + + emit GroupGuardianChanged(groupId, guardian); + } + + /** + * @dev Internal version of {setGrantDelay} without access control. + * + * Emits a {GroupGrantDelayChanged} event + */ + function _setGrantDelay(uint64 groupId, uint32 newDelay) internal virtual { + if (groupId == PUBLIC_GROUP) { + revert AccessManagerLockedGroup(groupId); + } + + Time.Delay updated = _groups[groupId].delay.withUpdate(newDelay, minSetback()); + _groups[groupId].delay = updated; + + (, , uint48 effect) = updated.unpack(); + emit GroupGrantDelayChanged(groupId, newDelay, effect); + } + + // ============================================= FUNCTION MANAGEMENT ============================================== + /** + * @dev Set the level of permission (`group`) required to call functions identified by the `selectors` in the + * `target` contract. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {FunctionAllowedGroupUpdated} event per selector + */ + function setFamilyFunctionGroup( + uint64 familyId, bytes4[] calldata selectors, - uint8 group, - bool allowed - ) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - require(hasGroup(group), "AccessManager: unknown group"); - for (uint256 i = 0; i < selectors.length; i++) { - bytes4 selector = selectors[i]; - _allowedGroups[target][selector] = _withUpdatedGroup(_allowedGroups[target][selector], group, allowed); - emit GroupAllowed(target, selector, group, allowed); + uint64 groupId + ) public virtual onlyGroup(ADMIN_GROUP) withFamilyDelay(familyId) { + for (uint256 i = 0; i < selectors.length; ++i) { + _setFamilyFunctionGroup(familyId, selectors[i], groupId); } } /** - * @dev Returns the mode of the target contract, which may be custom (`0`), closed (`1`), or open (`2`). + * @dev Internal version of {setFunctionAllowedGroup} without access control. + * + * Emits a {FunctionAllowedGroupUpdated} event */ - function getContractMode(address target) public view virtual returns (AccessMode) { - return _contractMode[target]; + function _setFamilyFunctionGroup(uint64 familyId, bytes4 selector, uint64 groupId) internal virtual { + _checkValidFamilyId(familyId); + _families[familyId].allowedGroups[selector] = groupId; + emit FamilyFunctionGroupUpdated(familyId, selector, groupId); } /** - * @dev Sets the target contract to be in custom restricted mode. All restricted functions in the target contract - * will follow the group-based restrictions defined by the AccessManager. The caller must be the default admin. + * @dev Set the delay for management operations on a given family of contract. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {FunctionAllowedGroupUpdated} event per selector */ - function setContractModeCustom(address target) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - _setContractMode(target, AccessMode.Custom); + function setFamilyAdminDelay(uint64 familyId, uint32 newDelay) public virtual onlyGroup(ADMIN_GROUP) { + _setFamilyAdminDelay(familyId, newDelay); } /** - * @dev Sets the target contract to be in "open" mode. All restricted functions in the target contract will become - * callable by anyone. The caller must be the default admin. + * @dev Internal version of {setFamilyAdminDelay} without access control. + * + * Emits a {FamilyAdminDelayUpdated} event */ - function setContractModeOpen(address target) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - _setContractMode(target, AccessMode.Open); + function _setFamilyAdminDelay(uint64 familyId, uint32 newDelay) internal virtual { + _checkValidFamilyId(familyId); + Time.Delay updated = _families[familyId].adminDelay.withUpdate(newDelay, minSetback()); + _families[familyId].adminDelay = updated; + (, , uint48 effect) = updated.unpack(); + emit FamilyAdminDelayUpdated(familyId, newDelay, effect); } /** - * @dev Sets the target contract to be in "closed" mode. All restricted functions in the target contract will be - * closed down and disallowed to all. The caller must be the default admin. + * @dev Reverts if `familyId` is 0. */ - function setContractModeClosed(address target) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - _setContractMode(target, AccessMode.Closed); + function _checkValidFamilyId(uint64 familyId) private pure { + if (familyId == 0) { + revert AccessManagerInvalidFamily(familyId); + } + } + + // =============================================== MODE MANAGEMENT ================================================ + /** + * @dev Set the family of a contract. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {ContractFamilyUpdated} event. + */ + function setContractFamily( + address target, + uint64 familyId + ) public virtual onlyGroup(ADMIN_GROUP) withFamilyDelay(_getContractFamilyId(target)) { + _setContractFamily(target, familyId); } /** - * @dev Transfers a target contract onto a new authority. The caller must be the default admin. + * @dev Set the family of a contract. This is an internal setter with no access restrictions. + * + * Emits a {ContractFamilyUpdated} event. */ - function transferContractAuthority( + function _setContractFamily(address target, uint64 familyId) internal virtual { + _contractMode[target].familyId = familyId; + emit ContractFamilyUpdated(target, familyId); + } + + /** + * @dev Set the closed flag for a contract. + * + * Requirements: + * + * - the caller must be a global admin + * + * Emits a {ContractClosed} event. + */ + function setContractClosed(address target, bool closed) public virtual onlyGroup(ADMIN_GROUP) { + _setContractClosed(target, closed); + } + + /** + * @dev Set the closed flag for a contract. This is an internal setter with no access restrictions. + * + * Emits a {ContractClosed} event. + */ + function _setContractClosed(address target, bool closed) internal virtual { + _contractMode[target].closed = closed; + emit ContractClosed(target, closed); + } + + // ============================================== DELAYED OPERATIONS ============================================== + /** + * @dev Return the timepoint at which a scheduled operation will be ready for execution. This returns 0 if the + * operation is not yet scheduled, has expired, was executed, or was canceled. + */ + function getSchedule(bytes32 id) public view virtual returns (uint48) { + uint48 timepoint = _schedules[id]; + return _isExpired(timepoint) ? 0 : timepoint; + } + + /** + * @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. + * + * Emits a {OperationScheduled} event. + */ + function schedule(address target, bytes calldata data, uint48 when) public virtual returns (bytes32) { + address caller = _msgSender(); + + // Fetch restriction to that apply to the caller on the targeted function + (bool allowed, uint32 setback) = _canCallExtended(caller, target, data); + + uint48 minWhen = Time.timestamp() + setback; + + // If caller is not authorised, revert + if (!allowed && (setback == 0 || when.isSetAndPast(minWhen - 1))) { + revert AccessManagerUnauthorizedCall(caller, target, bytes4(data[0:4])); + } + + // If caller is authorised, schedule operation + bytes32 operationId = _hashOperation(caller, target, data); + + // Cannot reschedule unless the operation has expired + uint48 prevTimepoint = _schedules[operationId]; + if (prevTimepoint != 0 && !_isExpired(prevTimepoint)) { + revert AccessManagerAlreadyScheduled(operationId); + } + + uint48 timepoint = when == 0 ? minWhen : when; + _schedules[operationId] = timepoint; + emit OperationScheduled(operationId, timepoint, caller, target, data); + + return operationId; + } + + /** + * @dev Execute a function that is delay restricted, provided it was properly scheduled beforehand, or the + * execution delay is 0. + * + * Emits an {OperationExecuted} event only if the call was scheduled and delayed. + */ + function relay(address target, bytes calldata data) public payable virtual { + address caller = _msgSender(); + + // Fetch restriction to that apply to the caller on the targeted function + (bool allowed, uint32 setback) = _canCallExtended(caller, target, data); + + // If caller is not authorised, revert + if (!allowed && setback == 0) { + revert AccessManagerUnauthorizedCall(caller, target, bytes4(data)); + } + + // If caller is authorised, check operation was scheduled early enough + bytes32 operationId = _hashOperation(caller, target, data); + + if (setback != 0) { + _consumeScheduledOp(operationId); + } + + // Mark the target and selector as authorised + bytes32 relayIdentifierBefore = _relayIdentifier; + _relayIdentifier = _hashRelayIdentifier(target, bytes4(data)); + + // Perform call + Address.functionCallWithValue(target, data, msg.value); + + // Reset relay identifier + _relayIdentifier = relayIdentifierBefore; + } + + /** + * @dev Consume a scheduled operation targeting the caller. If such an operation exists, mark it as consumed + * (emit an {OperationExecuted} event and clean the state). Otherwise, throw an error. + * + * This is useful for contract that want to enforce that calls targeting them were scheduled on the manager, + * with all the verifications that it implies. + * + * Emit a {OperationExecuted} event + */ + function consumeScheduledOp(address caller, bytes calldata data) public virtual { + address target = _msgSender(); + require(IAccessManaged(target).isConsumingScheduledOp()); + _consumeScheduledOp(_hashOperation(caller, target, data)); + } + + /** + * @dev Internal variant of {consumeScheduledOp} that operates on bytes32 operationId. + */ + function _consumeScheduledOp(bytes32 operationId) internal virtual { + uint48 timepoint = _schedules[operationId]; + + if (timepoint == 0) { + revert AccessManagerNotScheduled(operationId); + } else if (timepoint > Time.timestamp()) { + revert AccessManagerNotReady(operationId); + } else if (_isExpired(timepoint)) { + revert AccessManagerExpired(operationId); + } + + delete _schedules[operationId]; + emit OperationExecuted(operationId, timepoint); + } + + /** + * @dev Cancel a scheduled (delayed) operation. + * + * Requirements: + * + * - the caller must be the proposer, or a guardian of the targeted function + * + * Emits a {OperationCanceled} event. + */ + function cancel(address caller, address target, bytes calldata data) public virtual { + address msgsender = _msgSender(); + bytes4 selector = bytes4(data[0:4]); + + bytes32 operationId = _hashOperation(caller, target, data); + if (_schedules[operationId] == 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. + (bool isAdmin, ) = hasGroup(ADMIN_GROUP, msgsender); + (bool isGuardian, ) = hasGroup( + getGroupGuardian(getFamilyFunctionGroup(_getContractFamilyId(target), selector)), + msgsender + ); + if (!isAdmin && !isGuardian) { + revert AccessManagerCannotCancel(msgsender, caller, target, selector); + } + } + + uint48 timepoint = _schedules[operationId]; + delete _schedules[operationId]; + emit OperationCanceled(operationId, timepoint); + } + + /** + * @dev Hashing function for delayed operations + */ + function _hashOperation(address caller, address target, bytes calldata data) private pure returns (bytes32) { + return keccak256(abi.encode(caller, target, data)); + } + + /** + * @dev Hashing function for relay protection + */ + function _hashRelayIdentifier(address target, bytes4 selector) private pure returns (bytes32) { + return keccak256(abi.encode(target, selector)); + } + + // ==================================================== OTHERS ==================================================== + /** + * @dev Change the AccessManager instance used by a contract that correctly uses this instance. + * + * Requirements: + * + * - the caller must be a global admin + */ + function updateAuthority( address target, address newAuthority - ) public virtual onlyRole(DEFAULT_ADMIN_ROLE) { - AccessManaged(target).setAuthority(IAuthority(newAuthority)); + ) public virtual onlyGroup(ADMIN_GROUP) withFamilyDelay(_getContractFamilyId(target)) { + IAccessManaged(target).setAuthority(newAuthority); } - /** - * @dev Creates a new group. - * - * Emits {GroupUpdated}. - */ - function _createGroup(uint8 group, string memory name) internal virtual { - require(!hasGroup(group), "AccessManager: existing group"); - _createdGroups = _withUpdatedGroup(_createdGroups, group, true); - emit GroupUpdated(group, name); - } - - /** - * @dev Augmented version of {AccessControl-_grantRole} that keeps track of user group bitmaps. - */ - function _grantRole(bytes32 role, address user) internal virtual override { - super._grantRole(role, user); - (bool isGroup, uint8 group) = _decodeGroupRole(role); - if (isGroup) { - require(hasGroup(group), "AccessManager: unknown group"); - _userGroups[user] = _withUpdatedGroup(_userGroups[user], group, true); + // =================================================== HELPERS ==================================================== + function _checkGroup(uint64 groupId) internal view virtual { + address account = _msgSender(); + (bool inGroup, ) = hasGroup(groupId, account); + if (!inGroup) { + revert AccessManagerUnauthorizedAccount(account, groupId); } } - /** - * @dev Augmented version of {AccessControl-_revokeRole} that keeps track of user group bitmaps. - */ - function _revokeRole(bytes32 role, address user) internal virtual override { - super._revokeRole(role, user); - (bool isGroup, uint8 group) = _decodeGroupRole(role); - if (isGroup) { - require(hasGroup(group), "AccessManager: unknown group"); - require(group != _GROUP_PUBLIC, "AccessManager: irrevocable group"); - _userGroups[user] = _withUpdatedGroup(_userGroups[user], group, false); + function _checkFamilyDelay(uint64 familyId) internal virtual { + uint32 delay = getFamilyAdminDelay(familyId); + if (delay > 0) { + _consumeScheduledOp(_hashOperation(_msgSender(), address(this), _msgData())); } } - /** - * @dev Sets the restricted mode of a target contract. - */ - function _setContractMode(address target, AccessMode mode) internal virtual { - AccessMode previousMode = _contractMode[target]; - _contractMode[target] = mode; - emit AccessModeUpdated(target, previousMode, mode); + function _getContractFamilyId(address target) private view returns (uint64 familyId) { + (familyId, ) = getContractFamily(target); } - /** - * @dev Returns the {AccessControl} role id that corresponds to a group. - * - * This role id starts with the ASCII characters `group:`, followed by zeroes, and ends with the single byte - * corresponding to the group number. - */ - function _encodeGroupRole(uint8 group) internal pure virtual returns (bytes32) { - return bytes32("group:") | bytes32(uint256(group)); - } - - /** - * @dev Decodes a role id into a group, if it is a role id of the kind returned by {_encodeGroupRole}. - */ - function _decodeGroupRole(bytes32 role) internal pure virtual returns (bool isGroup, uint8 group) { - bytes32 tagMask = ~bytes32(uint256(0xff)); - bytes32 tag = role & tagMask; - isGroup = tag == bytes32("group:"); - group = uint8(role[31]); - } - - /** - * @dev Returns a bit mask where the only non-zero bit is the group number bit. - */ - function _groupMask(uint8 group) private pure returns (bytes32) { - return bytes32(1 << group); - } - - /** - * @dev Returns the value of the group number bit in a bitmap. - */ - function _getGroup(bytes32 bitmap, uint8 group) private pure returns (bool) { - return bitmap & _groupMask(group) > 0; - } - - /** - * @dev Returns a new group bitmap where a specific group was updated. - */ - function _withUpdatedGroup(bytes32 bitmap, uint8 group, bool value) private pure returns (bytes32) { - bytes32 mask = _groupMask(group); - if (value) { - return bitmap | mask; + function _parseFamilyOperation(bytes calldata data) private view returns (bool, uint64) { + bytes4 selector = bytes4(data); + if (selector == this.updateAuthority.selector || selector == this.setContractFamily.selector) { + return (true, _getContractFamilyId(abi.decode(data[0x04:0x24], (address)))); + } else if (selector == this.setFamilyFunctionGroup.selector) { + return (true, abi.decode(data[0x04:0x24], (uint64))); } else { - return bitmap & ~mask; + return (false, 0); } } + + function _canCallExtended(address caller, address target, bytes calldata data) private view returns (bool, uint32) { + if (target == address(this)) { + (bool isFamilyOperation, uint64 familyId) = _parseFamilyOperation(data); + uint32 delay = getFamilyAdminDelay(familyId); + (bool inGroup, ) = hasGroup(ADMIN_GROUP, caller); + return (inGroup && isFamilyOperation && delay == 0, delay); + } else { + bytes4 selector = bytes4(data); + return canCall(caller, target, selector); + } + } + + function _isExpired(uint48 timepoint) private view returns (bool) { + return timepoint + expiration() <= Time.timestamp(); + } } diff --git a/contracts/access/manager/AccessManagerAdapter.sol b/contracts/access/manager/AccessManagerAdapter.sol deleted file mode 100644 index afa92264a..000000000 --- a/contracts/access/manager/AccessManagerAdapter.sol +++ /dev/null @@ -1,54 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "./AccessManager.sol"; -import "./AccessManaged.sol"; - -/** - * @dev This contract can be used to migrate existing {Ownable} or {AccessControl} contracts into an {AccessManager} - * system. - * - * Ownable contracts can have their ownership transferred to an instance of this adapter. AccessControl contracts can - * grant all roles to the adapter, while ideally revoking them from all other accounts. Subsequently, the permissions - * for those contracts can be managed centrally and with function granularity in the {AccessManager} instance the - * adapter is connected to. - * - * Permissioned interactions with thus migrated contracts must go through the adapter's {relay} function and will - * proceed if the function is allowed for the caller in the AccessManager instance. - */ -contract AccessManagerAdapter is AccessManaged { - bytes32 private constant _DEFAULT_ADMIN_ROLE = 0; - - /** - * @dev Initializes an adapter connected to an AccessManager instance. - */ - constructor(AccessManager manager) AccessManaged(manager) {} - - /** - * @dev Relays a function call to the target contract. The call will be relayed if the AccessManager allows the - * caller access to this function in the target contract, i.e. if the caller is in a team that is allowed for the - * function, or if the caller is the default admin for the AccessManager. The latter is meant to be used for - * ad hoc operations such as asset recovery. - */ - function relay(address target, bytes memory data) external payable { - bytes4 sig = bytes4(data); - AccessManager manager = AccessManager(address(authority())); - require( - manager.canCall(msg.sender, target, sig) || manager.hasRole(_DEFAULT_ADMIN_ROLE, msg.sender), - "AccessManagerAdapter: caller not allowed" - ); - (bool ok, bytes memory result) = target.call{value: msg.value}(data); - assembly { - let result_pointer := add(32, result) - let result_size := mload(result) - switch ok - case true { - return(result_pointer, result_size) - } - default { - revert(result_pointer, result_size) - } - } - } -} diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol new file mode 100644 index 000000000..49ba74ea2 --- /dev/null +++ b/contracts/access/manager/AuthorityUtils.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IAuthority} from "./IAuthority.sol"; + +library AuthorityUtils { + /** + * @dev Since `AccessManager` implements an extended IAuthority interface, invoking `canCall` with backwards compatibility + * for the preexisting `IAuthority` interface requires special care to avoid reverting on insufficient return data. + * This helper function takes care of invoking `canCall` in a backwards compatible way without reverting. + */ + function canCallWithDelay( + address authority, + address caller, + address target, + bytes4 selector + ) internal view returns (bool allowed, uint32 delay) { + (bool success, bytes memory data) = authority.staticcall( + abi.encodeCall(IAuthority.canCall, (caller, target, selector)) + ); + if (success) { + if (data.length >= 0x40) { + (allowed, delay) = abi.decode(data, (bool, uint32)); + } else if (data.length >= 0x20) { + allowed = abi.decode(data, (bool)); + } + } + return (allowed, delay); + } +} diff --git a/contracts/access/manager/IAccessManaged.sol b/contracts/access/manager/IAccessManaged.sol new file mode 100644 index 000000000..947e5bf5d --- /dev/null +++ b/contracts/access/manager/IAccessManaged.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +interface IAccessManaged { + event AuthorityUpdated(address authority); + + error AccessManagedUnauthorized(address caller); + error AccessManagedRequiredDelay(address caller, uint32 delay); + error AccessManagedInvalidAuthority(address authority); + + function authority() external view returns (address); + + function setAuthority(address) external; + + function isConsumingScheduledOp() external view returns (bool); +} diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol new file mode 100644 index 000000000..659b4c5cd --- /dev/null +++ b/contracts/access/manager/IAccessManager.sol @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IAccessManaged} from "./IAccessManaged.sol"; +import {Time} from "../../utils/types/Time.sol"; + +interface IAccessManager { + /** + * @dev A delayed operation was scheduled. + */ + event OperationScheduled(bytes32 indexed operationId, uint48 schedule, address caller, address target, bytes data); + + /** + * @dev A scheduled operation was executed. + */ + event OperationExecuted(bytes32 indexed operationId, uint48 schedule); + + /** + * @dev A scheduled operation was canceled. + */ + event OperationCanceled(bytes32 indexed operationId, uint48 schedule); + + event GroupLabel(uint64 indexed groupId, string label); + event GroupGranted(uint64 indexed groupId, address indexed account, uint48 since, uint32 delay); + event GroupRevoked(uint64 indexed groupId, address indexed account); + event GroupExecutionDelayUpdated(uint64 indexed groupId, address indexed account, uint32 delay, uint48 from); + event GroupAdminChanged(uint64 indexed groupId, uint64 indexed admin); + event GroupGuardianChanged(uint64 indexed groupId, uint64 indexed guardian); + event GroupGrantDelayChanged(uint64 indexed groupId, uint32 delay, uint48 from); + + event ContractFamilyUpdated(address indexed target, uint64 indexed familyId); + event ContractClosed(address indexed target, bool closed); + + event FamilyFunctionGroupUpdated(uint64 indexed familyId, bytes4 selector, uint64 indexed groupId); + event FamilyAdminDelayUpdated(uint64 indexed familyId, uint32 delay, uint48 from); + + error AccessManagerAlreadyScheduled(bytes32 operationId); + error AccessManagerNotScheduled(bytes32 operationId); + error AccessManagerNotReady(bytes32 operationId); + error AccessManagerExpired(bytes32 operationId); + error AccessManagerLockedGroup(uint64 groupId); + error AccessManagerInvalidFamily(uint64 familyId); + error AccessManagerAccountAlreadyInGroup(uint64 groupId, address account); + error AccessManagerAccountNotInGroup(uint64 groupId, address account); + error AccessManagerBadConfirmation(); + error AccessManagerUnauthorizedAccount(address msgsender, uint64 groupId); + error AccessManagerUnauthorizedCall(address caller, address target, bytes4 selector); + error AccessManagerCannotCancel(address msgsender, address caller, address target, bytes4 selector); + + function canCall( + address caller, + address target, + bytes4 selector + ) external view returns (bool allowed, uint32 delay); + + function expiration() external returns (uint32); + + function getContractFamily(address target) external view returns (uint64 familyId, bool closed); + + function getFamilyFunctionGroup(uint64 familyId, bytes4 selector) external view returns (uint64); + + function getFamilyAdminDelay(uint64 familyId) external view returns (uint32); + + function getGroupAdmin(uint64 groupId) external view returns (uint64); + + function getGroupGuardian(uint64 groupId) external view returns (uint64); + + function getGroupGrantDelay(uint64 groupId) external view returns (uint32); + + function getAccess(uint64 groupId, address account) external view returns (uint48, uint32, uint32, uint48); + + function hasGroup(uint64 groupId, address account) external view returns (bool, uint32); + + function labelGroup(uint64 groupId, string calldata label) external; + + function grantGroup(uint64 groupId, address account, uint32 executionDelay) external; + + function revokeGroup(uint64 groupId, address account) external; + + function renounceGroup(uint64 groupId, address callerConfirmation) external; + + function setExecuteDelay(uint64 groupId, address account, uint32 newDelay) external; + + function setGroupAdmin(uint64 groupId, uint64 admin) external; + + function setGroupGuardian(uint64 groupId, uint64 guardian) external; + + function setGrantDelay(uint64 groupId, uint32 newDelay) external; + + function setFamilyFunctionGroup(uint64 familyId, bytes4[] calldata selectors, uint64 groupId) external; + + function setFamilyAdminDelay(uint64 familyId, uint32 newDelay) external; + + function setContractFamily(address target, uint64 familyId) external; + + function setContractClosed(address target, bool closed) external; + + function getSchedule(bytes32 id) external returns (uint48); + + function schedule(address target, bytes calldata data, uint48 when) external returns (bytes32); + + function relay(address target, bytes calldata data) external payable; + + function cancel(address caller, address target, bytes calldata data) external; + + function consumeScheduledOp(address caller, bytes calldata data) external; + + function updateAuthority(address target, address newAuthority) external; +} diff --git a/contracts/access/manager/IAuthority.sol b/contracts/access/manager/IAuthority.sol index 61a85d2f9..175b967f8 100644 --- a/contracts/access/manager/IAuthority.sol +++ b/contracts/access/manager/IAuthority.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity ^0.8.20; /** * @dev Standard interface for permissioning originally defined in Dappsys. diff --git a/contracts/mocks/AccessManagedTarget.sol b/contracts/mocks/AccessManagedTarget.sol new file mode 100644 index 000000000..305c989f6 --- /dev/null +++ b/contracts/mocks/AccessManagedTarget.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {AccessManaged} from "../access/manager/AccessManaged.sol"; + +abstract contract AccessManagedTarget is AccessManaged { + event CalledRestricted(address caller); + event CalledUnrestricted(address caller); + + function fnRestricted() public restricted { + emit CalledRestricted(msg.sender); + } + + function fnUnrestricted() public { + emit CalledUnrestricted(msg.sender); + } +} diff --git a/contracts/mocks/AccessManagerMocks.sol b/contracts/mocks/AccessManagerMocks.sol deleted file mode 100644 index caf81366e..000000000 --- a/contracts/mocks/AccessManagerMocks.sol +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.13; - -import "../access/manager/IAuthority.sol"; -import "../access/manager/AccessManaged.sol"; - -contract SimpleAuthority is IAuthority { - address _allowedCaller; - address _allowedTarget; - bytes4 _allowedSelector; - - function setAllowed(address allowedCaller, address allowedTarget, bytes4 allowedSelector) public { - _allowedCaller = allowedCaller; - _allowedTarget = allowedTarget; - _allowedSelector = allowedSelector; - } - - function canCall(address caller, address target, bytes4 selector) external view override returns (bool) { - return caller == _allowedCaller && target == _allowedTarget && selector == _allowedSelector; - } -} - -abstract contract AccessManagedMock is AccessManaged { - event RestrictedRan(); - - function restrictedFunction() external restricted { - emit RestrictedRan(); - } - - function otherRestrictedFunction() external restricted { - emit RestrictedRan(); - } -} diff --git a/contracts/utils/types/Time.sol b/contracts/utils/types/Time.sol new file mode 100644 index 000000000..07a3a68b1 --- /dev/null +++ b/contracts/utils/types/Time.sol @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Math} from "../math/Math.sol"; +import {SafeCast} from "../math/SafeCast.sol"; + +/** + * @dev This library provides helpers for manipulating time-related objects. + * + * It uses the following types: + * - `uint48` for timepoints + * - `uint32` for durations + * + * While the library doesn't provide specific types for timepoints and duration, it does provide: + * - a `Delay` type to represent duration that can be programmed to change value automatically at a given point + * - additional helper functions + */ +library Time { + using Time for *; + + /** + * @dev Get the block timestamp as a Timepoint. + */ + function timestamp() internal view returns (uint48) { + return SafeCast.toUint48(block.timestamp); + } + + /** + * @dev Get the block number as a Timepoint. + */ + function blockNumber() internal view returns (uint48) { + return SafeCast.toUint48(block.number); + } + + /** + * @dev Check if a timepoint is set, and in the past. + */ + function isSetAndPast(uint48 timepoint, uint48 ref) internal pure returns (bool) { + return timepoint != 0 && timepoint <= ref; + } + + // ==================================================== Delay ===================================================== + /** + * @dev A `Delay` is a uint32 duration that can be programmed to change value automatically at a given point in the + * future. The "effect" timepoint describes when the transitions happens from the "old" value to the "new" value. + * This allows updating the delay applied to some operation while keeping so guarantees. + * + * In particular, the {update} function guarantees that is the delay is reduced, the old delay still applies for + * some time. For example if the delay is currently 7 days to do an upgrade, the admin should not be able to set + * the delay to 0 and upgrade immediately. If the admin wants to reduce the delay, the old delay (7 days) should + * still apply for some time. + * + * + * The `Delay` type is 128 bits long, and packs the following: + * [000:031] uint32 for the current value (duration) + * [032:063] uint32 for the pending value (duration) + * [064:111] uint48 for the effect date (timepoint) + * + * NOTE: The {get} and {update} function operate using timestamps. Block number based delays should use the + * {getAt} and {withUpdateAt} variants of these functions. + */ + type Delay is uint112; + + /** + * @dev Wrap a duration into a Delay to add the one-step "update in the future" feature + */ + function toDelay(uint32 duration) internal pure returns (Delay) { + return Delay.wrap(duration); + } + + /** + * @dev Get the value at a given timepoint plus the pending value and effect timepoint if there is a scheduled + * change after this timepoint. If the effect timepoint is 0, then the pending value should not be considered. + */ + function getFullAt(Delay self, uint48 timepoint) internal pure returns (uint32, uint32, uint48) { + (uint32 oldValue, uint32 newValue, uint48 effect) = self.unpack(); + return effect.isSetAndPast(timepoint) ? (newValue, 0, 0) : (oldValue, newValue, effect); + } + + /** + * @dev Get the current value plus the pending value and effect timepoint if there is a scheduled change. If the + * effect timepoint is 0, then the pending value should not be considered. + */ + function getFull(Delay self) internal view returns (uint32, uint32, uint48) { + return self.getFullAt(timestamp()); + } + + /** + * @dev Get the value the Delay will be at a given timepoint. + */ + function getAt(Delay self, uint48 timepoint) internal pure returns (uint32) { + (uint32 delay, , ) = getFullAt(self, timepoint); + return delay; + } + + /** + * @dev Get the current value. + */ + function get(Delay self) internal view returns (uint32) { + return self.getAt(timestamp()); + } + + /** + * @dev Update a Delay object so that a new duration takes effect at a given timepoint. + */ + function withUpdateAt(Delay self, uint32 newValue, uint48 effect) internal view returns (Delay) { + return pack(self.get(), newValue, effect); + } + + /** + * @dev Update a Delay object so that it takes a new duration after at a timepoint that is automatically computed + * to enforce the old delay at the moment of the update. + */ + function withUpdate(Delay self, uint32 newValue, uint32 minSetback) internal view returns (Delay) { + uint32 value = self.get(); + uint32 setback = uint32(Math.max(minSetback, value > newValue ? value - newValue : 0)); + return self.withUpdateAt(newValue, timestamp() + setback); + } + + /** + * @dev Split a delay into its components: oldValue, newValue and effect (transition timepoint). + */ + function unpack(Delay self) internal pure returns (uint32, uint32, uint48) { + uint112 raw = Delay.unwrap(self); + return ( + uint32(raw), // oldValue + uint32(raw >> 32), // newValue + uint48(raw >> 64) // effect + ); + } + + /** + * @dev pack the components into a Delay object. + */ + function pack(uint32 oldValue, uint32 newValue, uint48 effect) internal pure returns (Delay) { + return Delay.wrap(uint112(oldValue) | (uint112(newValue) << 32) | (uint112(effect) << 64)); + } +} diff --git a/package-lock.json b/package-lock.json index 22fe93cb7..60fa391df 100644 --- a/package-lock.json +++ b/package-lock.json @@ -45,7 +45,7 @@ "solhint": "^3.3.6", "solhint-plugin-openzeppelin": "file:scripts/solhint-custom", "solidity-ast": "^0.4.25", - "solidity-coverage": "^0.8.0", + "solidity-coverage": "^0.8.4", "solidity-docgen": "^0.6.0-beta.29", "undici": "^5.22.1", "web3": "^1.3.0", @@ -12796,9 +12796,9 @@ } }, "node_modules/solidity-coverage/node_modules/@solidity-parser/parser": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.0.tgz", - "integrity": "sha512-ESipEcHyRHg4Np4SqBCfcXwyxxna1DgFVz69bgpLV8vzl/NP1DtcKsJ4dJZXWQhY/Z4J2LeKBiOkOVZn9ct33Q==", + "version": "0.16.1", + "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.1.tgz", + "integrity": "sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==", "dev": true, "dependencies": { "antlr4ts": "^0.5.0-alpha.4" @@ -25272,9 +25272,9 @@ }, "dependencies": { "@solidity-parser/parser": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.0.tgz", - "integrity": "sha512-ESipEcHyRHg4Np4SqBCfcXwyxxna1DgFVz69bgpLV8vzl/NP1DtcKsJ4dJZXWQhY/Z4J2LeKBiOkOVZn9ct33Q==", + "version": "0.16.1", + "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.1.tgz", + "integrity": "sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==", "dev": true, "requires": { "antlr4ts": "^0.5.0-alpha.4" diff --git a/test/access/manager/AccessManaged.test.js b/test/access/manager/AccessManaged.test.js deleted file mode 100644 index caf1eea7e..000000000 --- a/test/access/manager/AccessManaged.test.js +++ /dev/null @@ -1,55 +0,0 @@ -const { - expectEvent, - expectRevert, - constants: { ZERO_ADDRESS }, -} = require('@openzeppelin/test-helpers'); - -const AccessManaged = artifacts.require('$AccessManagedMock'); -const SimpleAuthority = artifacts.require('SimpleAuthority'); - -contract('AccessManaged', function (accounts) { - const [authority, other, user] = accounts; - it('construction', async function () { - const managed = await AccessManaged.new(authority); - expectEvent.inConstruction(managed, 'AuthorityUpdated', { - oldAuthority: ZERO_ADDRESS, - newAuthority: authority, - }); - expect(await managed.authority()).to.equal(authority); - }); - - describe('setAuthority', function () { - it(`current authority can change managed's authority`, async function () { - const managed = await AccessManaged.new(authority); - const set = await managed.setAuthority(other, { from: authority }); - expectEvent(set, 'AuthorityUpdated', { - sender: authority, - newAuthority: other, - }); - expect(await managed.authority()).to.equal(other); - }); - - it(`other account cannot change managed's authority`, async function () { - const managed = await AccessManaged.new(authority); - await expectRevert(managed.setAuthority(other, { from: other }), 'AccessManaged: not current authority'); - }); - }); - - describe('restricted', function () { - const selector = web3.eth.abi.encodeFunctionSignature('restrictedFunction()'); - - it('allows if authority returns true', async function () { - const authority = await SimpleAuthority.new(); - const managed = await AccessManaged.new(authority.address); - await authority.setAllowed(user, managed.address, selector); - const restricted = await managed.restrictedFunction({ from: user }); - expectEvent(restricted, 'RestrictedRan'); - }); - - it('reverts if authority returns false', async function () { - const authority = await SimpleAuthority.new(); - const managed = await AccessManaged.new(authority.address); - await expectRevert(managed.restrictedFunction({ from: user }), 'AccessManaged: authority rejected'); - }); - }); -}); diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js index 13c14e6e0..2d2f61d3e 100644 --- a/test/access/manager/AccessManager.test.js +++ b/test/access/manager/AccessManager.test.js @@ -1,514 +1,1144 @@ -const { - expectEvent, - expectRevert, - time: { duration }, -} = require('@openzeppelin/test-helpers'); -const { AccessMode } = require('../../helpers/enums'); - -const AccessManager = artifacts.require('AccessManager'); -const AccessManagerAdapter = artifacts.require('AccessManagerAdapter'); -const AccessManaged = artifacts.require('$AccessManagedMock'); +const { web3 } = require('hardhat'); +const { expectEvent, time } = require('@openzeppelin/test-helpers'); +const { expectRevertCustomError } = require('../../helpers/customError'); +const { selector } = require('../../helpers/methods'); +const { clockFromReceipt } = require('../../helpers/time'); +const AccessManager = artifacts.require('$AccessManager'); +const AccessManagedTarget = artifacts.require('$AccessManagedTarget'); const Ownable = artifacts.require('$Ownable'); -const AccessControl = artifacts.require('$AccessControl'); -const groupUtils = { - mask: group => 1n << BigInt(group), - decodeBitmap: hexBitmap => { - const m = BigInt(hexBitmap); - const allGroups = new Array(256).fill().map((_, i) => i.toString()); - return allGroups.filter(i => (m & groupUtils.mask(i)) !== 0n); - }, - role: group => web3.utils.asciiToHex('group:').padEnd(64, '0') + group.toString(16).padStart(2, '0'), +const MAX_UINT64 = web3.utils.toBN((2n ** 64n - 1n).toString()); + +const GROUPS = { + ADMIN: web3.utils.toBN(0), + SOME_ADMIN: web3.utils.toBN(17), + SOME: web3.utils.toBN(42), + PUBLIC: MAX_UINT64, }; +Object.assign(GROUPS, Object.fromEntries(Object.entries(GROUPS).map(([key, value]) => [value, key]))); -const PUBLIC_GROUP = '255'; +const familyId = web3.utils.toBN(1); +const executeDelay = web3.utils.toBN(10); +const grantDelay = web3.utils.toBN(10); + +const formatAccess = access => [access[0], access[1].toString()]; contract('AccessManager', function (accounts) { - const [admin, nonAdmin, user1, user2, otherAuthority] = accounts; - beforeEach('deploy', async function () { - this.delay = duration.days(1); - this.manager = await AccessManager.new(this.delay, admin); + const [admin, manager, member, user, other] = accounts; + + beforeEach(async function () { + this.manager = await AccessManager.new(admin); + + // add member to group + await this.manager.$_setGroupAdmin(GROUPS.SOME, GROUPS.SOME_ADMIN); + await this.manager.$_setGroupGuardian(GROUPS.SOME, GROUPS.SOME_ADMIN); + await this.manager.$_grantGroup(GROUPS.SOME_ADMIN, manager, 0, 0); + await this.manager.$_grantGroup(GROUPS.SOME, member, 0, 0); }); - it('configures default admin rules', async function () { - expect(await this.manager.defaultAdmin()).to.equal(admin); - expect(await this.manager.defaultAdminDelay()).to.be.bignumber.equal(this.delay); + it('groups are correctly initialized', async function () { + // group admin + expect(await this.manager.getGroupAdmin(GROUPS.ADMIN)).to.be.bignumber.equal(GROUPS.ADMIN); + expect(await this.manager.getGroupAdmin(GROUPS.SOME_ADMIN)).to.be.bignumber.equal(GROUPS.ADMIN); + expect(await this.manager.getGroupAdmin(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.SOME_ADMIN); + expect(await this.manager.getGroupAdmin(GROUPS.PUBLIC)).to.be.bignumber.equal(GROUPS.ADMIN); + // group guardian + expect(await this.manager.getGroupGuardian(GROUPS.ADMIN)).to.be.bignumber.equal(GROUPS.ADMIN); + expect(await this.manager.getGroupGuardian(GROUPS.SOME_ADMIN)).to.be.bignumber.equal(GROUPS.ADMIN); + expect(await this.manager.getGroupGuardian(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.SOME_ADMIN); + expect(await this.manager.getGroupGuardian(GROUPS.PUBLIC)).to.be.bignumber.equal(GROUPS.ADMIN); + // group members + expect(await this.manager.hasGroup(GROUPS.ADMIN, admin).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.ADMIN, manager).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME_ADMIN, admin).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME_ADMIN, manager).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME_ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME_ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME, admin).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME, manager).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + expect(await this.manager.hasGroup(GROUPS.PUBLIC, admin).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.PUBLIC, manager).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.PUBLIC, member).then(formatAccess)).to.be.deep.equal([true, '0']); + expect(await this.manager.hasGroup(GROUPS.PUBLIC, user).then(formatAccess)).to.be.deep.equal([true, '0']); }); - describe('groups', function () { - const group = '0'; - const name = 'dao'; - const otherGroup = '1'; - const otherName = 'council'; - - describe('public group', function () { - it('is created automatically', async function () { - await expectEvent.inConstruction(this.manager, 'GroupUpdated', { - group: PUBLIC_GROUP, - name: 'public', + describe('Groups management', function () { + describe('label group', function () { + it('admin can emit a label event', async function () { + expectEvent(await this.manager.labelGroup(GROUPS.SOME, 'Some label', { from: admin }), 'GroupLabel', { + groupId: GROUPS.SOME, + label: 'Some label', }); }); - it('includes all users automatically', async function () { - const groups = groupUtils.decodeBitmap(await this.manager.getUserGroups(user1)); - expect(groups).to.include(PUBLIC_GROUP); - }); - }); + it('admin can re-emit a label event', async function () { + await this.manager.labelGroup(GROUPS.SOME, 'Some label', { from: admin }); - describe('creating', function () { - it('admin can create groups', async function () { - const created = await this.manager.createGroup(group, name, { from: admin }); - expectEvent(created, 'GroupUpdated', { group, name }); - expect(await this.manager.hasGroup(group)).to.equal(true); - expect(await this.manager.hasGroup(otherGroup)).to.equal(false); + expectEvent(await this.manager.labelGroup(GROUPS.SOME, 'Updated label', { from: admin }), 'GroupLabel', { + groupId: GROUPS.SOME, + label: 'Updated label', + }); }); - it('non-admin cannot create groups', async function () { - await expectRevert(this.manager.createGroup(group, name, { from: nonAdmin }), 'missing role'); - }); - - it('cannot recreate a group', async function () { - await this.manager.createGroup(group, name, { from: admin }); - await expectRevert(this.manager.createGroup(group, name, { from: admin }), 'AccessManager: existing group'); - }); - }); - - describe('updating', function () { - beforeEach('create group', async function () { - await this.manager.createGroup(group, name, { from: admin }); - }); - - it('admin can update group', async function () { - const updated = await this.manager.updateGroupName(group, otherName, { from: admin }); - expectEvent(updated, 'GroupUpdated', { group, name: otherName }); - }); - - it('non-admin cannot update group', async function () { - await expectRevert(this.manager.updateGroupName(group, name, { from: nonAdmin }), 'missing role'); - }); - - it('cannot update built in group', async function () { - await expectRevert( - this.manager.updateGroupName(PUBLIC_GROUP, name, { from: admin }), - 'AccessManager: built-in group', - ); - }); - - it('cannot update nonexistent group', async function () { - await expectRevert( - this.manager.updateGroupName(otherGroup, name, { from: admin }), - 'AccessManager: unknown group', + it('emitting a label is restricted', async function () { + await expectRevertCustomError( + this.manager.labelGroup(GROUPS.SOME, 'Invalid label', { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.ADMIN], ); }); }); - describe('granting', function () { - beforeEach('create group', async function () { - await this.manager.createGroup(group, name, { from: admin }); + describe('grant group', function () { + describe('without a grant delay', function () { + it('without an execute delay', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'GroupGranted', { groupId: GROUPS.SOME, account: user, since: timestamp, delay: '0' }); + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal(timestamp); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); + + it('with an execute delay', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, user, executeDelay, { from: manager }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'GroupGranted', { + groupId: GROUPS.SOME, + account: user, + since: timestamp, + delay: executeDelay, + }); + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([ + true, + executeDelay.toString(), + ]); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal(timestamp); // inGroupSince + expect(access[1]).to.be.bignumber.equal(executeDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); + + it('to a user that is already in the group', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); + + await expectRevertCustomError( + this.manager.grantGroup(GROUPS.SOME, member, 0, { from: manager }), + 'AccessManagerAccountAlreadyInGroup', + [GROUPS.SOME, member], + ); + }); + + it('to a user that is scheduled for joining the group', async function () { + await this.manager.$_grantGroup(GROUPS.SOME, user, 10, 0); // grant delay 10 + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + await expectRevertCustomError( + this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }), + 'AccessManagerAccountAlreadyInGroup', + [GROUPS.SOME, user], + ); + }); + + it('grant group is restricted', async function () { + await expectRevertCustomError( + this.manager.grantGroup(GROUPS.SOME, user, 0, { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.SOME_ADMIN], + ); + }); }); - it('admin can grant group', async function () { - const granted = await this.manager.grantGroup(group, user1, { from: admin }); - expectEvent(granted, 'RoleGranted', { account: user1, role: groupUtils.role(group) }); - const groups = groupUtils.decodeBitmap(await this.manager.getUserGroups(user1)); - expect(groups).to.include(group); - }); + describe('with a grant delay', function () { + beforeEach(async function () { + await this.manager.$_setGrantDelay(GROUPS.SOME, grantDelay); + }); - it('non-admin cannot grant group', async function () { - await expectRevert(this.manager.grantGroup(group, user1, { from: nonAdmin }), 'missing role'); - }); + it('granted group is not active immediatly', async function () { + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'GroupGranted', { + groupId: GROUPS.SOME, + account: user, + since: timestamp.add(grantDelay), + delay: '0', + }); - it('cannot grant nonexistent group', async function () { - await expectRevert(this.manager.grantGroup(otherGroup, user1, { from: admin }), 'AccessManager: unknown group'); + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); + + it('granted group is active after the delay', async function () { + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'GroupGranted', { + groupId: GROUPS.SOME, + account: user, + since: timestamp.add(grantDelay), + delay: '0', + }); + + await time.increase(grantDelay); + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); }); }); - describe('revoking & renouncing', function () { - beforeEach('create and grant group', async function () { - await this.manager.createGroup(group, name, { from: admin }); - await this.manager.grantGroup(group, user1, { from: admin }); + describe('revoke group', function () { + it('from a user that is already in the group', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); + + const { receipt } = await this.manager.revokeGroup(GROUPS.SOME, member, { from: manager }); + expectEvent(receipt, 'GroupRevoked', { groupId: GROUPS.SOME, account: member }); + + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal('0'); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect }); - it('admin can revoke group', async function () { - await this.manager.revokeGroup(group, user1, { from: admin }); - const groups = groupUtils.decodeBitmap(await this.manager.getUserGroups(user1)); - expect(groups).to.not.include(group); + it('from a user that is scheduled for joining the group', async function () { + await this.manager.$_grantGroup(GROUPS.SOME, user, 10, 0); // grant delay 10 + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const { receipt } = await this.manager.revokeGroup(GROUPS.SOME, user, { from: manager }); + expectEvent(receipt, 'GroupRevoked', { groupId: GROUPS.SOME, account: user }); + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal('0'); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect }); - it('non-admin cannot revoke group', async function () { - await expectRevert(this.manager.revokeGroup(group, user1, { from: nonAdmin }), 'missing role'); - }); + it('from a user that is not in the group', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - it('user can renounce group', async function () { - await this.manager.renounceGroup(group, user1, { from: user1 }); - const groups = groupUtils.decodeBitmap(await this.manager.getUserGroups(user1)); - expect(groups).to.not.include(group); - }); - - it(`user cannot renounce other user's groups`, async function () { - await expectRevert( - this.manager.renounceGroup(group, user1, { from: user2 }), - 'can only renounce roles for self', - ); - await expectRevert( - this.manager.renounceGroup(group, user2, { from: user1 }), - 'can only renounce roles for self', + await expectRevertCustomError( + this.manager.revokeGroup(GROUPS.SOME, user, { from: manager }), + 'AccessManagerAccountNotInGroup', + [GROUPS.SOME, user], ); }); - it('cannot revoke public group', async function () { - await expectRevert( - this.manager.revokeGroup(PUBLIC_GROUP, user1, { from: admin }), - 'AccessManager: irrevocable group', - ); - }); - - it('cannot revoke nonexistent group', async function () { - await expectRevert( - this.manager.revokeGroup(otherGroup, user1, { from: admin }), - 'AccessManager: unknown group', - ); - await expectRevert( - this.manager.renounceGroup(otherGroup, user1, { from: user1 }), - 'AccessManager: unknown group', + it('revoke group is restricted', async function () { + await expectRevertCustomError( + this.manager.revokeGroup(GROUPS.SOME, member, { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.SOME_ADMIN], ); }); }); - describe('querying', function () { - it('returns expected groups', async function () { - const getGroups = () => this.manager.getUserGroups(user1); + describe('renounce group', function () { + it('for a user that is already in the group', async function () { + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - // only public group initially - expect(await getGroups()).to.equal('0x8000000000000000000000000000000000000000000000000000000000000000'); + const { receipt } = await this.manager.renounceGroup(GROUPS.SOME, member, { from: member }); + expectEvent(receipt, 'GroupRevoked', { groupId: GROUPS.SOME, account: member }); - await this.manager.createGroup('0', '0', { from: admin }); - await this.manager.grantGroup('0', user1, { from: admin }); - expect(await getGroups()).to.equal('0x8000000000000000000000000000000000000000000000000000000000000001'); + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']); - await this.manager.createGroup('1', '1', { from: admin }); - await this.manager.grantGroup('1', user1, { from: admin }); - expect(await getGroups()).to.equal('0x8000000000000000000000000000000000000000000000000000000000000003'); + const access = await this.manager.getAccess(GROUPS.SOME, member); + expect(access[0]).to.be.bignumber.equal('0'); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); - await this.manager.createGroup('16', '16', { from: admin }); - await this.manager.grantGroup('16', user1, { from: admin }); - expect(await getGroups()).to.equal('0x8000000000000000000000000000000000000000000000000000000000010003'); + it('for a user that is schedule for joining the group', async function () { + await this.manager.$_grantGroup(GROUPS.SOME, user, 10, 0); // grant delay 10 + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const { receipt } = await this.manager.renounceGroup(GROUPS.SOME, user, { from: user }); + expectEvent(receipt, 'GroupRevoked', { groupId: GROUPS.SOME, account: user }); + + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + + const access = await this.manager.getAccess(GROUPS.SOME, user); + expect(access[0]).to.be.bignumber.equal('0'); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + }); + + it('for a user that is not in the group', async function () { + await expectRevertCustomError( + this.manager.renounceGroup(GROUPS.SOME, user, { from: user }), + 'AccessManagerAccountNotInGroup', + [GROUPS.SOME, user], + ); + }); + + it('bad user confirmation', async function () { + await expectRevertCustomError( + this.manager.renounceGroup(GROUPS.SOME, member, { from: user }), + 'AccessManagerBadConfirmation', + [], + ); + }); + }); + + describe('change group admin', function () { + it("admin can set any group's admin", async function () { + expect(await this.manager.getGroupAdmin(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.SOME_ADMIN); + + const { receipt } = await this.manager.setGroupAdmin(GROUPS.SOME, GROUPS.ADMIN, { from: admin }); + expectEvent(receipt, 'GroupAdminChanged', { groupId: GROUPS.SOME, admin: GROUPS.ADMIN }); + + expect(await this.manager.getGroupAdmin(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.ADMIN); + }); + + it("setting a group's admin is restricted", async function () { + await expectRevertCustomError( + this.manager.setGroupAdmin(GROUPS.SOME, GROUPS.SOME, { from: manager }), + 'AccessManagerUnauthorizedAccount', + [manager, GROUPS.ADMIN], + ); + }); + }); + + describe('change group guardian', function () { + it("admin can set any group's admin", async function () { + expect(await this.manager.getGroupGuardian(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.SOME_ADMIN); + + const { receipt } = await this.manager.setGroupGuardian(GROUPS.SOME, GROUPS.ADMIN, { from: admin }); + expectEvent(receipt, 'GroupGuardianChanged', { groupId: GROUPS.SOME, guardian: GROUPS.ADMIN }); + + expect(await this.manager.getGroupGuardian(GROUPS.SOME)).to.be.bignumber.equal(GROUPS.ADMIN); + }); + + it("setting a group's admin is restricted", async function () { + await expectRevertCustomError( + this.manager.setGroupGuardian(GROUPS.SOME, GROUPS.SOME, { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.ADMIN], + ); + }); + }); + + describe('change execution delay', function () { + it('increassing the delay has immediate effect', async function () { + const oldDelay = web3.utils.toBN(10); + const newDelay = web3.utils.toBN(100); + + await this.manager.$_setExecuteDelay(GROUPS.SOME, member, oldDelay); + + const accessBefore = await this.manager.getAccess(GROUPS.SOME, member); + expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay + expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect + + const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, member, newDelay, { + from: manager, + }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'GroupExecutionDelayUpdated', { + groupId: GROUPS.SOME, + account: member, + delay: newDelay, + from: timestamp, + }); + + // immediate effect + const accessAfter = await this.manager.getAccess(GROUPS.SOME, member); + expect(accessAfter[1]).to.be.bignumber.equal(newDelay); // currentDelay + expect(accessAfter[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(accessAfter[3]).to.be.bignumber.equal('0'); // effect + }); + + it('decreassing the delay takes time', async function () { + const oldDelay = web3.utils.toBN(100); + const newDelay = web3.utils.toBN(10); + + await this.manager.$_setExecuteDelay(GROUPS.SOME, member, oldDelay); + + const accessBefore = await this.manager.getAccess(GROUPS.SOME, member); + expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay + expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect + + const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, member, newDelay, { + from: manager, + }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'GroupExecutionDelayUpdated', { + groupId: GROUPS.SOME, + account: member, + delay: newDelay, + from: timestamp.add(oldDelay).sub(newDelay), + }); + + // delayed effect + const accessAfter = await this.manager.getAccess(GROUPS.SOME, member); + expect(accessAfter[1]).to.be.bignumber.equal(oldDelay); // currentDelay + expect(accessAfter[2]).to.be.bignumber.equal(newDelay); // pendingDelay + expect(accessAfter[3]).to.be.bignumber.equal(timestamp.add(oldDelay).sub(newDelay)); // effect + }); + + it('cannot set the delay of a non member', async function () { + await expectRevertCustomError( + this.manager.setExecuteDelay(GROUPS.SOME, other, executeDelay, { from: manager }), + 'AccessManagerAccountNotInGroup', + [GROUPS.SOME, other], + ); + }); + + it('cannot set the delay of public and admin groups', async function () { + for (const group of [GROUPS.PUBLIC, GROUPS.ADMIN]) { + await expectRevertCustomError( + this.manager.$_setExecuteDelay(group, other, executeDelay, { from: manager }), + 'AccessManagerLockedGroup', + [group], + ); + } + }); + + it('can set a user execution delay during the grant delay', async function () { + await this.manager.$_grantGroup(GROUPS.SOME, other, 10, 0); + + const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, other, executeDelay, { from: manager }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'GroupExecutionDelayUpdated', { + groupId: GROUPS.SOME, + account: other, + delay: executeDelay, + from: timestamp, + }); + }); + + it('changing the execution delay is restricted', async function () { + await expectRevertCustomError( + this.manager.setExecuteDelay(GROUPS.SOME, member, executeDelay, { from: other }), + 'AccessManagerUnauthorizedAccount', + [GROUPS.SOME_ADMIN, other], + ); + }); + }); + + describe('change grant delay', function () { + it('increassing the delay has immediate effect', async function () { + const oldDelay = web3.utils.toBN(10); + const newDelay = web3.utils.toBN(100); + await this.manager.$_setGrantDelay(GROUPS.SOME, oldDelay); + + expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(oldDelay); + + const { receipt } = await this.manager.setGrantDelay(GROUPS.SOME, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'GroupGrantDelayChanged', { groupId: GROUPS.SOME, delay: newDelay, from: timestamp }); + + expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(newDelay); + }); + + it('increassing the delay has delay effect', async function () { + const oldDelay = web3.utils.toBN(100); + const newDelay = web3.utils.toBN(10); + await this.manager.$_setGrantDelay(GROUPS.SOME, oldDelay); + + expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(oldDelay); + + const { receipt } = await this.manager.setGrantDelay(GROUPS.SOME, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'GroupGrantDelayChanged', { + groupId: GROUPS.SOME, + delay: newDelay, + from: timestamp.add(oldDelay).sub(newDelay), + }); + + expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(oldDelay); + + await time.increase(oldDelay.sub(newDelay)); + + expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(newDelay); + }); + + it('changing the grant delay is restricted', async function () { + await expectRevertCustomError( + this.manager.setGrantDelay(GROUPS.SOME, grantDelay, { from: other }), + 'AccessManagerUnauthorizedAccount', + [GROUPS.ADMIN, other], + ); }); }); }); - describe('allowing', function () { - const group = '1'; - const otherGroup = '2'; - const groupMember = user1; - const selector = web3.eth.abi.encodeFunctionSignature('restrictedFunction()'); - const otherSelector = web3.eth.abi.encodeFunctionSignature('otherRestrictedFunction()'); - - beforeEach('deploying managed contract', async function () { - await this.manager.createGroup(group, '', { from: admin }); - await this.manager.grantGroup(group, groupMember, { from: admin }); - this.managed = await AccessManaged.new(this.manager.address); + describe('with AccessManaged target contract', function () { + beforeEach('deploy target contract', async function () { + this.target = await AccessManagedTarget.new(this.manager.address); + // helpers for indirect calls + this.callData = selector('fnRestricted()'); + this.call = [this.target.address, this.callData]; + this.opId = web3.utils.keccak256( + web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [user, ...this.call]), + ); + this.direct = (opts = {}) => this.target.fnRestricted({ from: user, ...opts }); + this.schedule = (opts = {}) => this.manager.schedule(...this.call, 0, { from: user, ...opts }); + this.relay = (opts = {}) => this.manager.relay(...this.call, { from: user, ...opts }); + this.cancel = (opts = {}) => this.manager.cancel(user, ...this.call, { from: user, ...opts }); }); - it('non-admin cannot change allowed groups', async function () { - await expectRevert( - this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, true, { from: nonAdmin }), - 'missing role', + describe('Change function permissions', function () { + const sigs = ['someFunction()', 'someOtherFunction(uint256)', 'oneMoreFunction(address,uint8)'].map(selector); + + it('admin can set function group', async function () { + for (const sig of sigs) { + expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal(GROUPS.ADMIN); + } + + const { receipt: receipt1 } = await this.manager.setFamilyFunctionGroup(familyId, sigs, GROUPS.SOME, { + from: admin, + }); + + for (const sig of sigs) { + expectEvent(receipt1, 'FamilyFunctionGroupUpdated', { + familyId, + selector: sig, + groupId: GROUPS.SOME, + }); + expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal(GROUPS.SOME); + } + + const { receipt: receipt2 } = await this.manager.setFamilyFunctionGroup( + familyId, + [sigs[1]], + GROUPS.SOME_ADMIN, + { from: admin }, + ); + expectEvent(receipt2, 'FamilyFunctionGroupUpdated', { + familyId, + selector: sigs[1], + groupId: GROUPS.SOME_ADMIN, + }); + + for (const sig of sigs) { + expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal( + sig == sigs[1] ? GROUPS.SOME_ADMIN : GROUPS.SOME, + ); + } + }); + + it('non-admin cannot set function group', async function () { + await expectRevertCustomError( + this.manager.setFamilyFunctionGroup(familyId, sigs, GROUPS.SOME, { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.ADMIN], + ); + }); + }); + + // WIP + describe('Calling restricted & unrestricted functions', function () { + const product = (...arrays) => arrays.reduce((a, b) => a.flatMap(ai => b.map(bi => [...ai, bi])), [[]]); + + for (const [callerGroups, fnGroup, closed, delay] of product( + [[], [GROUPS.SOME]], + [undefined, GROUPS.ADMIN, GROUPS.SOME, GROUPS.PUBLIC], + [false, true], + [null, executeDelay], + )) { + // can we call with a delay ? + const indirectSuccess = (fnGroup == GROUPS.PUBLIC || callerGroups.includes(fnGroup)) && !closed; + + // can we call without a delay ? + const directSuccess = (fnGroup == GROUPS.PUBLIC || (callerGroups.includes(fnGroup) && !delay)) && !closed; + + const description = [ + 'Caller in groups', + '[' + (callerGroups ?? []).map(groupId => GROUPS[groupId]).join(', ') + ']', + delay ? 'with a delay' : 'without a delay', + '+', + 'functions open to groups', + '[' + (GROUPS[fnGroup] ?? '') + ']', + closed ? `(closed)` : '', + ].join(' '); + + describe(description, function () { + beforeEach(async function () { + // setup + await Promise.all([ + this.manager.$_setContractClosed(this.target.address, closed), + this.manager.$_setContractFamily(this.target.address, familyId), + fnGroup && this.manager.$_setFamilyFunctionGroup(familyId, selector('fnRestricted()'), fnGroup), + fnGroup && this.manager.$_setFamilyFunctionGroup(familyId, selector('fnUnrestricted()'), fnGroup), + ...callerGroups + .filter(groupId => groupId != GROUPS.PUBLIC) + .map(groupId => this.manager.$_grantGroup(groupId, user, 0, delay ?? 0)), + ]); + + // post setup checks + const result = await this.manager.getContractFamily(this.target.address); + expect(result[0]).to.be.bignumber.equal(familyId); + expect(result[1]).to.be.equal(closed); + + if (fnGroup) { + expect( + await this.manager.getFamilyFunctionGroup(familyId, selector('fnRestricted()')), + ).to.be.bignumber.equal(fnGroup); + expect( + await this.manager.getFamilyFunctionGroup(familyId, selector('fnUnrestricted()')), + ).to.be.bignumber.equal(fnGroup); + } + + for (const groupId of callerGroups) { + const access = await this.manager.getAccess(groupId, user); + if (groupId == GROUPS.PUBLIC) { + expect(access[0]).to.be.bignumber.equal('0'); // inGroupSince + expect(access[1]).to.be.bignumber.equal('0'); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + } else { + expect(access[0]).to.be.bignumber.gt('0'); // inGroupSince + expect(access[1]).to.be.bignumber.eq(String(delay ?? 0)); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // effect + } + } + }); + + it('canCall', async function () { + const result = await this.manager.canCall(user, this.target.address, selector('fnRestricted()')); + expect(result[0]).to.be.equal(directSuccess); + expect(result[1]).to.be.bignumber.equal(!directSuccess && indirectSuccess ? delay ?? '0' : '0'); + }); + + it('Calling a non restricted function never revert', async function () { + expectEvent(await this.target.fnUnrestricted({ from: user }), 'CalledUnrestricted', { + caller: user, + }); + }); + + it(`Calling a restricted function directly should ${ + directSuccess ? 'succeed' : 'revert' + }`, async function () { + const promise = this.direct(); + + if (directSuccess) { + expectEvent(await promise, 'CalledRestricted', { caller: user }); + } else if (indirectSuccess) { + await expectRevertCustomError(promise, 'AccessManagerNotScheduled', [this.opId]); + } else { + await expectRevertCustomError(promise, 'AccessManagedUnauthorized', [user]); + } + }); + + it('Calling indirectly: only relay', async function () { + // relay without schedule + if (directSuccess) { + const { receipt, tx } = await this.relay(); + expectEvent.notEmitted(receipt, 'OperationExecuted', { operationId: this.opId }); + await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); + } else if (indirectSuccess) { + await expectRevertCustomError(this.relay(), 'AccessManagerNotScheduled', [this.opId]); + } else { + await expectRevertCustomError(this.relay(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + }); + + it('Calling indirectly: schedule and relay', async function () { + if (directSuccess || indirectSuccess) { + const { receipt } = await this.schedule(); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + + // if can call directly, delay should be 0. Otherwise, the delay should be applied + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal( + timestamp.add(directSuccess ? web3.utils.toBN(0) : delay), + ); + + // execute without wait + if (directSuccess) { + const { receipt, tx } = await this.relay(); + await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); + if (delay && fnGroup !== GROUPS.PUBLIC) { + expectEvent(receipt, 'OperationExecuted', { operationId: this.opId }); + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); + } + } else if (indirectSuccess) { + await expectRevertCustomError(this.relay(), 'AccessManagerNotReady', [this.opId]); + } else { + await expectRevertCustomError(this.relay(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + } else { + await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + }); + + it('Calling indirectly: schedule wait and relay', async function () { + if (directSuccess || indirectSuccess) { + const { receipt } = await this.schedule(); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + + // if can call directly, delay should be 0. Otherwise, the delay should be applied + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal( + timestamp.add(directSuccess ? web3.utils.toBN(0) : delay), + ); + + // wait + await time.increase(delay ?? 0); + + // execute without wait + if (directSuccess || indirectSuccess) { + const { receipt, tx } = await this.relay(); + await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); + if (delay && fnGroup !== GROUPS.PUBLIC) { + expectEvent(receipt, 'OperationExecuted', { operationId: this.opId }); + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); + } + } else { + await expectRevertCustomError(this.relay(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + } else { + await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + }); + + it('Calling directly: schedule and call', async function () { + if (directSuccess || indirectSuccess) { + const { receipt } = await this.schedule(); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + + // if can call directly, delay should be 0. Otherwise, the delay should be applied + const schedule = timestamp.add(directSuccess ? web3.utils.toBN(0) : delay); + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule); + + // execute without wait + const promise = this.direct(); + if (directSuccess) { + expectEvent(await promise, 'CalledRestricted', { caller: user }); + + // schedule is not reset + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule); + } else if (indirectSuccess) { + await expectRevertCustomError(promise, 'AccessManagerNotReady', [this.opId]); + } else { + await expectRevertCustomError(promise, 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + } else { + await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + }); + + it('Calling directly: schedule wait and call', async function () { + if (directSuccess || indirectSuccess) { + const { receipt } = await this.schedule(); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + + // if can call directly, delay should be 0. Otherwise, the delay should be applied + const schedule = timestamp.add(directSuccess ? web3.utils.toBN(0) : delay); + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule); + + // wait + await time.increase(delay ?? 0); + + // execute without wait + const promise = await this.direct(); + if (directSuccess) { + expectEvent(await promise, 'CalledRestricted', { caller: user }); + + // schedule is not reset + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(schedule); + } else if (indirectSuccess) { + const receipt = await promise; + + expectEvent(receipt, 'CalledRestricted', { caller: user }); + await expectEvent.inTransaction(receipt.tx, this.manager, 'OperationExecuted', { + operationId: this.opId, + }); + + // schedule is reset + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); + } else { + await expectRevertCustomError(this.direct(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + } else { + await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); + } + }); + + it('Scheduling for later than needed'); // TODO + }); + } + }); + + describe('Indirect execution corner-cases', async function () { + beforeEach(async function () { + await this.manager.$_setContractFamily(this.target.address, familyId); + await this.manager.$_setFamilyFunctionGroup(familyId, this.callData, GROUPS.SOME); + await this.manager.$_grantGroup(GROUPS.SOME, user, 0, executeDelay); + }); + + it('Checking canCall when caller is the manager depend on the _relayIdentifier', async function () { + const result = await this.manager.canCall(this.manager.address, this.target.address, '0x00000000'); + expect(result[0]).to.be.false; + expect(result[1]).to.be.bignumber.equal('0'); + }); + + it('Cannot execute earlier', async function () { + const { receipt } = await this.schedule(); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal(timestamp.add(executeDelay)); + + // we need to set the clock 2 seconds before the value, because the increaseTo "consumes" the timestamp + // and the next transaction will be one after that (see check below) + await time.increaseTo(timestamp.add(executeDelay).subn(2)); + + // too early + await expectRevertCustomError(this.relay(), 'AccessManagerNotReady', [this.opId]); + + // the revert happened one second before the execution delay expired + expect(await time.latest()).to.be.bignumber.equal(timestamp.add(executeDelay).subn(1)); + + // ok + await this.relay(); + + // the success happened when the delay was reached (earliest possible) + expect(await time.latest()).to.be.bignumber.equal(timestamp.add(executeDelay)); + }); + + it('Cannot schedule an already scheduled operation', async function () { + const { receipt } = await this.schedule(); + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + + await expectRevertCustomError(this.schedule(), 'AccessManagerAlreadyScheduled', [this.opId]); + }); + + it('Cannot cancel an operation that is not scheduled', async function () { + await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]); + }); + + it('Cannot cancel an operation that is not already relayed', async function () { + await this.schedule(); + await time.increase(executeDelay); + await this.relay(); + + await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]); + }); + + it('Scheduler can cancel', async function () { + await this.schedule(); + + expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0'); + + expectEvent(await this.cancel({ from: manager }), 'OperationCanceled', { operationId: this.opId }); + + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); + }); + + it('Guardian can cancel', async function () { + await this.schedule(); + + expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0'); + + expectEvent(await this.cancel({ from: manager }), 'OperationCanceled', { operationId: this.opId }); + + expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); + }); + + it('Cancel is restricted', async function () { + await this.schedule(); + + expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0'); + + await expectRevertCustomError(this.cancel({ from: other }), 'AccessManagerCannotCancel', [ + other, + user, + ...this.call, + ]); + + expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0'); + }); + + it('Can re-schedule after execution', async function () { + await this.schedule(); + await time.increase(executeDelay); + await this.relay(); + + // reschedule + const { receipt } = await this.schedule(); + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + }); + + it('Can re-schedule after cancel', async function () { + await this.schedule(); + await this.cancel(); + + // reschedule + const { receipt } = await this.schedule(); + expectEvent(receipt, 'OperationScheduled', { + operationId: this.opId, + caller: user, + target: this.call[0], + data: this.call[1], + }); + }); + }); + }); + + describe('with Ownable target contract', function () { + const groupId = web3.utils.toBN(1); + + beforeEach(async function () { + this.ownable = await Ownable.new(this.manager.address); + + // add user to group + await this.manager.$_grantGroup(groupId, user, 0, 0); + }); + + it('initial state', async function () { + expect(await this.ownable.owner()).to.be.equal(this.manager.address); + }); + + describe('Contract is closed', function () { + beforeEach(async function () { + await this.manager.$_setContractClosed(this.ownable.address, true); + }); + + it('directly call: reverts', async function () { + await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [user]); + }); + + it('relayed call (with group): reverts', async function () { + await expectRevertCustomError( + this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: user }), + 'AccessManagerUnauthorizedCall', + [user, this.ownable.address, selector('$_checkOwner()')], + ); + }); + + it('relayed call (without group): reverts', async function () { + await expectRevertCustomError( + this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: other }), + 'AccessManagerUnauthorizedCall', + [other, this.ownable.address, selector('$_checkOwner()')], + ); + }); + }); + + describe('Contract is managed', function () { + beforeEach('add contract to family', async function () { + await this.manager.$_setContractFamily(this.ownable.address, familyId); + }); + + describe('function is open to specific group', function () { + beforeEach(async function () { + await this.manager.$_setFamilyFunctionGroup(familyId, selector('$_checkOwner()'), groupId); + }); + + it('directly call: reverts', async function () { + await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [ + user, + ]); + }); + + it('relayed call (with group): success', async function () { + await this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: user }); + }); + + it('relayed call (without group): reverts', async function () { + await expectRevertCustomError( + this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: other }), + 'AccessManagerUnauthorizedCall', + [other, this.ownable.address, selector('$_checkOwner()')], + ); + }); + }); + + describe('function is open to public group', function () { + beforeEach(async function () { + await this.manager.$_setFamilyFunctionGroup(familyId, selector('$_checkOwner()'), GROUPS.PUBLIC); + }); + + it('directly call: reverts', async function () { + await expectRevertCustomError(this.ownable.$_checkOwner({ from: user }), 'OwnableUnauthorizedAccount', [ + user, + ]); + }); + + it('relayed call (with group): success', async function () { + await this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: user }); + }); + + it('relayed call (without group): success', async function () { + await this.manager.relay(this.ownable.address, selector('$_checkOwner()'), { from: other }); + }); + }); + }); + }); + + describe('authority update', function () { + beforeEach(async function () { + this.newManager = await AccessManager.new(admin); + this.target = await AccessManagedTarget.new(this.manager.address); + }); + + it('admin can change authority', async function () { + expect(await this.target.authority()).to.be.equal(this.manager.address); + + const { tx } = await this.manager.updateAuthority(this.target.address, this.newManager.address, { from: admin }); + await expectEvent.inTransaction(tx, this.target, 'AuthorityUpdated', { authority: this.newManager.address }); + + expect(await this.target.authority()).to.be.equal(this.newManager.address); + }); + + it('cannot set an address without code as the authority', async function () { + await expectRevertCustomError( + this.manager.updateAuthority(this.target.address, user, { from: admin }), + 'AccessManagedInvalidAuthority', + [user], ); }); - it('single selector', async function () { - const receipt = await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, true, { - from: admin, - }); - - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: selector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4 - group, - allowed: true, - }); - - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([group]); - - const otherAllowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, otherSelector); - expect(groupUtils.decodeBitmap(otherAllowedGroups)).to.deep.equal([]); - - const restricted = await this.managed.restrictedFunction({ from: groupMember }); - expectEvent(restricted, 'RestrictedRan'); - - await expectRevert( - this.managed.otherRestrictedFunction({ from: groupMember }), - 'AccessManaged: authority rejected', + it('updateAuthority is restricted on manager', async function () { + await expectRevertCustomError( + this.manager.updateAuthority(this.target.address, this.newManager.address, { from: other }), + 'AccessManagerUnauthorizedAccount', + [other, GROUPS.ADMIN], ); }); - it('multiple selectors', async function () { - const receipt = await this.manager.setFunctionAllowedGroup( - this.managed.address, - [selector, otherSelector], - group, - true, - { from: admin }, - ); - - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: selector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4 - group, - allowed: true, - }); - - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: otherSelector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4 - group, - allowed: true, - }); - - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([group]); - - const otherAllowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, otherSelector); - expect(groupUtils.decodeBitmap(otherAllowedGroups)).to.deep.equal([group]); - - const restricted = await this.managed.restrictedFunction({ from: groupMember }); - expectEvent(restricted, 'RestrictedRan'); - - await this.managed.otherRestrictedFunction({ from: groupMember }); - expectEvent(restricted, 'RestrictedRan'); - }); - - it('works on open target', async function () { - await this.manager.setContractModeOpen(this.managed.address, { from: admin }); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { from: admin }); - }); - - it('works on closed target', async function () { - await this.manager.setContractModeClosed(this.managed.address, { from: admin }); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { from: admin }); - }); - - it('cannot allow nonexistent group', async function () { - await expectRevert( - this.manager.setFunctionAllowedGroup(this.managed.address, [selector], otherGroup, true, { from: admin }), - 'AccessManager: unknown group', + it('setAuthority is restricted on AccessManaged', async function () { + await expectRevertCustomError( + this.target.setAuthority(this.newManager.address, { from: admin }), + 'AccessManagedUnauthorized', + [admin], ); }); }); - describe('disallowing', function () { - const group = '1'; - const groupMember = user1; - const selector = web3.eth.abi.encodeFunctionSignature('restrictedFunction()'); - const otherSelector = web3.eth.abi.encodeFunctionSignature('otherRestrictedFunction()'); + // TODO: test all admin functions + describe('family delays', function () { + const otherFamilyId = '2'; + const delay = '1000'; - beforeEach('deploying managed contract', async function () { - await this.manager.createGroup(group, '', { from: admin }); - await this.manager.grantGroup(group, groupMember, { from: admin }); - this.managed = await AccessManaged.new(this.manager.address); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector, otherSelector], group, true, { - from: admin, - }); + beforeEach('set contract family', async function () { + this.target = await AccessManagedTarget.new(this.manager.address); + await this.manager.setContractFamily(this.target.address, familyId, { from: admin }); + + this.call = () => this.manager.setContractFamily(this.target.address, otherFamilyId, { from: admin }); + this.data = this.manager.contract.methods.setContractFamily(this.target.address, otherFamilyId).encodeABI(); }); - it('non-admin cannot change disallowed groups', async function () { - await expectRevert( - this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { from: nonAdmin }), - 'missing role', - ); + it('without delay: succeeds', async function () { + await this.call(); }); - it('single selector', async function () { - const receipt = await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { - from: admin, + // TODO: here we need to check increase and decrease. + // - Increasing should have immediate effect + // - Decreassing should take time. + describe('with delay', function () { + beforeEach('set admin delay', async function () { + this.tx = await this.manager.setFamilyAdminDelay(familyId, delay, { from: admin }); + this.opId = web3.utils.keccak256( + web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [admin, this.manager.address, this.data]), + ); }); - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: selector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4, - group, - allowed: false, + it('emits event and sets delay', async function () { + const from = await clockFromReceipt.timestamp(this.tx.receipt).then(web3.utils.toBN); + expectEvent(this.tx.receipt, 'FamilyAdminDelayUpdated', { familyId, delay, from }); + + expect(await this.manager.getFamilyAdminDelay(familyId)).to.be.bignumber.equal(delay); }); - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([]); - - const otherAllowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, otherSelector); - expect(groupUtils.decodeBitmap(otherAllowedGroups)).to.deep.equal([group]); - - await expectRevert(this.managed.restrictedFunction({ from: groupMember }), 'AccessManaged: authority rejected'); - - const otherRestricted = await this.managed.otherRestrictedFunction({ from: groupMember }); - expectEvent(otherRestricted, 'RestrictedRan'); - }); - - it('multiple selectors', async function () { - const receipt = await this.manager.setFunctionAllowedGroup( - this.managed.address, - [selector, otherSelector], - group, - false, - { from: admin }, - ); - - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: selector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4 - group, - allowed: false, + it('without prior scheduling: reverts', async function () { + await expectRevertCustomError(this.call(), 'AccessManagerNotScheduled', [this.opId]); }); - expectEvent(receipt, 'GroupAllowed', { - target: this.managed.address, - selector: otherSelector.padEnd(66, '0'), // there seems to be a bug in decoding the indexed bytes4 - group, - allowed: false, + describe('with prior scheduling', async function () { + beforeEach('schedule', async function () { + await this.manager.schedule(this.manager.address, this.data, 0, { from: admin }); + }); + + it('without delay: reverts', async function () { + await expectRevertCustomError(this.call(), 'AccessManagerNotReady', [this.opId]); + }); + + it('with delay: succeeds', async function () { + await time.increase(delay); + await this.call(); + }); }); - - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([]); - - const otherAllowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, otherSelector); - expect(groupUtils.decodeBitmap(otherAllowedGroups)).to.deep.equal([]); - - await expectRevert(this.managed.restrictedFunction({ from: groupMember }), 'AccessManaged: authority rejected'); - await expectRevert( - this.managed.otherRestrictedFunction({ from: groupMember }), - 'AccessManaged: authority rejected', - ); - }); - - it('works on open target', async function () { - await this.manager.setContractModeOpen(this.managed.address, { from: admin }); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { from: admin }); - }); - - it('works on closed target', async function () { - await this.manager.setContractModeClosed(this.managed.address, { from: admin }); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, false, { from: admin }); - }); - }); - - describe('modes', function () { - const group = '1'; - const selector = web3.eth.abi.encodeFunctionSignature('restrictedFunction()'); - - beforeEach('deploying managed contract', async function () { - this.managed = await AccessManaged.new(this.manager.address); - await this.manager.createGroup('1', 'a group', { from: admin }); - await this.manager.setFunctionAllowedGroup(this.managed.address, [selector], group, true, { from: admin }); - }); - - it('custom mode is default', async function () { - expect(await this.manager.getContractMode(this.managed.address)).to.bignumber.equal(AccessMode.Custom); - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([group]); - }); - - it('open mode', async function () { - const receipt = await this.manager.setContractModeOpen(this.managed.address, { from: admin }); - expectEvent(receipt, 'AccessModeUpdated', { - target: this.managed.address, - mode: AccessMode.Open, - }); - expect(await this.manager.getContractMode(this.managed.address)).to.bignumber.equal(AccessMode.Open); - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([PUBLIC_GROUP]); - }); - - it('closed mode', async function () { - const receipt = await this.manager.setContractModeClosed(this.managed.address, { from: admin }); - expectEvent(receipt, 'AccessModeUpdated', { - target: this.managed.address, - mode: AccessMode.Closed, - }); - expect(await this.manager.getContractMode(this.managed.address)).to.bignumber.equal(AccessMode.Closed); - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([]); - }); - - it('mode cycle', async function () { - await this.manager.setContractModeOpen(this.managed.address, { from: admin }); - await this.manager.setContractModeClosed(this.managed.address, { from: admin }); - await this.manager.setContractModeCustom(this.managed.address, { from: admin }); - expect(await this.manager.getContractMode(this.managed.address)).to.bignumber.equal(AccessMode.Custom); - const allowedGroups = await this.manager.getFunctionAllowedGroups(this.managed.address, selector); - expect(groupUtils.decodeBitmap(allowedGroups)).to.deep.equal([group]); - }); - - it('non-admin cannot change mode', async function () { - await expectRevert(this.manager.setContractModeCustom(this.managed.address), 'missing role'); - await expectRevert(this.manager.setContractModeOpen(this.managed.address), 'missing role'); - await expectRevert(this.manager.setContractModeClosed(this.managed.address), 'missing role'); - }); - }); - - describe('transfering authority', function () { - beforeEach('deploying managed contract', async function () { - this.managed = await AccessManaged.new(this.manager.address); - }); - - it('admin can transfer authority', async function () { - await this.manager.transferContractAuthority(this.managed.address, otherAuthority, { from: admin }); - expect(await this.managed.authority()).to.equal(otherAuthority); - }); - - it('non-admin cannot transfer authority', async function () { - await expectRevert( - this.manager.transferContractAuthority(this.managed.address, otherAuthority, { from: nonAdmin }), - 'missing role', - ); - }); - }); - - describe('adapter', function () { - const group = '0'; - - beforeEach('deploying adapter', async function () { - await this.manager.createGroup(group, 'a group', { from: admin }); - await this.manager.grantGroup(group, user1, { from: admin }); - this.adapter = await AccessManagerAdapter.new(this.manager.address); - }); - - it('with ownable', async function () { - const target = await Ownable.new(); - await target.transferOwnership(this.adapter.address); - - const { data } = await target.$_checkOwner.request(); - const selector = data.slice(0, 10); - - await expectRevert( - this.adapter.relay(target.address, data, { from: user1 }), - 'AccessManagerAdapter: caller not allowed', - ); - - await this.manager.setFunctionAllowedGroup(target.address, [selector], group, true, { from: admin }); - await this.adapter.relay(target.address, data, { from: user1 }); - }); - - it('with access control', async function () { - const ROLE = web3.utils.soliditySha3('ROLE'); - const target = await AccessControl.new(); - await target.$_grantRole(ROLE, this.adapter.address); - - const { data } = await target.$_checkRole.request(ROLE); - const selector = data.slice(0, 10); - - await expectRevert( - this.adapter.relay(target.address, data, { from: user1 }), - 'AccessManagerAdapter: caller not allowed', - ); - - await this.manager.setFunctionAllowedGroup(target.address, [selector], group, true, { from: admin }); - await this.adapter.relay(target.address, data, { from: user1 }); - }); - - it('transfer authority', async function () { - await this.manager.transferContractAuthority(this.adapter.address, otherAuthority, { from: admin }); - expect(await this.adapter.authority()).to.equal(otherAuthority); }); }); }); diff --git a/test/helpers/enums.js b/test/helpers/enums.js index 95857ec09..6280e0f31 100644 --- a/test/helpers/enums.js +++ b/test/helpers/enums.js @@ -7,6 +7,5 @@ module.exports = { ProposalState: Enum('Pending', 'Active', 'Canceled', 'Defeated', 'Succeeded', 'Queued', 'Expired', 'Executed'), VoteType: Enum('Against', 'For', 'Abstain'), Rounding: Enum('Floor', 'Ceil', 'Trunc', 'Expand'), - AccessMode: Enum('Custom', 'Closed', 'Open'), OperationState: Enum('Unset', 'Waiting', 'Ready', 'Done'), }; diff --git a/test/helpers/methods.js b/test/helpers/methods.js new file mode 100644 index 000000000..cb30d8727 --- /dev/null +++ b/test/helpers/methods.js @@ -0,0 +1,5 @@ +const { soliditySha3 } = require('web3-utils'); + +module.exports = { + selector: signature => soliditySha3(signature).substring(0, 10), +};