Fraxlend (Frax Finance) contest - oyc_109's results

[L-01] Multiply before divide

Due to rounding errors, multiplication should be done befire division

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L314

[L-02] Unspecific Compiler Version Pragma

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

2022-08-frax/src/contracts/FraxlendPair.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/FraxlendPairConstants.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/FraxlendPairCore.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/LinearInterestRate.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/VariableInterestRate.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/libraries/SafeERC20.sol::2 => pragma solidity ^0.8.15;
2022-08-frax/src/contracts/libraries/VaultAccount.sol::2 => pragma solidity ^0.8.15;

[L-03] Use of Block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

2022-08-frax/src/contracts/FraxlendPairCore.sol::321 => return maturityDate != 0 && block.timestamp > maturityDate;
2022-08-frax/src/contracts/FraxlendPairCore.sol::420 => if (_currentRateInfo.lastTimestamp == block.timestamp) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::434 => _currentRateInfo.lastTimestamp = uint64(block.timestamp);
2022-08-frax/src/contracts/FraxlendPairCore.sol::441 => uint256 _deltaTime = block.timestamp - _currentRateInfo.lastTimestamp;
2022-08-frax/src/contracts/FraxlendPairCore.sol::464 => _currentRateInfo.lastTimestamp = uint64(block.timestamp);
2022-08-frax/src/contracts/FraxlendPairCore.sol::518 => if (_exchangeRateInfo.lastTimestamp == block.timestamp) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::544 => _exchangeRateInfo.lastTimestamp = uint32(block.timestamp);
2022-08-frax/src/contracts/FraxlendPairCore.sol::955 => if (block.timestamp > _deadline) revert PastDeadline(block.timestamp, _deadline);

[L-04] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). Unless there is a compelling reason, abi.encode should be preferred. If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

2022-08-frax/src/contracts/FraxlendPairDeployer.sol::204 => bytes32 salt = keccak256(abi.encodePacked(_saltSeed, _configData));
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::329 => keccak256(abi.encodePacked("public")),
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::372 => keccak256(abi.encodePacked(_name)),

[L-05] approve should be replaced with safeApprove or safeIncreaseAllowance() / safeDecreaseAllowance()

approve is subject to a known front-running attack. Consider using safeApprove() or safeIncreaseAllowance() or safeDecreaseAllowance() instead

2022-08-frax/src/contracts/FraxlendPairCore.sol::1103 => _assetContract.approve(_swapperAddress, _borrowAmount);
2022-08-frax/src/contracts/FraxlendPairCore.sol::1184 => _collateralContract.approve(_swapperAddress, _collateralToSwap);

[L-06] _safeMint() should be used rather than _mint() wherever possible

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert. Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

2022-08-frax/src/contracts/FraxlendPairCore.sol::487 => _mint(address(this), _feesShare);
2022-08-frax/src/contracts/FraxlendPairCore.sol::570 => _mint(_receiver, _shares);

[N-01] Large multiples of ten should use scientific notation

Use (e.g. 1e6) rather than decimal literals (e.g. 1000000), for better code readability

2022-08-frax/src/contracts/FraxlendPairDeployer.sol::51 => uint256 public DEFAULT_LIQ_FEE = 10000; // 10% with 1e5 precision

[N-02] Event is missing indexed fields

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

2022-08-frax/src/contracts/FraxlendPair.sol::200 => event SetTimeLock(address _oldAddress, address _newAddress);
2022-08-frax/src/contracts/FraxlendPair.sol::211 => event ChangeFee(uint32 _newFee);
2022-08-frax/src/contracts/FraxlendPair.sol::228 => event WithdrawFees(uint128 _shares, address _recipient, uint256 _amountToTransfer);
2022-08-frax/src/contracts/FraxlendPair.sol::268 => event SetSwapper(address _swapper, bool _approval);
2022-08-frax/src/contracts/FraxlendPairCore.sol::389 => event UpdateRate(uint256 _ratePerSec, uint256 _deltaTime, uint256 _utilizationRate, uint256 _newRatePerSec);
2022-08-frax/src/contracts/FraxlendPairCore.sol::504 => event UpdateExchangeRate(uint256 _rate);

