Frax Ether Liquid Staking contest - tnevler's results

Low Risk

[L-01]: Loops may exceed gas limit

Context:

• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L84
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L129

 for (uint256 i = 0; i < length; i++) {
            _createClaimUnchecked(_recipients[i], _startTimestamps[i], _endTimestamps[i], _cliffReleaseTimestamps[i], _releaseIntervalsSecs[i], _linearVestAmounts[i], _cliffAmounts[i]);
        }

https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L353

Description:

Either explicitly or just due to normal operation, the number of iterations in a loop can grow beyond the block gas limit, which can cause the complete contract to be stalled at a certain point.

Non-Critical Issues

[N-01]: Floating Pragma

Context:

• https://github.com/code-423n4/2022-09-frax/blob/main/src/sfrxETH.sol#L2
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L2
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L2
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L2
• https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L4
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETH.sol#L2

Recommendation:

https://swcregistry.io/docs/SWC-103

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

[N-02]: Public function can be external

Context:

• sfrxETH.pricePerShare
• ERC20PermitPermissionedMint.minter_burn_from
• ERC20PermitPermissionedMint.minter_mint
• ERC20PermitPermissionedMint.addMinter
• ERC20PermitPermissionedMint.removeMinter
• ERC20PermitPermissionedMint.setTimelock
• OperatorRegistry.popValidators
• OperatorRegistry.removeValidator
• xERC4626.totalAssets
• xERC4626.syncRewards

Description:

Public functions can be declared external if they are not called by the contract.

Recommendation:

Declare these functions as external instead of public.

[N-03]: NatSpec is incomplete

1. https://github.com/code-423n4/2022-09-frax/blob/main/src/sfrxETH.sol (no '@param' and '@return' tags where they are needed)
2. https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol (NatSpec is missing)
3. https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol (no '@param' tags where they are needed)
4. https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol (no '@param' and '@return' tags where they are needed)
5. https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol (no '@param' and '@return' tags where they are needed)

Report

Gas Optimizations

[G-01]: X += Y (X -= Y) costs more gas than X = X + Y (X = X - Y)

Context:

• currentWithheldETH += withheld_amt;
• currentWithheldETH -= amount;
• storedTotalAssets += amount;
• storedTotalAssets -= amount;

Recommendation:

Change X += Y (X -= Y) to X = X + Y (X = X - Y).

[G-02]: Don't initialize variable with its default value

Context:

1. https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L84
2. https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L129
3. https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L63
4. https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L84
5. https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L114
6. https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L63
7. https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L64
8. https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L94

Description:

Default value of uint is 0. It's unnecessary and costs more gas to initialize uint variavles to 0.

Recommendation:

1-5. Change "uint256 i = 0;" to "uint256 i;".

6-7. Remove the line.

8. Change "uint256 withheld_amt = 0;" to "uint256 withheld_amt".

[G-03]: >0 costs more gas than !=0

Context:

• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L79
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L126

Description:

uint256 type will never be less than 0.

Recommendation:

Change > 0 to !=0.

[G-04]: i++ costs more gas than ++i

Context:

for (uint i = 0; i < minters_array.length; i++){ 

https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L84

Recommendation:

Change i++ to ++i.

[G-05]: Unchecked arithmetic

Context:

https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L34

Description:

Some gas can be saved by using an unchecked {} block if an overflow/underflow isn't possible because of a previous require() or if-statement.

Recommendation:

Place the arithmetic operations in an unchecked block.

[G-06]: Use new variable instead of reading array length in every loop of a for-loop

Context:

• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L84
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L114

Description:

If you read the length of the array at each iteration of the loop, this consumes a lot of gas.

Recommendation:

Store the array’s length in a variable before the for-loop, and use this new variable in the loop.

[G-07]: Use custom errors instead of revert strings

Custom errors are more gas efficient than using require with a string explanation.

• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L40
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L46
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L66
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L68
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L77
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L78
• https://github.com/code-423n4/2022-09-frax/blob/main/src/ERC20/ERC20PermitPermissionedMint.sol#L95
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L79
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L87
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L88
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L122
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L126
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L140
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L140
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L167
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L171
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L193
• https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L200
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L46
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L137
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L182
• https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L203