diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol index 2b5b5a07f..f2df043e7 100644 --- a/contracts/access/manager/AccessManager.sol +++ b/contracts/access/manager/AccessManager.sol @@ -14,23 +14,32 @@ import {Time} from "../../utils/types/Time.sol"; /** * @dev AccessManager is a central contract to store the permissions of a system. * - * The smart contracts under the control of an AccessManager instance will have a set of "restricted" functions, and the - * 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 "roles". + * A smart contract under the control of an AccessManager instance is known as a target, and will inherit from the + * {AccessManaged} contract, be connected to this contract as its manager and implement the {AccessManaged-restricted} + * modifier on a set of functions selected to be permissioned. Note that any function without this setup won't be + * effectively restricted. * - * An AccessManager instance will define a set of roles. Accounts can be added into any number of these roles. 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 {setFunctionAllowedRoles}. + * The restriction rules for such functions are defined in terms of "roles" identified by an `uint64` and scoped + * by target (`address`) and function selectors (`bytes4`). These roles are stored in this contract and can be + * configured by admins (`ADMIN_ROLE` members) after a delay (see {getTargetAdminDelay}). * - * Note that a function in a target contract may become permissioned in this way only when: 1) said contract is - * {AccessManaged} and is connected to this contract as its manager, and 2) said function is decorated with the - * `restricted` modifier. + * For each target contract, admins can configure the following without any delay: * - * There is a special role defined by default named "public" which all accounts automatically have. + * * The target's {AccessManaged-authority} via {updateAuthority}. + * * Close or open a target via {setTargetClosed} keeping the permissions intact. + * * The roles that are allowed (or disallowed) to call a given function (identified by its selector) through {setTargetFunctionRole}. * - * In addition to the access rules defined by each target's functions being assigned to roles, then entire target can - * be "closed". This "closed" mode is set/unset by the admin using {setTargetClosed} and can be used to lock a contract - * while permissions are being (re-)configured. + * By default every address is member of the `PUBLIC_ROLE` and every target function is restricted to the `ADMIN_ROLE` until configured otherwise. + * Additionally, each role has the following configuration options restricted to this manager's admins: + * + * * A role's admin role via {setRoleAdmin} who can grant or revoke roles. + * * A role's guardian role via {setRoleGuardian} who's allowed to cancel operations. + * * A delay in which a role takes effect after being granted through {setGrantDelay}. + * * A delay of any target's admin action via {setTargetAdminDelay}. + * * A role label for discoverability purposes with {labelRole}. + * + * Any account can be added and removed into any number of these roles by using the {grantRole} and {revokeRole} functions + * restricted to each role's admin (see {getRoleAdmin}). * * Since all the permissions of the managed system can be modified by the admins of this instance, it is expected that * they will be highly secured (e.g., a multisig or a well-configured DAO). @@ -61,28 +70,30 @@ contract AccessManager is Context, Multicall, IAccessManager { // Structure that stores the details for a role/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 role - // permission is not available. + // Timepoint at which the user gets the permission. + // If this is either 0 or in the future, then the role permission is not available. uint48 since; // Delay for execution. Only applies to restricted() / execute() calls. Time.Delay delay; } - // Structure that stores the details of a role, including: - // - the members of the role - // - the admin role (that can grant or revoke permissions) - // - the guardian role (that can cancel operations targeting functions that need this role) - // - the grand delay + // Structure that stores the details of a role. struct Role { + // Members of the role. mapping(address user => Access access) members; + // Admin who can grant or revoke permissions. uint64 admin; + // Guardian who can cancel operations targeting functions that need this role. uint64 guardian; + // Delay in which the role takes effect after being granted. Time.Delay grantDelay; } // Structure that stores the details for a scheduled operation. This structure fits into a single slot. struct Schedule { + // Moment at which the operation can be executed. uint48 timepoint; + // Operation nonce to allow third-party contracts to identify the operation. uint32 nonce; } @@ -93,6 +104,7 @@ contract AccessManager is Context, Multicall, IAccessManager { mapping(uint64 roleId => Role) private _roles; mapping(bytes32 operationId => Schedule) private _schedules; + // Used to identify operations that are currently being executed via {execute}. // This should be transient storage when supported by the EVM. bytes32 private _executionId; @@ -121,15 +133,19 @@ contract AccessManager is Context, Multicall, IAccessManager { * & {execute} workflow. * * 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. + * Therefore we only return true if the call can be performed without any delay. If the call is subject to a + * previously set delay (not zero), then the function should return false and the caller should schedule the operation + * for future execution. * - * 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? + * If `immediate` is true, the delay can be disregarded and the operation can be immediately executed, otherwise + * the operation can be executed if and only if delay is greater than 0. * * NOTE: The IAuthority interface does not include the `uint32` delay. This is an extension of that interface that * is backward compatible. Some contracts may thus ignore the second return argument. In that case they will fail * to identify the indirect workflow, and will consider calls that require a delay to be forbidden. + * + * NOTE: This function does not report the permissions of this manager itself. These are defined by the + * {_canCallSelf} function instead. */ function canCall( address caller, @@ -151,13 +167,16 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Expiration delay for scheduled proposals. Defaults to 1 week. + * + * IMPORTANT: Avoid overriding the expiration with 0. Otherwise every contract proposal will be expired immediately, + * disabling any scheduling usage. */ function expiration() public view virtual returns (uint32) { return 1 weeks; } /** - * @dev Minimum setback for all delay updates, with the exception of execution delays, which + * @dev Minimum setback for all delay updates, with the exception of execution delays. It * can be increased without setback (and in the event of an accidental increase can be reset * via {revokeRole}). Defaults to 5 days. */ @@ -166,7 +185,7 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Get the mode under which a contract is operating. + * @dev Get whether the contract is closed disabling any access. Otherwise role permissions are applied. */ function isTargetClosed(address target) public view virtual returns (bool) { return _targets[target].closed; @@ -187,7 +206,7 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Get the id of the role that acts as an admin for given role. + * @dev Get the id of the role that acts as an admin for the given role. * * The admin permission is required to grant the role, revoke the role and update the execution delay to execute * an operation that is restricted to this role. @@ -206,9 +225,10 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Get the role 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 - * {RoleGrantDelayChanged} event. + * @dev Get the role current grant delay. + * + * Its value may change at any point without an event emitted following a call to {setGrantDelay}. + * Changes to this value, including effect timepoint are notified in advance by the {RoleGrantDelayChanged} event. */ function getRoleGrantDelay(uint64 roleId) public view virtual returns (uint32) { return _roles[roleId].grantDelay.get(); @@ -238,8 +258,8 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Check if a given account currently had the permission level corresponding to a given role. Note that this - * permission might be associated with a delay. {getAccess} can provide more details. + * @dev Check if a given account currently has the permission level corresponding to a given role. Note that this + * permission might be associated with an execution delay. {getAccess} can provide more details. */ function hasRole( uint64 roleId, @@ -257,6 +277,10 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Give a label to a role, for improved role discoverabily by UIs. * + * Requirements: + * + * - the caller must be a global admin + * * Emits a {RoleLabel} event. */ function labelRole(uint64 roleId, string calldata label) public virtual onlyAuthorized { @@ -271,19 +295,20 @@ contract AccessManager is Context, Multicall, IAccessManager { * * This gives the account the authorization to call any function that is restricted to this role. An optional * execution delay (in seconds) can be set. If that delay is non 0, the user is required to schedule any operation - * that is restricted to members this role. The user will only be able to execute the operation after the delay has + * that is restricted to members of this role. The user will only be able to execute the operation after the delay has * passed, before it has expired. During this period, admin and guardians can cancel the operation (see {cancel}). * * If the account has already been granted this role, the execution delay will be updated. This update is not - * immediate and follows the delay rules. For example, If a user currently has a delay of 3 hours, and this is + * immediate and follows the delay rules. For example, if a user currently has a delay of 3 hours, and this is * called to reduce that delay to 1 hour, the new delay will take some time to take effect, enforcing that any * operation executed in the 3 hours that follows this update was indeed scheduled before this update. * * Requirements: * * - the caller must be an admin for the role (see {getRoleAdmin}) + * - granted role must not be the `PUBLIC_ROLE` * - * Emits a {RoleGranted} event + * Emits a {RoleGranted} event. */ function grantRole(uint64 roleId, address account, uint32 executionDelay) public virtual onlyAuthorized { _grantRole(roleId, account, getRoleGrantDelay(roleId), executionDelay); @@ -296,6 +321,7 @@ contract AccessManager is Context, Multicall, IAccessManager { * Requirements: * * - the caller must be an admin for the role (see {getRoleAdmin}) + * - revoked role must not be the `PUBLIC_ROLE` * * Emits a {RoleRevoked} event if the account had the role. */ @@ -304,8 +330,8 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Renounce role permissions for the calling account, with immediate effect. If the sender is not in - * the role, this call has no effect. + * @dev Renounce role permissions for the calling account with immediate effect. If the sender is not in + * the role this call has no effect. * * Requirements: * @@ -417,7 +443,10 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Internal version of {setRoleAdmin} without access control. * - * Emits a {RoleAdminChanged} event + * Emits a {RoleAdminChanged} event. + * + * NOTE: Setting the admin role as the `PUBLIC_ROLE` is allowed, but it will effectively allow + * anyone to set grant or revoke such role. */ function _setRoleAdmin(uint64 roleId, uint64 admin) internal virtual { if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) { @@ -432,7 +461,10 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Internal version of {setRoleGuardian} without access control. * - * Emits a {RoleGuardianChanged} event + * Emits a {RoleGuardianChanged} event. + * + * NOTE: Setting the guardian role as the `PUBLIC_ROLE` is allowed, but it will effectively allow + * anyone to cancel any scheduled operation for such role. */ function _setRoleGuardian(uint64 roleId, uint64 guardian) internal virtual { if (roleId == ADMIN_ROLE || roleId == PUBLIC_ROLE) { @@ -447,7 +479,7 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Internal version of {setGrantDelay} without access control. * - * Emits a {RoleGrantDelayChanged} event + * Emits a {RoleGrantDelayChanged} event. */ function _setGrantDelay(uint64 roleId, uint32 newDelay) internal virtual { if (roleId == PUBLIC_ROLE) { @@ -481,9 +513,9 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Internal version of {setFunctionAllowedRole} without access control. + * @dev Internal version of {setTargetFunctionRole} without access control. * - * Emits a {TargetFunctionRoleUpdated} event + * Emits a {TargetFunctionRoleUpdated} event. */ function _setTargetFunctionRole(address target, bytes4 selector, uint64 roleId) internal virtual { _targets[target].allowedRoles[selector] = roleId; @@ -497,7 +529,7 @@ contract AccessManager is Context, Multicall, IAccessManager { * * - the caller must be a global admin * - * Emits a {TargetAdminDelayUpdated} event per selector + * Emits a {TargetAdminDelayUpdated} event. */ function setTargetAdminDelay(address target, uint32 newDelay) public virtual onlyAuthorized { _setTargetAdminDelay(target, newDelay); @@ -506,7 +538,7 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Internal version of {setTargetAdminDelay} without access control. * - * Emits a {TargetAdminDelayUpdated} event + * Emits a {TargetAdminDelayUpdated} event. */ function _setTargetAdminDelay(address target, uint32 newDelay) internal virtual { uint48 effect; @@ -674,7 +706,7 @@ contract AccessManager is Context, Multicall, IAccessManager { * 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 + * Emit a {OperationExecuted} event. */ function consumeScheduledOp(address caller, bytes calldata data) public virtual { address target = _msgSender(); @@ -789,7 +821,7 @@ contract AccessManager is Context, Multicall, IAccessManager { * Returns: * - bool restricted: does this data match a restricted operation * - uint64: which role is this operation restricted to - * - uint32: minimum delay to enforce for that operation (on top of the admin's execution delay) + * - uint32: minimum delay to enforce for that operation (max between operation's delay and admin's execution delay) */ function _getAdminRestrictions( bytes calldata data @@ -835,14 +867,12 @@ contract AccessManager is Context, Multicall, IAccessManager { // =================================================== HELPERS ==================================================== /** - * @dev An extended version of {canCall} for internal use that considers restrictions for admin functions. + * @dev An extended version of {canCall} for internal usage that checks {_canCallSelf} + * when the target is this contract. * * Returns: * - bool immediate: whether the operation can be executed immediately (with no delay) * - uint32 delay: the execution delay - * - * If immediate is true, the delay can be disregarded and the operation can be immediately executed. - * If immediate is false, the operation can be executed if and only if delay is greater than 0. */ function _canCallExtended( address caller, diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol index 01ecedd1c..ca7aaa623 100644 --- a/contracts/access/manager/IAccessManager.sol +++ b/contracts/access/manager/IAccessManager.sol @@ -30,6 +30,13 @@ interface IAccessManager { event OperationCanceled(bytes32 indexed operationId, uint32 indexed nonce); event RoleLabel(uint64 indexed roleId, string label); + /** + * @dev Emitted when `account` is granted `roleId`. + * + * NOTE: The meaning of the `since` argument depends on the `newMember` argument. + * If the role is granted to a new member, the `since` argument indicates when the account becomes a member of the role, + * otherwise it indicates the execution delay for this account and roleId is updated. + */ event RoleGranted(uint64 indexed roleId, address indexed account, uint32 delay, uint48 since, bool newMember); event RoleRevoked(uint64 indexed roleId, address indexed account); event RoleAdminChanged(uint64 indexed roleId, uint64 indexed admin); diff --git a/contracts/governance/utils/Votes.sol b/contracts/governance/utils/Votes.sol index 56c8772f9..da23aac7d 100644 --- a/contracts/governance/utils/Votes.sol +++ b/contracts/governance/utils/Votes.sol @@ -27,7 +27,7 @@ import {Time} from "../../utils/types/Time.sol"; * * When using this module the derived contract must implement {_getVotingUnits} (for example, make it return * {ERC721-balanceOf}), and can use {_transferVotingUnits} to track a change in the distribution of those units (in the - * previous example, it would be included in {ERC721-_beforeTokenTransfer}). + * previous example, it would be included in {ERC721-_update}). */ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { using Checkpoints for Checkpoints.Trace208; diff --git a/contracts/mocks/AccessManagedTarget.sol b/contracts/mocks/AccessManagedTarget.sol index 0f7c7a193..673feedaa 100644 --- a/contracts/mocks/AccessManagedTarget.sol +++ b/contracts/mocks/AccessManagedTarget.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {AccessManaged} from "../access/manager/AccessManaged.sol"; +import {StorageSlot} from "../utils/StorageSlot.sol"; abstract contract AccessManagedTarget is AccessManaged { event CalledRestricted(address caller); @@ -17,6 +18,16 @@ abstract contract AccessManagedTarget is AccessManaged { emit CalledUnrestricted(msg.sender); } + function setIsConsumingScheduledOp(bool isConsuming, bytes32 slot) external { + // Memory layout is 0x....<_consumingSchedule (boolean)> + bytes32 mask = bytes32(uint256(1 << 160)); + if (isConsuming) { + StorageSlot.getBytes32Slot(slot).value |= mask; + } else { + StorageSlot.getBytes32Slot(slot).value &= ~mask; + } + } + fallback() external { emit CalledFallback(msg.sender); } diff --git a/contracts/mocks/AuthorityMock.sol b/contracts/mocks/AuthorityMock.sol new file mode 100644 index 000000000..bf2434b0a --- /dev/null +++ b/contracts/mocks/AuthorityMock.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IAccessManaged} from "../access/manager/IAccessManaged.sol"; +import {IAuthority} from "../access/manager/IAuthority.sol"; + +contract NotAuthorityMock is IAuthority { + function canCall(address /* caller */, address /* target */, bytes4 /* selector */) external pure returns (bool) { + revert("AuthorityNoDelayMock: not implemented"); + } +} + +contract AuthorityNoDelayMock is IAuthority { + bool _immediate; + + function canCall( + address /* caller */, + address /* target */, + bytes4 /* selector */ + ) external view returns (bool immediate) { + return _immediate; + } + + function _setImmediate(bool immediate) external { + _immediate = immediate; + } +} + +contract AuthorityDelayMock { + bool _immediate; + uint32 _delay; + + function canCall( + address /* caller */, + address /* target */, + bytes4 /* selector */ + ) external view returns (bool immediate, uint32 delay) { + return (_immediate, _delay); + } + + function _setImmediate(bool immediate) external { + _immediate = immediate; + } + + function _setDelay(uint32 delay) external { + _delay = delay; + } +} + +contract AuthorityNoResponse { + function canCall(address /* caller */, address /* target */, bytes4 /* selector */) external view {} +} + +contract AuthoritiyObserveIsConsuming { + event ConsumeScheduledOpCalled(address caller, bytes data, bytes4 isConsuming); + + function canCall( + address /* caller */, + address /* target */, + bytes4 /* selector */ + ) external pure returns (bool immediate, uint32 delay) { + return (false, 1); + } + + function consumeScheduledOp(address caller, bytes memory data) public { + emit ConsumeScheduledOpCalled(caller, data, IAccessManaged(msg.sender).isConsumingScheduledOp()); + } +} diff --git a/contracts/mocks/docs/ERC20WithAutoMinerReward.sol b/contracts/mocks/docs/ERC20WithAutoMinerReward.sol new file mode 100644 index 000000000..46be53238 --- /dev/null +++ b/contracts/mocks/docs/ERC20WithAutoMinerReward.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {ERC20} from "../../token/ERC20/ERC20.sol"; + +contract ERC20WithAutoMinerReward is ERC20 { + constructor() ERC20("Reward", "RWD") { + _mintMinerReward(); + } + + function _mintMinerReward() internal { + _mint(block.coinbase, 1000); + } + + function _update(address from, address to, uint256 value) internal virtual override { + if (!(from == address(0) && to == block.coinbase)) { + _mintMinerReward(); + } + super._update(from, to, value); + } +} diff --git a/contracts/mocks/docs/MyContractOwnable.sol b/contracts/mocks/docs/MyContractOwnable.sol new file mode 100644 index 000000000..01847c036 --- /dev/null +++ b/contracts/mocks/docs/MyContractOwnable.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Ownable} from "../../access/Ownable.sol"; + +contract MyContract is Ownable { + constructor(address initialOwner) Ownable(initialOwner) {} + + function normalThing() public { + // anyone can call this normalThing() + } + + function specialThing() public onlyOwner { + // only the owner can call specialThing()! + } +} diff --git a/contracts/utils/types/Time.sol b/contracts/utils/types/Time.sol index df4d0af91..67be9a836 100644 --- a/contracts/utils/types/Time.sol +++ b/contracts/utils/types/Time.sol @@ -97,22 +97,26 @@ library Time { * enforce the old delay at the moment of the update. Returns the updated Delay object and the timestamp when the * new delay becomes effective. */ - function withUpdate(Delay self, uint32 newValue, uint32 minSetback) internal view returns (Delay, uint48) { + function withUpdate( + Delay self, + uint32 newValue, + uint32 minSetback + ) internal view returns (Delay updatedDelay, uint48 effect) { uint32 value = self.get(); uint32 setback = uint32(Math.max(minSetback, value > newValue ? value - newValue : 0)); - uint48 effect = timestamp() + setback; + effect = timestamp() + setback; return (pack(value, newValue, effect), effect); } /** * @dev Split a delay into its components: valueBefore, valueAfter and effect (transition timepoint). */ - function unpack(Delay self) internal pure returns (uint32, uint32, uint48) { + function unpack(Delay self) internal pure returns (uint32 valueBefore, uint32 valueAfter, uint48 effect) { uint112 raw = Delay.unwrap(self); - uint32 valueAfter = uint32(raw); - uint32 valueBefore = uint32(raw >> 32); - uint48 effect = uint48(raw >> 64); + valueAfter = uint32(raw); + valueBefore = uint32(raw >> 32); + effect = uint48(raw >> 64); return (valueBefore, valueAfter, effect); } diff --git a/docs/modules/ROOT/pages/access-control.adoc b/docs/modules/ROOT/pages/access-control.adoc index a60a34388..2a977a2b1 100644 --- a/docs/modules/ROOT/pages/access-control.adoc +++ b/docs/modules/ROOT/pages/access-control.adoc @@ -9,24 +9,9 @@ The most common and basic form of access control is the concept of _ownership_: OpenZeppelin Contracts provides xref:api:access.adoc#Ownable[`Ownable`] for implementing ownership in your contracts. -[source,solidity] ----- -// contracts/MyContract.sol -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; - -contract MyContract is Ownable { - function normalThing() public { - // anyone can call this normalThing() - } - - function specialThing() public onlyOwner { - // only the owner can call specialThing()! - } -} ----- +```solidity +include::api:example$MyContractOwnable.sol[] +``` By default, the xref:api:access.adoc#Ownable-owner--[`owner`] of an `Ownable` contract is the account that deployed it, which is usually exactly what you want. diff --git a/docs/modules/ROOT/pages/erc20-supply.adoc b/docs/modules/ROOT/pages/erc20-supply.adoc index 44cbd73dd..bf6e24058 100644 --- a/docs/modules/ROOT/pages/erc20-supply.adoc +++ b/docs/modules/ROOT/pages/erc20-supply.adoc @@ -57,27 +57,13 @@ As we can see, `_mint` makes it super easy to do this correctly. [[automating-the-reward]] == Automating the Reward -So far our supply mechanism was triggered manually, but `ERC20` also allows us to extend the core functionality of the token through the xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`_beforeTokenTransfer`] hook (see xref:extending-contracts.adoc#using-hooks[Using Hooks]). +So far our supply mechanism was triggered manually, but `ERC20` also allows us to extend the core functionality of the token through the xref:api:token/ERC20.adoc#ERC20-_update-address-address-uint256-[`_update`] function. -Adding to the supply mechanism from the previous section, we can use this hook to mint a miner reward for every token transfer that is included in the blockchain. +Adding to the supply mechanism from the previous section, we can use this function to mint a miner reward for every token transfer that is included in the blockchain. -[source,solidity] ----- -contract ERC20WithAutoMinerReward is ERC20 { - constructor() ERC20("Reward", "RWD") {} - - function _mintMinerReward() internal { - _mint(block.coinbase, 1000); - } - - function _beforeTokenTransfer(address from, address to, uint256 value) internal virtual override { - if (!(from == address(0) && to == block.coinbase)) { - _mintMinerReward(); - } - super._beforeTokenTransfer(from, to, value); - } -} ----- +```solidity +include::api:example$ERC20WithAutoMinerReward.sol[] +``` [[wrapping-up]] == Wrapping Up diff --git a/docs/modules/ROOT/pages/extending-contracts.adoc b/docs/modules/ROOT/pages/extending-contracts.adoc index 1c13d6b4b..330d3a5bc 100644 --- a/docs/modules/ROOT/pages/extending-contracts.adoc +++ b/docs/modules/ROOT/pages/extending-contracts.adoc @@ -68,60 +68,6 @@ The `super.revokeRole` statement at the end will invoke ``AccessControl``'s orig NOTE: The same rule is implemented and extended in xref:api:access.adoc#AccessControlDefaultAdminRules[`AccessControlDefaultAdminRules`], an extension that also adds enforced security measures for the `DEFAULT_ADMIN_ROLE`. -[[using-hooks]] -== Using Hooks - -Sometimes, in order to extend a parent contract you will need to override multiple related functions, which leads to code duplication and increased likelihood of bugs. - -For example, consider implementing safe xref:api:token/ERC20.adoc#ERC20[`ERC20`] transfers in the style of xref:api:token/ERC721.adoc#IERC721Receiver[`IERC721Receiver`]. You may think overriding xref:api:token/ERC20.adoc#ERC20-transfer-address-uint256-[`transfer`] and xref:api:token/ERC20.adoc#ERC20-transferFrom-address-address-uint256-[`transferFrom`] would be enough, but what about xref:api:token/ERC20.adoc#ERC20-_transfer-address-address-uint256-[`_transfer`] and xref:api:token/ERC20.adoc#ERC20-_mint-address-uint256-[`_mint`]? To prevent you from having to deal with these details, we introduced **hooks**. - -Hooks are simply functions that are called before or after some action takes place. They provide a centralized point to _hook into_ and extend the original behavior. - -Here's how you would implement the `IERC721Receiver` pattern in `ERC20`, using the xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`_beforeTokenTransfer`] hook: - -```solidity -pragma solidity ^0.8.20; - -import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; - -contract ERC20WithSafeTransfer is ERC20 { - function _beforeTokenTransfer(address from, address to, uint256 amount) - internal virtual override - { - super._beforeTokenTransfer(from, to, amount); - - require(_validRecipient(to), "ERC20WithSafeTransfer: invalid recipient"); - } - - function _validRecipient(address to) private view returns (bool) { - ... - } - - ... -} -``` - -Using hooks this way leads to cleaner and safer code, without having to rely on a deep understanding of the parent's internals. - -=== Rules of Hooks - -There's a few guidelines you should follow when writing code that uses hooks in order to prevent issues. They are very simple, but do make sure you follow them: - -1. Whenever you override a parent's hook, re-apply the `virtual` attribute to the hook. That will allow child contracts to add more functionality to the hook. -2. **Always** call the parent's hook in your override using `super`. This will make sure all hooks in the inheritance tree are called: contracts like xref:api:token/ERC20.adoc#ERC20Pausable[`ERC20Pausable`] rely on this behavior. - -```solidity -contract MyToken is ERC20 { - function _beforeTokenTransfer(address from, address to, uint256 amount) - internal virtual override // Add virtual here! - { - super._beforeTokenTransfer(from, to, amount); // Call parent hook - ... - } -} -``` -That's it! Enjoy simpler code using hooks! - == Security The maintainers of OpenZeppelin Contracts are mainly concerned with the correctness and security of the code as published in the library, and the combinations of base contracts with the official extensions from the library. diff --git a/docs/modules/api/examples/ERC20WithAutoMinerReward.sol b/docs/modules/api/examples/ERC20WithAutoMinerReward.sol new file mode 100644 index 000000000..800a90a57 --- /dev/null +++ b/docs/modules/api/examples/ERC20WithAutoMinerReward.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +contract ERC20WithAutoMinerReward is ERC20 { + constructor() ERC20("Reward", "RWD") { + _mintMinerReward(); + } + + function _mintMinerReward() internal { + _mint(block.coinbase, 1000); + } + + function _update(address from, address to, uint256 value) internal virtual override { + if (!(from == address(0) && to == block.coinbase)) { + _mintMinerReward(); + } + super._update(from, to, value); + } +} diff --git a/docs/modules/api/examples/MyContractOwnable.sol b/docs/modules/api/examples/MyContractOwnable.sol new file mode 100644 index 000000000..c06dc9f98 --- /dev/null +++ b/docs/modules/api/examples/MyContractOwnable.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + +contract MyContract is Ownable { + constructor(address initialOwner) Ownable(initialOwner) {} + + function normalThing() public { + // anyone can call this normalThing() + } + + function specialThing() public onlyOwner { + // only the owner can call specialThing()! + } +} diff --git a/docs/modules/api/pages/access.adoc b/docs/modules/api/pages/access.adoc index e533d9698..d0551bec4 100644 --- a/docs/modules/api/pages/access.adoc +++ b/docs/modules/api/pages/access.adoc @@ -170,6 +170,8 @@ :AccessControl-_setRoleAdmin: pass:normal[xref:access.adoc#AccessControl-_setRoleAdmin-bytes32-bytes32-[`AccessControl._setRoleAdmin`]] :xref-IAuthority-canCall-address-address-bytes4-: xref:access.adoc#IAuthority-canCall-address-address-bytes4- :AccessManaged: pass:normal[xref:access.adoc#AccessManaged[`AccessManaged`]] +:AccessManaged-restricted: pass:normal[xref:access.adoc#AccessManaged-restricted--[`AccessManaged.restricted`]] +:AccessManaged-authority: pass:normal[xref:access.adoc#AccessManaged-authority--[`AccessManaged.authority`]] :IAuthority: pass:normal[xref:access.adoc#IAuthority[`IAuthority`]] :Ownable: pass:normal[xref:access.adoc#Ownable[`Ownable`]] :AccessManager: pass:normal[xref:access.adoc#AccessManager[`AccessManager`]] @@ -1922,23 +1924,32 @@ import "@openzeppelin/contracts/access/manager/AccessManager.sol"; AccessManager is a central contract to store the permissions of a system. -The smart contracts under the control of an AccessManager instance will have a set of "restricted" functions, and the -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 "roles". +A smart contract under the control of an AccessManager instance is known as a target, and will inherit from the +{AccessManaged} contract, be connected to this contract as its manager and implement the {AccessManaged-restricted} +modifier on a set of functions selected to be permissioned. Note that any function without this setup won't be +effectively restricted. -An AccessManager instance will define a set of roles. Accounts can be added into any number of these roles. 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 {setFunctionAllowedRoles}. +The restriction rules for such functions are defined in terms of "roles" identified by an `uint64` and scoped +by target (`address`) and function selectors (`bytes4`). These roles are stored in this contract and can be +configured by admins (`ADMIN_ROLE` members) after a delay (see {getTargetAdminDelay}). -Note that a function in a target contract may become permissioned in this way only when: 1) said contract is -{AccessManaged} and is connected to this contract as its manager, and 2) said function is decorated with the -`restricted` modifier. +For each target contract, admins can configure the following without any delay: -There is a special role defined by default named "public" which all accounts automatically have. +* The target's {AccessManaged-authority} via {updateAuthority}. +* Close or open a target via {setTargetClosed} keeping the permissions intact. +* The roles that are allowed (or disallowed) to call a given function (identified by its selector) through {setTargetFunctionRole}. -In addition to the access rules defined by each target's functions being assigned to roles, then entire target can -be "closed". This "closed" mode is set/unset by the admin using {setTargetClosed} and can be used to lock a contract -while permissions are being (re-)configured. +By default every address is member of the `PUBLIC_ROLE` and every target function is restricted to the `ADMIN_ROLE` until configured otherwise. +Additionally, each role has the following configuration options restricted to this manager's admins: + +* A role's admin role via {setRoleAdmin} who can grant or revoke roles. +* A role's guardian role via {setRoleGuardian} who's allowed to cancel operations. +* A delay in which a role takes effect after being granted through {setGrantDelay}. +* A delay of any target's admin action via {setTargetAdminDelay}. +* A role label for discoverability purposes with {labelRole}. + +Any account can be added and removed into any number of these roles by using the {grantRole} and {revokeRole} functions +restricted to each role's admin (see {getRoleAdmin}). Since all the permissions of the managed system can be modified by the admins of this instance, it is expected that they will be highly secured (e.g., a multisig or a well-configured DAO). @@ -2085,27 +2096,34 @@ no restriction). Additionally, it returns the delay needed to perform the call i & {execute} workflow. 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. +Therefore we only return true if the call can be performed without any delay. If the call is subject to a +previously set delay (not zero), then the function should return false and the caller should schedule the operation +for future execution. -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? +If `immediate` is true, the delay can be disregarded and the operation can be immediately executed, otherwise +the operation can be executed if and only if delay is greater than 0. NOTE: The IAuthority interface does not include the `uint32` delay. This is an extension of that interface that is backward compatible. Some contracts may thus ignore the second return argument. In that case they will fail to identify the indirect workflow, and will consider calls that require a delay to be forbidden. +NOTE: This function does not report the permissions of this manager itself. These are defined by the +{_canCallSelf} function instead. + [.contract-item] [[AccessManager-expiration--]] ==== `[.contract-item-name]#++expiration++#++() → uint32++` [.item-kind]#public# Expiration delay for scheduled proposals. Defaults to 1 week. +IMPORTANT: Avoid overriding the expiration with 0. Otherwise every contract proposal will be expired immediately, +disabling any scheduling usage. + [.contract-item] [[AccessManager-minSetback--]] ==== `[.contract-item-name]#++minSetback++#++() → uint32++` [.item-kind]#public# -Minimum setback for all delay updates, with the exception of execution delays, which +Minimum setback for all delay updates, with the exception of execution delays. It can be increased without setback (and in the event of an accidental increase can be reset via {revokeRole}). Defaults to 5 days. @@ -2113,7 +2131,7 @@ via {revokeRole}). Defaults to 5 days. [[AccessManager-isTargetClosed-address-]] ==== `[.contract-item-name]#++isTargetClosed++#++(address target) → bool++` [.item-kind]#public# -Get the mode under which a contract is operating. +Get whether the contract is closed disabling any access. Otherwise role permissions are applied. [.contract-item] [[AccessManager-getTargetFunctionRole-address-bytes4-]] @@ -2131,7 +2149,7 @@ Get the admin delay for a target contract. Changes to contract configuration are [[AccessManager-getRoleAdmin-uint64-]] ==== `[.contract-item-name]#++getRoleAdmin++#++(uint64 roleId) → uint64++` [.item-kind]#public# -Get the id of the role that acts as an admin for given role. +Get the id of the role that acts as an admin for the given role. The admin permission is required to grant the role, revoke the role and update the execution delay to execute an operation that is restricted to this role. @@ -2148,9 +2166,10 @@ The guardian permission allows canceling operations that have been scheduled und [[AccessManager-getRoleGrantDelay-uint64-]] ==== `[.contract-item-name]#++getRoleGrantDelay++#++(uint64 roleId) → uint32++` [.item-kind]#public# -Get the role 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 -{RoleGrantDelayChanged} event. +Get the role current grant delay. + +Its value may change at any point without an event emitted following a call to {setGrantDelay}. +Changes to this value, including effect timepoint are notified in advance by the {RoleGrantDelayChanged} event. [.contract-item] [[AccessManager-getAccess-uint64-address-]] @@ -2170,8 +2189,8 @@ Returns: [[AccessManager-hasRole-uint64-address-]] ==== `[.contract-item-name]#++hasRole++#++(uint64 roleId, address account) → bool isMember, uint32 executionDelay++` [.item-kind]#public# -Check if a given account currently had the permission level corresponding to a given role. Note that this -permission might be associated with a delay. {getAccess} can provide more details. +Check if a given account currently has the permission level corresponding to a given role. Note that this +permission might be associated with an execution delay. {getAccess} can provide more details. [.contract-item] [[AccessManager-labelRole-uint64-string-]] @@ -2179,6 +2198,10 @@ permission might be associated with a delay. {getAccess} can provide more detail Give a label to a role, for improved role discoverabily by UIs. +Requirements: + +- the caller must be a global admin + Emits a {RoleLabel} event. [.contract-item] @@ -2189,19 +2212,20 @@ Add `account` to `roleId`, or change its execution delay. This gives the account the authorization to call any function that is restricted to this role. An optional execution delay (in seconds) can be set. If that delay is non 0, the user is required to schedule any operation -that is restricted to members this role. The user will only be able to execute the operation after the delay has +that is restricted to members of this role. The user will only be able to execute the operation after the delay has passed, before it has expired. During this period, admin and guardians can cancel the operation (see {cancel}). If the account has already been granted this role, the execution delay will be updated. This update is not -immediate and follows the delay rules. For example, If a user currently has a delay of 3 hours, and this is +immediate and follows the delay rules. For example, if a user currently has a delay of 3 hours, and this is called to reduce that delay to 1 hour, the new delay will take some time to take effect, enforcing that any operation executed in the 3 hours that follows this update was indeed scheduled before this update. Requirements: - the caller must be an admin for the role (see {getRoleAdmin}) +- granted role must not be the `PUBLIC_ROLE` -Emits a {RoleGranted} event +Emits a {RoleGranted} event. [.contract-item] [[AccessManager-revokeRole-uint64-address-]] @@ -2213,6 +2237,7 @@ no effect. Requirements: - the caller must be an admin for the role (see {getRoleAdmin}) +- revoked role must not be the `PUBLIC_ROLE` Emits a {RoleRevoked} event if the account had the role. @@ -2220,8 +2245,8 @@ Emits a {RoleRevoked} event if the account had the role. [[AccessManager-renounceRole-uint64-address-]] ==== `[.contract-item-name]#++renounceRole++#++(uint64 roleId, address callerConfirmation)++` [.item-kind]#public# -Renounce role permissions for the calling account, with immediate effect. If the sender is not in -the role, this call has no effect. +Renounce role permissions for the calling account with immediate effect. If the sender is not in +the role this call has no effect. Requirements: @@ -2288,7 +2313,10 @@ Emits a {RoleRevoked} event if the account had the role. Internal version of {setRoleAdmin} without access control. -Emits a {RoleAdminChanged} event +Emits a {RoleAdminChanged} event. + +NOTE: Setting the admin role as the `PUBLIC_ROLE` is allowed, but it will effectively allow +anyone to set grant or revoke such role. [.contract-item] [[AccessManager-_setRoleGuardian-uint64-uint64-]] @@ -2296,7 +2324,10 @@ Emits a {RoleAdminChanged} event Internal version of {setRoleGuardian} without access control. -Emits a {RoleGuardianChanged} event +Emits a {RoleGuardianChanged} event. + +NOTE: Setting the guardian role as the `PUBLIC_ROLE` is allowed, but it will effectively allow +anyone to cancel any scheduled operation for such role. [.contract-item] [[AccessManager-_setGrantDelay-uint64-uint32-]] @@ -2304,7 +2335,7 @@ Emits a {RoleGuardianChanged} event Internal version of {setGrantDelay} without access control. -Emits a {RoleGrantDelayChanged} event +Emits a {RoleGrantDelayChanged} event. [.contract-item] [[AccessManager-setTargetFunctionRole-address-bytes4---uint64-]] @@ -2322,9 +2353,9 @@ Emits a {TargetFunctionRoleUpdated} event per selector. [[AccessManager-_setTargetFunctionRole-address-bytes4-uint64-]] ==== `[.contract-item-name]#++_setTargetFunctionRole++#++(address target, bytes4 selector, uint64 roleId)++` [.item-kind]#internal# -Internal version of {setFunctionAllowedRole} without access control. +Internal version of {setTargetFunctionRole} without access control. -Emits a {TargetFunctionRoleUpdated} event +Emits a {TargetFunctionRoleUpdated} event. [.contract-item] [[AccessManager-setTargetAdminDelay-address-uint32-]] @@ -2336,7 +2367,7 @@ Requirements: - the caller must be a global admin -Emits a {TargetAdminDelayUpdated} event per selector +Emits a {TargetAdminDelayUpdated} event. [.contract-item] [[AccessManager-_setTargetAdminDelay-address-uint32-]] @@ -2344,7 +2375,7 @@ Emits a {TargetAdminDelayUpdated} event per selector Internal version of {setTargetAdminDelay} without access control. -Emits a {TargetAdminDelayUpdated} event +Emits a {TargetAdminDelayUpdated} event. [.contract-item] [[AccessManager-setTargetClosed-address-bool-]] @@ -2420,7 +2451,7 @@ Consume a scheduled operation targeting the caller. If such an operation exists, 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 +Emit a {OperationExecuted} event. [.contract-item] [[AccessManager-_consumeScheduledOp-bytes32-]] diff --git a/docs/modules/api/pages/governance.adoc b/docs/modules/api/pages/governance.adoc index 55252f410..3a9de4652 100644 --- a/docs/modules/api/pages/governance.adoc +++ b/docs/modules/api/pages/governance.adoc @@ -937,6 +937,7 @@ :IGovernor-cancel: pass:normal[xref:governance.adoc#IGovernor-cancel-address---uint256---bytes---bytes32-[`IGovernor.cancel`]] :ERC721Votes: pass:normal[xref:token/ERC721.adoc#ERC721Votes[`ERC721Votes`]] :ERC721-balanceOf: pass:normal[xref:token/ERC721.adoc#ERC721-balanceOf-address-[`ERC721.balanceOf`]] +:ERC721-_update: pass:normal[xref:token/ERC721.adoc#ERC721-_update-address-uint256-address-[`ERC721._update`]] :xref-Votes-clock--: xref:governance.adoc#Votes-clock-- :xref-Votes-CLOCK_MODE--: xref:governance.adoc#Votes-CLOCK_MODE-- :xref-Votes-getVotes-address-: xref:governance.adoc#Votes-getVotes-address- @@ -4517,7 +4518,7 @@ cost of this history tracking optional. When using this module the derived contract must implement {_getVotingUnits} (for example, make it return {ERC721-balanceOf}), and can use {_transferVotingUnits} to track a change in the distribution of those units (in the -previous example, it would be included in {ERC721-_beforeTokenTransfer}). +previous example, it would be included in {ERC721-_update}). [.contract-index] .Functions diff --git a/hardhat/env-contract.js b/hardhat/env-contract.js index 74d54cfbb..c615249a3 100644 --- a/hardhat/env-contract.js +++ b/hardhat/env-contract.js @@ -2,9 +2,24 @@ extendEnvironment(env => { const { contract } = env; env.contract = function (name, body) { - // remove the default account from the accounts list used in tests, in order - // to protect tests against accidentally passing due to the contract - // deployer being used subsequently as function caller - contract(name, accounts => body(accounts.slice(1))); + const { takeSnapshot } = require('@nomicfoundation/hardhat-network-helpers'); + + contract(name, accounts => { + // reset the state of the chain in between contract test suites + let snapshot; + + before(async function () { + snapshot = await takeSnapshot(); + }); + + after(async function () { + await snapshot.restore(); + }); + + // remove the default account from the accounts list used in tests, in order + // to protect tests against accidentally passing due to the contract + // deployer being used subsequently as function caller + body(accounts.slice(1)); + }); }; }); diff --git a/scripts/upgradeable/transpile.sh b/scripts/upgradeable/transpile.sh index f2126936c..ebc8c3219 100644 --- a/scripts/upgradeable/transpile.sh +++ b/scripts/upgradeable/transpile.sh @@ -6,7 +6,7 @@ VERSION="$(jq -r .version contracts/package.json)" DIRNAME="$(dirname -- "${BASH_SOURCE[0]}")" bash "$DIRNAME/patch-apply.sh" -sed -i "s//$VERSION/g" contracts/package.json +sed -i'' -e "s//$VERSION/g" "contracts/package.json" git add contracts/package.json npm run clean diff --git a/test/access/manager/AccessManaged.test.js b/test/access/manager/AccessManaged.test.js new file mode 100644 index 000000000..9e94af615 --- /dev/null +++ b/test/access/manager/AccessManaged.test.js @@ -0,0 +1,142 @@ +const { expectEvent, time, expectRevert } = require('@openzeppelin/test-helpers'); +const { selector } = require('../../helpers/methods'); +const { expectRevertCustomError } = require('../../helpers/customError'); +const { + time: { setNextBlockTimestamp }, +} = require('@nomicfoundation/hardhat-network-helpers'); +const { impersonate } = require('../../helpers/account'); + +const AccessManaged = artifacts.require('$AccessManagedTarget'); +const AccessManager = artifacts.require('$AccessManager'); + +const AuthoritiyObserveIsConsuming = artifacts.require('$AuthoritiyObserveIsConsuming'); + +contract('AccessManaged', function (accounts) { + const [admin, roleMember, other] = accounts; + + beforeEach(async function () { + this.authority = await AccessManager.new(admin); + this.managed = await AccessManaged.new(this.authority.address); + }); + + it('sets authority and emits AuthorityUpdated event during construction', async function () { + await expectEvent.inConstruction(this.managed, 'AuthorityUpdated', { + authority: this.authority.address, + }); + expect(await this.managed.authority()).to.eq(this.authority.address); + }); + + describe('restricted modifier', function () { + const method = 'fnRestricted()'; + + beforeEach(async function () { + this.selector = selector(method); + this.role = web3.utils.toBN(42); + await this.authority.$_setTargetFunctionRole(this.managed.address, this.selector, this.role); + await this.authority.$_grantRole(this.role, roleMember, 0, 0); + }); + + it('succeeds when role is granted without execution delay', async function () { + await this.managed.methods[method]({ from: roleMember }); + }); + + it('reverts when role is not granted', async function () { + await expectRevertCustomError(this.managed.methods[method]({ from: other }), 'AccessManagedUnauthorized', [ + other, + ]); + }); + + it('panics in short calldata', async function () { + // We avoid adding the `restricted` modifier to the fallback function because other tests may depend on it + // being accessible without restrictions. We check for the internal `_checkCanCall` instead. + await expectRevert.unspecified(this.managed.$_checkCanCall(other, '0x1234')); + }); + + describe('when role is granted with execution delay', function () { + beforeEach(async function () { + const executionDelay = web3.utils.toBN(911); + await this.authority.$_grantRole(this.role, roleMember, 0, executionDelay); + }); + + it('reverts if the operation is not scheduled', async function () { + const calldata = await this.managed.contract.methods[method]().encodeABI(); + const opId = await this.authority.hashOperation(roleMember, this.managed.address, calldata); + + await expectRevertCustomError(this.managed.methods[method]({ from: roleMember }), 'AccessManagerNotScheduled', [ + opId, + ]); + }); + + it('succeeds if the operation is scheduled', async function () { + // Arguments + const delay = time.duration.hours(12); + const calldata = await this.managed.contract.methods[method]().encodeABI(); + + // Schedule + const timestamp = await time.latest(); + const scheduledAt = timestamp.addn(1); + const when = scheduledAt.add(delay); + await setNextBlockTimestamp(scheduledAt); + await this.authority.schedule(this.managed.address, calldata, when, { + from: roleMember, + }); + + // Set execution date + await setNextBlockTimestamp(when); + + // Shouldn't revert + await this.managed.methods[method]({ from: roleMember }); + }); + }); + }); + + describe('setAuthority', function () { + beforeEach(async function () { + this.newAuthority = await AccessManager.new(admin); + }); + + it('reverts if the caller is not the authority', async function () { + await expectRevertCustomError(this.managed.setAuthority(other, { from: other }), 'AccessManagedUnauthorized', [ + other, + ]); + }); + + it('reverts if the new authority is not a valid authority', async function () { + await impersonate(this.authority.address); + await expectRevertCustomError( + this.managed.setAuthority(other, { from: this.authority.address }), + 'AccessManagedInvalidAuthority', + [other], + ); + }); + + it('sets authority and emits AuthorityUpdated event', async function () { + await impersonate(this.authority.address); + const { receipt } = await this.managed.setAuthority(this.newAuthority.address, { from: this.authority.address }); + await expectEvent(receipt, 'AuthorityUpdated', { + authority: this.newAuthority.address, + }); + expect(await this.managed.authority()).to.eq(this.newAuthority.address); + }); + }); + + describe('isConsumingScheduledOp', function () { + beforeEach(async function () { + this.authority = await AuthoritiyObserveIsConsuming.new(); + this.managed = await AccessManaged.new(this.authority.address); + }); + + it('returns bytes4(0) when not consuming operation', async function () { + expect(await this.managed.isConsumingScheduledOp()).to.eq('0x00000000'); + }); + + it('returns isConsumingScheduledOp selector when consuming operation', async function () { + const receipt = await this.managed.fnRestricted({ from: other }); + await expectEvent.inTransaction(receipt.tx, this.authority, 'ConsumeScheduledOpCalled', { + caller: other, + data: this.managed.contract.methods.fnRestricted().encodeABI(), + isConsuming: selector('isConsumingScheduledOp()'), + }); + }); + }); +}); diff --git a/test/access/manager/AccessManager.behavior.js b/test/access/manager/AccessManager.behavior.js new file mode 100644 index 000000000..d528ffb48 --- /dev/null +++ b/test/access/manager/AccessManager.behavior.js @@ -0,0 +1,711 @@ +const { time } = require('@openzeppelin/test-helpers'); +const { + time: { setNextBlockTimestamp }, + setStorageAt, + mine, +} = require('@nomicfoundation/hardhat-network-helpers'); +const { impersonate } = require('../../helpers/account'); +const { expectRevertCustomError } = require('../../helpers/customError'); +const { EXPIRATION, EXECUTION_ID_STORAGE_SLOT } = require('../../helpers/access-manager'); + +// ============ COMMON PATHS ============ + +const COMMON_IS_EXECUTING_PATH = { + executing() { + it('succeeds', async function () { + await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }); + }); + }, + notExecuting() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, this.role.id], + ); + }); + }, +}; + +const COMMON_GET_ACCESS_PATH = { + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, this.role.id], + ); + }); + }, + afterGrantDelay: undefined, // Diverges if there's an operation delay or not + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, this.role.id], + ); + }); + }, + afterGrantDelay() { + it('succeeds called directly', async function () { + await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }); + }); + + it('succeeds via execute', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay: undefined, // Diverges if there's an operation to schedule or not + callerHasNoExecutionDelay() { + it('succeeds called directly', async function () { + await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }); + }); + + it('succeeds via execute', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, + }, + }, + requiredRoleIsNotGranted() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, this.role.id], + ); + }); + }, +}; + +const COMMON_SCHEDULABLE_PATH = { + scheduled: { + before() { + it('reverts as AccessManagerNotReady', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerNotReady', + [this.operationId], + ); + }); + }, + after() { + it('succeeds called directly', async function () { + await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }); + }); + + it('succeeds via execute', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, + expired() { + it('reverts as AccessManagerExpired', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerExpired', + [this.operationId], + ); + }); + }, + }, + notScheduled() { + it('reverts as AccessManagerNotScheduled', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerNotScheduled', + [this.operationId], + ); + }); + }, +}; + +const COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY = { + scheduled: { + before() { + it.skip('is not reachable without a delay'); + }, + after() { + it.skip('is not reachable without a delay'); + }, + expired() { + it.skip('is not reachable without a delay'); + }, + }, + notScheduled() { + it('succeeds', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, +}; + +// ============ MODE HELPERS ============ + +/** + * @requires this.{manager,target} + */ +function shouldBehaveLikeClosable({ closed, open }) { + describe('when the manager is closed', function () { + beforeEach('close', async function () { + await this.manager.$_setTargetClosed(this.target.address, true); + }); + + closed(); + }); + + describe('when the manager is open', function () { + beforeEach('open', async function () { + await this.manager.$_setTargetClosed(this.target.address, false); + }); + + open(); + }); +} + +// ============ DELAY HELPERS ============ + +/** + * @requires this.{delay} + */ +function shouldBehaveLikeDelay(type, { before, after }) { + beforeEach('define timestamp when delay takes effect', async function () { + const timestamp = await time.latest(); + this.delayEffect = timestamp.add(this.delay); + }); + + describe(`when ${type} delay has not taken effect yet`, function () { + beforeEach(`set next block timestamp before ${type} takes effect`, async function () { + await setNextBlockTimestamp(this.delayEffect.subn(1)); + }); + + before(); + }); + + describe(`when ${type} delay has taken effect`, function () { + beforeEach(`set next block timestamp when ${type} takes effect`, async function () { + await setNextBlockTimestamp(this.delayEffect); + }); + + after(); + }); +} + +// ============ OPERATION HELPERS ============ + +/** + * @requires this.{manager,scheduleIn,caller,target,calldata} + */ +function shouldBehaveLikeSchedulableOperation({ scheduled: { before, after, expired }, notScheduled }) { + describe('when operation is scheduled', function () { + beforeEach('schedule operation', async function () { + await impersonate(this.caller); // May be a contract + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.scheduleIn, + }); + this.operationId = operationId; + }); + + describe('when operation is not ready for execution', function () { + beforeEach('set next block time before operation is ready', async function () { + this.scheduledAt = await time.latest(); + const schedule = await this.manager.getSchedule(this.operationId); + await setNextBlockTimestamp(schedule.subn(1)); + }); + + before(); + }); + + describe('when operation is ready for execution', function () { + beforeEach('set next block time when operation is ready for execution', async function () { + this.scheduledAt = await time.latest(); + const schedule = await this.manager.getSchedule(this.operationId); + await setNextBlockTimestamp(schedule); + }); + + after(); + }); + + describe('when operation has expired', function () { + beforeEach('set next block time when operation expired', async function () { + this.scheduledAt = await time.latest(); + const schedule = await this.manager.getSchedule(this.operationId); + await setNextBlockTimestamp(schedule.add(EXPIRATION)); + }); + + expired(); + }); + }); + + describe('when operation is not scheduled', function () { + beforeEach('set expected operationId', async function () { + this.operationId = await this.manager.hashOperation(this.caller, this.target.address, this.calldata); + + // Assert operation is not scheduled + expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal(web3.utils.toBN(0)); + }); + + notScheduled(); + }); +} + +/** + * @requires this.{manager,roles,target,calldata} + */ +function shouldBehaveLikeARestrictedOperation({ callerIsNotTheManager, callerIsTheManager }) { + describe('when the call comes from the manager (msg.sender == manager)', function () { + beforeEach('define caller as manager', async function () { + this.caller = this.manager.address; + await impersonate(this.caller); + }); + + shouldBehaveLikeCanCallExecuting(callerIsTheManager); + }); + + describe('when the call does not come from the manager (msg.sender != manager)', function () { + beforeEach('define non manager caller', function () { + this.caller = this.roles.SOME.members[0]; + }); + + callerIsNotTheManager(); + }); +} + +/** + * @requires this.{manager,roles,executionDelay,operationDelay,target} + */ +function shouldBehaveLikeDelayedOperation() { + describe('with operation delay', function () { + describe('when operation delay is greater than execution delay', function () { + beforeEach('set operation delay', async function () { + this.operationDelay = this.executionDelay.add(time.duration.hours(1)); + await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay); + this.scheduleIn = this.operationDelay; // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }); + + describe('when operation delay is shorter than execution delay', function () { + beforeEach('set operation delay', async function () { + this.operationDelay = this.executionDelay.sub(time.duration.hours(1)); + await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }); + }); + + describe('without operation delay', function () { + beforeEach('set operation delay', async function () { + this.operationDelay = web3.utils.toBN(0); + await this.manager.$_setTargetAdminDelay(this.target.address, this.operationDelay); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }); +} + +// ============ METHOD HELPERS ============ + +/** + * @requires this.{manager,roles,role,target,calldata} + */ +function shouldBehaveLikeCanCall({ + closed, + open: { + callerIsTheManager, + callerIsNotTheManager: { publicRoleIsRequired, specificRoleIsRequired }, + }, +}) { + shouldBehaveLikeClosable({ + closed, + open() { + shouldBehaveLikeARestrictedOperation({ + callerIsTheManager, + callerIsNotTheManager() { + shouldBehaveLikeHasRole({ + publicRoleIsRequired, + specificRoleIsRequired, + }); + }, + }); + }, + }); +} + +/** + * @requires this.{target,calldata} + */ +function shouldBehaveLikeCanCallExecuting({ executing, notExecuting }) { + describe('when _executionId is in storage for target and selector', function () { + beforeEach('set _executionId flag from calldata and target', async function () { + const executionId = await web3.utils.keccak256( + web3.eth.abi.encodeParameters(['address', 'bytes4'], [this.target.address, this.calldata.substring(0, 10)]), + ); + await setStorageAt(this.manager.address, EXECUTION_ID_STORAGE_SLOT, executionId); + }); + + executing(); + }); + + describe('when _executionId does not match target and selector', notExecuting); +} + +/** + * @requires this.{target,calldata,roles,role} + */ +function shouldBehaveLikeHasRole({ publicRoleIsRequired, specificRoleIsRequired }) { + describe('when the function requires the caller to be granted with the PUBLIC_ROLE', function () { + beforeEach('set target function role as PUBLIC_ROLE', async function () { + this.role = this.roles.PUBLIC; + await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, { + from: this.roles.ADMIN.members[0], + }); + }); + + publicRoleIsRequired(); + }); + + describe('when the function requires the caller to be granted with a role other than PUBLIC_ROLE', function () { + beforeEach('set target function role as PUBLIC_ROLE', async function () { + await this.manager.$_setTargetFunctionRole(this.target.address, this.calldata.substring(0, 10), this.role.id, { + from: this.roles.ADMIN.members[0], + }); + }); + + shouldBehaveLikeGetAccess(specificRoleIsRequired); + }); +} + +/** + * @requires this.{manager,role,caller} + */ +function shouldBehaveLikeGetAccess({ + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + // Because both grant and execution delay are set within the same $_grantRole call + // it's not possible to create a set of tests that diverge between grant and execution delay. + // Therefore, the shouldBehaveLikeDelay arguments are renamed for clarity: + // before => beforeGrantDelay + // after => afterGrantDelay + callerHasAnExecutionDelay: { beforeGrantDelay: case1, afterGrantDelay: case2 }, + callerHasNoExecutionDelay: { beforeGrantDelay: case3, afterGrantDelay: case4 }, + }, + roleGrantingIsNotDelayed: { callerHasAnExecutionDelay: case5, callerHasNoExecutionDelay: case6 }, + }, + requiredRoleIsNotGranted, +}) { + describe('when the required role is granted to the caller', function () { + describe('when role granting is delayed', function () { + beforeEach('define delay', function () { + this.grantDelay = time.duration.minutes(3); + this.delay = this.grantDelay; // For shouldBehaveLikeDelay + }); + + describe('when caller has an execution delay', function () { + beforeEach('set role and delay', async function () { + this.executionDelay = time.duration.hours(10); + this.delay = this.grantDelay; + await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay); + }); + + shouldBehaveLikeDelay('grant', { before: case1, after: case2 }); + }); + + describe('when caller has no execution delay', function () { + beforeEach('set role and delay', async function () { + this.executionDelay = web3.utils.toBN(0); + await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay); + }); + + shouldBehaveLikeDelay('grant', { before: case3, after: case4 }); + }); + }); + + describe('when role granting is not delayed', function () { + beforeEach('define delay', function () { + this.grantDelay = web3.utils.toBN(0); + }); + + describe('when caller has an execution delay', function () { + beforeEach('set role and delay', async function () { + this.executionDelay = time.duration.hours(10); + await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay); + }); + + case5(); + }); + + describe('when caller has no execution delay', function () { + beforeEach('set role and delay', async function () { + this.executionDelay = web3.utils.toBN(0); + await this.manager.$_grantRole(this.role.id, this.caller, this.grantDelay, this.executionDelay); + }); + + case6(); + }); + }); + }); + + describe('when role is not granted', function () { + // Because this helper can be composed with other helpers, it's possible + // that role has been set already by another helper. + // Although this is highly unlikely, we check for it here to avoid false positives. + beforeEach('assert role is unset', async function () { + const { since } = await this.manager.getAccess(this.role.id, this.caller); + expect(since).to.be.bignumber.equal(web3.utils.toBN(0)); + }); + + requiredRoleIsNotGranted(); + }); +} + +// ============ ADMIN OPERATION HELPERS ============ + +/** + * @requires this.{manager,roles,calldata,role} + */ +function shouldBehaveLikeDelayedAdminOperation() { + const getAccessPath = COMMON_GET_ACCESS_PATH; + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + shouldBehaveLikeDelayedOperation(); + }; + getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () { + beforeEach('set execution delay', async function () { + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeDelayedOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + + beforeEach('set target as manager', function () { + this.target = this.manager; + }); + + shouldBehaveLikeARestrictedOperation({ + callerIsTheManager: COMMON_IS_EXECUTING_PATH, + callerIsNotTheManager() { + shouldBehaveLikeHasRole({ + publicRoleIsRequired() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [ + this.caller, + this.roles.ADMIN.id, // Although PUBLIC is required, target function role doesn't apply to admin ops + ], + ); + }); + }, + specificRoleIsRequired: getAccessPath, + }); + }, + }); +} + +/** + * @requires this.{manager,roles,calldata,role} + */ +function shouldBehaveLikeNotDelayedAdminOperation() { + const getAccessPath = COMMON_GET_ACCESS_PATH; + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () { + beforeEach('set execution delay', async function () { + await mine(); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () { + beforeEach('set execution delay', async function () { + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + + beforeEach('set target as manager', function () { + this.target = this.manager; + }); + + shouldBehaveLikeARestrictedOperation({ + callerIsTheManager: COMMON_IS_EXECUTING_PATH, + callerIsNotTheManager() { + shouldBehaveLikeHasRole({ + publicRoleIsRequired() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, this.roles.ADMIN.id], // Although PUBLIC_ROLE is required, admin ops are not subject to target function roles + ); + }); + }, + specificRoleIsRequired: getAccessPath, + }); + }, + }); +} + +/** + * @requires this.{manager,roles,calldata,role} + */ +function shouldBehaveLikeRoleAdminOperation(roleAdmin) { + const getAccessPath = COMMON_GET_ACCESS_PATH; + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () { + beforeEach('set operation delay', async function () { + await mine(); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () { + beforeEach('set execution delay', async function () { + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + + beforeEach('set target as manager', function () { + this.target = this.manager; + }); + + shouldBehaveLikeARestrictedOperation({ + callerIsTheManager: COMMON_IS_EXECUTING_PATH, + callerIsNotTheManager() { + shouldBehaveLikeHasRole({ + publicRoleIsRequired() { + it('reverts as AccessManagerUnauthorizedAccount', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagerUnauthorizedAccount', + [this.caller, roleAdmin], // Role admin ops require the role's admin + ); + }); + }, + specificRoleIsRequired: getAccessPath, + }); + }, + }); +} + +// ============ RESTRICTED OPERATION HELPERS ============ + +/** + * @requires this.{manager,roles,calldata,role} + */ +function shouldBehaveLikeAManagedRestrictedOperation() { + function revertUnauthorized() { + it('reverts as AccessManagedUnauthorized', async function () { + await expectRevertCustomError( + web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }), + 'AccessManagedUnauthorized', + [this.caller], + ); + }); + } + + const getAccessPath = COMMON_GET_ACCESS_PATH; + + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.beforeGrantDelay = + revertUnauthorized; + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasNoExecutionDelay.beforeGrantDelay = + revertUnauthorized; + getAccessPath.requiredRoleIsNotGranted = revertUnauthorized; + + getAccessPath.requiredRoleIsGranted.roleGrantingIsDelayed.callerHasAnExecutionDelay.afterGrantDelay = function () { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + getAccessPath.requiredRoleIsGranted.roleGrantingIsNotDelayed.callerHasAnExecutionDelay = function () { + beforeEach('consume previously set grant delay', async function () { + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }; + + const isExecutingPath = COMMON_IS_EXECUTING_PATH; + isExecutingPath.notExecuting = revertUnauthorized; + + shouldBehaveLikeCanCall({ + closed: revertUnauthorized, + open: { + callerIsTheManager: isExecutingPath, + callerIsNotTheManager: { + publicRoleIsRequired() { + it('succeeds called directly', async function () { + await web3.eth.sendTransaction({ to: this.target.address, data: this.calldata, from: this.caller }); + }); + + it('succeeds via execute', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, + specificRoleIsRequired: getAccessPath, + }, + }, + }); +} + +// ============ HELPERS ============ + +/** + * @requires this.{manager, caller, target, calldata} + */ +async function scheduleOperation(manager, { caller, target, calldata, delay }) { + const timestamp = await time.latest(); + const scheduledAt = timestamp.addn(1); + await setNextBlockTimestamp(scheduledAt); // Fix next block timestamp for predictability + const { receipt } = await manager.schedule(target, calldata, scheduledAt.add(delay), { + from: caller, + }); + + return { + receipt, + scheduledAt, + operationId: await manager.hashOperation(caller, target, calldata), + }; +} + +module.exports = { + // COMMON PATHS + COMMON_SCHEDULABLE_PATH, + COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY, + // MODE HELPERS + shouldBehaveLikeClosable, + // DELAY HELPERS + shouldBehaveLikeDelay, + // OPERATION HELPERS + shouldBehaveLikeSchedulableOperation, + // METHOD HELPERS + shouldBehaveLikeCanCall, + shouldBehaveLikeGetAccess, + shouldBehaveLikeHasRole, + // ADMIN OPERATION HELPERS + shouldBehaveLikeDelayedAdminOperation, + shouldBehaveLikeNotDelayedAdminOperation, + shouldBehaveLikeRoleAdminOperation, + // RESTRICTED OPERATION HELPERS + shouldBehaveLikeAManagedRestrictedOperation, + // HELPERS + scheduleOperation, +}; diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js index 5d8ed5de9..705af1a8a 100644 --- a/test/access/manager/AccessManager.test.js +++ b/test/access/manager/AccessManager.test.js @@ -1,1030 +1,2592 @@ const { web3 } = require('hardhat'); -const { constants, expectEvent, time } = require('@openzeppelin/test-helpers'); +const { constants, expectEvent, time, expectRevert } = require('@openzeppelin/test-helpers'); const { expectRevertCustomError } = require('../../helpers/customError'); const { selector } = require('../../helpers/methods'); const { clockFromReceipt } = require('../../helpers/time'); -const { product } = require('../../helpers/iterate'); -const helpers = require('@nomicfoundation/hardhat-network-helpers'); +const { + buildBaseRoles, + formatAccess, + EXPIRATION, + MINSETBACK, + EXECUTION_ID_STORAGE_SLOT, + CONSUMING_SCHEDULE_STORAGE_SLOT, +} = require('../../helpers/access-manager'); +const { + // COMMON PATHS + COMMON_SCHEDULABLE_PATH, + COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY, + // MODE HELPERS + shouldBehaveLikeClosable, + // DELAY HELPERS + shouldBehaveLikeDelay, + // OPERATION HELPERS + shouldBehaveLikeSchedulableOperation, + // METHOD HELPERS + shouldBehaveLikeCanCall, + shouldBehaveLikeGetAccess, + shouldBehaveLikeHasRole, + // ADMIN OPERATION HELPERS + shouldBehaveLikeDelayedAdminOperation, + shouldBehaveLikeNotDelayedAdminOperation, + shouldBehaveLikeRoleAdminOperation, + // RESTRICTED OPERATION HELPERS + shouldBehaveLikeAManagedRestrictedOperation, + // HELPERS + scheduleOperation, +} = require('./AccessManager.behavior'); +const { default: Wallet } = require('ethereumjs-wallet'); +const { + mine, + time: { setNextBlockTimestamp }, + getStorageAt, +} = require('@nomicfoundation/hardhat-network-helpers'); +const { MAX_UINT48 } = require('../../helpers/constants'); +const { impersonate } = require('../../helpers/account'); const AccessManager = artifacts.require('$AccessManager'); const AccessManagedTarget = artifacts.require('$AccessManagedTarget'); const Ownable = artifacts.require('$Ownable'); -const MAX_UINT64 = web3.utils.toBN((2n ** 64n - 1n).toString()); - -const ROLES = { - ADMIN: web3.utils.toBN(0), - SOME_ADMIN: web3.utils.toBN(17), - SOME: web3.utils.toBN(42), - PUBLIC: MAX_UINT64, -}; -Object.assign(ROLES, Object.fromEntries(Object.entries(ROLES).map(([key, value]) => [value, key]))); - -const executeDelay = web3.utils.toBN(10); -const grantDelay = web3.utils.toBN(10); -const MINSETBACK = time.duration.days(5); - -const formatAccess = access => [access[0], access[1].toString()]; +const someAddress = Wallet.generate().getChecksumAddressString(); contract('AccessManager', function (accounts) { - const [admin, manager, member, user, other] = accounts; + const [admin, manager, guardian, member, user, other] = accounts; beforeEach(async function () { + this.roles = buildBaseRoles(); + + // Add members + this.roles.ADMIN.members = [admin]; + this.roles.SOME_ADMIN.members = [manager]; + this.roles.SOME_GUARDIAN.members = [guardian]; + this.roles.SOME.members = [member]; + this.roles.PUBLIC.members = [admin, manager, guardian, member, user, other]; + this.manager = await AccessManager.new(admin); + this.target = await AccessManagedTarget.new(this.manager.address); - // add member to role - await this.manager.$_setRoleAdmin(ROLES.SOME, ROLES.SOME_ADMIN); - await this.manager.$_setRoleGuardian(ROLES.SOME, ROLES.SOME_ADMIN); - await this.manager.$_grantRole(ROLES.SOME_ADMIN, manager, 0, 0); - await this.manager.$_grantRole(ROLES.SOME, member, 0, 0); + for (const { id: roleId, admin, guardian, members } of Object.values(this.roles)) { + if (roleId === this.roles.PUBLIC.id) continue; // Every address belong to public and is locked + if (roleId === this.roles.ADMIN.id) continue; // Admin set during construction and is locked + + // Set admin role avoiding default + if (admin.id !== this.roles.ADMIN.id) { + await this.manager.$_setRoleAdmin(roleId, admin.id); + } + + // Set guardian role avoiding default + if (guardian.id !== this.roles.ADMIN.id) { + await this.manager.$_setRoleGuardian(roleId, guardian.id); + } + + // Grant role to members + for (const member of members) { + await this.manager.$_grantRole(roleId, member, 0, 0); + } + } }); - it('rejects zero address for initialAdmin', async function () { - await expectRevertCustomError(AccessManager.new(constants.ZERO_ADDRESS), 'AccessManagerInvalidInitialAdmin', [ - constants.ZERO_ADDRESS, - ]); - }); - - it('default minsetback is 1 day', async function () { - expect(await this.manager.minSetback()).to.be.bignumber.equal(MINSETBACK); - }); - - it('roles are correctly initialized', async function () { - // role admin - expect(await this.manager.getRoleAdmin(ROLES.ADMIN)).to.be.bignumber.equal(ROLES.ADMIN); - expect(await this.manager.getRoleAdmin(ROLES.SOME_ADMIN)).to.be.bignumber.equal(ROLES.ADMIN); - expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN); - expect(await this.manager.getRoleAdmin(ROLES.PUBLIC)).to.be.bignumber.equal(ROLES.ADMIN); - // role guardian - expect(await this.manager.getRoleGuardian(ROLES.ADMIN)).to.be.bignumber.equal(ROLES.ADMIN); - expect(await this.manager.getRoleGuardian(ROLES.SOME_ADMIN)).to.be.bignumber.equal(ROLES.ADMIN); - expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN); - expect(await this.manager.getRoleGuardian(ROLES.PUBLIC)).to.be.bignumber.equal(ROLES.ADMIN); - // role members - expect(await this.manager.hasRole(ROLES.ADMIN, admin).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.ADMIN, manager).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME_ADMIN, admin).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME_ADMIN, manager).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.SOME_ADMIN, member).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME_ADMIN, user).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME, admin).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME, manager).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - expect(await this.manager.hasRole(ROLES.PUBLIC, admin).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.PUBLIC, manager).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.PUBLIC, member).then(formatAccess)).to.be.deep.equal([true, '0']); - expect(await this.manager.hasRole(ROLES.PUBLIC, user).then(formatAccess)).to.be.deep.equal([true, '0']); - }); - - describe('Roles management', function () { - describe('label role', function () { - it('admin can emit a label event', async function () { - expectEvent(await this.manager.labelRole(ROLES.SOME, 'Some label', { from: admin }), 'RoleLabel', { - roleId: ROLES.SOME, - label: 'Some label', - }); - }); - - it('admin can re-emit a label event', async function () { - await this.manager.labelRole(ROLES.SOME, 'Some label', { from: admin }); - - expectEvent(await this.manager.labelRole(ROLES.SOME, 'Updated label', { from: admin }), 'RoleLabel', { - roleId: ROLES.SOME, - label: 'Updated label', - }); - }); - - it('emitting a label is restricted', async function () { - await expectRevertCustomError( - this.manager.labelRole(ROLES.SOME, 'Invalid label', { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.ADMIN], - ); - }); + describe('during construction', function () { + it('grants admin role to initialAdmin', async function () { + const manager = await AccessManager.new(other); + expect(await manager.hasRole(this.roles.ADMIN.id, other).then(formatAccess)).to.be.deep.equal([true, '0']); }); - describe('grant role', function () { - describe('without a grant delay', function () { - it('without an execute delay', async function () { - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); + it('rejects zero address for initialAdmin', async function () { + await expectRevertCustomError(AccessManager.new(constants.ZERO_ADDRESS), 'AccessManagerInvalidInitialAdmin', [ + constants.ZERO_ADDRESS, + ]); + }); - const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: user, - since: timestamp, - delay: '0', - newMember: true, - }); + it('initializes setup roles correctly', async function () { + for (const { id: roleId, admin, guardian, members } of Object.values(this.roles)) { + expect(await this.manager.getRoleAdmin(roleId)).to.be.bignumber.equal(admin.id); + expect(await this.manager.getRoleGuardian(roleId)).to.be.bignumber.equal(guardian.id); - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal(timestamp); // inRoleSince - 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.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const { receipt } = await this.manager.grantRole(ROLES.SOME, user, executeDelay, { from: manager }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: user, - since: timestamp, - delay: executeDelay, - newMember: true, - }); - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([ - true, - executeDelay.toString(), + for (const user of this.roles.PUBLIC.members) { + expect(await this.manager.hasRole(roleId, user).then(formatAccess)).to.be.deep.equal([ + members.includes(user), + '0', ]); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal(timestamp); // inRoleSince - 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 role', async function () { - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - await this.manager.grantRole(ROLES.SOME, member, 0, { from: manager }); - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - }); - - it('to a user that is scheduled for joining the role', async function () { - await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10 - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager }); - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - }); - - it('grant role is restricted', async function () { - await expectRevertCustomError( - this.manager.grantRole(ROLES.SOME, user, 0, { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.SOME_ADMIN], - ); - }); - }); - - describe('with a grant delay', function () { - beforeEach(async function () { - await this.manager.$_setGrantDelay(ROLES.SOME, grantDelay); - await time.increase(MINSETBACK); - }); - - it('granted role is not active immediately', async function () { - const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: user, - since: timestamp.add(grantDelay), - delay: '0', - newMember: true, - }); - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inRoleSince - 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 role is active after the delay', async function () { - const { receipt } = await this.manager.grantRole(ROLES.SOME, user, 0, { from: manager }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: user, - since: timestamp.add(grantDelay), - delay: '0', - newMember: true, - }); - - await time.increase(grantDelay); - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([true, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal(timestamp.add(grantDelay)); // inRoleSince - 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('cannot grant public role', async function () { - await expectRevertCustomError( - this.manager.$_grantRole(ROLES.PUBLIC, other, 0, executeDelay, { from: manager }), - 'AccessManagerLockedRole', - [ROLES.PUBLIC], - ); - }); - }); - - describe('revoke role', function () { - it('from a user that is already in the role', async function () { - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - - const { receipt } = await this.manager.revokeRole(ROLES.SOME, member, { from: manager }); - expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: member }); - - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince - 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('from a user that is scheduled for joining the role', async function () { - await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10 - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const { receipt } = await this.manager.revokeRole(ROLES.SOME, user, { from: manager }); - expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: user }); - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince - 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('from a user that is not in the role', async function () { - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - await this.manager.revokeRole(ROLES.SOME, user, { from: manager }); - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - }); - - it('revoke role is restricted', async function () { - await expectRevertCustomError( - this.manager.revokeRole(ROLES.SOME, member, { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.SOME_ADMIN], - ); - }); - }); - - describe('renounce role', function () { - it('for a user that is already in the role', async function () { - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - - const { receipt } = await this.manager.renounceRole(ROLES.SOME, member, { from: member }); - expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: member }); - - expect(await this.manager.hasRole(ROLES.SOME, member).then(formatAccess)).to.be.deep.equal([false, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, member); - expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince - 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 schedule for joining the role', async function () { - await this.manager.$_grantRole(ROLES.SOME, user, 10, 0); // grant delay 10 - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const { receipt } = await this.manager.renounceRole(ROLES.SOME, user, { from: user }); - expectEvent(receipt, 'RoleRevoked', { roleId: ROLES.SOME, account: user }); - - expect(await this.manager.hasRole(ROLES.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - const access = await this.manager.getAccess(ROLES.SOME, user); - expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince - 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 role', async function () { - await this.manager.renounceRole(ROLES.SOME, user, { from: user }); - }); - - it('bad user confirmation', async function () { - await expectRevertCustomError( - this.manager.renounceRole(ROLES.SOME, member, { from: user }), - 'AccessManagerBadConfirmation', - [], - ); - }); - }); - - describe('change role admin', function () { - it("admin can set any role's admin", async function () { - expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN); - - const { receipt } = await this.manager.setRoleAdmin(ROLES.SOME, ROLES.ADMIN, { from: admin }); - expectEvent(receipt, 'RoleAdminChanged', { roleId: ROLES.SOME, admin: ROLES.ADMIN }); - - expect(await this.manager.getRoleAdmin(ROLES.SOME)).to.be.bignumber.equal(ROLES.ADMIN); - }); - - it("setting a role's admin is restricted", async function () { - await expectRevertCustomError( - this.manager.setRoleAdmin(ROLES.SOME, ROLES.SOME, { from: manager }), - 'AccessManagerUnauthorizedAccount', - [manager, ROLES.ADMIN], - ); - }); - }); - - describe('change role guardian', function () { - it("admin can set any role's admin", async function () { - expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.SOME_ADMIN); - - const { receipt } = await this.manager.setRoleGuardian(ROLES.SOME, ROLES.ADMIN, { from: admin }); - expectEvent(receipt, 'RoleGuardianChanged', { roleId: ROLES.SOME, guardian: ROLES.ADMIN }); - - expect(await this.manager.getRoleGuardian(ROLES.SOME)).to.be.bignumber.equal(ROLES.ADMIN); - }); - - it("setting a role's admin is restricted", async function () { - await expectRevertCustomError( - this.manager.setRoleGuardian(ROLES.SOME, ROLES.SOME, { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.ADMIN], - ); - }); - }); - - describe('change execution delay', function () { - it('increasing the delay has immediate effect', async function () { - const oldDelay = web3.utils.toBN(10); - const newDelay = web3.utils.toBN(100); - - // role is already granted (with no delay) in the initial setup. this update takes time. - await this.manager.$_grantRole(ROLES.SOME, member, 0, oldDelay); - - const accessBefore = await this.manager.getAccess(ROLES.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.grantRole(ROLES.SOME, member, newDelay, { - from: manager, - }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: member, - since: timestamp, - delay: newDelay, - newMember: false, - }); - - // immediate effect - const accessAfter = await this.manager.getAccess(ROLES.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('decreasing the delay takes time', async function () { - const oldDelay = web3.utils.toBN(100); - const newDelay = web3.utils.toBN(10); - - // role is already granted (with no delay) in the initial setup. this update takes time. - await this.manager.$_grantRole(ROLES.SOME, member, 0, oldDelay); - - const accessBefore = await this.manager.getAccess(ROLES.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.grantRole(ROLES.SOME, member, newDelay, { - from: manager, - }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - const setback = oldDelay.sub(newDelay); - - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: member, - since: timestamp.add(setback), - delay: newDelay, - newMember: false, - }); - - // no immediate effect - const accessAfter = await this.manager.getAccess(ROLES.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(setback)); // effect - - // delayed effect - await time.increase(setback); - const accessAfterSetback = await this.manager.getAccess(ROLES.SOME, member); - expect(accessAfterSetback[1]).to.be.bignumber.equal(newDelay); // currentDelay - expect(accessAfterSetback[2]).to.be.bignumber.equal('0'); // pendingDelay - expect(accessAfterSetback[3]).to.be.bignumber.equal('0'); // effect - }); - - it('can set a user execution delay during the grant delay', async function () { - await this.manager.$_grantRole(ROLES.SOME, other, 10, 0); - // here: "other" is pending to get the role, but doesn't yet have it. - - const { receipt } = await this.manager.grantRole(ROLES.SOME, other, executeDelay, { from: manager }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - - // increasing the execution delay from 0 to executeDelay is immediate - expectEvent(receipt, 'RoleGranted', { - roleId: ROLES.SOME, - account: other, - since: timestamp, - delay: executeDelay, - newMember: false, - }); - }); - }); - - describe('change grant delay', function () { - it('increasing the delay has immediate effect', async function () { - const oldDelay = web3.utils.toBN(10); - const newDelay = web3.utils.toBN(100); - - await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay); - await time.increase(MINSETBACK); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - - const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay)); - - expect(setback).to.be.bignumber.equal(MINSETBACK); - expectEvent(receipt, 'RoleGrantDelayChanged', { - roleId: ROLES.SOME, - delay: newDelay, - since: timestamp.add(setback), - }); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - await time.increase(setback); - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay); - }); - - it('increasing the delay has delay effect #1', async function () { - const oldDelay = web3.utils.toBN(100); - const newDelay = web3.utils.toBN(10); - - await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay); - await time.increase(MINSETBACK); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - - const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay)); - - expect(setback).to.be.bignumber.equal(MINSETBACK); - expectEvent(receipt, 'RoleGrantDelayChanged', { - roleId: ROLES.SOME, - delay: newDelay, - since: timestamp.add(setback), - }); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - await time.increase(setback); - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay); - }); - - it('increasing the delay has delay effect #2', async function () { - const oldDelay = time.duration.days(30); // more than the minsetback - const newDelay = web3.utils.toBN(10); - - await this.manager.$_setGrantDelay(ROLES.SOME, oldDelay); - await time.increase(MINSETBACK); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - - const { receipt } = await this.manager.setGrantDelay(ROLES.SOME, newDelay, { from: admin }); - const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - const setback = web3.utils.BN.max(MINSETBACK, oldDelay.sub(newDelay)); - - expect(setback).to.be.bignumber.gt(MINSETBACK); - expectEvent(receipt, 'RoleGrantDelayChanged', { - roleId: ROLES.SOME, - delay: newDelay, - since: timestamp.add(setback), - }); - - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(oldDelay); - await time.increase(setback); - expect(await this.manager.getRoleGrantDelay(ROLES.SOME)).to.be.bignumber.equal(newDelay); - }); - - it('changing the grant delay is restricted', async function () { - await expectRevertCustomError( - this.manager.setGrantDelay(ROLES.SOME, grantDelay, { from: other }), - 'AccessManagerUnauthorizedAccount', - [ROLES.ADMIN, other], - ); - }); + } + } }); }); - 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.execute = (opts = {}) => this.manager.execute(...this.call, { from: user, ...opts }); - this.cancel = (opts = {}) => this.manager.cancel(user, ...this.call, { from: user, ...opts }); - }); + describe('getters', function () { + describe('#canCall', function () { + beforeEach('set calldata', function () { + this.calldata = '0x12345678'; + this.role = { id: web3.utils.toBN(379204) }; + }); - describe('Change function permissions', function () { - const sigs = ['someFunction()', 'someOtherFunction(uint256)', 'oneMoreFunction(address,uint8)'].map(selector); - - it('admin can set function role', async function () { - for (const sig of sigs) { - expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal(ROLES.ADMIN); - } - - const { receipt: receipt1 } = await this.manager.setTargetFunctionRole(this.target.address, sigs, ROLES.SOME, { - from: admin, - }); - - for (const sig of sigs) { - expectEvent(receipt1, 'TargetFunctionRoleUpdated', { - target: this.target.address, - selector: sig, - roleId: ROLES.SOME, + shouldBehaveLikeCanCall({ + closed() { + it('should return false and no delay', async function () { + const { immediate, delay } = await this.manager.canCall( + someAddress, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal('0'); }); - expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal(ROLES.SOME); - } - - const { receipt: receipt2 } = await this.manager.setTargetFunctionRole( - this.target.address, - [sigs[1]], - ROLES.SOME_ADMIN, - { - from: admin, + }, + open: { + callerIsTheManager: { + executing() { + it('should return true and no delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(true); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + notExecuting() { + it('should return false and no delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }, }, - ); - expectEvent(receipt2, 'TargetFunctionRoleUpdated', { - target: this.target.address, - selector: sigs[1], - roleId: ROLES.SOME_ADMIN, - }); + callerIsNotTheManager: { + publicRoleIsRequired() { + it('should return true and no delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(true); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + specificRoleIsRequired: { + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); - for (const sig of sigs) { - expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal( - sig == sigs[1] ? ROLES.SOME_ADMIN : ROLES.SOME, - ); - } + it('should return false and no execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + this.scheduleIn = this.executionDelay; // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation({ + scheduled: { + before() { + beforeEach('consume previously set delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('should return false and execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal(this.executionDelay); + }); + }, + after() { + beforeEach('consume previously set delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('should return false and execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal(this.executionDelay); + }); + }, + expired() { + beforeEach('consume previously set delay', async function () { + // Consume previously set delay + await mine(); + }); + it('should return false and execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal(this.executionDelay); + }); + }, + }, + notScheduled() { + it('should return false and execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal(this.executionDelay); + }); + }, + }); + }, + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('should return false and no execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('should return true and no execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(true); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay() { + it('should return false and execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal(this.executionDelay); + }); + }, + callerHasNoExecutionDelay() { + it('should return true and no execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(true); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + }, + }, + requiredRoleIsNotGranted() { + it('should return false and no execution delay', async function () { + const { immediate, delay } = await this.manager.canCall( + this.caller, + this.target.address, + this.calldata.substring(0, 10), + ); + expect(immediate).to.be.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }, + }, + }, + }, + }); + }); + + describe('#expiration', function () { + it('has a 7 days default expiration', async function () { + expect(await this.manager.expiration()).to.be.bignumber.equal(EXPIRATION); + }); + }); + + describe('#minSetback', function () { + it('has a 5 days default minimum setback', async function () { + expect(await this.manager.minSetback()).to.be.bignumber.equal(MINSETBACK); + }); + }); + + describe('#isTargetClosed', function () { + shouldBehaveLikeClosable({ + closed() { + it('returns true', async function () { + expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(true); + }); + }, + open() { + it('returns false', async function () { + expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(false); + }); + }, + }); + }); + + describe('#getTargetFunctionRole', function () { + const methodSelector = selector('something(address,bytes)'); + + it('returns the target function role', async function () { + const roleId = web3.utils.toBN(21498); + await this.manager.$_setTargetFunctionRole(this.target.address, methodSelector, roleId); + + expect(await this.manager.getTargetFunctionRole(this.target.address, methodSelector)).to.be.bignumber.equal( + roleId, + ); }); - it('non-admin cannot set function role', async function () { - await expectRevertCustomError( - this.manager.setTargetFunctionRole(this.target.address, sigs, ROLES.SOME, { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.ADMIN], + it('returns the ADMIN role if not set', async function () { + expect(await this.manager.getTargetFunctionRole(this.target.address, methodSelector)).to.be.bignumber.equal( + this.roles.ADMIN.id, ); }); }); - // WIP - describe('Calling restricted & unrestricted functions', function () { - for (const [callerRoles, fnRole, closed, delay] of product( - [[], [ROLES.SOME]], - [undefined, ROLES.ADMIN, ROLES.SOME, ROLES.PUBLIC], - [false, true], - [null, executeDelay], - )) { - // can we call with a delay ? - const indirectSuccess = (fnRole == ROLES.PUBLIC || callerRoles.includes(fnRole)) && !closed; + describe('#getTargetAdminDelay', function () { + describe('when the target admin delay is setup', function () { + beforeEach('set target admin delay', async function () { + this.oldDelay = await this.manager.getTargetAdminDelay(this.target.address); + this.newDelay = time.duration.days(10); - // can we call without a delay ? - const directSuccess = (fnRole == ROLES.PUBLIC || (callerRoles.includes(fnRole) && !delay)) && !closed; + await this.manager.$_setTargetAdminDelay(this.target.address, this.newDelay); + this.delay = MINSETBACK; // For shouldBehaveLikeDelay + }); - const description = [ - 'Caller in roles', - '[' + (callerRoles ?? []).map(roleId => ROLES[roleId]).join(', ') + ']', - delay ? 'with a delay' : 'without a delay', - '+', - 'functions open to roles', - '[' + (ROLES[fnRole] ?? '') + ']', - closed ? `(closed)` : '', - ].join(' '); + shouldBehaveLikeDelay('effect', { + before() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); - describe(description, function () { - beforeEach(async function () { - if (!delay || fnRole === ROLES.PUBLIC) this.skip(); // TODO: Fixed in #4613 + it('returns the old target admin delay', async function () { + expect(await this.manager.getTargetAdminDelay(this.target.address)).to.be.bignumber.equal(this.oldDelay); + }); + }, + after() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); - // setup - await Promise.all([ - this.manager.$_setTargetClosed(this.target.address, closed), - fnRole && this.manager.$_setTargetFunctionRole(this.target.address, selector('fnRestricted()'), fnRole), - fnRole && this.manager.$_setTargetFunctionRole(this.target.address, selector('fnUnrestricted()'), fnRole), - ...callerRoles - .filter(roleId => roleId != ROLES.PUBLIC) - .map(roleId => this.manager.$_grantRole(roleId, user, 0, delay ?? 0)), - ]); + it('returns the new target admin delay', async function () { + expect(await this.manager.getTargetAdminDelay(this.target.address)).to.be.bignumber.equal(this.newDelay); + }); + }, + }); + }); - // post setup checks - expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(closed); + it('returns the 0 if not set', async function () { + expect(await this.manager.getTargetAdminDelay(this.target.address)).to.be.bignumber.equal('0'); + }); + }); - if (fnRole) { - expect( - await this.manager.getTargetFunctionRole(this.target.address, selector('fnRestricted()')), - ).to.be.bignumber.equal(fnRole); - expect( - await this.manager.getTargetFunctionRole(this.target.address, selector('fnUnrestricted()')), - ).to.be.bignumber.equal(fnRole); - } + describe('#getRoleAdmin', function () { + const roleId = web3.utils.toBN(5234907); - for (const roleId of callerRoles) { - const access = await this.manager.getAccess(roleId, user); - if (roleId == ROLES.PUBLIC) { - expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince + it('returns the role admin', async function () { + const adminId = web3.utils.toBN(789433); + + await this.manager.$_setRoleAdmin(roleId, adminId); + + expect(await this.manager.getRoleAdmin(roleId)).to.be.bignumber.equal(adminId); + }); + + it('returns the ADMIN role if not set', async function () { + expect(await this.manager.getRoleAdmin(roleId)).to.be.bignumber.equal(this.roles.ADMIN.id); + }); + }); + + describe('#getRoleGuardian', function () { + const roleId = web3.utils.toBN(5234907); + + it('returns the role guardian', async function () { + const guardianId = web3.utils.toBN(789433); + + await this.manager.$_setRoleGuardian(roleId, guardianId); + + expect(await this.manager.getRoleGuardian(roleId)).to.be.bignumber.equal(guardianId); + }); + + it('returns the ADMIN role if not set', async function () { + expect(await this.manager.getRoleGuardian(roleId)).to.be.bignumber.equal(this.roles.ADMIN.id); + }); + }); + + describe('#getRoleGrantDelay', function () { + const roleId = web3.utils.toBN(9248439); + + describe('when the grant admin delay is setup', function () { + beforeEach('set grant admin delay', async function () { + this.oldDelay = await this.manager.getRoleGrantDelay(roleId); + this.newDelay = time.duration.days(11); + + await this.manager.$_setGrantDelay(roleId, this.newDelay); + this.delay = MINSETBACK; // For shouldBehaveLikeDelay + }); + + shouldBehaveLikeDelay('grant', { + before() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('returns the old role grant delay', async function () { + expect(await this.manager.getRoleGrantDelay(roleId)).to.be.bignumber.equal(this.oldDelay); + }); + }, + after() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('returns the new role grant delay', async function () { + expect(await this.manager.getRoleGrantDelay(roleId)).to.be.bignumber.equal(this.newDelay); + }); + }, + }); + }); + + it('returns 0 if delay is not set', async function () { + expect(await this.manager.getTargetAdminDelay(this.target.address)).to.be.bignumber.equal('0'); + }); + }); + + describe('#getAccess', function () { + beforeEach('set role', function () { + this.role = { id: web3.utils.toBN(9452) }; + this.caller = user; + }); + + shouldBehaveLikeGetAccess({ + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('role is not in effect and execution delay is set', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal(this.delayEffect); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Not in effect yet + expect(await time.latest()).to.be.bignumber.lt(access[0]); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('access has role in effect and execution delay is set', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + + expect(access[0]).to.be.bignumber.equal(this.delayEffect); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await time.latest()).to.be.bignumber.equal(access[0]); + }); + }, + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('access has role not in effect without execution delay', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal(this.delayEffect); // inEffectSince + 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'); // pendingDelayEffect + + // Not in effect yet + expect(await time.latest()).to.be.bignumber.lt(access[0]); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('role is in effect without execution delay', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal(this.delayEffect); // inEffectSince + 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'); // pendingDelayEffect + + // Already in effect + expect(await time.latest()).to.be.bignumber.equal(access[0]); + }); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay() { + it('access has role in effect and execution delay is set', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal(await time.latest()); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await time.latest()).to.be.bignumber.equal(access[0]); + }); + }, + callerHasNoExecutionDelay() { + it('access has role in effect without execution delay', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal(await time.latest()); // inEffectSince 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'); // inRoleSince - 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 - } - } + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await time.latest()).to.be.bignumber.equal(access[0]); + }); + }, + }, + }, + requiredRoleIsNotGranted() { + it('has empty access', async function () { + const access = await this.manager.getAccess(this.role.id, this.caller); + expect(access[0]).to.be.bignumber.equal('0'); // inEffectSince + 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'); // pendingDelayEffect + }); + }, + }); + }); + + describe('#hasRole', function () { + beforeEach('setup shouldBehaveLikeHasRole', function () { + this.role = { id: web3.utils.toBN(49832) }; + this.calldata = '0x1234'; + this.caller = user; + }); + + shouldBehaveLikeHasRole({ + publicRoleIsRequired() { + it('has PUBLIC role', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.true; + expect(executionDelay).to.be.bignumber.eq('0'); + }); + }, + specificRoleIsRequired: { + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('does not have role but execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.false; + expect(executionDelay).to.be.bignumber.eq(this.executionDelay); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('has role and execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.true; + expect(executionDelay).to.be.bignumber.eq(this.executionDelay); + }); + }, + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('does not have role nor execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.false; + expect(executionDelay).to.be.bignumber.eq('0'); + }); + }, + afterGrantDelay() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('has role and no execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.true; + expect(executionDelay).to.be.bignumber.eq('0'); + }); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay() { + it('has role and execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.true; + expect(executionDelay).to.be.bignumber.eq(this.executionDelay); + }); + }, + callerHasNoExecutionDelay() { + it('has role and no execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.true; + expect(executionDelay).to.be.bignumber.eq('0'); + }); + }, + }, + }, + requiredRoleIsNotGranted() { + it('has no role and no execution delay', async function () { + const { isMember, executionDelay } = await this.manager.hasRole(this.role.id, this.caller); + expect(isMember).to.be.false; + expect(executionDelay).to.be.bignumber.eq('0'); + }); + }, + }, + }); + }); + + describe('#getSchedule', function () { + beforeEach('set role and calldata', async function () { + const method = 'fnRestricted()'; + this.caller = user; + this.role = { id: web3.utils.toBN(493590) }; + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay + + this.calldata = await this.target.contract.methods[method]().encodeABI(); + this.scheduleIn = time.duration.days(10); // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation({ + scheduled: { + before() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('returns schedule in the future', async function () { + const schedule = await this.manager.getSchedule(this.operationId); + expect(schedule).to.be.bignumber.equal(this.scheduledAt.add(this.scheduleIn)); + expect(schedule).to.be.bignumber.gt(await time.latest()); + }); + }, + after() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('returns schedule', async function () { + const schedule = await this.manager.getSchedule(this.operationId); + expect(schedule).to.be.bignumber.equal(this.scheduledAt.add(this.scheduleIn)); + expect(schedule).to.be.bignumber.eq(await time.latest()); + }); + }, + expired() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('returns 0', async function () { + expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal('0'); + }); + }, + }, + notScheduled() { + it('defaults to 0', async function () { + expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal('0'); + }); + }, + }); + }); + + describe('#getNonce', function () { + describe('when operation is scheduled', function () { + beforeEach('schedule operation', async function () { + const method = 'fnRestricted()'; + this.caller = user; + this.role = { id: web3.utils.toBN(4209043) }; + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay + + this.calldata = await this.target.contract.methods[method]().encodeABI(); + this.delay = time.duration.days(10); + + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + this.operationId = operationId; + }); + + it('returns nonce', async function () { + expect(await this.manager.getNonce(this.operationId)).to.be.bignumber.equal('1'); + }); + }); + + describe('when is not scheduled', function () { + it('returns default 0', async function () { + expect(await this.manager.getNonce(web3.utils.keccak256('operation'))).to.be.bignumber.equal('0'); + }); + }); + }); + + describe('#hashOperation', function () { + it('returns an operationId', async function () { + const calldata = '0x123543'; + const address = someAddress; + + const args = [user, address, calldata]; + + expect(await this.manager.hashOperation(...args)).to.be.bignumber.eq( + await web3.utils.keccak256(web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], args)), + ); + }); + }); + }); + + describe('admin operations', function () { + beforeEach('set required role', function () { + this.role = this.roles.ADMIN; + }); + + describe('subject to a delay', function () { + describe('#labelRole', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'labelRole(uint64,string)'; + const args = [123443, 'TEST']; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); }); - 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'); + shouldBehaveLikeDelayedAdminOperation(); + }); + + it('emits an event with the label', async function () { + expectEvent(await this.manager.labelRole(this.roles.SOME.id, 'Some label', { from: admin }), 'RoleLabel', { + roleId: this.roles.SOME.id, + label: 'Some label', + }); + }); + + it('updates label on a second call', async function () { + await this.manager.labelRole(this.roles.SOME.id, 'Some label', { from: admin }); + + expectEvent(await this.manager.labelRole(this.roles.SOME.id, 'Updated label', { from: admin }), 'RoleLabel', { + roleId: this.roles.SOME.id, + label: 'Updated label', + }); + }); + + it('reverts labeling PUBLIC_ROLE', async function () { + await expectRevertCustomError( + this.manager.labelRole(this.roles.PUBLIC.id, 'Some label', { from: admin }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + it('reverts labeling ADMIN_ROLE', async function () { + await expectRevertCustomError( + this.manager.labelRole(this.roles.ADMIN.id, 'Some label', { from: admin }), + 'AccessManagerLockedRole', + [this.roles.ADMIN.id], + ); + }); + }); + + describe('#setRoleAdmin', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setRoleAdmin(uint64,uint64)'; + const args = [93445, 84532]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); }); - it('Calling a non restricted function never revert', async function () { - expectEvent(await this.target.fnUnrestricted({ from: user }), 'CalledUnrestricted', { - caller: user, + shouldBehaveLikeDelayedAdminOperation(); + }); + + it("sets any role's admin if called by an admin", async function () { + expect(await this.manager.getRoleAdmin(this.roles.SOME.id)).to.be.bignumber.equal(this.roles.SOME_ADMIN.id); + + const { receipt } = await this.manager.setRoleAdmin(this.roles.SOME.id, this.roles.ADMIN.id, { from: admin }); + expectEvent(receipt, 'RoleAdminChanged', { roleId: this.roles.SOME.id, admin: this.roles.ADMIN.id }); + + expect(await this.manager.getRoleAdmin(this.roles.SOME.id)).to.be.bignumber.equal(this.roles.ADMIN.id); + }); + + it('reverts setting PUBLIC_ROLE admin', async function () { + await expectRevertCustomError( + this.manager.setRoleAdmin(this.roles.PUBLIC.id, this.roles.ADMIN.id, { from: admin }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + it('reverts setting ADMIN_ROLE admin', async function () { + await expectRevertCustomError( + this.manager.setRoleAdmin(this.roles.ADMIN.id, this.roles.ADMIN.id, { from: admin }), + 'AccessManagerLockedRole', + [this.roles.ADMIN.id], + ); + }); + }); + + describe('#setRoleGuardian', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setRoleGuardian(uint64,uint64)'; + const args = [93445, 84532]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); + + shouldBehaveLikeDelayedAdminOperation(); + }); + + it("sets any role's guardian if called by an admin", async function () { + expect(await this.manager.getRoleGuardian(this.roles.SOME.id)).to.be.bignumber.equal( + this.roles.SOME_GUARDIAN.id, + ); + + const { receipt } = await this.manager.setRoleGuardian(this.roles.SOME.id, this.roles.ADMIN.id, { + from: admin, + }); + expectEvent(receipt, 'RoleGuardianChanged', { roleId: this.roles.SOME.id, guardian: this.roles.ADMIN.id }); + + expect(await this.manager.getRoleGuardian(this.roles.SOME.id)).to.be.bignumber.equal(this.roles.ADMIN.id); + }); + + it('reverts setting PUBLIC_ROLE admin', async function () { + await expectRevertCustomError( + this.manager.setRoleGuardian(this.roles.PUBLIC.id, this.roles.ADMIN.id, { from: admin }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + it('reverts setting ADMIN_ROLE admin', async function () { + await expectRevertCustomError( + this.manager.setRoleGuardian(this.roles.ADMIN.id, this.roles.ADMIN.id, { from: admin }), + 'AccessManagerLockedRole', + [this.roles.ADMIN.id], + ); + }); + }); + + describe('#setGrantDelay', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setGrantDelay(uint64,uint32)'; + const args = [984910, time.duration.days(2)]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); + + shouldBehaveLikeDelayedAdminOperation(); + }); + + it('reverts setting grant delay for the PUBLIC_ROLE', async function () { + await expectRevertCustomError( + this.manager.setGrantDelay(this.roles.PUBLIC.id, web3.utils.toBN(69), { from: admin }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + describe('when increasing the delay', function () { + const oldDelay = web3.utils.toBN(10); + const newDelay = web3.utils.toBN(100); + + beforeEach('sets old delay', async function () { + this.role = this.roles.SOME; + await this.manager.$_setGrantDelay(this.role.id, oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(oldDelay); + }); + + it('increases the delay after minsetback', async function () { + const { receipt } = await this.manager.setGrantDelay(this.role.id, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'RoleGrantDelayChanged', { + roleId: this.role.id, + delay: newDelay, + since: timestamp.add(MINSETBACK), + }); + + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(newDelay); + }); + }); + + describe('when reducing the delay', function () { + const oldDelay = time.duration.days(10); + + beforeEach('sets old delay', async function () { + this.role = this.roles.SOME; + await this.manager.$_setGrantDelay(this.role.id, oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(oldDelay); + }); + + describe('when the delay difference is shorter than minimum setback', function () { + const newDelay = oldDelay.subn(1); + + it('increases the delay after minsetback', async function () { + const { receipt } = await this.manager.setGrantDelay(this.role.id, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'RoleGrantDelayChanged', { + roleId: this.role.id, + delay: newDelay, + since: timestamp.add(MINSETBACK), + }); + + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(newDelay); }); }); - it(`Calling a restricted function directly should ${ - directSuccess ? 'succeed' : 'revert' - }`, async function () { - const promise = this.direct(); + describe('when the delay difference is longer than minimum setback', function () { + const newDelay = web3.utils.toBN(1); - if (directSuccess) { - expectEvent(await promise, 'CalledRestricted', { caller: user }); - } else if (indirectSuccess) { - await expectRevertCustomError(promise, 'AccessManagerNotScheduled', [this.opId]); - } else { - await expectRevertCustomError(promise, 'AccessManagedUnauthorized', [user]); - } - }); + beforeEach('assert delay difference is higher than minsetback', function () { + expect(oldDelay.sub(newDelay)).to.be.bignumber.gt(MINSETBACK); + }); - it('Calling indirectly: only execute', async function () { - // execute without schedule - if (directSuccess) { - const nonceBefore = await this.manager.getNonce(this.opId); - const { receipt, tx } = await this.execute(); - - expectEvent.notEmitted(receipt, 'OperationExecuted', { operationId: this.opId }); - await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); - - // nonce is not modified - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore); - } else if (indirectSuccess) { - await expectRevertCustomError(this.execute(), 'AccessManagerNotScheduled', [this.opId]); - } else { - await expectRevertCustomError(this.execute(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); - } - }); - - it('Calling indirectly: schedule and execute', async function () { - if (directSuccess || indirectSuccess) { - const nonceBefore = await this.manager.getNonce(this.opId); - const { receipt } = await this.schedule(); + it('increases the delay after delay difference', async function () { + const setback = oldDelay.sub(newDelay); + const { receipt } = await this.manager.setGrantDelay(this.role.id, newDelay, { from: admin }); 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], + expectEvent(receipt, 'RoleGrantDelayChanged', { + roleId: this.role.id, + delay: newDelay, + since: timestamp.add(setback), }); - // 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), - ); - - // nonce is incremented - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - - // execute without wait - if (directSuccess) { - const { receipt, tx } = await this.execute(); - - await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); - if (delay && fnRole !== ROLES.PUBLIC) { - expectEvent(receipt, 'OperationExecuted', { operationId: this.opId }); - expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); - } - - // nonce is not modified by execute - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - } else if (indirectSuccess) { - await expectRevertCustomError(this.execute(), 'AccessManagerNotReady', [this.opId]); - } else { - await expectRevertCustomError(this.execute(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); - } - } else { - await expectRevertCustomError(this.schedule(), 'AccessManagerUnauthorizedCall', [user, ...this.call]); - } + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(oldDelay); + await time.increase(setback); + expect(await this.manager.getRoleGrantDelay(this.role.id)).to.be.bignumber.equal(newDelay); + }); }); - - it('Calling indirectly: schedule wait and execute', async function () { - if (directSuccess || indirectSuccess) { - const nonceBefore = await this.manager.getNonce(this.opId); - 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), - ); - - // nonce is incremented - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - - // wait - await time.increase(delay ?? 0); - - // execute without wait - if (directSuccess || indirectSuccess) { - const { receipt, tx } = await this.execute(); - - await expectEvent.inTransaction(tx, this.target, 'CalledRestricted', { caller: this.manager.address }); - if (delay && fnRole !== ROLES.PUBLIC) { - expectEvent(receipt, 'OperationExecuted', { operationId: this.opId }); - expect(await this.manager.getSchedule(this.opId)).to.be.bignumber.equal('0'); - } - - // nonce is not modified by execute - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - } else { - await expectRevertCustomError(this.execute(), '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 nonceBefore = await this.manager.getNonce(this.opId); - 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); - - // nonce is incremented - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - - // 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); - - // nonce is not modified by execute - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - } 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 nonceBefore = await this.manager.getNonce(this.opId); - 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); - - // nonce is incremented - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - - // 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); - - // nonce is not modified by execute - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - } 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'); - - // nonce is not modified by execute - expect(await this.manager.getNonce(this.opId)).to.be.bignumber.equal(nonceBefore.addn(1)); - } 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('#setTargetAdminDelay', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setTargetAdminDelay(address,uint32)'; + const args = [someAddress, time.duration.days(3)]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); + + shouldBehaveLikeDelayedAdminOperation(); + }); + + describe('when increasing the delay', function () { + const oldDelay = time.duration.days(10); + const newDelay = time.duration.days(11); + const target = someAddress; + + beforeEach('sets old delay', async function () { + await this.manager.$_setTargetAdminDelay(target, oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(oldDelay); + }); + + it('increases the delay after minsetback', async function () { + const { receipt } = await this.manager.setTargetAdminDelay(target, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'TargetAdminDelayUpdated', { + target, + delay: newDelay, + since: timestamp.add(MINSETBACK), + }); + + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(newDelay); + }); + }); + + describe('when reducing the delay', function () { + const oldDelay = time.duration.days(10); + const target = someAddress; + + beforeEach('sets old delay', async function () { + await this.manager.$_setTargetAdminDelay(target, oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(oldDelay); + }); + + describe('when the delay difference is shorter than minimum setback', function () { + const newDelay = oldDelay.subn(1); + + it('increases the delay after minsetback', async function () { + const { receipt } = await this.manager.setTargetAdminDelay(target, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'TargetAdminDelayUpdated', { + target, + delay: newDelay, + since: timestamp.add(MINSETBACK), + }); + + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(oldDelay); + await time.increase(MINSETBACK); + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(newDelay); + }); + }); + + describe('when the delay difference is longer than minimum setback', function () { + const newDelay = web3.utils.toBN(1); + + beforeEach('assert delay difference is higher than minsetback', function () { + expect(oldDelay.sub(newDelay)).to.be.bignumber.gt(MINSETBACK); + }); + + it('increases the delay after delay difference', async function () { + const setback = oldDelay.sub(newDelay); + const { receipt } = await this.manager.setTargetAdminDelay(target, newDelay, { from: admin }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'TargetAdminDelayUpdated', { + target, + delay: newDelay, + since: timestamp.add(setback), + }); + + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(oldDelay); + await time.increase(setback); + expect(await this.manager.getTargetAdminDelay(target)).to.be.bignumber.equal(newDelay); + }); + }); + }); + }); }); - describe('Indirect execution corner-cases', async function () { - beforeEach(async function () { - await this.manager.$_setTargetFunctionRole(this.target.address, this.callData, ROLES.SOME); - await this.manager.$_grantRole(ROLES.SOME, user, 0, executeDelay); - }); - - it('Checking canCall when caller is the manager depend on the _executionId', 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)); - - // too early - await helpers.time.setNextBlockTimestamp(timestamp.add(executeDelay).subn(1)); - await expectRevertCustomError(this.execute(), '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 helpers.time.setNextBlockTimestamp(timestamp.add(executeDelay)); - await this.execute(); - - // 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], + describe('not subject to a delay', function () { + describe('#updateAuthority', function () { + beforeEach('create a target and a new authority', async function () { + this.newAuthority = await AccessManager.new(admin); + this.newManagedTarget = await AccessManagedTarget.new(this.manager.address); }); - await expectRevertCustomError(this.schedule(), 'AccessManagerAlreadyScheduled', [this.opId]); - }); + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'updateAuthority(address,address)'; + const args = [this.newManagedTarget.address, this.newAuthority.address]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); - it('Cannot cancel an operation that is not scheduled', async function () { - await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]); - }); + shouldBehaveLikeNotDelayedAdminOperation(); + }); - it('Cannot cancel an operation that is already executed', async function () { - await this.schedule(); - await time.increase(executeDelay); - await this.execute(); + it('changes the authority', async function () { + expect(await this.newManagedTarget.authority()).to.be.equal(this.manager.address); - await expectRevertCustomError(this.cancel(), 'AccessManagerNotScheduled', [this.opId]); - }); + const { tx } = await this.manager.updateAuthority(this.newManagedTarget.address, this.newAuthority.address, { + from: admin, + }); - it('Scheduler can cancel', async function () { - await this.schedule(); + // Managed contract is responsible of notifying the change through an event + await expectEvent.inTransaction(tx, this.newManagedTarget, 'AuthorityUpdated', { + authority: this.newAuthority.address, + }); - 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 }), 'AccessManagerUnauthorizedCancel', [ - 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.execute(); - - // reschedule - const { receipt } = await this.schedule(); - expectEvent(receipt, 'OperationScheduled', { - operationId: this.opId, - caller: user, - target: this.call[0], - data: this.call[1], + expect(await this.newManagedTarget.authority()).to.be.equal(this.newAuthority.address); }); }); - it('Can re-schedule after cancel', async function () { - await this.schedule(); - await this.cancel(); + describe('#setTargetClosed', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setTargetClosed(address,bool)'; + const args = [someAddress, true]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); - // reschedule - const { receipt } = await this.schedule(); - expectEvent(receipt, 'OperationScheduled', { - operationId: this.opId, - caller: user, - target: this.call[0], - data: this.call[1], + shouldBehaveLikeNotDelayedAdminOperation(); + }); + + it('closes and opens a target', async function () { + const close = await this.manager.setTargetClosed(this.target.address, true, { from: admin }); + expectEvent(close.receipt, 'TargetClosed', { target: this.target.address, closed: true }); + + expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(true); + + const open = await this.manager.setTargetClosed(this.target.address, false, { from: admin }); + expectEvent(open.receipt, 'TargetClosed', { target: this.target.address, closed: false }); + expect(await this.manager.isTargetClosed(this.target.address)).to.be.equal(false); + }); + + it('reverts if closing the manager', async function () { + await expectRevertCustomError( + this.manager.setTargetClosed(this.manager.address, true, { from: admin }), + 'AccessManagerLockedAccount', + [this.manager.address], + ); }); }); + + describe('#setTargetFunctionRole', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'setTargetFunctionRole(address,bytes4[],uint64)'; + const args = [someAddress, ['0x12345678'], 443342]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); + + shouldBehaveLikeNotDelayedAdminOperation(); + }); + + const sigs = ['someFunction()', 'someOtherFunction(uint256)', 'oneMoreFunction(address,uint8)'].map(selector); + + it('sets function roles', async function () { + for (const sig of sigs) { + expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal( + this.roles.ADMIN.id, + ); + } + + const { receipt: receipt1 } = await this.manager.setTargetFunctionRole( + this.target.address, + sigs, + this.roles.SOME.id, + { + from: admin, + }, + ); + + for (const sig of sigs) { + expectEvent(receipt1, 'TargetFunctionRoleUpdated', { + target: this.target.address, + selector: sig, + roleId: this.roles.SOME.id, + }); + expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal( + this.roles.SOME.id, + ); + } + + const { receipt: receipt2 } = await this.manager.setTargetFunctionRole( + this.target.address, + [sigs[1]], + this.roles.SOME_ADMIN.id, + { + from: admin, + }, + ); + expectEvent(receipt2, 'TargetFunctionRoleUpdated', { + target: this.target.address, + selector: sigs[1], + roleId: this.roles.SOME_ADMIN.id, + }); + + for (const sig of sigs) { + expect(await this.manager.getTargetFunctionRole(this.target.address, sig)).to.be.bignumber.equal( + sig == sigs[1] ? this.roles.SOME_ADMIN.id : this.roles.SOME.id, + ); + } + }); + }); + + describe('role admin operations', function () { + const ANOTHER_ADMIN = web3.utils.toBN(0xdeadc0de1); + const ANOTHER_ROLE = web3.utils.toBN(0xdeadc0de2); + + beforeEach('set required role', async function () { + // Make admin a member of ANOTHER_ADMIN + await this.manager.$_grantRole(ANOTHER_ADMIN, admin, 0, 0); + await this.manager.$_setRoleAdmin(ANOTHER_ROLE, ANOTHER_ADMIN); + + this.role = { id: ANOTHER_ADMIN }; + this.user = user; + await this.manager.$_grantRole(this.role.id, this.user, 0, 0); + }); + + describe('#grantRole', function () { + describe('restrictions', function () { + beforeEach('set method and args', function () { + const method = 'grantRole(uint64,address,uint32)'; + const args = [ANOTHER_ROLE, someAddress, 0]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + }); + + shouldBehaveLikeRoleAdminOperation(ANOTHER_ADMIN); + }); + + it('reverts when granting PUBLIC_ROLE', async function () { + await expectRevertCustomError( + this.manager.grantRole(this.roles.PUBLIC.id, user, 0, { + from: admin, + }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + describe('when the user is not a role member', function () { + describe('with grant delay', function () { + beforeEach('set grant delay and grant role', async function () { + // Delay granting + this.grantDelay = time.duration.weeks(2); + await this.manager.$_setGrantDelay(ANOTHER_ROLE, this.grantDelay); + await time.increase(MINSETBACK); + + // Grant role + this.executionDelay = time.duration.days(3); + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.executionDelay, { + from: admin, + }); + + this.receipt = receipt; + this.delay = this.grantDelay; // For shouldBehaveLikeDelay + }); + + shouldBehaveLikeDelay('grant', { + before() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('does not grant role to the user yet', async function () { + const timestamp = await clockFromReceipt.timestamp(this.receipt).then(web3.utils.toBN); + expectEvent(this.receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: timestamp.add(this.grantDelay), + delay: this.executionDelay, + newMember: true, + }); + + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(timestamp.add(this.grantDelay)); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Not in effect yet + const currentTimestamp = await time.latest(); + expect(currentTimestamp).to.be.a.bignumber.lt(access[0]); + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + false, + this.executionDelay.toString(), + ]); + }); + }, + after() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('grants role to the user', async function () { + const timestamp = await clockFromReceipt.timestamp(this.receipt).then(web3.utils.toBN); + expectEvent(this.receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: timestamp.add(this.grantDelay), + delay: this.executionDelay, + newMember: true, + }); + + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(timestamp.add(this.grantDelay)); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + const currentTimestamp = await time.latest(); + expect(currentTimestamp).to.be.a.bignumber.equal(access[0]); + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.executionDelay.toString(), + ]); + }); + }, + }); + }); + + describe('without grant delay', function () { + beforeEach('set granting delay', async function () { + // Delay granting + this.grantDelay = 0; + await this.manager.$_setGrantDelay(ANOTHER_ROLE, this.grantDelay); + await time.increase(MINSETBACK); + }); + + it('immediately grants the role to the user', async function () { + this.executionDelay = time.duration.days(6); + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.executionDelay, { + from: admin, + }); + + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + expectEvent(receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: timestamp, + delay: this.executionDelay, + newMember: true, + }); + + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(timestamp); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.executionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + const currentTimestamp = await time.latest(); + expect(currentTimestamp).to.be.a.bignumber.equal(access[0]); + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.executionDelay.toString(), + ]); + }); + }); + }); + + describe('when the user is already a role member', function () { + beforeEach('make user role member', async function () { + this.previousExecutionDelay = time.duration.days(6); + await this.manager.$_grantRole(ANOTHER_ROLE, this.user, 0, this.previousExecutionDelay); + this.oldAccess = await this.manager.getAccess(ANOTHER_ROLE, user); + }); + + describe('with grant delay', function () { + beforeEach('set granting delay', async function () { + // Delay granting + const grantDelay = time.duration.weeks(2); + await this.manager.$_setGrantDelay(ANOTHER_ROLE, grantDelay); + await time.increase(MINSETBACK); + }); + + describe('when increasing the execution delay', function () { + beforeEach('set increased new execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + + this.newExecutionDelay = this.previousExecutionDelay.add(time.duration.days(4)); + }); + + it('emits event and immediately changes the execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay, { + from: admin, + }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: timestamp, + delay: this.newExecutionDelay, + newMember: false, + }); + + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.newExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.newExecutionDelay.toString(), + ]); + }); + }); + + describe('when decreasing the execution delay', function () { + beforeEach('decrease execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + + this.newExecutionDelay = this.previousExecutionDelay.sub(time.duration.days(4)); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay, { + from: admin, + }); + this.grantTimestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + this.receipt = receipt; + this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For shouldBehaveLikeDelay + }); + + it('emits event', function () { + expectEvent(this.receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: this.grantTimestamp.add(this.delay), + delay: this.newExecutionDelay, + newMember: false, + }); + }); + + shouldBehaveLikeDelay('execution delay effect', { + before() { + beforeEach('consume effect delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('does not change the execution delay yet', async function () { + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.previousExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal(this.newExecutionDelay); // pendingDelay + expect(access[3]).to.be.bignumber.equal(this.grantTimestamp.add(this.delay)); // pendingDelayEffect + + // Not in effect yet + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + }); + }, + after() { + beforeEach('consume effect delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('changes the execution delay', async function () { + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.newExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.newExecutionDelay.toString(), + ]); + }); + }, + }); + }); + }); + + describe('without grant delay', function () { + beforeEach('set granting delay', async function () { + // Delay granting + const grantDelay = 0; + await this.manager.$_setGrantDelay(ANOTHER_ROLE, grantDelay); + await time.increase(MINSETBACK); + }); + + describe('when increasing the execution delay', function () { + beforeEach('set increased new execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + + this.newExecutionDelay = this.previousExecutionDelay.add(time.duration.days(4)); + }); + + it('emits event and immediately changes the execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay, { + from: admin, + }); + const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + expectEvent(receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: timestamp, + delay: this.newExecutionDelay, + newMember: false, + }); + + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.newExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.newExecutionDelay.toString(), + ]); + }); + }); + + describe('when decreasing the execution delay', function () { + beforeEach('decrease execution delay', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, this.user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + + this.newExecutionDelay = this.previousExecutionDelay.sub(time.duration.days(4)); + const { receipt } = await this.manager.grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay, { + from: admin, + }); + this.grantTimestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); + + this.receipt = receipt; + this.delay = this.previousExecutionDelay.sub(this.newExecutionDelay); // For shouldBehaveLikeDelay + }); + + it('emits event', function () { + expectEvent(this.receipt, 'RoleGranted', { + roleId: ANOTHER_ROLE, + account: this.user, + since: this.grantTimestamp.add(this.delay), + delay: this.newExecutionDelay, + newMember: false, + }); + }); + + shouldBehaveLikeDelay('execution delay effect', { + before() { + beforeEach('consume effect delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('does not change the execution delay yet', async function () { + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.previousExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal(this.newExecutionDelay); // pendingDelay + expect(access[3]).to.be.bignumber.equal(this.grantTimestamp.add(this.delay)); // pendingDelayEffect + + // Not in effect yet + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.previousExecutionDelay.toString(), + ]); + }); + }, + after() { + beforeEach('consume effect delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('changes the execution delay', async function () { + // Access is correctly stored + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + + expect(access[0]).to.be.bignumber.equal(this.oldAccess[0]); // inEffectSince + expect(access[1]).to.be.bignumber.equal(this.newExecutionDelay); // currentDelay + expect(access[2]).to.be.bignumber.equal('0'); // pendingDelay + expect(access[3]).to.be.bignumber.equal('0'); // pendingDelayEffect + + // Already in effect + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + this.newExecutionDelay.toString(), + ]); + }); + }, + }); + }); + }); + }); + }); + + describe('#revokeRole', function () { + describe('restrictions', function () { + beforeEach('set method and args', async function () { + const method = 'revokeRole(uint64,address)'; + const args = [ANOTHER_ROLE, someAddress]; + this.calldata = this.manager.contract.methods[method](...args).encodeABI(); + + // Need to be set before revoking + await this.manager.$_grantRole(...args, 0, 0); + }); + + shouldBehaveLikeRoleAdminOperation(ANOTHER_ADMIN); + }); + + describe('when role has been granted', function () { + beforeEach('grant role with grant delay', async function () { + this.grantDelay = time.duration.weeks(1); + await this.manager.$_grantRole(ANOTHER_ROLE, user, this.grantDelay, 0); + + this.delay = this.grantDelay; // For shouldBehaveLikeDelay + }); + + shouldBehaveLikeDelay('grant', { + before() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('revokes a granted role that will take effect in the future', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + + const { receipt } = await this.manager.revokeRole(ANOTHER_ROLE, user, { from: admin }); + expectEvent(receipt, 'RoleRevoked', { roleId: ANOTHER_ROLE, account: user }); + + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince + 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 + }); + }, + after() { + beforeEach('consume previously set grant delay', async function () { + // Consume previously set delay + await mine(); + }); + + it('revokes a granted role that already took effect', async function () { + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + true, + '0', + ]); + + const { receipt } = await this.manager.revokeRole(ANOTHER_ROLE, user, { from: admin }); + expectEvent(receipt, 'RoleRevoked', { roleId: ANOTHER_ROLE, account: user }); + + expect(await this.manager.hasRole(ANOTHER_ROLE, user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + + const access = await this.manager.getAccess(ANOTHER_ROLE, user); + expect(access[0]).to.be.bignumber.equal('0'); // inRoleSince + 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('when role has not been granted', function () { + it('has no effect', async function () { + expect(await this.manager.hasRole(this.roles.SOME.id, user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + const { receipt } = await this.manager.revokeRole(this.roles.SOME.id, user, { from: manager }); + expectEvent.notEmitted(receipt, 'RoleRevoked', { roleId: ANOTHER_ROLE, account: user }); + expect(await this.manager.hasRole(this.roles.SOME.id, user).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + }); + }); + + it('reverts revoking PUBLIC_ROLE', async function () { + await expectRevertCustomError( + this.manager.revokeRole(this.roles.PUBLIC.id, user, { from: admin }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + }); + }); + + describe('self role operations', function () { + describe('#renounceRole', function () { + beforeEach('grant role', async function () { + this.role = { id: web3.utils.toBN(783164) }; + this.caller = user; + await this.manager.$_grantRole(this.role.id, this.caller, 0, 0); + }); + + it('renounces a role', async function () { + expect(await this.manager.hasRole(this.role.id, this.caller).then(formatAccess)).to.be.deep.equal([ + true, + '0', + ]); + const { receipt } = await this.manager.renounceRole(this.role.id, this.caller, { + from: this.caller, + }); + expectEvent(receipt, 'RoleRevoked', { + roleId: this.role.id, + account: this.caller, + }); + expect(await this.manager.hasRole(this.role.id, this.caller).then(formatAccess)).to.be.deep.equal([ + false, + '0', + ]); + }); + + it('reverts if renouncing the PUBLIC_ROLE', async function () { + await expectRevertCustomError( + this.manager.renounceRole(this.roles.PUBLIC.id, this.caller, { + from: this.caller, + }), + 'AccessManagerLockedRole', + [this.roles.PUBLIC.id], + ); + }); + + it('reverts if renouncing with bad caller confirmation', async function () { + await expectRevertCustomError( + this.manager.renounceRole(this.role.id, someAddress, { + from: this.caller, + }), + 'AccessManagerBadConfirmation', + [], + ); + }); + }); + }); + }); + }); + + describe('access managed target operations', function () { + describe('when calling a restricted target function', function () { + const method = 'fnRestricted()'; + + beforeEach('set required role', function () { + this.role = { id: web3.utils.toBN(3597243) }; + this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + }); + + describe('restrictions', function () { + beforeEach('set method and args', function () { + this.calldata = this.target.contract.methods[method]().encodeABI(); + this.caller = user; + }); + + shouldBehaveLikeAManagedRestrictedOperation(); + }); + + it('succeeds called by a role member', async function () { + await this.manager.$_grantRole(this.role.id, user, 0, 0); + + const { receipt } = await this.target.methods[method]({ + data: this.calldata, + from: user, + }); + expectEvent(receipt, 'CalledRestricted', { + caller: user, + }); + }); + }); + + describe('when calling a non-restricted target function', function () { + const method = 'fnUnrestricted()'; + + beforeEach('set required role', async function () { + this.role = { id: web3.utils.toBN(879435) }; + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + }); + + it('succeeds called by anyone', async function () { + const { receipt } = await this.target.methods[method]({ + data: this.calldata, + from: user, + }); + expectEvent(receipt, 'CalledUnrestricted', { + caller: user, + }); + }); + }); + }); + + describe('#schedule', function () { + const method = 'fnRestricted()'; + + beforeEach('set target function role', async function () { + this.role = { id: web3.utils.toBN(498305) }; + this.caller = user; + + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay + + this.calldata = this.target.contract.methods[method]().encodeABI(); + this.delay = time.duration.weeks(2); + }); + + describe('restrictions', function () { + shouldBehaveLikeCanCall({ + closed() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + open: { + callerIsTheManager: { + executing() { + it.skip('is not reachable because schedule is not restrictable'); + }, + notExecuting() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + callerIsNotTheManager: { + publicRoleIsRequired() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await expectRevertCustomError( + this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + specificRoleIsRequired: { + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await expectRevertCustomError( + this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + afterGrantDelay() { + it('succeeds', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }); + }); + }, + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await expectRevertCustomError( + this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + afterGrantDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await expectRevertCustomError( + this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay() { + it('succeeds', async function () { + await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + }); + }, + callerHasNoExecutionDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + // scheduleOperation is not used here because it alters the next block timestamp + await expectRevertCustomError( + this.manager.schedule(this.target.address, this.calldata, MAX_UINT48, { + from: this.caller, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + }, + requiredRoleIsNotGranted() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + }, + }, + }); + }); + + it('schedules an operation at the specified execution date if it is larger than caller execution delay', async function () { + const { operationId, scheduledAt, receipt } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + + expect(await this.manager.getSchedule(operationId)).to.be.bignumber.equal(scheduledAt.add(this.delay)); + expectEvent(receipt, 'OperationScheduled', { + operationId, + nonce: '1', + schedule: scheduledAt.add(this.delay), + target: this.target.address, + data: this.calldata, + }); + }); + + it('schedules an operation at the minimum execution date if no specified execution date (when == 0)', async function () { + const executionDelay = await time.duration.hours(72); + await this.manager.$_grantRole(this.role.id, this.caller, 0, executionDelay); + + const timestamp = await time.latest(); + const scheduledAt = timestamp.addn(1); + await setNextBlockTimestamp(scheduledAt); + const { receipt } = await this.manager.schedule(this.target.address, this.calldata, 0, { + from: this.caller, + }); + + const operationId = await this.manager.hashOperation(this.caller, this.target.address, this.calldata); + + expect(await this.manager.getSchedule(operationId)).to.be.bignumber.equal(scheduledAt.add(executionDelay)); + expectEvent(receipt, 'OperationScheduled', { + operationId, + nonce: '1', + schedule: scheduledAt.add(executionDelay), + target: this.target.address, + data: this.calldata, + }); + }); + + it('increases the nonce of an operation scheduled more than once', async function () { + // Setup and check initial nonce + const expectedOperationId = await web3.utils.keccak256( + web3.eth.abi.encodeParameters( + ['address', 'address', 'bytes'], + [this.caller, this.target.address, this.calldata], + ), + ); + expect(await this.manager.getNonce(expectedOperationId)).to.be.bignumber.eq('0'); + + // Schedule + const op1 = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + expectEvent(op1.receipt, 'OperationScheduled', { + operationId: op1.operationId, + nonce: '1', + schedule: op1.scheduledAt.add(this.delay), + target: this.target.address, + data: this.calldata, + }); + expect(expectedOperationId).to.eq(op1.operationId); + + // Consume + await time.increase(this.delay); + await this.manager.$_consumeScheduledOp(expectedOperationId); + + // Check nonce + expect(await this.manager.getNonce(expectedOperationId)).to.be.bignumber.eq('1'); + + // Schedule again + const op2 = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + expectEvent(op2.receipt, 'OperationScheduled', { + operationId: op2.operationId, + nonce: '2', + schedule: op2.scheduledAt.add(this.delay), + target: this.target.address, + data: this.calldata, + }); + expect(expectedOperationId).to.eq(op2.operationId); + + // Check final nonce + expect(await this.manager.getNonce(expectedOperationId)).to.be.bignumber.eq('2'); + }); + + it('reverts if the specified execution date is before the current timestamp + caller execution delay', async function () { + const executionDelay = time.duration.weeks(1).add(this.delay); + await this.manager.$_grantRole(this.role.id, this.caller, 0, executionDelay); + + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + + it('reverts if an operation is already schedule', async function () { + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }); + + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.delay, + }), + 'AccessManagerAlreadyScheduled', + [operationId], + ); + }); + + it('panics scheduling calldata with less than 4 bytes', async function () { + const calldata = '0x1234'; // 2 bytes + + // Managed contract + await expectRevert.unspecified( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: calldata, + delay: this.delay, + }), + ); + + // Manager contract + await expectRevert.unspecified( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.manager.address, + calldata: calldata, + delay: this.delay, + }), + ); + }); + + it('reverts scheduling an unknown operation to the manager', async function () { + const calldata = '0x12345678'; + + await expectRevertCustomError( + scheduleOperation(this.manager, { + caller: this.caller, + target: this.manager.address, + calldata, + delay: this.delay, + }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.manager.address, calldata], + ); + }); + }); + + describe('#execute', function () { + const method = 'fnRestricted()'; + + beforeEach('set target function role', async function () { + this.role = { id: web3.utils.toBN(9825430) }; + this.caller = user; + + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 0); + + this.calldata = this.target.contract.methods[method]().encodeABI(); + }); + + describe('restrictions', function () { + shouldBehaveLikeCanCall({ + closed() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + open: { + callerIsTheManager: { + executing() { + it('succeeds', async function () { + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + }); + }, + notExecuting() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + callerIsNotTheManager: { + publicRoleIsRequired() { + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY); + }, + specificRoleIsRequired: { + requiredRoleIsGranted: { + roleGrantingIsDelayed: { + callerHasAnExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + afterGrantDelay() { + beforeEach('define schedule delay', async function () { + // Consume previously set delay + await mine(); + this.scheduleIn = time.duration.days(21); + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }, + }, + callerHasNoExecutionDelay: { + beforeGrantDelay() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + afterGrantDelay() { + beforeEach('define schedule delay', async function () { + // Consume previously set delay + await mine(); + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY); + }, + }, + }, + roleGrantingIsNotDelayed: { + callerHasAnExecutionDelay() { + beforeEach('define schedule delay', async function () { + this.scheduleIn = time.duration.days(15); + }); + + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH); + }, + callerHasNoExecutionDelay() { + shouldBehaveLikeSchedulableOperation(COMMON_SCHEDULABLE_PATH_IF_ZERO_DELAY); + }, + }, + }, + requiredRoleIsNotGranted() { + it('reverts as AccessManagerUnauthorizedCall', async function () { + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedCall', + [this.caller, this.target.address, this.calldata.substring(0, 10)], + ); + }); + }, + }, + }, + }, + }); + }); + + it('executes with a delay consuming the scheduled operation', async function () { + const delay = time.duration.hours(4); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // Execution delay is needed so the operation is consumed + + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay, + }); + await time.increase(delay); + const { receipt } = await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + expectEvent(receipt, 'OperationExecuted', { + operationId, + nonce: '1', + }); + expect(await this.manager.getSchedule(operationId)).to.be.bignumber.equal('0'); + }); + + it('executes with no delay consuming a scheduled operation', async function () { + const delay = time.duration.hours(4); + + // give caller an execution delay + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); + + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay, + }); + + // remove the execution delay + await this.manager.$_grantRole(this.role.id, this.caller, 0, 0); + + await time.increase(delay); + const { receipt } = await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + expectEvent(receipt, 'OperationExecuted', { + operationId, + nonce: '1', + }); + expect(await this.manager.getSchedule(operationId)).to.be.bignumber.equal('0'); + }); + + it('keeps the original _executionId after finishing the call', async function () { + const executionIdBefore = await getStorageAt(this.manager.address, EXECUTION_ID_STORAGE_SLOT); + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + const executionIdAfter = await getStorageAt(this.manager.address, EXECUTION_ID_STORAGE_SLOT); + expect(executionIdBefore).to.be.bignumber.equal(executionIdAfter); + }); + + it('reverts executing twice', async function () { + const delay = time.duration.hours(2); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // Execution delay is needed so the operation is consumed + + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay, + }); + await time.increase(delay); + await this.manager.execute(this.target.address, this.calldata, { from: this.caller }); + await expectRevertCustomError( + this.manager.execute(this.target.address, this.calldata, { from: this.caller }), + 'AccessManagerNotScheduled', + [operationId], + ); + }); + }); + + describe('#consumeScheduledOp', function () { + beforeEach('define scheduling parameters', async function () { + const method = 'fnRestricted()'; + this.caller = this.target.address; + this.calldata = this.target.contract.methods[method]().encodeABI(); + this.role = { id: web3.utils.toBN(9834983) }; + + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.role.id); + await this.manager.$_grantRole(this.role.id, this.caller, 0, 1); // nonzero execution delay + + this.scheduleIn = time.duration.hours(10); // For shouldBehaveLikeSchedulableOperation + }); + + describe('when caller is not consuming scheduled operation', function () { + beforeEach('set consuming false', async function () { + await this.target.setIsConsumingScheduledOp(false, `0x${CONSUMING_SCHEDULE_STORAGE_SLOT.toString(16)}`); + }); + + it('reverts as AccessManagerUnauthorizedConsume', async function () { + await impersonate(this.caller); + await expectRevertCustomError( + this.manager.consumeScheduledOp(this.caller, this.calldata, { from: this.caller }), + 'AccessManagerUnauthorizedConsume', + [this.caller], + ); + }); + }); + + describe('when caller is consuming scheduled operation', function () { + beforeEach('set consuming true', async function () { + await this.target.setIsConsumingScheduledOp(true, `0x${CONSUMING_SCHEDULE_STORAGE_SLOT.toString(16)}`); + }); + + shouldBehaveLikeSchedulableOperation({ + scheduled: { + before() { + it('reverts as AccessManagerNotReady', async function () { + await impersonate(this.caller); + await expectRevertCustomError( + this.manager.consumeScheduledOp(this.caller, this.calldata, { from: this.caller }), + 'AccessManagerNotReady', + [this.operationId], + ); + }); + }, + after() { + it('consumes the scheduled operation and resets timepoint', async function () { + expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal( + this.scheduledAt.add(this.scheduleIn), + ); + await impersonate(this.caller); + const { receipt } = await this.manager.consumeScheduledOp(this.caller, this.calldata, { + from: this.caller, + }); + expectEvent(receipt, 'OperationExecuted', { + operationId: this.operationId, + nonce: '1', + }); + expect(await this.manager.getSchedule(this.operationId)).to.be.bignumber.equal('0'); + }); + }, + expired() { + it('reverts as AccessManagerExpired', async function () { + await impersonate(this.caller); + await expectRevertCustomError( + this.manager.consumeScheduledOp(this.caller, this.calldata, { from: this.caller }), + 'AccessManagerExpired', + [this.operationId], + ); + }); + }, + }, + notScheduled() { + it('reverts as AccessManagerNotScheduled', async function () { + await impersonate(this.caller); + await expectRevertCustomError( + this.manager.consumeScheduledOp(this.caller, this.calldata, { from: this.caller }), + 'AccessManagerNotScheduled', + [this.operationId], + ); + }); + }, + }); + }); + }); + + describe('#cancelScheduledOp', function () { + const method = 'fnRestricted()'; + + beforeEach('setup scheduling', async function () { + this.caller = this.roles.SOME.members[0]; + await this.manager.$_setTargetFunctionRole(this.target.address, selector(method), this.roles.SOME.id); + await this.manager.$_grantRole(this.roles.SOME.id, this.caller, 0, 1); // nonzero execution delay + + this.calldata = await this.target.contract.methods[method]().encodeABI(); + this.scheduleIn = time.duration.days(10); // For shouldBehaveLikeSchedulableOperation + }); + + shouldBehaveLikeSchedulableOperation({ + scheduled: { + before() { + describe('when caller is the scheduler', function () { + it('succeeds', async function () { + await this.manager.cancel(this.caller, this.target.address, this.calldata, { from: this.caller }); + }); + }); + + describe('when caller is an admin', function () { + it('succeeds', async function () { + await this.manager.cancel(this.caller, this.target.address, this.calldata, { + from: this.roles.ADMIN.members[0], + }); + }); + }); + + describe('when caller is the role guardian', function () { + it('succeeds', async function () { + await this.manager.cancel(this.caller, this.target.address, this.calldata, { + from: this.roles.SOME_GUARDIAN.members[0], + }); + }); + }); + + describe('when caller is any other account', function () { + it('reverts as AccessManagerUnauthorizedCancel', async function () { + await expectRevertCustomError( + this.manager.cancel(this.caller, this.target.address, this.calldata, { from: other }), + 'AccessManagerUnauthorizedCancel', + [other, this.caller, this.target.address, selector(method)], + ); + }); + }); + }, + after() { + it('succeeds', async function () { + await this.manager.cancel(this.caller, this.target.address, this.calldata, { from: this.caller }); + }); + }, + expired() { + it('succeeds', async function () { + await this.manager.cancel(this.caller, this.target.address, this.calldata, { from: this.caller }); + }); + }, + }, + notScheduled() { + it('reverts as AccessManagerNotScheduled', async function () { + await expectRevertCustomError( + this.manager.cancel(this.caller, this.target.address, this.calldata), + 'AccessManagerNotScheduled', + [this.operationId], + ); + }); + }, + }); + + it('cancels an operation and resets schedule', async function () { + const { operationId } = await scheduleOperation(this.manager, { + caller: this.caller, + target: this.target.address, + calldata: this.calldata, + delay: this.scheduleIn, + }); + const { receipt } = await this.manager.cancel(this.caller, this.target.address, this.calldata, { + from: this.caller, + }); + expectEvent(receipt, 'OperationCanceled', { + operationId, + nonce: '1', + }); + expect(await this.manager.getSchedule(operationId)).to.be.bignumber.eq('0'); }); }); @@ -1095,7 +2657,11 @@ contract('AccessManager', function (accounts) { describe('function is open to public role', function () { beforeEach(async function () { - await this.manager.$_setTargetFunctionRole(this.ownable.address, selector('$_checkOwner()'), ROLES.PUBLIC); + await this.manager.$_setTargetFunctionRole( + this.ownable.address, + selector('$_checkOwner()'), + this.roles.PUBLIC.id, + ); }); it('directly call: reverts', async function () { @@ -1114,50 +2680,4 @@ contract('AccessManager', function (accounts) { }); }); }); - - 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('updateAuthority is restricted on manager', async function () { - await expectRevertCustomError( - this.manager.updateAuthority(this.target.address, this.newManager.address, { from: other }), - 'AccessManagerUnauthorizedAccount', - [other, ROLES.ADMIN], - ); - }); - - it('setAuthority is restricted on AccessManaged', async function () { - await expectRevertCustomError( - this.target.setAuthority(this.newManager.address, { from: admin }), - 'AccessManagedUnauthorized', - [admin], - ); - }); - }); - - // TODO: - // - check opening/closing a contract - // - check updating the contract delay - // - check the delay applies to admin function - describe.skip('contract modes', function () {}); }); diff --git a/test/access/manager/AuthorityUtils.test.js b/test/access/manager/AuthorityUtils.test.js new file mode 100644 index 000000000..346be030b --- /dev/null +++ b/test/access/manager/AuthorityUtils.test.js @@ -0,0 +1,91 @@ +require('@openzeppelin/test-helpers'); + +const AuthorityUtils = artifacts.require('$AuthorityUtils'); +const NotAuthorityMock = artifacts.require('NotAuthorityMock'); +const AuthorityNoDelayMock = artifacts.require('AuthorityNoDelayMock'); +const AuthorityDelayMock = artifacts.require('AuthorityDelayMock'); +const AuthorityNoResponse = artifacts.require('AuthorityNoResponse'); + +contract('AuthorityUtils', function (accounts) { + const [user, other] = accounts; + + beforeEach(async function () { + this.mock = await AuthorityUtils.new(); + }); + + describe('canCallWithDelay', function () { + describe('when authority does not have a canCall function', function () { + beforeEach(async function () { + this.authority = await NotAuthorityMock.new(); + }); + + it('returns (immediate = 0, delay = 0)', async function () { + const { immediate, delay } = await this.mock.$canCallWithDelay( + this.authority.address, + user, + other, + '0x12345678', + ); + expect(immediate).to.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }); + + describe('when authority has no delay', function () { + beforeEach(async function () { + this.authority = await AuthorityNoDelayMock.new(); + this.immediate = true; + await this.authority._setImmediate(this.immediate); + }); + + it('returns (immediate, delay = 0)', async function () { + const { immediate, delay } = await this.mock.$canCallWithDelay( + this.authority.address, + user, + other, + '0x12345678', + ); + expect(immediate).to.equal(this.immediate); + expect(delay).to.be.bignumber.equal('0'); + }); + }); + + describe('when authority replies with a delay', function () { + beforeEach(async function () { + this.authority = await AuthorityDelayMock.new(); + this.immediate = true; + this.delay = web3.utils.toBN(42); + await this.authority._setImmediate(this.immediate); + await this.authority._setDelay(this.delay); + }); + + it('returns (immediate, delay)', async function () { + const { immediate, delay } = await this.mock.$canCallWithDelay( + this.authority.address, + user, + other, + '0x12345678', + ); + expect(immediate).to.equal(this.immediate); + expect(delay).to.be.bignumber.equal(this.delay); + }); + }); + + describe('when authority replies with empty data', function () { + beforeEach(async function () { + this.authority = await AuthorityNoResponse.new(); + }); + + it('returns (immediate = 0, delay = 0)', async function () { + const { immediate, delay } = await this.mock.$canCallWithDelay( + this.authority.address, + user, + other, + '0x12345678', + ); + expect(immediate).to.equal(false); + expect(delay).to.be.bignumber.equal('0'); + }); + }); + }); +}); diff --git a/test/governance/extensions/GovernorTimelockAccess.test.js b/test/governance/extensions/GovernorTimelockAccess.test.js index 9734a2f5c..68df99e85 100644 --- a/test/governance/extensions/GovernorTimelockAccess.test.js +++ b/test/governance/extensions/GovernorTimelockAccess.test.js @@ -1,4 +1,4 @@ -const { expectEvent } = require('@openzeppelin/test-helpers'); +const { expectEvent, time } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); const Enums = require('../../helpers/enums'); @@ -12,7 +12,7 @@ const Governor = artifacts.require('$GovernorTimelockAccessMock'); const AccessManagedTarget = artifacts.require('$AccessManagedTarget'); const TOKENS = [ - // { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' }, + { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' }, { Token: artifacts.require('$ERC20VotesTimestampMock'), mode: 'timestamp' }, ]; @@ -20,7 +20,7 @@ const hashOperation = (caller, target, data) => web3.utils.keccak256(web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [caller, target, data])); contract('GovernorTimelockAccess', function (accounts) { - const [admin, voter1, voter2, voter3, voter4] = accounts; + const [admin, voter1, voter2, voter3, voter4, other] = accounts; const name = 'OZ-Governor'; const version = '1'; @@ -112,6 +112,139 @@ contract('GovernorTimelockAccess', function (accounts) { expect(await this.mock.accessManager()).to.be.equal(this.manager.address); }); + it('sets base delay (seconds)', async function () { + const baseDelay = time.duration.hours(10); + + // Only through governance + await expectRevertCustomError( + this.mock.setBaseDelaySeconds(baseDelay, { from: voter1 }), + 'GovernorOnlyExecutor', + [voter1], + ); + + this.proposal = await this.helper.setProposal( + [ + { + target: this.mock.address, + value: '0', + data: this.mock.contract.methods.setBaseDelaySeconds(baseDelay).encodeABI(), + }, + ], + 'descr', + ); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + const receipt = await this.helper.execute(); + + expectEvent(receipt, 'BaseDelaySet', { + oldBaseDelaySeconds: '0', + newBaseDelaySeconds: baseDelay, + }); + + expect(await this.mock.baseDelaySeconds()).to.be.bignumber.eq(baseDelay); + }); + + it('sets access manager ignored', async function () { + const selectors = ['0x12345678', '0x87654321', '0xabcdef01']; + + // Only through governance + await expectRevertCustomError( + this.mock.setAccessManagerIgnored(other, selectors, true, { from: voter1 }), + 'GovernorOnlyExecutor', + [voter1], + ); + + // Ignore + const helperIgnore = new GovernorHelper(this.mock, mode); + await helperIgnore.setProposal( + [ + { + target: this.mock.address, + value: '0', + data: this.mock.contract.methods.setAccessManagerIgnored(other, selectors, true).encodeABI(), + }, + ], + 'descr', + ); + + await helperIgnore.propose(); + await helperIgnore.waitForSnapshot(); + await helperIgnore.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await helperIgnore.waitForDeadline(); + const ignoreReceipt = await helperIgnore.execute(); + + for (const selector of selectors) { + expectEvent(ignoreReceipt, 'AccessManagerIgnoredSet', { + target: other, + selector, + ignored: true, + }); + expect(await this.mock.isAccessManagerIgnored(other, selector)).to.be.true; + } + + // Unignore + const helperUnignore = new GovernorHelper(this.mock, mode); + await helperUnignore.setProposal( + [ + { + target: this.mock.address, + value: '0', + data: this.mock.contract.methods.setAccessManagerIgnored(other, selectors, false).encodeABI(), + }, + ], + 'descr', + ); + + await helperUnignore.propose(); + await helperUnignore.waitForSnapshot(); + await helperUnignore.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await helperUnignore.waitForDeadline(); + const unignoreReceipt = await helperUnignore.execute(); + + for (const selector of selectors) { + expectEvent(unignoreReceipt, 'AccessManagerIgnoredSet', { + target: other, + selector, + ignored: false, + }); + expect(await this.mock.isAccessManagerIgnored(other, selector)).to.be.false; + } + }); + + it('sets access manager ignored when target is the governor', async function () { + const other = this.mock.address; + const selectors = ['0x12345678', '0x87654321', '0xabcdef01']; + + await this.helper.setProposal( + [ + { + target: this.mock.address, + value: '0', + data: this.mock.contract.methods.setAccessManagerIgnored(other, selectors, true).encodeABI(), + }, + ], + 'descr', + ); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + const receipt = await this.helper.execute(); + + for (const selector of selectors) { + expectEvent(receipt, 'AccessManagerIgnoredSet', { + target: other, + selector, + ignored: true, + }); + expect(await this.mock.isAccessManagerIgnored(other, selector)).to.be.true; + } + }); + describe('base delay only', function () { for (const [delay, queue] of [ [0, true], @@ -124,10 +257,15 @@ contract('GovernorTimelockAccess', function (accounts) { this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr'); await this.helper.propose(); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([false]); + expect(withDelay).to.deep.eq([false]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); - if (queue) { + if (await this.mock.proposalNeedsQueuing(this.proposal.id)) { const txQueue = await this.helper.queue(); expectEvent(txQueue, 'ProposalQueued', { proposalId: this.proposal.id }); } @@ -141,6 +279,82 @@ contract('GovernorTimelockAccess', function (accounts) { } }); + it('reverts when an operation is executed before eta', async function () { + const delay = time.duration.hours(2); + await this.mock.$_setBaseDelaySeconds(delay); + + this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr'); + + await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([false]); + expect(withDelay).to.deep.eq([false]); + + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + await this.helper.queue(); + await expectRevertCustomError(this.helper.execute(), 'GovernorUnmetDelay', [ + this.proposal.id, + await this.mock.proposalEta(this.proposal.id), + ]); + }); + + it('reverts with a proposal including multiple operations but one of those was cancelled in the manager', async function () { + const delay = time.duration.hours(2); + const roleId = '1'; + + await this.manager.setTargetFunctionRole(this.receiver.address, [this.restricted.selector], roleId, { + from: admin, + }); + await this.manager.grantRole(roleId, this.mock.address, delay, { from: admin }); + + // Set proposals + const original = new GovernorHelper(this.mock, mode); + await original.setProposal([this.restricted.operation, this.unrestricted.operation], 'descr'); + + // Go through all the governance process + await original.propose(); + expect(await this.mock.proposalNeedsQueuing(original.currentProposal.id)).to.be.eq(true); + const { + delay: planDelay, + indirect, + withDelay, + } = await this.mock.proposalExecutionPlan(original.currentProposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([true, false]); + expect(withDelay).to.deep.eq([true, false]); + await original.waitForSnapshot(); + await original.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await original.waitForDeadline(); + await original.queue(); + await original.waitForEta(); + + // Suddenly cancel one of the proposed operations in the manager + await this.manager.cancel(this.mock.address, this.restricted.operation.target, this.restricted.operation.data, { + from: admin, + }); + + // Reschedule the same operation in a different proposal to avoid "AccessManagerNotScheduled" error + const rescheduled = new GovernorHelper(this.mock, mode); + await rescheduled.setProposal([this.restricted.operation], 'descr'); + await rescheduled.propose(); + await rescheduled.waitForSnapshot(); + await rescheduled.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await rescheduled.waitForDeadline(); + await rescheduled.queue(); // This will schedule it again in the manager + await rescheduled.waitForEta(); + + // Attempt to execute + await expectRevertCustomError(original.execute(), 'GovernorMismatchedNonce', [ + original.currentProposal.id, + 1, + 2, + ]); + }); + it('single operation with access manager delay', async function () { const delay = 1000; const roleId = '1'; @@ -153,6 +367,12 @@ contract('GovernorTimelockAccess', function (accounts) { this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr'); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([true]); + expect(withDelay).to.deep.eq([true]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); @@ -197,6 +417,12 @@ contract('GovernorTimelockAccess', function (accounts) { ); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(baseDelay)); + expect(indirect).to.deep.eq([true, false, false]); + expect(withDelay).to.deep.eq([true, false, false]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); @@ -235,10 +461,16 @@ contract('GovernorTimelockAccess', function (accounts) { await this.manager.grantRole(roleId, this.mock.address, delay, { from: admin }); }); - it('cancellation after queue (internal)', async function () { + it('cancels restricted with delay after queue (internal)', async function () { this.proposal = await this.helper.setProposal([this.restricted.operation], 'descr'); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(true); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([true]); + expect(withDelay).to.deep.eq([true]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); @@ -259,7 +491,114 @@ contract('GovernorTimelockAccess', function (accounts) { ]); }); - it('cancel calls already canceled by guardian', async function () { + it('cancels restricted with queueing if the same operation is part of a more recent proposal (internal)', async function () { + // Set proposals + const original = new GovernorHelper(this.mock, mode); + await original.setProposal([this.restricted.operation], 'descr'); + + // Go through all the governance process + await original.propose(); + expect(await this.mock.proposalNeedsQueuing(original.currentProposal.id)).to.be.eq(true); + const { + delay: planDelay, + indirect, + withDelay, + } = await this.mock.proposalExecutionPlan(original.currentProposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN(delay)); + expect(indirect).to.deep.eq([true]); + expect(withDelay).to.deep.eq([true]); + await original.waitForSnapshot(); + await original.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await original.waitForDeadline(); + await original.queue(); + + // Cancel the operation in the manager + await this.manager.cancel( + this.mock.address, + this.restricted.operation.target, + this.restricted.operation.data, + { from: admin }, + ); + + // Another proposal is added with the same operation + const rescheduled = new GovernorHelper(this.mock, mode); + await rescheduled.setProposal([this.restricted.operation], 'another descr'); + + // Queue the new proposal + await rescheduled.propose(); + await rescheduled.waitForSnapshot(); + await rescheduled.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await rescheduled.waitForDeadline(); + await rescheduled.queue(); // This will schedule it again in the manager + + // Cancel + const eta = await this.mock.proposalEta(rescheduled.currentProposal.id); + const txCancel = await original.cancel('internal'); + expectEvent(txCancel, 'ProposalCanceled', { proposalId: original.currentProposal.id }); + + await time.increase(eta); // waitForEta() + await expectRevertCustomError(original.execute(), 'GovernorUnexpectedProposalState', [ + original.currentProposal.id, + Enums.ProposalState.Canceled, + proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]), + ]); + }); + + it('cancels unrestricted with queueing (internal)', async function () { + this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr'); + + await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0')); + expect(indirect).to.deep.eq([false]); + expect(withDelay).to.deep.eq([false]); + + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + await this.helper.queue(); + + const eta = await this.mock.proposalEta(this.proposal.id); + const txCancel = await this.helper.cancel('internal'); + expectEvent(txCancel, 'ProposalCanceled', { proposalId: this.proposal.id }); + + await time.increase(eta); // waitForEta() + await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [ + this.proposal.id, + Enums.ProposalState.Canceled, + proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]), + ]); + }); + + it('cancels unrestricted without queueing (internal)', async function () { + this.proposal = await this.helper.setProposal([this.unrestricted.operation], 'descr'); + + await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0')); + expect(indirect).to.deep.eq([false]); + expect(withDelay).to.deep.eq([false]); + + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + // await this.helper.queue(); + + // const eta = await this.mock.proposalEta(this.proposal.id); + const txCancel = await this.helper.cancel('internal'); + expectEvent(txCancel, 'ProposalCanceled', { proposalId: this.proposal.id }); + + // await time.increase(eta); // waitForEta() + await expectRevertCustomError(this.helper.execute(), 'GovernorUnexpectedProposalState', [ + this.proposal.id, + Enums.ProposalState.Canceled, + proposalStatesToBitMap([Enums.ProposalState.Succeeded, Enums.ProposalState.Queued]), + ]); + }); + + it('cancels calls already canceled by guardian', async function () { const operationA = { target: this.receiver.address, data: this.restricted.selector + '00' }; const operationB = { target: this.receiver.address, data: this.restricted.selector + '01' }; const operationC = { target: this.receiver.address, data: this.restricted.selector + '02' }; @@ -353,6 +692,12 @@ contract('GovernorTimelockAccess', function (accounts) { ); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false); + const { delay: planDelay, indirect, withDelay } = await this.mock.proposalExecutionPlan(this.proposal.id); + expect(planDelay).to.be.bignumber.eq(web3.utils.toBN('0')); + expect(indirect).to.deep.eq([]); // Governor operations ignore access manager + expect(withDelay).to.deep.eq([]); // Governor operations ignore access manager + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); @@ -393,8 +738,14 @@ contract('GovernorTimelockAccess', function (accounts) { await this.manager.setTargetFunctionRole(target, [selector], roleId, { from: admin }); await this.manager.grantRole(roleId, this.mock.address, 0, { from: admin }); - await this.helper.setProposal([{ target, data, value: '0' }], '1'); + const proposal = await this.helper.setProposal([{ target, data, value: '0' }], '1'); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false); + const plan = await this.mock.proposalExecutionPlan(proposal.id); + expect(plan.delay).to.be.bignumber.eq(web3.utils.toBN('0')); + expect(plan.indirect).to.deep.eq([true]); + expect(plan.withDelay).to.deep.eq([false]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); @@ -406,8 +757,14 @@ contract('GovernorTimelockAccess', function (accounts) { await this.mock.$_setAccessManagerIgnored(target, selector, true); - await this.helper.setProposal([{ target, data, value: '0' }], '2'); + const proposalIgnored = await this.helper.setProposal([{ target, data, value: '0' }], '2'); await this.helper.propose(); + expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.eq(false); + const planIgnored = await this.mock.proposalExecutionPlan(proposalIgnored.id); + expect(planIgnored.delay).to.be.bignumber.eq(web3.utils.toBN('0')); + expect(planIgnored.indirect).to.deep.eq([false]); + expect(planIgnored.withDelay).to.deep.eq([false]); + await this.helper.waitForSnapshot(); await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); await this.helper.waitForDeadline(); diff --git a/test/helpers/access-manager.js b/test/helpers/access-manager.js new file mode 100644 index 000000000..7dfc4c33d --- /dev/null +++ b/test/helpers/access-manager.js @@ -0,0 +1,69 @@ +const { time } = require('@openzeppelin/test-helpers'); +const { MAX_UINT64 } = require('./constants'); +const { artifacts } = require('hardhat'); + +function buildBaseRoles() { + const roles = { + ADMIN: { + id: web3.utils.toBN(0), + }, + SOME_ADMIN: { + id: web3.utils.toBN(17), + }, + SOME_GUARDIAN: { + id: web3.utils.toBN(35), + }, + SOME: { + id: web3.utils.toBN(42), + }, + PUBLIC: { + id: MAX_UINT64, + }, + }; + + // Names + Object.entries(roles).forEach(([name, role]) => (role.name = name)); + + // Defaults + for (const role of Object.keys(roles)) { + roles[role].admin = roles.ADMIN; + roles[role].guardian = roles.ADMIN; + } + + // Admins + roles.SOME.admin = roles.SOME_ADMIN; + + // Guardians + roles.SOME.guardian = roles.SOME_GUARDIAN; + + return roles; +} + +const formatAccess = access => [access[0], access[1].toString()]; + +const MINSETBACK = time.duration.days(5); +const EXPIRATION = time.duration.weeks(1); + +let EXECUTION_ID_STORAGE_SLOT = 3n; +let CONSUMING_SCHEDULE_STORAGE_SLOT = 0n; +try { + // Try to get the artifact paths, will throw if it doesn't exist + artifacts._getArtifactPathSync('AccessManagerUpgradeable'); + artifacts._getArtifactPathSync('AccessManagedUpgradeable'); + + // ERC-7201 namespace location for AccessManager + EXECUTION_ID_STORAGE_SLOT += 0x40c6c8c28789853c7efd823ab20824bbd71718a8a5915e855f6f288c9a26ad00n; + // ERC-7201 namespace location for AccessManaged + CONSUMING_SCHEDULE_STORAGE_SLOT += 0xf3177357ab46d8af007ab3fdb9af81da189e1068fefdc0073dca88a2cab40a00n; +} catch (_) { + // eslint-disable-next-line no-empty +} + +module.exports = { + buildBaseRoles, + formatAccess, + MINSETBACK, + EXPIRATION, + EXECUTION_ID_STORAGE_SLOT, + CONSUMING_SCHEDULE_STORAGE_SLOT, +}; diff --git a/test/metatx/ERC2771Forwarder.test.js b/test/metatx/ERC2771Forwarder.test.js index 209c84b2f..0e0998832 100644 --- a/test/metatx/ERC2771Forwarder.test.js +++ b/test/metatx/ERC2771Forwarder.test.js @@ -16,7 +16,6 @@ contract('ERC2771Forwarder', function (accounts) { from: another, value: web3.utils.toWei('0.5'), data: '0x1742', - deadline: 0xdeadbeef, }; beforeEach(async function () { diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 499cf0628..fa66785f0 100644 --- a/test/token/ERC20/extensions/ERC4626.test.js +++ b/test/token/ERC20/extensions/ERC4626.test.js @@ -88,7 +88,7 @@ contract('ERC4626', function (accounts) { const sharesForDeposit = await vault.previewDeposit(value, { from: holder }); const sharesForReenter = await vault.previewDeposit(reenterValue, { from: holder }); - // Do deposit normally, triggering the _beforeTokenTransfer hook + // Deposit normally, reentering before the internal `_update` const receipt = await vault.deposit(value, holder, { from: holder }); // Main deposit event @@ -170,7 +170,7 @@ contract('ERC4626', function (accounts) { // Price before const sharesBefore = await vault.previewDeposit(value); - // Deposit, triggering the _beforeTokenTransfer hook + // Deposit, reentering before the internal `_update` const receipt = await vault.deposit(value, holder, { from: holder }); // Price is as previewed