AsAccessControl Code Style Findings

AAC-01C: Inefficient Usage of Utility Functions

Type	Severity	Location
Gas Optimization		AsAccessControl.sol:L126, L137, L149

Description:

In the referenced instances, the _roles[role] lookup will be redundantly performed multiple times due to using utility functions that also fetch its storage location.

Example:

Copy
143/**
144 * @dev Internal function to revoke a role from an account.
145 * @param role The role to revoke.
146 * @param account The account to revoke the role from.
147 */
148function _revokeRole(bytes32 role, address account) internal virtual {
149    if (hasRole(role, account)) {
150        _roles[role].members.remove(account.toBytes32());
151        emit RoleRevoked(role, account, msg.sender);
152    }
153}

Recommendation:

We advise the function invocations to be replaced by their statements directly, caching the result of _roles[role] to a local RoleState storage variable that can be re-used and thus optimize the gas cost of the functions.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsAccessControl implementation has been sunset by the standalone AccessController implementation which incorporates a significant portion of the original code.

The ported implementations of AccessController::_setRoleAdmin, AccessController::_grantRole, and AccessController::_revokeRole properly incorporate the optimization outlined by no longer utilizing utility functions.

As a result, we consider this exhibit alleviated in the implementation that supersedes the original.

AAC-02C: Redundant Input Argument

Type	Severity	Location
Gas Optimization		AsAccessControl.sol:L106

Description:

THe AsAccessControl::renounceRole function will accept an input argument that will always be mandated as equal to the msg.sender.

Example:

Copy
101/**
102 * @dev Renounce a role for the sender account.
103 * @param role The role to renounce.
104 * @param account The account renouncing the role.
105 */
106function renounceRole(bytes32 role, address account) external virtual {
107    if (account != msg.sender) revert Unauthorized();
108    _revokeRole(role, account);
109}

Recommendation:

We advise the referenced input argument to be omitted, ensuring that a role renunciation only requires the role that is being renounced.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsAccessControl implementation has been sunset by the standalone AccessController implementation which incorporates a significant portion of the original code.

The ported implementation of AccessController::renounceRole incorporates our recommendation to omit the input argument and replacing it with direct use of the msg.sender.

As a result, we consider this exhibit alleviated in the implementation that supersedes the original.

AAC-03C: Redundant Local Variable

Type	Severity	Location
Gas Optimization		AsAccessControl.sol:L126

Description:

The referenced statement will declare a previousAdminRole local variable that is solely utilized once within the code block.

Example:

Copy
120/**
121 * @dev Internal function to set the admin role for a given role.
122 * @param role The role to set the admin role of.
123 * @param adminRole The admin role to be set.
124 */
125function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
126    bytes32 previousAdminRole = getRoleAdmin(role);
127    _roles[role].adminRole = adminRole;
128    emit RoleAdminChanged(role, previousAdminRole, adminRole);
129}
130
131/**
132 * @dev Internal function to grant a role to an account.
133 * @param role The role to grant.
134 * @param account The account to grant the role to.
135 */
136function _grantRole(bytes32 role, address account) internal virtual {
137    if (!hasRole(role, account)) {
138        _roles[role].members.push(account.toBytes32());
139        emit RoleGranted(role, account, msg.sender);

Recommendation:

We advise the AsAccessControl::getRoleAdmin evaluation to be directly utilized as input to the RoleAdminChanged event, and the event's emission to be relocated prior to the _roles data entry's adjustment.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsAccessControl implementation has been sunset by the standalone AccessController implementation which incorporates a significant portion of the original code.

The ported implementation of AccessController::_setRoleAdmin properly applies our recommended optimization by emitting the RoleAdminChanged event before mutating the role.adminRole data entry.

As a result, we consider this exhibit alleviated in the implementation that supersedes the original.