code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

Gas Optimizations #272

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)

!= 0 should be used where possible since > 0 costs more gas

frxETHMinter.sol: L79

        require(sfrxeth_recieved > 0, 'No sfrxETH was returned');

Recommendation: Change sfrxeth_recieved > 0 to sfrxeth_recieved != 0

Note: The spelling error (recieved) was addressed under Typos in the QA Report (low / non-critical)



State variables should not be initialized to their default values

In particular, initializing uint variables to their default value of 0 is unnecessary and costs gas


frxETHMinter.sol: L50

        uint256 withheld_amt = 0;

Change to:

        uint256 withheld_amt;


Require revert string is too long

The revert string below can be shortened to 32 characters (as shown) to save gas


frxETHMinter.sol: L167

        require(amount <= currentWithheldETH, "Not enough withheld ETH in contract");

Change message to Not enough contract withheld ETH



Array length should not be looked up during every iteration of a for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop, as demonstrated below


ERC20PermitPermissionedMint.sol: L84-89

        for (uint i = 0; i < minters_array.length; i++){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;
            }
        }

Suggestion:

        uint256 totalMintersLength = minters_array.length; 
        for (uint i = 0; i < totalMintersLength; i++){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;
            }
        }

Similarly for the following for loop:

OperatorRegistry.sol: L114-118



Use ++i instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i

ERC20PermitPermissionedMint.sol: L84-89

        for (uint i = 0; i < minters_array.length; i++){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;
            }
        }

Suggestion:

        uint256 totalMintersLength = minters_array.length; 
        for (uint i = 0; i < totalMintersLength; ++i){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;
            }
        }

Note that I have included the previous correction (avoiding the array length look up during every for loop iteration) in the suggestion



Avoid use of default "checked" behavior in a for loop

Underflow/overflow checks are made every time ++i (or equivalent counter) is called. Such checks are unnecessary since i is already limited. Therefore, use `unchecked {++i} instead, as shown below:

ERC20PermitPermissionedMint.sol: L84-89

        for (uint i = 0; i < minters_array.length; i++){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;
            }
        }

Suggestion:

        uint256 totalMintersLength = minters_array.length; 
        for (uint i = 0; i < totalMintersLength;){ 
            if (minters_array[i] == minter_address) {
                minters_array[i] = address(0); // This will leave a null in the array and keep the indices the same
                break;

                unchecked {
                  ++i;
              }
            }
        }

Similarly for the following four for loops:

frxETHMinter.sol: L129-154

OperatorRegistry.sol: L63-65

OperatorRegistry.sol: L84-86

OperatorRegistry.sol: L114-117