Fraxlend (Frax Finance) contest - CodingNameKiki's results

1. Missing checks for address(0x0) when assigning values to address state variables

src/contracts/FraxlendPairCore.sol 172: CIRCUIT_BREAKER_ADDRESS = _circuitBreaker; 173: COMPTROLLER_ADDRESS = _comptrollerAddress; 174: TIME_LOCK_ADDRESS = _timeLockAddress; 175: FRAXLEND_WHITELIST_ADDRESS = _fraxlendWhitelistAddress;

2. Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything.

src/contracts/FraxlendPairCore.sol 1103: _assetContract.approve(_swapperAddress, _borrowAmount); 1184: _collateralContract.approve(_swapperAddress, _collateralToSwap);

3. Front-runable initializer

There is nothing preventing another account from calling the initializer before the contract owner. In the best case, the owner is forced to waste gas and re-deploy. In the worst case, the owner does not notice that his/her call reverts, and everyone starts using a contract under the control of an attacker.

src/contracts/FraxlendPairCore.sol 245: function initialize(

4. Avoid the use of sensitive terms

Use alternative variants, e.g. allowlist/denylist instead of whitelist/blacklist

src/contracts/FraxlendPairCore.sol 89: address public immutable FRAXLEND_WHITELIST_ADDRESS; 133: bool public immutable borrowerWhitelistActive; 136: bool public immutable lenderWhitelistActive;

5. Constants should be defined rather than using magic numbers

src/contracts/FraxlendPairCore.sol 194: dirtyLiquidationFee = (_liquidationFee * 9000) / LIQ_PRECISION;

6. Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

src/contracts/FraxlendPairCore.sol 376: event AddInterest( 389: event UpdateRate(uint256 _ratePerSec, uint256 _deltaTime, uint256 _utilizationRate, uint256 _newRatePerSec); 504: event UpdateExchangeRate(uint256 _rate); 696: event BorrowAsset( 760: event AddCollateral(address indexed _sender, address indexed _borrower, uint256 _collateralAmount); 846: event RepayAsset(address indexed _sender, address indexed _borrower, uint256 _amountToRepay, uint256 _shares); 897: event Liquidate( 1045: event LeveragedPosition( 1143: event RepayAssetWithCollateral(

7. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

src/contracts/FraxlendPairCore.sol 320: function _isPastMaturity() internal view returns (bool) {

8. Adding a return statement when the function defines a named return variable, is redundant

src/contracts/FraxlendPairCore.sol 422: return (_interestEarned, _feesAmount, _feesShare, _newRate);

9. Variable names that consist of all capital letters should be reserved for const/immutable variables

src/contracts/FraxlendPairCore.sol 86: address public TIME_LOCK_ADDRESS;

1. Use != 0 instead of > 0 for unsigned integer comparison

When dealing with unsigned integer types, comparisons with != 0 are cheaper than with > 0 as we can save 3 gas

src/contracts/FraxlendPairCore.sol 477: if (_currentRateInfo.feeToProtocolRate > 0) { 754: if (_collateralAmount > 0) { 1002: if (_leftoverBorrowShares > 0) { 1094: if (_initialCollateralAmount > 0) {

2. No need to initialize variables with their defaults.

If a variable is not set/initialized, it is assumed to have the default value (0 for uint/int, false for bool, address(0) for address and so on). Explicitly initializing it with its default value is an anti-pattern and wastes gas as It costs more gas to initialize variables to zero than to let the default of zero be applied

src/contracts/FraxlendPairCore.sol 265: for (uint256 i = 0; i < _approvedBorrowers.length; ++i) { 270: for (uint256 i = 0; i < _approvedLenders.length; ++i) {

3. Use short circuit

The operators “||” and “&&” apply the common short-circuiting rules.

src/contracts/FraxlendPairCore.sol (Before) 308: if (maxLTV == 0) return true; uint256 _borrowerAmount = totalBorrow.toAmount(userBorrowShares[_borrower], true); if (_borrowerAmount == 0) return true; uint256 _collateralAmount = userCollateralBalance[_borrower];

(After) 308: if (maxLTV == 0 || _borrowerAmount == 0) return true; uint256 _borrowerAmount = totalBorrow.toAmount(userBorrowShares[_borrower], true); uint256 _collateralAmount = userCollateralBalance[_borrower];

4. For-Loops: Cache array length outside of loops

Reading an array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration.

src/contracts/FraxlendPairCore.sol (Before) 265: for (uint256 i = 0; i < _approvedBorrowers.length; ++i) { (After) 265: uint256 len = _approvedBorrowers.length for (uint256 i = 0; i < len; ++i) {

(Before) 270: for (uint256 i = 0; i < _approvedLenders.length; ++i) { (After) 270: uint256 len = _approvedLenders.length for (uint256 i = 0; i < len; ++i) {

5. For-Loops: Index increments can be left unchecked

For-Loops: Index increments can be left unchecked From Solidity v0.8 onwards, all arithmetic operations come with implicit overflow and underflow checks. In for-loops, as it is impossible for the index to overflow, it can be left unchecked to save gas every iteration.

src/contracts/FraxlendPairCore.sol (Before) 265: for (uint256 i = 0; i < _approvedBorrowers.length; ++i) { (After) 265: for (uint256 i = 0; i < _approvedBorrowers.length;) { unchecked { ++i; }

(Before) 270: for (uint256 i = 0; i < _approvedLenders.length; ++i) { (After) 270: for (uint256 i = 0; i < _approvedLenders.length;) { unchecked { ++i; }