diff --git a/contracts/access/manager/AccessManaged.sol b/contracts/access/manager/AccessManaged.sol index 087aee4c4..59265017a 100644 --- a/contracts/access/manager/AccessManaged.sol +++ b/contracts/access/manager/AccessManaged.sol @@ -90,7 +90,8 @@ abstract contract AccessManaged is Context, IAccessManaged { } /** - * @dev Transfers control to a new authority. Internal function with no access restriction. + * @dev Transfers control to a new authority. Internal function with no access restriction. Allows bypassing the + * permissions set by the current authority. */ function _setAuthority(address newAuthority) internal virtual { _authority = newAuthority; diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol index 6ecad9921..929a6736b 100644 --- a/contracts/access/manager/AccessManager.sol +++ b/contracts/access/manager/AccessManager.sol @@ -52,7 +52,7 @@ contract AccessManager is Context, Multicall, IAccessManager { using Time for *; struct AccessMode { - uint64 familyId; + uint64 classId; bool closed; } @@ -78,7 +78,7 @@ contract AccessManager is Context, Multicall, IAccessManager { Time.Delay delay; // delay for granting } - struct Family { + struct Class { mapping(bytes4 selector => uint64 groupId) allowedGroups; Time.Delay adminDelay; } @@ -87,7 +87,7 @@ contract AccessManager is Context, Multicall, IAccessManager { uint64 public constant PUBLIC_GROUP = type(uint64).max; // 2**64-1 mapping(address target => AccessMode mode) private _contractMode; - mapping(uint64 familyId => Family) private _families; + mapping(uint64 classId => Class) private _classes; mapping(uint64 groupId => Group) private _groups; mapping(bytes32 operationId => uint48 schedule) private _schedules; mapping(bytes4 selector => Time.Delay delay) private _adminDelays; @@ -127,7 +127,7 @@ contract AccessManager is Context, Multicall, IAccessManager { * to identify the indirect workflow, and will consider call that require a delay to be forbidden. */ function canCall(address caller, address target, bytes4 selector) public view virtual returns (bool, uint32) { - (uint64 familyId, bool closed) = getContractFamily(target); + (uint64 classId, bool closed) = getContractClass(target); if (closed) { return (false, 0); } else if (caller == address(this)) { @@ -135,7 +135,7 @@ contract AccessManager is Context, Multicall, IAccessManager { // verify that the call "identifier", which is set during the relay call, is correct. return (_relayIdentifier == _hashRelayIdentifier(target, selector), 0); } else { - uint64 groupId = getFamilyFunctionGroup(familyId, selector); + uint64 groupId = getClassFunctionGroup(classId, selector); (bool inGroup, uint32 currentDelay) = hasGroup(groupId, caller); return inGroup ? (currentDelay == 0, currentDelay) : (false, 0); } @@ -158,21 +158,21 @@ contract AccessManager is Context, Multicall, IAccessManager { /** * @dev Get the mode under which a contract is operating. */ - function getContractFamily(address target) public view virtual returns (uint64, bool) { + function getContractClass(address target) public view virtual returns (uint64, bool) { AccessMode storage mode = _contractMode[target]; - return (mode.familyId, mode.closed); + return (mode.classId, mode.closed); } /** * @dev Get the permission level (group) required to call a function. This only applies for contract that are * operating under the `Custom` mode. */ - function getFamilyFunctionGroup(uint64 familyId, bytes4 selector) public view virtual returns (uint64) { - return _families[familyId].allowedGroups[selector]; + function getClassFunctionGroup(uint64 classId, bytes4 selector) public view virtual returns (uint64) { + return _classes[classId].allowedGroups[selector]; } - function getFamilyAdminDelay(uint64 familyId) public view virtual returns (uint32) { - return _families[familyId].adminDelay.get(); + function getClassAdminDelay(uint64 classId) public view virtual returns (uint32) { + return _classes[classId].adminDelay.get(); } /** @@ -250,11 +250,17 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Add `account` to `groupId`. This gives him the authorization to call any function that is restricted to - * this group. An optional execution delay (in seconds) can be set. If that delay is non 0, the user is required - * to schedule any operation that is restricted to members this group. The user will only be able to execute the - * operation after the delay expires. During this delay, admin and guardians can cancel the operation (see - * {cancel}). + * @dev Add `account` to `groupId`, or change its execution delay. + * + * This gives the account the authorization to call any function that is restricted to this group. An optional + * execution delay (in seconds) can be set. If that delay is non 0, the user is required to schedule any operation + * that is restricted to members this group. The user will only be able to execute the operation after the delay 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 group, 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 + * 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: * @@ -267,7 +273,8 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Remove an account for a group, with immediate effect. + * @dev Remove an account for a group, with immediate effect. If the sender is not in the group, this call has no + * effect. * * Requirements: * @@ -280,7 +287,8 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Renounce group permissions for the calling account, with immediate effect. + * @dev Renounce group permissions for the calling account, with immediate effect. If the sender is not in + * the group, this call has no effect. * * Requirements: * @@ -295,22 +303,6 @@ contract AccessManager is Context, Multicall, IAccessManager { _revokeGroup(groupId, callerConfirmation); } - /** - * @dev Set the execution delay for a given account in a given group. This update is not immediate and follows the - * delay rules. For example, If a user currently has a delay of 3 hours, and this is called to reduce that delay to - * 1 hour, the new delay will take some time to take effect, enforcing that any operation executed in the 3 hours - * that follows this update was indeed scheduled before this update. - * - * Requirements: - * - * - the caller must be in the group's admins - * - * Emits a {GroupExecutionDelayUpdated} event - */ - function setExecuteDelay(uint64 groupId, address account, uint32 newDelay) public virtual onlyAuthorized { - _setExecuteDelay(groupId, account, newDelay); - } - /** * @dev Change admin group for a given group. * @@ -351,57 +343,57 @@ contract AccessManager is Context, Multicall, IAccessManager { } /** - * @dev Internal version of {grantGroup} without access control. + * @dev Internal version of {grantGroup} without access control. Returns true if the group was newly granted. * * Emits a {GroupGranted} event */ - function _grantGroup(uint64 groupId, address account, uint32 grantDelay, uint32 executionDelay) internal virtual { + function _grantGroup( + uint64 groupId, + address account, + uint32 grantDelay, + uint32 executionDelay + ) internal virtual returns (bool) { if (groupId == PUBLIC_GROUP) { revert AccessManagerLockedGroup(groupId); - } else if (_groups[groupId].members[account].since != 0) { - revert AccessManagerAccountAlreadyInGroup(groupId, account); } - uint48 since = Time.timestamp() + grantDelay; - _groups[groupId].members[account] = Access({since: since, delay: executionDelay.toDelay()}); + bool inGroup = _groups[groupId].members[account].since != 0; - emit GroupGranted(groupId, account, since, executionDelay); + uint48 since; + + if (inGroup) { + (_groups[groupId].members[account].delay, since) = _groups[groupId].members[account].delay.withUpdate( + executionDelay, + minSetback() + ); + } else { + since = Time.timestamp() + grantDelay; + _groups[groupId].members[account] = Access({since: since, delay: executionDelay.toDelay()}); + } + + emit GroupGranted(groupId, account, executionDelay, since); + return !inGroup; } /** * @dev Internal version of {revokeGroup} without access control. This logic is also used by {renounceGroup}. + * Returns true if the group was previously granted. * * Emits a {GroupRevoked} event */ - function _revokeGroup(uint64 groupId, address account) internal virtual { + function _revokeGroup(uint64 groupId, address account) internal virtual returns (bool) { if (groupId == PUBLIC_GROUP) { revert AccessManagerLockedGroup(groupId); - } else if (_groups[groupId].members[account].since == 0) { - revert AccessManagerAccountNotInGroup(groupId, account); + } + + if (_groups[groupId].members[account].since == 0) { + return false; } delete _groups[groupId].members[account]; emit GroupRevoked(groupId, account); - } - - /** - * @dev Internal version of {setExecuteDelay} without access control. - * - * Emits a {GroupExecutionDelayUpdated} event. - */ - function _setExecuteDelay(uint64 groupId, address account, uint32 newDuration) internal virtual { - if (groupId == PUBLIC_GROUP || groupId == ADMIN_GROUP) { - revert AccessManagerLockedGroup(groupId); - } else if (_groups[groupId].members[account].since == 0) { - revert AccessManagerAccountNotInGroup(groupId, account); - } - - Time.Delay updated = _groups[groupId].members[account].delay.withUpdate(newDuration, minSetback()); - _groups[groupId].members[account].delay = updated; - - (, , uint48 effect) = updated.unpack(); - emit GroupExecutionDelayUpdated(groupId, account, newDuration, effect); + return true; } /** @@ -444,10 +436,9 @@ contract AccessManager is Context, Multicall, IAccessManager { revert AccessManagerLockedGroup(groupId); } - Time.Delay updated = _groups[groupId].delay.withUpdate(newDelay, minSetback()); + (Time.Delay updated, uint48 effect) = _groups[groupId].delay.withUpdate(newDelay, minSetback()); _groups[groupId].delay = updated; - (, , uint48 effect) = updated.unpack(); emit GroupGrantDelayChanged(groupId, newDelay, effect); } @@ -462,13 +453,13 @@ contract AccessManager is Context, Multicall, IAccessManager { * * Emits a {FunctionAllowedGroupUpdated} event per selector */ - function setFamilyFunctionGroup( - uint64 familyId, + function setClassFunctionGroup( + uint64 classId, bytes4[] calldata selectors, uint64 groupId ) public virtual onlyAuthorized { for (uint256 i = 0; i < selectors.length; ++i) { - _setFamilyFunctionGroup(familyId, selectors[i], groupId); + _setClassFunctionGroup(classId, selectors[i], groupId); } } @@ -477,14 +468,14 @@ contract AccessManager is Context, Multicall, IAccessManager { * * Emits a {FunctionAllowedGroupUpdated} event */ - function _setFamilyFunctionGroup(uint64 familyId, bytes4 selector, uint64 groupId) internal virtual { - _checkValidFamilyId(familyId); - _families[familyId].allowedGroups[selector] = groupId; - emit FamilyFunctionGroupUpdated(familyId, selector, groupId); + function _setClassFunctionGroup(uint64 classId, bytes4 selector, uint64 groupId) internal virtual { + _checkValidClassId(classId); + _classes[classId].allowedGroups[selector] = groupId; + emit ClassFunctionGroupUpdated(classId, selector, groupId); } /** - * @dev Set the delay for management operations on a given family of contract. + * @dev Set the delay for management operations on a given class of contract. * * Requirements: * @@ -492,54 +483,57 @@ contract AccessManager is Context, Multicall, IAccessManager { * * Emits a {FunctionAllowedGroupUpdated} event per selector */ - function setFamilyAdminDelay(uint64 familyId, uint32 newDelay) public virtual onlyAuthorized { - _setFamilyAdminDelay(familyId, newDelay); + function setClassAdminDelay(uint64 classId, uint32 newDelay) public virtual onlyAuthorized { + _setClassAdminDelay(classId, newDelay); } /** - * @dev Internal version of {setFamilyAdminDelay} without access control. + * @dev Internal version of {setClassAdminDelay} without access control. * - * Emits a {FamilyAdminDelayUpdated} event + * Emits a {ClassAdminDelayUpdated} event */ - function _setFamilyAdminDelay(uint64 familyId, uint32 newDelay) internal virtual { - _checkValidFamilyId(familyId); - Time.Delay updated = _families[familyId].adminDelay.withUpdate(newDelay, minSetback()); - _families[familyId].adminDelay = updated; - (, , uint48 effect) = updated.unpack(); - emit FamilyAdminDelayUpdated(familyId, newDelay, effect); + function _setClassAdminDelay(uint64 classId, uint32 newDelay) internal virtual { + _checkValidClassId(classId); + (Time.Delay updated, uint48 effect) = _classes[classId].adminDelay.withUpdate(newDelay, minSetback()); + _classes[classId].adminDelay = updated; + emit ClassAdminDelayUpdated(classId, newDelay, effect); } /** - * @dev Reverts if `familyId` is 0. + * @dev Reverts if `classId` is 0. This is the default class id given to contracts and it should not have any + * configurations. */ - function _checkValidFamilyId(uint64 familyId) private pure { - if (familyId == 0) { - revert AccessManagerInvalidFamily(familyId); + function _checkValidClassId(uint64 classId) private pure { + if (classId == 0) { + revert AccessManagerInvalidClass(classId); } } // =============================================== MODE MANAGEMENT ================================================ /** - * @dev Set the family of a contract. + * @dev Set the class of a contract. * * Requirements: * * - the caller must be a global admin * - * Emits a {ContractFamilyUpdated} event. + * Emits a {ContractClassUpdated} event. */ - function setContractFamily(address target, uint64 familyId) public virtual onlyAuthorized { - _setContractFamily(target, familyId); + function setContractClass(address target, uint64 classId) public virtual onlyAuthorized { + _setContractClass(target, classId); } /** - * @dev Set the family of a contract. This is an internal setter with no access restrictions. + * @dev Set the class of a contract. This is an internal setter with no access restrictions. * - * Emits a {ContractFamilyUpdated} event. + * Emits a {ContractClassUpdated} event. */ - function _setContractFamily(address target, uint64 familyId) internal virtual { - _contractMode[target].familyId = familyId; - emit ContractFamilyUpdated(target, familyId); + function _setContractClass(address target, uint64 classId) internal virtual { + if (target == address(this)) { + revert AccessManagerLockedAccount(target); + } + _contractMode[target].classId = classId; + emit ContractClassUpdated(target, classId); } /** @@ -561,6 +555,9 @@ contract AccessManager is Context, Multicall, IAccessManager { * Emits a {ContractClosed} event. */ function _setContractClosed(address target, bool closed) internal virtual { + if (target == address(this)) { + revert AccessManagerLockedAccount(target); + } _contractMode[target].closed = closed; emit ContractClosed(target, closed); } @@ -700,9 +697,9 @@ contract AccessManager is Context, Multicall, IAccessManager { revert AccessManagerNotScheduled(operationId); } else if (caller != msgsender) { // calls can only be canceled by the account that scheduled them, a global admin, or by a guardian of the required group. - (uint64 familyId, ) = getContractFamily(target); + (uint64 classId, ) = getContractClass(target); (bool isAdmin, ) = hasGroup(ADMIN_GROUP, msgsender); - (bool isGuardian, ) = hasGroup(getGroupGuardian(getFamilyFunctionGroup(familyId, selector)), msgsender); + (bool isGuardian, ) = hasGroup(getGroupGuardian(getClassFunctionGroup(classId, selector)), msgsender); if (!isAdmin && !isGuardian) { revert AccessManagerCannotCancel(msgsender, caller, target, selector); } @@ -762,32 +759,30 @@ contract AccessManager is Context, Multicall, IAccessManager { function _getAdminRestrictions(bytes calldata data) private view returns (bool, uint64, uint32) { bytes4 selector = bytes4(data); - if (selector == this.updateAuthority.selector || selector == this.setContractFamily.selector) { - // First argument is a target. Restricted to ADMIN with the family delay corresponding to the target's family + if (data.length < 4) { + return (false, 0, 0); + } else if (selector == this.updateAuthority.selector || selector == this.setContractClass.selector) { + // First argument is a target. Restricted to ADMIN with the class delay corresponding to the target's class address target = abi.decode(data[0x04:0x24], (address)); - (uint64 familyId, ) = getContractFamily(target); - uint32 delay = getFamilyAdminDelay(familyId); + (uint64 classId, ) = getContractClass(target); + uint32 delay = getClassAdminDelay(classId); return (true, ADMIN_GROUP, delay); - } else if (selector == this.setFamilyFunctionGroup.selector) { - // First argument is a family. Restricted to ADMIN with the family delay corresponding to the family - uint64 familyId = abi.decode(data[0x04:0x24], (uint64)); - uint32 delay = getFamilyAdminDelay(familyId); + } else if (selector == this.setClassFunctionGroup.selector) { + // First argument is a class. Restricted to ADMIN with the class delay corresponding to the class + uint64 classId = abi.decode(data[0x04:0x24], (uint64)); + uint32 delay = getClassAdminDelay(classId); return (true, ADMIN_GROUP, delay); } else if ( selector == this.labelGroup.selector || selector == this.setGroupAdmin.selector || selector == this.setGroupGuardian.selector || selector == this.setGrantDelay.selector || - selector == this.setFamilyAdminDelay.selector || + selector == this.setClassAdminDelay.selector || selector == this.setContractClosed.selector ) { // Restricted to ADMIN with no delay beside any execution delay the caller may have return (true, ADMIN_GROUP, 0); - } else if ( - selector == this.grantGroup.selector || - selector == this.revokeGroup.selector || - selector == this.setExecuteDelay.selector - ) { + } else if (selector == this.grantGroup.selector || selector == this.revokeGroup.selector) { // First argument is a groupId. Restricted to that group's admin with no delay beside any execution delay the caller may have. uint64 groupId = abi.decode(data[0x04:0x24], (uint64)); uint64 groupAdminId = getGroupAdmin(groupId); diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol index 659b4c5cd..312b4da74 100644 --- a/contracts/access/manager/IAccessManager.sol +++ b/contracts/access/manager/IAccessManager.sol @@ -22,27 +22,25 @@ interface IAccessManager { event OperationCanceled(bytes32 indexed operationId, uint48 schedule); event GroupLabel(uint64 indexed groupId, string label); - event GroupGranted(uint64 indexed groupId, address indexed account, uint48 since, uint32 delay); + event GroupGranted(uint64 indexed groupId, address indexed account, uint32 delay, uint48 since); event GroupRevoked(uint64 indexed groupId, address indexed account); - event GroupExecutionDelayUpdated(uint64 indexed groupId, address indexed account, uint32 delay, uint48 from); event GroupAdminChanged(uint64 indexed groupId, uint64 indexed admin); event GroupGuardianChanged(uint64 indexed groupId, uint64 indexed guardian); - event GroupGrantDelayChanged(uint64 indexed groupId, uint32 delay, uint48 from); + event GroupGrantDelayChanged(uint64 indexed groupId, uint32 delay, uint48 since); - event ContractFamilyUpdated(address indexed target, uint64 indexed familyId); + event ContractClassUpdated(address indexed target, uint64 indexed classId); event ContractClosed(address indexed target, bool closed); - event FamilyFunctionGroupUpdated(uint64 indexed familyId, bytes4 selector, uint64 indexed groupId); - event FamilyAdminDelayUpdated(uint64 indexed familyId, uint32 delay, uint48 from); + event ClassFunctionGroupUpdated(uint64 indexed classId, bytes4 selector, uint64 indexed groupId); + event ClassAdminDelayUpdated(uint64 indexed classId, uint32 delay, uint48 since); error AccessManagerAlreadyScheduled(bytes32 operationId); error AccessManagerNotScheduled(bytes32 operationId); error AccessManagerNotReady(bytes32 operationId); error AccessManagerExpired(bytes32 operationId); + error AccessManagerLockedAccount(address account); error AccessManagerLockedGroup(uint64 groupId); - error AccessManagerInvalidFamily(uint64 familyId); - error AccessManagerAccountAlreadyInGroup(uint64 groupId, address account); - error AccessManagerAccountNotInGroup(uint64 groupId, address account); + error AccessManagerInvalidClass(uint64 classId); error AccessManagerBadConfirmation(); error AccessManagerUnauthorizedAccount(address msgsender, uint64 groupId); error AccessManagerUnauthorizedCall(address caller, address target, bytes4 selector); @@ -56,11 +54,11 @@ interface IAccessManager { function expiration() external returns (uint32); - function getContractFamily(address target) external view returns (uint64 familyId, bool closed); + function getContractClass(address target) external view returns (uint64 classId, bool closed); - function getFamilyFunctionGroup(uint64 familyId, bytes4 selector) external view returns (uint64); + function getClassFunctionGroup(uint64 classId, bytes4 selector) external view returns (uint64); - function getFamilyAdminDelay(uint64 familyId) external view returns (uint32); + function getClassAdminDelay(uint64 classId) external view returns (uint32); function getGroupAdmin(uint64 groupId) external view returns (uint64); @@ -80,19 +78,17 @@ interface IAccessManager { function renounceGroup(uint64 groupId, address callerConfirmation) external; - function setExecuteDelay(uint64 groupId, address account, uint32 newDelay) external; - function setGroupAdmin(uint64 groupId, uint64 admin) external; function setGroupGuardian(uint64 groupId, uint64 guardian) external; function setGrantDelay(uint64 groupId, uint32 newDelay) external; - function setFamilyFunctionGroup(uint64 familyId, bytes4[] calldata selectors, uint64 groupId) external; + function setClassFunctionGroup(uint64 classId, bytes4[] calldata selectors, uint64 groupId) external; - function setFamilyAdminDelay(uint64 familyId, uint32 newDelay) external; + function setClassAdminDelay(uint64 classId, uint32 newDelay) external; - function setContractFamily(address target, uint64 familyId) external; + function setContractClass(address target, uint64 classId) external; function setContractClosed(address target, bool closed) external; diff --git a/contracts/utils/types/Time.sol b/contracts/utils/types/Time.sol index 07a3a68b1..9f640ec33 100644 --- a/contracts/utils/types/Time.sol +++ b/contracts/utils/types/Time.sol @@ -53,9 +53,13 @@ library Time { * * * The `Delay` type is 128 bits long, and packs the following: - * [000:031] uint32 for the current value (duration) - * [032:063] uint32 for the pending value (duration) - * [064:111] uint48 for the effect date (timepoint) + * + * ``` + * | [uint48]: effect date (timepoint) + * | | [uint32]: current value (duration) + * ↓ ↓ ↓ [uint32]: pending value (duration) + * 0xAAAAAAAAAAAABBBBBBBBCCCCCCCC + * ``` * * NOTE: The {get} and {update} function operate using timestamps. Block number based delays should use the * {getAt} and {withUpdateAt} variants of these functions. @@ -110,12 +114,14 @@ library Time { /** * @dev Update a Delay object so that it takes a new duration after at a timepoint that is automatically computed - * to enforce the old delay at the moment of the update. + * to 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) { + function withUpdate(Delay self, uint32 newValue, uint32 minSetback) internal view returns (Delay, uint48) { uint32 value = self.get(); uint32 setback = uint32(Math.max(minSetback, value > newValue ? value - newValue : 0)); - return self.withUpdateAt(newValue, timestamp() + setback); + uint48 effect = timestamp() + setback; + return (self.withUpdateAt(newValue, effect), effect); } /** diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js index 2d2f61d3e..538120e00 100644 --- a/test/access/manager/AccessManager.test.js +++ b/test/access/manager/AccessManager.test.js @@ -18,7 +18,7 @@ const GROUPS = { }; Object.assign(GROUPS, Object.fromEntries(Object.entries(GROUPS).map(([key, value]) => [value, key]))); -const familyId = web3.utils.toBN(1); +const classId = web3.utils.toBN(1); const executeDelay = web3.utils.toBN(10); const grantDelay = web3.utils.toBN(10); @@ -138,24 +138,15 @@ contract('AccessManager', function (accounts) { it('to a user that is already in the group', async function () { expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); - - await expectRevertCustomError( - this.manager.grantGroup(GROUPS.SOME, member, 0, { from: manager }), - 'AccessManagerAccountAlreadyInGroup', - [GROUPS.SOME, member], - ); + await this.manager.grantGroup(GROUPS.SOME, member, 0, { from: manager }); + expect(await this.manager.hasGroup(GROUPS.SOME, member).then(formatAccess)).to.be.deep.equal([true, '0']); }); it('to a user that is scheduled for joining the group', async function () { await this.manager.$_grantGroup(GROUPS.SOME, user, 10, 0); // grant delay 10 - expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - await expectRevertCustomError( - this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }), - 'AccessManagerAccountAlreadyInGroup', - [GROUPS.SOME, user], - ); + await this.manager.grantGroup(GROUPS.SOME, user, 0, { from: manager }); + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); }); it('grant group is restricted', async function () { @@ -212,6 +203,14 @@ contract('AccessManager', function (accounts) { expect(access[3]).to.be.bignumber.equal('0'); // effect }); }); + + it('cannot grant public group', async function () { + await expectRevertCustomError( + this.manager.$_grantGroup(GROUPS.PUBLIC, other, 0, executeDelay, { from: manager }), + 'AccessManagerLockedGroup', + [GROUPS.PUBLIC], + ); + }); }); describe('revoke group', function () { @@ -249,12 +248,8 @@ contract('AccessManager', function (accounts) { it('from a user that is not in the group', async function () { expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); - - await expectRevertCustomError( - this.manager.revokeGroup(GROUPS.SOME, user, { from: manager }), - 'AccessManagerAccountNotInGroup', - [GROUPS.SOME, user], - ); + await this.manager.revokeGroup(GROUPS.SOME, user, { from: manager }); + expect(await this.manager.hasGroup(GROUPS.SOME, user).then(formatAccess)).to.be.deep.equal([false, '0']); }); it('revoke group is restricted', async function () { @@ -300,11 +295,7 @@ contract('AccessManager', function (accounts) { }); it('for a user that is not in the group', async function () { - await expectRevertCustomError( - this.manager.renounceGroup(GROUPS.SOME, user, { from: user }), - 'AccessManagerAccountNotInGroup', - [GROUPS.SOME, user], - ); + await this.manager.renounceGroup(GROUPS.SOME, user, { from: user }); }); it('bad user confirmation', async function () { @@ -355,27 +346,27 @@ contract('AccessManager', function (accounts) { }); describe('change execution delay', function () { - it('increassing the delay has immediate effect', async 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.$_setExecuteDelay(GROUPS.SOME, member, oldDelay); + await this.manager.$_grantGroup(GROUPS.SOME, member, 0, oldDelay); const accessBefore = await this.manager.getAccess(GROUPS.SOME, member); expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect - const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, member, newDelay, { + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, member, newDelay, { from: manager, }); const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'GroupExecutionDelayUpdated', { + expectEvent(receipt, 'GroupGranted', { groupId: GROUPS.SOME, account: member, + since: timestamp, delay: newDelay, - from: timestamp, }); // immediate effect @@ -385,27 +376,27 @@ contract('AccessManager', function (accounts) { expect(accessAfter[3]).to.be.bignumber.equal('0'); // effect }); - it('decreassing the delay takes time', async function () { + it('decreasing the delay takes time', async function () { const oldDelay = web3.utils.toBN(100); const newDelay = web3.utils.toBN(10); - await this.manager.$_setExecuteDelay(GROUPS.SOME, member, oldDelay); + await this.manager.$_grantGroup(GROUPS.SOME, member, 0, oldDelay); const accessBefore = await this.manager.getAccess(GROUPS.SOME, member); expect(accessBefore[1]).to.be.bignumber.equal(oldDelay); // currentDelay expect(accessBefore[2]).to.be.bignumber.equal('0'); // pendingDelay expect(accessBefore[3]).to.be.bignumber.equal('0'); // effect - const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, member, newDelay, { + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, member, newDelay, { from: manager, }); const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'GroupExecutionDelayUpdated', { + expectEvent(receipt, 'GroupGranted', { groupId: GROUPS.SOME, account: member, + since: timestamp.add(oldDelay).sub(newDelay), delay: newDelay, - from: timestamp.add(oldDelay).sub(newDelay), }); // delayed effect @@ -415,49 +406,23 @@ contract('AccessManager', function (accounts) { expect(accessAfter[3]).to.be.bignumber.equal(timestamp.add(oldDelay).sub(newDelay)); // effect }); - it('cannot set the delay of a non member', async function () { - await expectRevertCustomError( - this.manager.setExecuteDelay(GROUPS.SOME, other, executeDelay, { from: manager }), - 'AccessManagerAccountNotInGroup', - [GROUPS.SOME, other], - ); - }); - - it('cannot set the delay of public and admin groups', async function () { - for (const group of [GROUPS.PUBLIC, GROUPS.ADMIN]) { - await expectRevertCustomError( - this.manager.$_setExecuteDelay(group, other, executeDelay, { from: manager }), - 'AccessManagerLockedGroup', - [group], - ); - } - }); - it('can set a user execution delay during the grant delay', async function () { await this.manager.$_grantGroup(GROUPS.SOME, other, 10, 0); - const { receipt } = await this.manager.setExecuteDelay(GROUPS.SOME, other, executeDelay, { from: manager }); + const { receipt } = await this.manager.grantGroup(GROUPS.SOME, other, executeDelay, { from: manager }); const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'GroupExecutionDelayUpdated', { + expectEvent(receipt, 'GroupGranted', { groupId: GROUPS.SOME, account: other, + since: timestamp, delay: executeDelay, - from: timestamp, }); }); - - it('changing the execution delay is restricted', async function () { - await expectRevertCustomError( - this.manager.setExecuteDelay(GROUPS.SOME, member, executeDelay, { from: other }), - 'AccessManagerUnauthorizedAccount', - [GROUPS.SOME_ADMIN, other], - ); - }); }); describe('change grant delay', function () { - it('increassing the delay has immediate effect', async function () { + 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(GROUPS.SOME, oldDelay); @@ -467,12 +432,12 @@ contract('AccessManager', function (accounts) { const { receipt } = await this.manager.setGrantDelay(GROUPS.SOME, newDelay, { from: admin }); const timestamp = await clockFromReceipt.timestamp(receipt).then(web3.utils.toBN); - expectEvent(receipt, 'GroupGrantDelayChanged', { groupId: GROUPS.SOME, delay: newDelay, from: timestamp }); + expectEvent(receipt, 'GroupGrantDelayChanged', { groupId: GROUPS.SOME, delay: newDelay, since: timestamp }); expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(newDelay); }); - it('increassing the delay has delay effect', async function () { + it('increasing the delay has delay effect', async function () { const oldDelay = web3.utils.toBN(100); const newDelay = web3.utils.toBN(10); await this.manager.$_setGrantDelay(GROUPS.SOME, oldDelay); @@ -485,7 +450,7 @@ contract('AccessManager', function (accounts) { expectEvent(receipt, 'GroupGrantDelayChanged', { groupId: GROUPS.SOME, delay: newDelay, - from: timestamp.add(oldDelay).sub(newDelay), + since: timestamp.add(oldDelay).sub(newDelay), }); expect(await this.manager.getGroupGrantDelay(GROUPS.SOME)).to.be.bignumber.equal(oldDelay); @@ -525,36 +490,33 @@ contract('AccessManager', function (accounts) { it('admin can set function group', async function () { for (const sig of sigs) { - expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal(GROUPS.ADMIN); + expect(await this.manager.getClassFunctionGroup(classId, sig)).to.be.bignumber.equal(GROUPS.ADMIN); } - const { receipt: receipt1 } = await this.manager.setFamilyFunctionGroup(familyId, sigs, GROUPS.SOME, { + const { receipt: receipt1 } = await this.manager.setClassFunctionGroup(classId, sigs, GROUPS.SOME, { from: admin, }); for (const sig of sigs) { - expectEvent(receipt1, 'FamilyFunctionGroupUpdated', { - familyId, + expectEvent(receipt1, 'ClassFunctionGroupUpdated', { + classId, selector: sig, groupId: GROUPS.SOME, }); - expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal(GROUPS.SOME); + expect(await this.manager.getClassFunctionGroup(classId, sig)).to.be.bignumber.equal(GROUPS.SOME); } - const { receipt: receipt2 } = await this.manager.setFamilyFunctionGroup( - familyId, - [sigs[1]], - GROUPS.SOME_ADMIN, - { from: admin }, - ); - expectEvent(receipt2, 'FamilyFunctionGroupUpdated', { - familyId, + const { receipt: receipt2 } = await this.manager.setClassFunctionGroup(classId, [sigs[1]], GROUPS.SOME_ADMIN, { + from: admin, + }); + expectEvent(receipt2, 'ClassFunctionGroupUpdated', { + classId, selector: sigs[1], groupId: GROUPS.SOME_ADMIN, }); for (const sig of sigs) { - expect(await this.manager.getFamilyFunctionGroup(familyId, sig)).to.be.bignumber.equal( + expect(await this.manager.getClassFunctionGroup(classId, sig)).to.be.bignumber.equal( sig == sigs[1] ? GROUPS.SOME_ADMIN : GROUPS.SOME, ); } @@ -562,7 +524,7 @@ contract('AccessManager', function (accounts) { it('non-admin cannot set function group', async function () { await expectRevertCustomError( - this.manager.setFamilyFunctionGroup(familyId, sigs, GROUPS.SOME, { from: other }), + this.manager.setClassFunctionGroup(classId, sigs, GROUPS.SOME, { from: other }), 'AccessManagerUnauthorizedAccount', [other, GROUPS.ADMIN], ); @@ -600,25 +562,25 @@ contract('AccessManager', function (accounts) { // setup await Promise.all([ this.manager.$_setContractClosed(this.target.address, closed), - this.manager.$_setContractFamily(this.target.address, familyId), - fnGroup && this.manager.$_setFamilyFunctionGroup(familyId, selector('fnRestricted()'), fnGroup), - fnGroup && this.manager.$_setFamilyFunctionGroup(familyId, selector('fnUnrestricted()'), fnGroup), + this.manager.$_setContractClass(this.target.address, classId), + fnGroup && this.manager.$_setClassFunctionGroup(classId, selector('fnRestricted()'), fnGroup), + fnGroup && this.manager.$_setClassFunctionGroup(classId, selector('fnUnrestricted()'), fnGroup), ...callerGroups .filter(groupId => groupId != GROUPS.PUBLIC) .map(groupId => this.manager.$_grantGroup(groupId, user, 0, delay ?? 0)), ]); // post setup checks - const result = await this.manager.getContractFamily(this.target.address); - expect(result[0]).to.be.bignumber.equal(familyId); + const result = await this.manager.getContractClass(this.target.address); + expect(result[0]).to.be.bignumber.equal(classId); expect(result[1]).to.be.equal(closed); if (fnGroup) { expect( - await this.manager.getFamilyFunctionGroup(familyId, selector('fnRestricted()')), + await this.manager.getClassFunctionGroup(classId, selector('fnRestricted()')), ).to.be.bignumber.equal(fnGroup); expect( - await this.manager.getFamilyFunctionGroup(familyId, selector('fnUnrestricted()')), + await this.manager.getClassFunctionGroup(classId, selector('fnUnrestricted()')), ).to.be.bignumber.equal(fnGroup); } @@ -832,8 +794,8 @@ contract('AccessManager', function (accounts) { describe('Indirect execution corner-cases', async function () { beforeEach(async function () { - await this.manager.$_setContractFamily(this.target.address, familyId); - await this.manager.$_setFamilyFunctionGroup(familyId, this.callData, GROUPS.SOME); + await this.manager.$_setContractClass(this.target.address, classId); + await this.manager.$_setClassFunctionGroup(classId, this.callData, GROUPS.SOME); await this.manager.$_grantGroup(GROUPS.SOME, user, 0, executeDelay); }); @@ -996,13 +958,13 @@ contract('AccessManager', function (accounts) { }); describe('Contract is managed', function () { - beforeEach('add contract to family', async function () { - await this.manager.$_setContractFamily(this.ownable.address, familyId); + beforeEach('add contract to class', async function () { + await this.manager.$_setContractClass(this.ownable.address, classId); }); describe('function is open to specific group', function () { beforeEach(async function () { - await this.manager.$_setFamilyFunctionGroup(familyId, selector('$_checkOwner()'), groupId); + await this.manager.$_setClassFunctionGroup(classId, selector('$_checkOwner()'), groupId); }); it('directly call: reverts', async function () { @@ -1026,7 +988,7 @@ contract('AccessManager', function (accounts) { describe('function is open to public group', function () { beforeEach(async function () { - await this.manager.$_setFamilyFunctionGroup(familyId, selector('$_checkOwner()'), GROUPS.PUBLIC); + await this.manager.$_setClassFunctionGroup(classId, selector('$_checkOwner()'), GROUPS.PUBLIC); }); it('directly call: reverts', async function () { @@ -1087,16 +1049,16 @@ contract('AccessManager', function (accounts) { }); // TODO: test all admin functions - describe('family delays', function () { - const otherFamilyId = '2'; + describe('class delays', function () { + const otherClassId = '2'; const delay = '1000'; - beforeEach('set contract family', async function () { + beforeEach('set contract class', async function () { this.target = await AccessManagedTarget.new(this.manager.address); - await this.manager.setContractFamily(this.target.address, familyId, { from: admin }); + await this.manager.setContractClass(this.target.address, classId, { from: admin }); - this.call = () => this.manager.setContractFamily(this.target.address, otherFamilyId, { from: admin }); - this.data = this.manager.contract.methods.setContractFamily(this.target.address, otherFamilyId).encodeABI(); + this.call = () => this.manager.setContractClass(this.target.address, otherClassId, { from: admin }); + this.data = this.manager.contract.methods.setContractClass(this.target.address, otherClassId).encodeABI(); }); it('without delay: succeeds', async function () { @@ -1105,20 +1067,20 @@ contract('AccessManager', function (accounts) { // TODO: here we need to check increase and decrease. // - Increasing should have immediate effect - // - Decreassing should take time. + // - Decreasing should take time. describe('with delay', function () { beforeEach('set admin delay', async function () { - this.tx = await this.manager.setFamilyAdminDelay(familyId, delay, { from: admin }); + this.tx = await this.manager.setClassAdminDelay(classId, delay, { from: admin }); this.opId = web3.utils.keccak256( web3.eth.abi.encodeParameters(['address', 'address', 'bytes'], [admin, this.manager.address, this.data]), ); }); it('emits event and sets delay', async function () { - const from = await clockFromReceipt.timestamp(this.tx.receipt).then(web3.utils.toBN); - expectEvent(this.tx.receipt, 'FamilyAdminDelayUpdated', { familyId, delay, from }); + const since = await clockFromReceipt.timestamp(this.tx.receipt).then(web3.utils.toBN); + expectEvent(this.tx.receipt, 'ClassAdminDelayUpdated', { classId, delay, since }); - expect(await this.manager.getFamilyAdminDelay(familyId)).to.be.bignumber.equal(delay); + expect(await this.manager.getClassAdminDelay(classId)).to.be.bignumber.equal(delay); }); it('without prior scheduling: reverts', async function () {