AccessControlFacetStorage Code Style Findings

ACS-01C: Improper Role Definitions

Type	Severity	Location
Code Style		AccessControlFacetStorage.sol: • I-1: L50 • I-2: L51

Description:

The referenced role definitions are text literals that are converted to their bytes32 underlying type and are prefixed with 0x misleadingly.

As an example the ADMIN_ROLE would give the indication that it is the 0x00..00 data entry whilst it is not as it contains the ASCII characters "0x00".

Example:

Copy
50bytes32 public constant ADMIN_ROLE = "0x00";
51bytes32 public constant OPEN_ROLE = "0x100"; // Every user has this role

Recommendation:

We advise either proper names to be utilized or correct bytes32 literal declarations, either of which we consider an adequate resolution to this exhibit.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

The role definitions have been corrected to more standardized formats, addressing this exhibit.

ACS-02C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		AccessControlFacetStorage.sol: • I-1: L257 • I-2: L285, L286 • I-3: L335 • I-4: L364, L365

Description:

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

Example:

Copy
284for (uint256 i = 0; i < length; i++) {
285    l.userRolesForId[campaignId][accounts[i]].add(role);
286    l.usersWithRoleForId[campaignId][role].add(accounts[i]);
287}

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.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

All referenced inefficient mapping lookups have been optimized to the greatest extent possible, significantly reducing the gas cost of the functions the statements were located in.

ACS-03C: Non-Standard Storage Slot Definition

Type	Severity	Location
Standard Conformity		AccessControlFacetStorage.sol:L48

Description:

The referenced declaration will define a storage slot for use by a facet of the system's main EIP-2535 Diamond, however, the way it is declared does not adhere to the latest standards.

Example:

Copy
48bytes32 internal constant STORAGE_SLOT = keccak256("Evergonlabs.Tmi-Tokenizer.storage.AccessControlFacetStorage");

Recommendation:

We advise the EIP-7201 name-spaced layout approach to be adhered to similarly to OpenZeppelin and other relevant standard libraries, ensuring consistency among the ecosystem's widely utilized libraries and conforming to the latest standards.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

The referenced slot definition has been updated to its standardized EIP-7201 representation, addressing this exhibit.

ACS-04C: Redundant Logical Structures

Type	Severity	Location
Gas Optimization		AccessControlFacetStorage.sol: • I-1: L143, L145 • I-2: L158, L160

Description:

The referenced logical structures are extraneous as they can be compressed to a single return statement each.

Example:

Copy
142function hasRole(Layout storage l, bytes32 role, address account) internal view returns (bool) {
143    if (role == OPEN_ROLE) return true;
144
145    return l.userRoles[account].contains(role);
146}

Recommendation:

We advise this optimization to be applied, minimizing the number of statements that need to be executed in each function.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

The logical structures were optimized as advised, minimizing the number of statements executed within each function.