[N-03] Missing NatSpec

Code should include NatSpec

2022-08-frax/src/contracts/FraxlendPairConstants.sol::1 => // SPDX-License-Identifier: ISC

[N-04] Constants should be defined rather than using magic numbers

It is bad practice to use numbers directly in code without explanation

2022-08-frax/src/contracts/FraxlendPairCore.sol::194 => dirtyLiquidationFee = (_liquidationFee * 9000) / LIQ_PRECISION; // 90% of clean fee
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::171 => bytes memory _firstHalf = BytesLib.slice(_creationCode, 0, 13000);
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::173 => if (_creationCode.length > 13000) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::174 => bytes memory _secondHalf = BytesLib.slice(_creationCode, 13000, _creationCode.length - 13000);tional 1e36 to make math simpler

[N-05] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

2022-08-frax/src/contracts/FraxlendPair.sol::74 => function name() public view override(ERC20, IERC20Metadata) returns (string memory) {
2022-08-frax/src/contracts/FraxlendPair.sol::78 => function symbol() public view override(ERC20, IERC20Metadata) returns (string memory) {
2022-08-frax/src/contracts/FraxlendPair.sol::84 => function decimals() public pure override(ERC20, IERC20Metadata) returns (uint8) {
2022-08-frax/src/contracts/FraxlendPair.sol::89 => function totalSupply() public view override(ERC20, IERC20) returns (uint256) {
2022-08-frax/src/contracts/FraxlendPair.sol::100 => function totalAssets() public view virtual returns (uint256) {

[N-06] Adding a return statement when the function defines a named return variable, is redundant

It is not necessary to have both a named return and a return statement.

2022-08-frax/src/contracts/FraxlendPairCore.sol::516 => function _updateExchangeRate() internal returns (uint256 _exchangeRate) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::201 => ) private returns (address _pairAddress) {
2022-08-frax/src/contracts/LinearInterestRate.sol::46 => function getConstants() external pure returns (bytes memory _calldata) {
2022-08-frax/src/contracts/VariableInterestRate.sol::52 => function getConstants() external pure returns (bytes memory _calldata) {

[N-07] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length

2022-08-frax/src/contracts/FraxlendPairCore.sol::144 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::184 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::239 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::268 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::308 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::348 => /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

[G-01] Don't Initialize Variables with Default Value

Uninitialized variables are assigned with the types default value. Explicitly initializing a variable with it's default value costs unnecesary gas.

2022-08-frax/src/contracts/FraxlendPair.sol::289 => for (uint256 i = 0; i < _lenders.length; i++) {
2022-08-frax/src/contracts/FraxlendPair.sol::308 => for (uint256 i = 0; i < _borrowers.length; i++) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::265 => for (uint256 i = 0; i < _approvedBorrowers.length; ++i) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::270 => for (uint256 i = 0; i < _approvedLenders.length; ++i) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::402 => for (uint256 i = 0; i < _lengthOfArray; ) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::51 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::66 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::81 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/libraries/SafeERC20.sol::22 => uint8 i = 0;

[G-02] Cache Array Length Outside of Loop

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.

2022-08-frax/src/contracts/FraxlendPair.sol::289 => for (uint256 i = 0; i < _lenders.length; i++) {
2022-08-frax/src/contracts/FraxlendPair.sol::308 => for (uint256 i = 0; i < _borrowers.length; i++) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::265 => for (uint256 i = 0; i < _approvedBorrowers.length; ++i) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::270 => for (uint256 i = 0; i < _approvedLenders.length; ++i) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::51 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::66 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::81 => for (uint256 i = 0; i < _addresses.length; i++) {

[G-03] Long Revert Strings

Shortening revert strings to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.

If the contract(s) in scope allow using Solidity >=0.8.4, consider using Custom Errors as they are more gas efficient while allowing developers to describe the error in detail using NatSpec.

2022-08-frax/src/contracts/FraxlendPairDeployer.sol::205 => require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::228 => require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::253 => require(deployedPairsByName[_name] == address(0), "FraxlendPairDeployer: Pair name must be unique");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::365 => require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");

[G-04] Use calldata instead of memory

Use calldata instead of memory for function parameters saves gas if the function argument is only read.

2022-08-frax/src/contracts/FraxlendPairDeployer.sol::310 => function deploy(bytes memory _configData) external returns (address _pairAddress) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::398 => function globalPause(address[] memory _addresses) external returns (address[] memory _updatedAddresses) {
2022-08-frax/src/contracts/libraries/SafeERC20.sol::18 => function returnDataToString(bytes memory data) internal pure returns (string memory) {

[G-05] Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

2022-08-frax/src/contracts/FraxlendPairCore.sol::59 => IERC20 public immutable collateralContract;
2022-08-frax/src/contracts/FraxlendPairCore.sol::62 => address public immutable oracleMultiply;
2022-08-frax/src/contracts/FraxlendPairCore.sol::63 => address public immutable oracleDivide;
2022-08-frax/src/contracts/FraxlendPairCore.sol::64 => uint256 public immutable oracleNormalization;
2022-08-frax/src/contracts/FraxlendPairCore.sol::67 => uint256 public immutable maxLTV;
2022-08-frax/src/contracts/FraxlendPairCore.sol::70 => uint256 public immutable cleanLiquidationFee;
2022-08-frax/src/contracts/FraxlendPairCore.sol::71 => uint256 public immutable dirtyLiquidationFee;
2022-08-frax/src/contracts/FraxlendPairCore.sol::74 => IRateCalculator public immutable rateContract; // For complex rate calculations
2022-08-frax/src/contracts/FraxlendPairCore.sol::81 => address public immutable DEPLOYER_ADDRESS;
2022-08-frax/src/contracts/FraxlendPairCore.sol::84 => address public immutable CIRCUIT_BREAKER_ADDRESS;
2022-08-frax/src/contracts/FraxlendPairCore.sol::85 => address public immutable COMPTROLLER_ADDRESS;
2022-08-frax/src/contracts/FraxlendPairCore.sol::89 => address public immutable FRAXLEND_WHITELIST_ADDRESS;
2022-08-frax/src/contracts/FraxlendPairCore.sol::95 => uint256 public immutable maturityDate;
2022-08-frax/src/contracts/FraxlendPairCore.sol::96 => uint256 public immutable penaltyRate;
2022-08-frax/src/contracts/FraxlendPairCore.sol::133 => bool public immutable borrowerWhitelistActive;
2022-08-frax/src/contracts/FraxlendPairCore.sol::136 => bool public immutable lenderWhitelistActive;

[G-06] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

2022-08-frax/src/contracts/FraxlendPair.sol::204 => function setTimeLock(address _newAddress) external onlyOwner {
2022-08-frax/src/contracts/FraxlendPair.sol::234 => function withdrawFees(uint128 _shares, address _recipient) external onlyOwner returns (uint256 _amountToTransfer) {
2022-08-frax/src/contracts/FraxlendPair.sol::274 => function setSwapper(address _swapper, bool _approval) external onlyOwner {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::170 => function setCreationCode(bytes calldata _creationCode) external onlyOwner {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::50 => function setOracleContractWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::65 => function setRateContractWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::80 => function setFraxlendDeployerWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {

[G-07] Empty blocks should be removed or emit something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

2022-08-frax/src/contracts/FraxlendWhitelist.sol::40 => constructor() Ownable() {}
2022-08-frax/src/contracts/VariableInterestRate.sol::57 => function requireValidInitData(bytes calldata _initData) external pure {}

[G-08] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

2022-08-frax/src/contracts/FraxlendPairConstants.sol::41 => uint64 internal constant DEFAULT_INT = 158247046; // 0.5% annual rate 1e18 precision
2022-08-frax/src/contracts/FraxlendPairConstants.sol::47 => uint16 internal constant DEFAULT_PROTOCOL_FEE = 0; // 1e5 precision
2022-08-frax/src/contracts/FraxlendPairCore.sol::106 => uint64 lastBlock;
2022-08-frax/src/contracts/FraxlendPairCore.sol::107 => uint64 feeToProtocolRate; // Fee amount 1e5 precision
2022-08-frax/src/contracts/FraxlendPairCore.sol::108 => uint64 lastTimestamp;
2022-08-frax/src/contracts/FraxlendPairCore.sol::109 => uint64 ratePerSec;
2022-08-frax/src/contracts/FraxlendPairCore.sol::116 => uint32 lastTimestamp;
2022-08-frax/src/contracts/FraxlendPairCore.sol::967 => uint128 _borrowerShares = userBorrowShares[_borrower].toUint128();
2022-08-frax/src/contracts/FraxlendPairCore.sol::993 => uint128 _amountLiquidatorToRepay = (_totalBorrow.toAmount(_sharesToLiquidate, true)).toUint128();
2022-08-frax/src/contracts/FraxlendPairCore.sol::997 => uint128 _sharesToAdjust;
2022-08-frax/src/contracts/FraxlendPairCore.sol::998 => uint128 _amountToAdjust;
2022-08-frax/src/contracts/FraxlendPairCore.sol::1100 => uint256 _borrowShares = _borrowAsset(_borrowAmount.toUint128(), address(this));
2022-08-frax/src/contracts/VariableInterestRate.sol::35 => uint32 private constant MIN_UTIL = 75000; // 75%
2022-08-frax/src/contracts/VariableInterestRate.sol::36 => uint32 private constant MAX_UTIL = 85000; // 85%
2022-08-frax/src/contracts/VariableInterestRate.sol::37 => uint32 private constant UTIL_PREC = 1e5; // 5 decimals
2022-08-frax/src/contracts/VariableInterestRate.sol::40 => uint64 private constant MIN_INT = 79123523; // 0.25% annual rate
2022-08-frax/src/contracts/VariableInterestRate.sol::41 => uint64 private constant MAX_INT = 146248508681; // 10,000% annual rate
2022-08-frax/src/contracts/libraries/VaultAccount.sol::5 => uint128 amount; // Total amount, analogous to market cap
2022-08-frax/src/contracts/libraries/VaultAccount.sol::6 => uint128 shares; // Total shares, analogous to shares outstanding

[G-09] Using bools for storage incurs overhead

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled. Use uint256(1) and uint256(2) for true/false instead

2022-08-frax/src/contracts/FraxlendPairCore.sol::78 => mapping(address => bool) public swappers; // approved swapper addresses
2022-08-frax/src/contracts/FraxlendPairCore.sol::133 => bool public immutable borrowerWhitelistActive;
2022-08-frax/src/contracts/FraxlendPairCore.sol::134 => mapping(address => bool) public approvedBorrowers;
2022-08-frax/src/contracts/FraxlendPairCore.sol::136 => bool public immutable lenderWhitelistActive;
2022-08-frax/src/contracts/FraxlendPairCore.sol::137 => mapping(address => bool) public approvedLenders;
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::94 => mapping(address => bool) public deployedPairCustomStatusByAddress;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::32 => mapping(address => bool) public oracleContractWhitelist;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::35 => mapping(address => bool) public rateContractWhitelist;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::38 => mapping(address => bool) public fraxlendDeployerWhitelist;

[G-10] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, for example when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

2022-08-frax/src/contracts/FraxlendPair.sol::289 => for (uint256 i = 0; i < _lenders.length; i++) {
2022-08-frax/src/contracts/FraxlendPair.sol::308 => for (uint256 i = 0; i < _borrowers.length; i++) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::265 => for (uint256 i = 0; i < _approvedBorrowers.length; ++i) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::270 => for (uint256 i = 0; i < _approvedLenders.length; ++i) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::51 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::66 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::81 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/libraries/SafeERC20.sol::27 => for (i = 0; i < 32 && data[i] != 0; i++) {

[G-11] <x> += <y> costs more gas than <x> = <x> + <y> for state variables

use <x> = <x> + <y> or <x> = <x> - <y> instead to save gas

2022-08-frax/src/contracts/FraxlendPair.sol::252 => _totalAsset.amount -= uint128(_amountToTransfer);
2022-08-frax/src/contracts/FraxlendPair.sol::253 => _totalAsset.shares -= _shares;
2022-08-frax/src/contracts/FraxlendPairCore.sol::475 => _totalBorrow.amount += uint128(_interestEarned);
2022-08-frax/src/contracts/FraxlendPairCore.sol::476 => _totalAsset.amount += uint128(_interestEarned);
2022-08-frax/src/contracts/FraxlendPairCore.sol::484 => _totalAsset.shares += uint128(_feesShare);
2022-08-frax/src/contracts/FraxlendPairCore.sol::566 => _totalAsset.amount += _amount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::567 => _totalAsset.shares += _shares;
2022-08-frax/src/contracts/FraxlendPairCore.sol::643 => _totalAsset.amount -= _amountToReturn;
2022-08-frax/src/contracts/FraxlendPairCore.sol::644 => _totalAsset.shares -= _shares;
2022-08-frax/src/contracts/FraxlendPairCore.sol::718 => _totalBorrow.amount += _borrowAmount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::719 => _totalBorrow.shares += uint128(_sharesAdded);
2022-08-frax/src/contracts/FraxlendPairCore.sol::724 => userBorrowShares[msg.sender] += _sharesAdded;
2022-08-frax/src/contracts/FraxlendPairCore.sol::772 => userCollateralBalance[_borrower] += _collateralAmount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::773 => totalCollateral += _collateralAmount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::813 => userCollateralBalance[_borrower] -= _collateralAmount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::815 => totalCollateral -= _collateralAmount;
2022-08-frax/src/contracts/FraxlendPairCore.sol::863 => _totalBorrow.amount -= _amountToRepay;
2022-08-frax/src/contracts/FraxlendPairCore.sol::864 => _totalBorrow.shares -= _shares;
2022-08-frax/src/contracts/FraxlendPairCore.sol::867 => userBorrowShares[_borrower] -= _shares;
2022-08-frax/src/contracts/FraxlendPairCore.sol::1008 => totalAsset.amount -= _amountToAdjust;
2022-08-frax/src/contracts/FraxlendPairCore.sol::1011 => _totalBorrow.amount -= _amountToAdjust;
2022-08-frax/src/contracts/FraxlendPairCore.sol::1012 => _totalBorrow.shares -= _sharesToAdjust;

[G-12] abi.encode() is less efficient than abi.encodePacked()

use abi.encodePacked() where possible to save gas

2022-08-frax/src/contracts/FraxlendPairCore.sol::450 => bytes memory _rateData = abi.encode(
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::213 => abi.encode(
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::331 => abi.encode(CIRCUIT_BREAKER_ADDRESS, COMPTROLLER_ADDRESS, TIME_LOCK_ADDRESS, FRAXLEND_WHITELIST_ADDRESS),
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::374 => abi.encode(CIRCUIT_BREAKER_ADDRESS, COMPTROLLER_ADDRESS, TIME_LOCK_ADDRESS, FRAXLEND_WHITELIST_ADDRESS),
2022-08-frax/src/contracts/LinearInterestRate.sol::47 => return abi.encode(MIN_INT, MAX_INT, MAX_VERTEX_UTIL, UTIL_PREC);
2022-08-frax/src/contracts/VariableInterestRate.sol::53 => return abi.encode(MIN_UTIL, MAX_UTIL, UTIL_PREC, MIN_INT, MAX_INT, INT_HALF_LIFE);

[G-13] Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

2022-08-frax/src/contracts/FraxlendPairDeployer.sol::205 => require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::228 => require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::253 => require(deployedPairsByName[_name] == address(0), "FraxlendPairDeployer: Pair name must be unique");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::365 => require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::399 => require(msg.sender == CIRCUIT_BREAKER_ADDRESS, "Circuit Breaker only");

[G-14] Prefix increments cheaper than Postfix increments

++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) Saves 5 gas PER LOOP

2022-08-frax/src/contracts/FraxlendPair.sol::289 => for (uint256 i = 0; i < _lenders.length; i++) {
2022-08-frax/src/contracts/FraxlendPair.sol::308 => for (uint256 i = 0; i < _borrowers.length; i++) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::130 => i++;
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::158 => i++;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::51 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::66 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/FraxlendWhitelist.sol::81 => for (uint256 i = 0; i < _addresses.length; i++) {
2022-08-frax/src/contracts/libraries/SafeERC20.sol::24 => i++;
2022-08-frax/src/contracts/libraries/SafeERC20.sol::27 => for (i = 0; i < 32 && data[i] != 0; i++) {

[G-15] Use bytes32 instead of string

Use bytes32 instead of string to save gas whenever possible. String is a dynamic data structure and therefore is more gas consuming then bytes32.

2022-08-frax/src/contracts/FraxlendPairCore.sol::51 => string public version = "1.0.0";

[G-16] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public and can save gas by doing so.

2022-08-frax/src/contracts/FraxlendPair.sol::74 => function name() public view override(ERC20, IERC20Metadata) returns (string memory) {
2022-08-frax/src/contracts/FraxlendPair.sol::78 => function symbol() public view override(ERC20, IERC20Metadata) returns (string memory) {
2022-08-frax/src/contracts/FraxlendPair.sol::84 => function decimals() public pure override(ERC20, IERC20Metadata) returns (uint8) {
2022-08-frax/src/contracts/FraxlendPair.sol::89 => function totalSupply() public view override(ERC20, IERC20) returns (uint256) {
2022-08-frax/src/contracts/FraxlendPair.sol::100 => function totalAssets() public view virtual returns (uint256) {

[G-17] Not using the named return variables when a function returns, wastes deployment gas

It is not necessary to have both a named return and a return statement.

2022-08-frax/src/contracts/FraxlendPairCore.sol::516 => function _updateExchangeRate() internal returns (uint256 _exchangeRate) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::201 => ) private returns (address _pairAddress) {
2022-08-frax/src/contracts/LinearInterestRate.sol::46 => function getConstants() external pure returns (bytes memory _calldata) {
2022-08-frax/src/contracts/VariableInterestRate.sol::52 => function getConstants() external pure returns (bytes memory _calldata) {

[G-18] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

2022-08-frax/src/contracts/FraxlendPairCore.sol::78 => mapping(address => bool) public swappers; // approved swapper addresses
2022-08-frax/src/contracts/FraxlendPairCore.sol::127 => mapping(address => uint256) public userCollateralBalance; // amount of collateral each user is backed
2022-08-frax/src/contracts/FraxlendPairCore.sol::129 => mapping(address => uint256) public userBorrowShares; // represents the shares held by individuals
2022-08-frax/src/contracts/FraxlendPairCore.sol::134 => mapping(address => bool) public approvedBorrowers;
2022-08-frax/src/contracts/FraxlendPairCore.sol::137 => mapping(address => bool) public approvedLenders;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::32 => mapping(address => bool) public oracleContractWhitelist;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::35 => mapping(address => bool) public rateContractWhitelist;
2022-08-frax/src/contracts/FraxlendWhitelist.sol::38 => mapping(address => bool) public fraxlendDeployerWhitelist;

[G-19] Use assembly to check for address(0)

Saves 6 gas per instance if using assembly to check for address(0)

e.g.

assembly {
 if iszero(_addr) {
  mstore(0x00, "zero address")
  revert(0x00, 0x20)
 }
}

instances:

2022-08-frax/src/contracts/FraxlendPairCore.sol::206 => if (_oracleMultiply != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleMultiply)) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::210 => if (_oracleDivide != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleDivide)) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::523 => if (oracleMultiply != address(0)) {
2022-08-frax/src/contracts/FraxlendPairCore.sol::531 => if (oracleDivide != address(0)) {
2022-08-frax/src/contracts/FraxlendPairDeployer.sol::228 => require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");

[G-20] State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

FraxlendPairDeployer.sol

        CIRCUIT_BREAKER_ADDRESS = _circuitBreaker;
        COMPTROLLER_ADDRESS = _comptroller;
        TIME_LOCK_ADDRESS = _timelock;