AccessControlBase Code Style Findings

ACB-01C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		AccessControlBase.sol:L98-L99, L119-L120

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
116function _revokeRoleAccount(bytes32 role, address account) internal virtual {
117  AccessControlStorage storage acs = _acs();
118
119  if (acs.roles[role].members[account]) {
120    acs.roles[role].members[account] = false;
121    emit RoleAccountRevoked(role, account, msg.sender);
122  }
123}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

All referenced mapping lookups have been optimized as advised, utilizing interim variables for iterative mapping declarations and caching values to local variables where applicable.

ACB-02C: Loop Iterator Optimization

Type	Severity	Location
Gas Optimization		AccessControlBase.sol:L177

Description:

The linked for loop increments / decrements the iterator "safely" due to Solidity's built-in safe arithmetics (post-0.8.X).

Example:

Copy
177for (uint256 i = 0; i < length; i++) {

Recommendation:

We advise the increment / decrement operation to be performed in an unchecked code block as the last statement within the for loop to optimize its execution cost.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The referenced iterator increment statement has been optimized as advised, performing it within an unchecked code block to save gas.

ACB-03C: Redundant Repetitive Invocations of Getter Function

Type	Severity	Location
Gas Optimization		AccessControlBase.sol:L178

Description:

The Element::db getter function is redundantly invoked multiple times within AccessControlBase::_hasRole on each iteration.

Example:

Copy
161/// @notice Checks if the account has the role
162/// @param role The role the account is checked against
163/// @param account The
164/// @return If the account has the role
165function _hasRole(
166  bytes32 role,
167  address account
168) internal view virtual returns (bool) {
169  AccessControlStorage storage acs = _acs();
170  RoleData storage roleData = acs.roles[role];
171
172  if (roleData.members[account]) return true;
173
174  bytes32 addr = bytes32(bytes20(account));
175  uint256 length = roleData.keys.length();
176
177  for (uint256 i = 0; i < length; i++) {
178    if (_db().hasPair(roleData.keys.at(i), addr)) return true;
179  }
180
181  return false;
182}

Recommendation:

We advise it to be invoked once and stored to a local variable that is consequently utilized within the for loop, optimizing the code's gas cost.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The result of the _db call is properly cached outside the for loop in the updated code, optimizing each iteration's gas cost.