Different Solidity Versions and use of requires

[demis1997]

src/common/AuthControl.sol and src/common/AuthRolesol These have different solidity version and use require statements. You can instead use custom errors or the new require format from the solidity version 0.8.26 and above which use both requires and custom errors for readability and gas reduction.

Example: Current:

    function transferAdmin(address newAdmin) public virtual onlyOwner {
        require(newAdmin != address(0), "Accessible: zero address");
        require(msg.sender != newAdmin, "Accessible: same admin");

        grantRole(ADMIN_ROLE, newAdmin);
        renounceRole(ADMIN_ROLE, msg.sender);
    }

Fix 1:

error ZeroAddress();
error SameAdmin();

function transferAdmin(address newAdmin) public virtual onlyOwner {
    if (newAdmin == address(0)) {
        revert ZeroAddress();
    }

    if (msg.sender == newAdmin) {
        revert SameAdmin();
    }

    grantRole(ADMIN_ROLE, newAdmin);
    renounceRole(ADMIN_ROLE, msg.sender);
}

Fix 2:

error ZeroAddress();
error SameAdmin();

function transferAdmin(address newAdmin) public virtual onlyOwner {
    require(newAdmin != address(0), ZeroAddress());
    require(msg.sender != newAdmin, SameAdmin());

    grantRole(ADMIN_ROLE, newAdmin);
    renounceRole(ADMIN_ROLE, msg.sender);
}

[mehdi-defiesta]

These types of suggestions were already raised during previous code reviews and were never taken into account. I've ran multiple time the Foundry Gas Report and the difference was not significant (see foundry doc ). @suahnkim @Zena-park What are your thoughts ?

[suahnkim]

I don't think I have enough experience with this to make meaningful comments, but I have heard the similar thing about how there is almost no gas cost difference.

The custom gas error is definitely an improvement from code readability for me, but that is my personal opinion.

Within the team, there seems to be difference in opinions as well.

Let's hear from Zena

[demis1997]

I think regardless you should use the same solidity version throughout your files, it has nothing to do with gas but more of a safety procedure (for example underflows and overflows are only handled correctly in solidity version 0.8 and above) For gas efficiency (caching array lengths, using ++i, using custom errors instead of requires, avoiding && etc.) will be raised by an auditing company unless you ask them otherwise so I thought they were worth mentioning. There are multiple instances where these optimisations can be applied and I understand we are talking about miniscule gas costs but it is still an optimization you can make

[mehdi-defiesta]

I agreed on the solc version and I will modify it. However, let's wait for Zena's opinion regarding require statements.

[Zena-park]

In my experience, handling errors with revert rather than require saves a little gas costs. It's a very small difference, but I think fixing it like 'Fix1' would help reduce gas costs.

Thank you

[mehdi-defiesta]

fixed. Here is the link You can close the issue if you find it right.

[mehdi-defiesta]

@demis1997

[demis1997]

src/L1/L1WrappedStakedTON.sol

Please check this contract for require statements, length caching and loop optimisations The other files are okay