Skip to content
Open
2 changes: 1 addition & 1 deletion snapshots/Hub.Operations.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"draw": "105931",
"eliminateDeficit: full": "59781",
"eliminateDeficit: partial": "69429",
"mintFeeShares": "86130",
"mintFeeShares": "86218",
"payFee": "72302",
"refreshPremium": "71999",
"remove: full": "76993",
Expand Down
173 changes: 161 additions & 12 deletions src/access/AccessManagerEnumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,77 @@ import {IAccessManagerEnumerable} from 'src/access/interfaces/IAccessManagerEnum
contract AccessManagerEnumerable is AccessManager, IAccessManagerEnumerable {
using EnumerableSet for EnumerableSet.AddressSet;
using EnumerableSet for EnumerableSet.Bytes32Set;
using EnumerableSet for EnumerableSet.UintSet;

/// @dev Set of all role identifiers.
EnumerableSet.UintSet private _rolesSet;

/// @dev Set of all admin role identifiers.
EnumerableSet.UintSet private _adminRolesSet;

/// @dev Map of role identifiers to their respective member sets.
mapping(uint64 roleId => EnumerableSet.AddressSet) private _roleMembers;

/// @dev Map of admin role identifiers to their respective role identifier sets.
mapping(uint64 roleId => EnumerableSet.UintSet) private _adminOfRoles;

/// @dev Map of role identifiers to their respective target contract addresses.
mapping(uint64 roleId => EnumerableSet.AddressSet) private _roleTargets;

/// @dev Map of target contract addresses and function selectors to their assigned role identifier.
mapping(address target => mapping(bytes4 selector => uint64 roleId)) private _targetSelectorRoles;

/// @dev Map of role identifiers and target contract addresses to their respective set of function selectors.
mapping(uint64 roleId => mapping(address target => EnumerableSet.Bytes32Set))
private _roleTargetFunctions;
private _roleTargetSelectors;

/// @dev Constructor.
/// @param initialAdmin_ The address of the initial admin.
constructor(address initialAdmin_) AccessManager(initialAdmin_) {
// Track the ADMIN_ROLE by default.
// (already tracked as a default role via AccessManager constructor)
_adminRolesSet.add(ADMIN_ROLE);
}

/// @inheritdoc IAccessManagerEnumerable
function getRole(uint256 index) external view returns (uint64) {
return uint64(_rolesSet.at(index));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do safecast here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the RoleId that is returned here is always passed as uint64 before being added in the Set, so I don't see a scenario were we can overflow the uint64 when fetching it back.
Kind of like we did with getRoleTargetFunction

}

/// @inheritdoc IAccessManagerEnumerable
function getRoleCount() external view returns (uint256) {
return _rolesSet.length();
}

/// @inheritdoc IAccessManagerEnumerable
function getRoles(uint256 start, uint256 end) external view returns (uint64[] memory) {
uint256[] memory listedRoles = _rolesSet.values(start, end);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not just return _rolesSet.values(start, end)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if yes, do we need this fn anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roleIds are uint64 everywhere else, so I think better to return in the same uint64 format here

uint64[] memory roles;
assembly ('memory-safe') {
roles := listedRoles
}
return roles;
}

constructor(address initialAdmin_) AccessManager(initialAdmin_) {}
/// @inheritdoc IAccessManagerEnumerable
function getAdminRole(uint256 index) external view returns (uint64) {
return uint64(_adminRolesSet.at(index));
}

/// @inheritdoc IAccessManagerEnumerable
function getAdminRoleCount() external view returns (uint256) {
return _adminRolesSet.length();
}

/// @inheritdoc IAccessManagerEnumerable
function getAdminRoles(uint256 start, uint256 end) external view returns (uint64[] memory) {
uint256[] memory listedAdminRoles = _adminRolesSet.values(start, end);
uint64[] memory adminRoles;
assembly ('memory-safe') {
adminRoles := listedAdminRoles
}
return adminRoles;
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleMember(uint64 roleId, uint256 index) external view returns (address) {
Expand All @@ -42,38 +104,122 @@ contract AccessManagerEnumerable is AccessManager, IAccessManagerEnumerable {
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargetFunction(
function getAdminOfRole(uint64 adminRoleId, uint256 index) external view returns (uint64) {
return uint64(_adminOfRoles[adminRoleId].at(index));
}

/// @inheritdoc IAccessManagerEnumerable
function getAdminOfRoleCount(uint64 adminRoleId) external view returns (uint256) {
return _adminOfRoles[adminRoleId].length();
}

/// @inheritdoc IAccessManagerEnumerable
function getAdminOfRoles(
uint64 adminRoleId,
uint256 start,
uint256 end
) external view returns (uint64[] memory) {
uint256[] memory listedRoles = _adminOfRoles[adminRoleId].values(start, end);
uint64[] memory roles;
assembly ('memory-safe') {
roles := listedRoles
}
return roles;
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTarget(uint64 roleId, uint256 index) external view returns (address) {
return _roleTargets[roleId].at(index);
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargetCount(uint64 roleId) external view returns (uint256) {
return _roleTargets[roleId].length();
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargets(
uint64 roleId,
uint256 start,
uint256 end
) external view returns (address[] memory) {
return _roleTargets[roleId].values(start, end);
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargetSelector(
uint64 roleId,
address target,
uint256 index
) external view returns (bytes4) {
return bytes4(_roleTargetFunctions[roleId][target].at(index));
return bytes4(_roleTargetSelectors[roleId][target].at(index));
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargetFunctionCount(
function getRoleTargetSelectorCount(
uint64 roleId,
address target
) external view returns (uint256) {
return _roleTargetFunctions[roleId][target].length();
return _roleTargetSelectors[roleId][target].length();
}

/// @inheritdoc IAccessManagerEnumerable
function getRoleTargetFunctions(
function getRoleTargetSelectors(
uint64 roleId,
address target,
uint256 start,
uint256 end
) external view returns (bytes4[] memory) {
bytes32[] memory targetFunctions = _roleTargetFunctions[roleId][target].values(start, end);
bytes32[] memory targetFunctions = _roleTargetSelectors[roleId][target].values(start, end);
bytes4[] memory targetFunctionSelectors;
assembly ('memory-safe') {
targetFunctionSelectors := targetFunctions
}
return targetFunctionSelectors;
}

/// @dev Override AccessManager `_grantRole` function to track role members.
/// @dev Tracks all role identifiers when a new role is created.
function _trackRole(uint64 roleId) internal {
_rolesSet.add(uint256(roleId));
}

/// @dev Tracks all admin role identifiers when a new admin role is set.
function _trackAdminRole(uint64 roleId, uint64 admin) internal {
_adminRolesSet.add(uint256(admin));
uint64 oldAdmin = getRoleAdmin(roleId);
_adminOfRoles[oldAdmin].remove(uint256(roleId));
_adminOfRoles[admin].add(uint256(roleId));
}

/// @dev Tracks all targets where a selector was assigned to a role.
function _trackRoleTarget(uint64 roleId, address target, bytes4 selector) internal {
uint64 oldRole = _targetSelectorRoles[target][selector];
if (oldRole == roleId) {
return;
}
if (oldRole != ADMIN_ROLE && _roleTargetSelectors[oldRole][target].length() == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's weird/error-prone the fact that we are checking smt that it ve just updated right before this call (in _setTargetFunctionRole).

_roleTargets[oldRole].remove(target);
}
if (roleId != ADMIN_ROLE) {
_roleTargets[roleId].add(target);
}
_targetSelectorRoles[target][selector] = roleId;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels incosistent... we are adding ADMIN_ROLE here, but not in _roleTargets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because by default _targetSelectorRoles for a given target & selector will return ADMIN_ROLE (0), so if they are set back to ADMIN_ROLE we need to track it accordingly here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good pt. It makes sense also not to do it in _roleTargets because by default all contracts are assigned to ADMIN_ROLE. Worth adding a comment in their getter functions

}

/// @dev Override AccessManager `_setRoleAdmin` function to track created roles.
function _setRoleAdmin(uint64 roleId, uint64 admin) internal override {
_trackRole(roleId);
Copy link
Member

@miguelmtzinf miguelmtzinf Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but we do not know which role is the admin role of a given role.
same for guardian

Copy link
Contributor Author

@Kogaroshi Kogaroshi Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

=> can fetch with :
function getRoleAdmin(uint64 roleId) public view virtual returns (uint64) {

function getRoleGuardian(uint64 roleId) public view virtual returns (uint64) {

Copy link
Member

@miguelmtzinf miguelmtzinf Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we will have all roles stored in a single mapping, no matter whether they are admin/guardian roles or not. Then, it's only possible to detect if the role is admin/guardian role by fetching all existing roles and calling getRoleAdmin and getRoleGuardian for each one, correct?

Copy link
Contributor Author

@Kogaroshi Kogaroshi Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's currently the only way.
Do we also want to introduce Enum sets for the Guardians & Admins, returning a list of roles[] for a given guardian or admin, and the lists of all roles that are set as admin or guardian to another role ? (that would still mean we need to fetch from the admin/guardian to get the list of concerned roles, but would allow to fetch from both direction) ?

_trackAdminRole(roleId, admin);
super._setRoleAdmin(roleId, admin);
}

/// @dev Override AccessManager `_setRoleGuardian` function to track created roles.
function _setRoleGuardian(uint64 roleId, uint64 guardian) internal override {
_trackRole(roleId);
super._setRoleGuardian(roleId, guardian);
}

/// @dev Override AccessManager `_grantRole` function to track roles' membership.
function _grantRole(
uint64 roleId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if roleId is ADMIN_ROLE or PUBLIC_ROLE? No need to track ig

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will track the ADMIN_ROLE, and it's members, since the constructor does _grantRole(ADMIN_ROLE, initialAdmin, 0, 0)
The PUBLIC_ROLE is not track indeed, which is okay since by naming it's public, so defined by default and accepts all members.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you are right. ADMIN_ROLE is always being tracked.
PUBLIC_ROLE does not matter as all role-related functions will revert if they are triggered with this role id.
Exception is _setTargetFunctionRole, which allows PUBLIC_ROLE and the internal method _trackRoleTarget keeps track of all targets assigned to this role. Do not see any test case covering this aspect, can you add please?
Also, should we say in the roles getters that PUBLIC_ROLE is never returned?

address account,
Expand All @@ -82,12 +228,13 @@ contract AccessManagerEnumerable is AccessManager, IAccessManagerEnumerable {
) internal override returns (bool) {
bool granted = super._grantRole(roleId, account, grantDelay, executionDelay);
if (granted) {
_trackRole(roleId);
_roleMembers[roleId].add(account);
}
return granted;
}

/// @dev Override AccessManager `_revokeRole` function to remove from tracked role members.
/// @dev Override AccessManager `_revokeRole` function to remove from tracked roles' membership.
function _revokeRole(uint64 roleId, address account) internal override returns (bool) {
bool revoked = super._revokeRole(roleId, account);
if (revoked) {
Expand All @@ -105,10 +252,12 @@ contract AccessManagerEnumerable is AccessManager, IAccessManagerEnumerable {
uint64 oldRoleId = getTargetFunctionRole(target, selector);
super._setTargetFunctionRole(target, selector, roleId);
if (oldRoleId != ADMIN_ROLE) {
Comment on lines 252 to 254
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint64 oldRoleId = getTargetFunctionRole(target, selector);
super._setTargetFunctionRole(target, selector, roleId);
if (oldRoleId != ADMIN_ROLE) {
uint64 oldRoleId = getTargetFunctionRole(target, selector);
super._setTargetFunctionRole(target, selector, roleId);
if (oldRoleId != ADMIN_ROLE) {

_roleTargetFunctions[oldRoleId][target].remove(bytes32(selector));
_roleTargetSelectors[oldRoleId][target].remove(bytes32(selector));
}
if (roleId != ADMIN_ROLE) {
_roleTargetFunctions[roleId][target].add(bytes32(selector));
_roleTargetSelectors[roleId][target].add(bytes32(selector));
}
Comment on lines 254 to 259
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it d make sense to extract methods so on every overriden function we clearly see:

  • preparation (if any)
  • call super
  • tracking

// also track the target under the role (will be added if not already present)
_trackRoleTarget(roleId, target, selector);
}
}
83 changes: 79 additions & 4 deletions src/access/interfaces/IAccessManagerEnumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,37 @@ import {IAccessManager} from 'src/dependencies/openzeppelin/IAccessManager.sol';
/// @author Aave Labs
/// @notice Interface for AccessManagerEnumerable extension.
interface IAccessManagerEnumerable is IAccessManager {
/// @notice Returns the identifier of the role at a specified index.
/// @param index The index in the role list.
/// @return The identifier of the role.
function getRole(uint256 index) external view returns (uint64);

/// @notice Returns the total number of existing roles.
/// @dev Does not account for the built-in `PUBLIC_ROLE` role.
/// @return The number of roles.
function getRoleCount() external view returns (uint256);

/// @notice Returns the list of role identifiers between the specified indexes.
/// @param start The starting index for the role list.
/// @param end The ending index for the role list.
/// @return The list of role identifiers.
function getRoles(uint256 start, uint256 end) external view returns (uint64[] memory);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the main motivation that this just comes for free with the data structure? I imagine that it will be used either to get a specific role or all roles.

What if we just made these private variables public and just expose the simple getters like at an index, or all the roles? Then integrators can just do this kind of access on their own

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could remove given that we have getters for items and length, but I'm fine adding this getter for ease of integration (the set structure provides a function to do it and makes sense to make integrators' lives easier)


/// @notice Returns the identifier of the admin role at a specified index.
/// @param index The index in the admin role list.
/// @return The identifier of the admin role.
function getAdminRole(uint256 index) external view returns (uint64);

/// @notice Returns the total number of existing admin roles.
/// @return The number of admin roles.
function getAdminRoleCount() external view returns (uint256);

/// @notice Returns the list of admin role identifiers between the specified indexes.
/// @param start The starting index for the admin role list.
/// @param end The ending index for the admin role list.
/// @return The list of admin role identifiers.
function getAdminRoles(uint256 start, uint256 end) external view returns (uint64[] memory);

/// @notice Returns the address of the role member at a specified index.
/// @param roleId The identifier of the role.
/// @param index The index in the role member list.
Expand All @@ -19,7 +50,7 @@ interface IAccessManagerEnumerable is IAccessManager {
/// @return The number of members for the role.
function getRoleMemberCount(uint64 roleId) external view returns (uint256);

/// @notice Returns the list of members for a specified role.
/// @notice Returns the list of members for a specified role between the specified indexes.
/// @param roleId The identifier of the role.
/// @param start The starting index for the member list.
/// @param end The ending index for the member list.
Expand All @@ -30,12 +61,56 @@ interface IAccessManagerEnumerable is IAccessManager {
uint256 end
) external view returns (address[] memory);

/// @notice Returns the role identifier of the listed roles for a specified admin role at a specified index.
/// @param adminRoleId The identifier of the admin role.
/// @param index The index in the admin controlled role list.
/// @return The indentifier of the role.
function getAdminOfRole(uint64 adminRoleId, uint256 index) external view returns (uint64);

/// @notice Returns the number of members for a specified role.
/// @param adminRoleId The identifier of the admin role.
/// @return The number of members for the role.
function getAdminOfRoleCount(uint64 adminRoleId) external view returns (uint256);

/// @notice Returns the list of role identifiers controlled by a specified admin role between the specified indexes.
/// @param adminRoleId The identifier of the admin role.
/// @param start The starting index for the admin controlled role list.
/// @param end The ending index for the admin controlled role list.
/// @return The list of admin controlled role indentifiers for the admin role.
function getAdminOfRoles(
uint64 adminRoleId,
uint256 start,
uint256 end
) external view returns (uint64[] memory);

/// @notice Returns the address of the target contract for a specified role and index.
/// @param roleId The identifier of the role.
/// @param index The index in the role target list.
/// @return The address of the target contract.
function getRoleTarget(uint64 roleId, uint256 index) external view returns (address);

/// @notice Returns the number of target contracts for a specified role.
/// @param roleId The identifier of the role.
/// @return The number of target contracts for the role.
function getRoleTargetCount(uint64 roleId) external view returns (uint256);

/// @notice Returns the list of target contracts for a specified role between the specified indexes.
/// @param roleId The identifier of the role.
/// @param start The starting index for the role target list.
/// @param end The ending index for the role target list.
/// @return The list of target contracts for the role.
function getRoleTargets(
uint64 roleId,
uint256 start,
uint256 end
) external view returns (address[] memory);

/// @notice Returns the function selector assigned to a given role at the specified index.
/// @param roleId The identifier of the role.
/// @param target The address of the target contract.
/// @param index The index in the role member list.
/// @return The selector at the index.
function getRoleTargetFunction(
function getRoleTargetSelector(
uint64 roleId,
address target,
uint256 index
Expand All @@ -45,7 +120,7 @@ interface IAccessManagerEnumerable is IAccessManager {
/// @param roleId The identifier of the role.
/// @param target The address of the target contract.
/// @return The number of selectors assigned to the role.
function getRoleTargetFunctionCount(
function getRoleTargetSelectorCount(
uint64 roleId,
address target
) external view returns (uint256);
Expand All @@ -56,7 +131,7 @@ interface IAccessManagerEnumerable is IAccessManager {
/// @param start The starting index for the selector list.
/// @param end The ending index for the selector list.
/// @return The list of selectors assigned to the role.
function getRoleTargetFunctions(
function getRoleTargetSelectors(
uint64 roleId,
address target,
uint256 start,
Expand Down
Loading
Loading