Fraxlend (Frax Finance) contest - IllIllI's results

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L204-L207 https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L274-L277

Vulnerability details

Impact

A malicious admin can bypass the timelock and change the protocol fee to the maximum as soon as they see a very large transaction hit the mempool, or can set a swapper address to a contract that always reverts, breaking the functionality of the protocol (repayAssetWithCollateral() and leveragedPosition())

Even if the user is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation.

Proof of Concept

The setTimeLock() function does not itself verify that the change is being done by the timelock, so to change the fee, the admin can change the timelock to him/herself and change the fee, in the same transaction:

File: /src/contracts/FraxlendPair.sol   #1

204      function setTimeLock(address _newAddress) external onlyOwner {
205          emit SetTimeLock(TIME_LOCK_ADDRESS, _newAddress);
206          TIME_LOCK_ADDRESS = _newAddress;
207      }
208  
209      /// @notice The ```ChangeFee``` event first when the fee is changed
210      /// @param _newFee The new fee
211      event ChangeFee(uint32 _newFee);
212  
213      /// @notice The ```changeFee``` function changes the protocol fee, max 50%
214      /// @param _newFee The new fee
215      function changeFee(uint32 _newFee) external whenNotPaused {
216          if (msg.sender != TIME_LOCK_ADDRESS) revert OnlyTimeLock();
217          if (_newFee > MAX_PROTOCOL_FEE) {
218              revert BadProtocolFee();
219          }
220          currentRateInfo.feeToProtocolRate = _newFee;
221          emit ChangeFee(_newFee);
222:     }

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L204-L222

The setSwapper() function does not have a timelock check at all, can specific swappers can be set to false during a sandwich attack by the admin:

File: /src/contracts/FraxlendPair.sol   #2

274      function setSwapper(address _swapper, bool _approval) external onlyOwner {
275          swappers[_swapper] = _approval;
276          emit SetSwapper(_swapper, _approval);
277:     }

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L274-L277

Tools Used

Code inspection

Recommended Mitigation Steps

Add timelock checks to setSwapper(), and setTimeLock()

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L355-L383

Vulnerability details

Impact

Pair creation can be front-run by using custom pairs, griefing legitimate pair creations

Proof of Concept

Normal pair creation generates the pair name using features of the tokens involved, along with a counter (line 324):

File: /src/contracts/FraxlendPairDeployer.sol   #1

310      function deploy(bytes memory _configData) external returns (address _pairAddress) {
311          (address _asset, address _collateral, , , , address _rateContract, ) = abi.decode(
312              _configData,
313              (address, address, address, address, uint256, address, bytes)
314          );
315          string memory _name = string(
316              abi.encodePacked(
317                  "FraxlendV1 - ",
318                  IERC20(_collateral).safeName(),
319                  "/",
320                  IERC20(_asset).safeName(),
321                  " - ",
322                  IRateCalculator(_rateContract).name(),
323                  " - ",
324                  (deployedPairsArray.length + 1).toString()
325              )
326          );
327  
328          _pairAddress = _deployFirst(
329              keccak256(abi.encodePacked("public")),
330              _configData,
331              abi.encode(CIRCUIT_BREAKER_ADDRESS, COMPTROLLER_ADDRESS, TIME_LOCK_ADDRESS, FRAXLEND_WHITELIST_ADDRESS),
332              DEFAULT_MAX_LTV,
333              DEFAULT_LIQ_FEE,
334              0,
335              0,
336              false,
337              false
338          );
339  
340:         _deploySecond(_name, _pairAddress, _configData, new address[](0), new address[](0));

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L310-L340

deployCustom() does no validations on the name passed to it, and a malicious user can monitor the mempool for calls to deploy(), determine the expected _name, and use that in a call to deployCustom(), and specify an _approvedBorrowers/_approvedLenders that rejects everyone:

File: /src/contracts/FraxlendPairDeployer.sol   #2

355      function deployCustom(
356          string memory _name,
357          bytes memory _configData,
358          uint256 _maxLTV,
359          uint256 _liquidationFee,
360          uint256 _maturityDate,
361          uint256 _penaltyRate,
362          address[] memory _approvedBorrowers,
363          address[] memory _approvedLenders
364      ) external returns (address _pairAddress) {
365          require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");
366          require(
367              IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS).fraxlendDeployerWhitelist(msg.sender),
368              "FraxlendPairDeployer: Only whitelisted addresses"
369          );
370  
371          _pairAddress = _deployFirst(
372              keccak256(abi.encodePacked(_name)),
373              _configData,
374              abi.encode(CIRCUIT_BREAKER_ADDRESS, COMPTROLLER_ADDRESS, TIME_LOCK_ADDRESS, FRAXLEND_WHITELIST_ADDRESS),
375              _maxLTV,
376              _liquidationFee,
377              _maturityDate,
378              _penaltyRate,
379              _approvedBorrowers.length > 0,
380              _approvedLenders.length > 0
381          );
382  
383:         _deploySecond(_name, _pairAddress, _configData, _approvedBorrowers, _approvedLenders);

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L355-L383

_deploySecond(), called by both of the above functions, requires that the name provided does not match a prior name:

File: /src/contracts/FraxlendPairDeployer.sol   #3

242      function _deploySecond(
243          string memory _name,
244          address _pairAddress,
245          bytes memory _configData,
246          address[] memory _approvedBorrowers,
247          address[] memory _approvedLenders
248      ) private {
249          (, , , , , , bytes memory _rateInitData) = abi.decode(
250              _configData,
251              (address, address, address, address, uint256, address, bytes)
252          );
253:         require(deployedPairsByName[_name] == address(0), "FraxlendPairDeployer: Pair name must be unique");

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L242-L253

Tools Used

Code inspection

Recommended Mitigation Steps

Prefix all deployCustom() names with a known prefix, and add the same sort of counter to the resulting name

Summary

Low Risk Issues

Total: 10 instances over 4 issues

Non-critical Issues

Total: 84 instances over 14 issues

Low Risk Issues

[L‑01] Allow-list not required if maxLTV is zero

According to _isSolvent(), if maxLTV is zero, the solvency check will always pass. The documentation states When making under-collateralized loans, lenders know and trust the counter-party so the check below should include a check for zero

There is 1 instance of this issue:

File: /src/contracts/FraxlendPairCore.sol

196              if (_maxLTV >= LTV_PRECISION && !_isBorrowerWhitelistActive) revert BorrowerWhitelistRequired();
197:             maxLTV = _maxLTV;

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

[L‑02] Exchange data not fetched once per block once timestamp passes type(uint32).max

The comments say that the exchange data is only fetched once per block. Elsewhere timestamps are cast to uint64, but here they're cast to uint32 so it's inconsistent, and one will wrap before the other. Once the _exchangeRateInfo.lastTimestamp passes type(uint32).max, the block.timestamp will never match the stored value, and exchange rate data will be looked up any time the function is called.

There is 1 instance of this issue:

File: /src/contracts/FraxlendPairCore.sol

544:         _exchangeRateInfo.lastTimestamp = uint32(block.timestamp);

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

[L‑03] Missing checks for address(0x0) when assigning values to address state variables

There are 5 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

104:          CIRCUIT_BREAKER_ADDRESS = _circuitBreaker;

105:          COMPTROLLER_ADDRESS = _comptroller;

106:          TIME_LOCK_ADDRESS = _timelock;

107:          FRAXLEND_WHITELIST_ADDRESS = _fraxlendWhitelist;

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

File: src/contracts/FraxlendPair.sol

206:          TIME_LOCK_ADDRESS = _newAddress;

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

[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.

If all arguments are strings and or bytes, bytes.concat() should be used instead

There are 3 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

204:              bytes32 salt = keccak256(abi.encodePacked(_saltSeed, _configData));

329:              keccak256(abi.encodePacked("public")),

372:              keccak256(abi.encodePacked(_name)),

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

Non-critical Issues

[N‑01] Inconsistent constant naming pattern

In the other files UTIL_PREC is specified as it is here, so that seems to be the pattern. However, the other variables type out the full word. They should all be made to be consistent

There is 1 instance of this issue:

File: /src/contracts/FraxlendPairConstants.sol

34       uint256 internal constant LTV_PRECISION = 1e5; // 5 decimals
35       uint256 internal constant LIQ_PRECISION = 1e5;
36       uint256 internal constant UTIL_PREC = 1e5;
37       uint256 internal constant FEE_PRECISION = 1e5;
38:      uint256 internal constant EXCHANGE_PRECISION = 1e18;

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairConstants.sol#L34-L38

[N‑02] 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

There are 2 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

1103:         _assetContract.approve(_swapperAddress, _borrowAmount);

1184:         _collateralContract.approve(_swapperAddress, _collateralToSwap);

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

[N‑03] The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against reentrancy in other modifiers

There are 4 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

747:          nonReentrant

914:          nonReentrant

954:      ) external whenNotPaused nonReentrant approvedLender(msg.sender) returns (uint256 _collateralForLiquidator) {

1071:         nonReentrant

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

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

There is 1 instance of this issue:

File: src/contracts/FraxlendPairDeployer.sol

233:          return _pairAddress;

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

[N‑05] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 16 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit 9000
194:              dirtyLiquidationFee = (_liquidationFee * 9000) / LIQ_PRECISION; // 90% of clean fee

/// @audit 1e18
468:              _interestEarned = (_deltaTime * _totalBorrow.amount * _currentRateInfo.ratePerSec) / 1e18;

/// @audit 1e36
522:          uint256 _price = uint256(1e36);

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

File: src/contracts/FraxlendPairDeployer.sol

/// @audit 13000
171:          bytes memory _firstHalf = BytesLib.slice(_creationCode, 0, 13000);

/// @audit 13000
173:          if (_creationCode.length > 13000) {

/// @audit 13000
/// @audit 13000
174:              bytes memory _secondHalf = BytesLib.slice(_creationCode, 13000, _creationCode.length - 13000);

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

File: src/contracts/FraxlendPair.sol

/// @audit 1e18
105:          _assetsPerUnitShare = totalAsset.toAmount(1e18, false);

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

File: src/contracts/libraries/SafeERC20.sol

/// @audit 64
19:           if (data.length >= 64) {

/// @audit 32
21:           } else if (data.length == 32) {

/// @audit 32
23:               while (i < 32 && data[i] != 0) {

/// @audit 32
27:               for (i = 0; i < 32 && data[i] != 0; i++) {

/// @audit 32
/// @audit 18
57:           return success && data.length == 32 ? abi.decode(data, (uint8)) : 18;

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/libraries/SafeERC20.sol#L19

File: src/contracts/VariableInterestRate.sol

/// @audit 1e18
69:               uint256 _deltaUtilization = ((MIN_UTIL - _utilization) * 1e18) / MIN_UTIL;

/// @audit 1e18
76:               uint256 _deltaUtilization = ((_utilization - MAX_UTIL) * 1e18) / (UTIL_PREC - MAX_UTIL);

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

[N‑06] Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used

There is 1 instance of this issue:

File: src/contracts/VariableInterestRate.sol

/// @audit 43200e36
42:       uint256 private constant INT_HALF_LIFE = 43200e36; // given in seconds, equal to 12 hours, additional 1e36 to make math simpler

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

[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

There are 6 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

144:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

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

File: src/contracts/FraxlendPairDeployer.sol

184:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

239:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

268:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

308:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

348:      /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)

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

[N‑08] Variable names that consist of all capital letters should be reserved for constant/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

There are 15 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

86:       address public TIME_LOCK_ADDRESS;

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

File: src/contracts/FraxlendPairDeployer.sol

49:       uint256 public DEFAULT_MAX_LTV = 75000; // 75% with 1e5 precision

50:       uint256 public GLOBAL_MAX_LTV = 1e8; // 1000x (100,000%) with 1e5 precision, protects from rounding errors in LTV calc

51:       uint256 public DEFAULT_LIQ_FEE = 10000; // 10% with 1e5 precision

57:       address public CIRCUIT_BREAKER_ADDRESS;

58:       address public COMPTROLLER_ADDRESS;

59:       address public TIME_LOCK_ADDRESS;

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

File: src/contracts/FraxlendPair.sol

160:              uint256 _LTV_PRECISION,

161:              uint256 _LIQ_PRECISION,

162:              uint256 _UTIL_PREC,

163:              uint256 _FEE_PRECISION,

164:              uint256 _EXCHANGE_PRECISION,

165:              uint64 _DEFAULT_INT,

166:              uint16 _DEFAULT_PROTOCOL_FEE,

167:              uint256 _MAX_PROTOCOL_FEE

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

[N‑09] Non-library/interface files should use fixed compiler versions, not floating ones

There are 5 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

2:    pragma solidity ^0.8.15;

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

File: src/contracts/FraxlendPair.sol

2:    pragma solidity ^0.8.15;

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

File: src/contracts/FraxlendWhitelist.sol

2:    pragma solidity ^0.8.15;

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

File: src/contracts/LinearInterestRate.sol

2:    pragma solidity ^0.8.15;

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

File: src/contracts/VariableInterestRate.sol

2:    pragma solidity ^0.8.15;

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

[N‑10] File is missing NatSpec

There is 1 instance of this issue:

File: src/contracts/FraxlendPairConstants.sol

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

[N‑11] NatSpec is incomplete

There are 7 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit Missing: '@param _immutables'
143       /// @notice The ```constructor``` function is called on deployment
144       /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
145       /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
146       /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
147       /// @param _maturityDate The maturityDate date of the Pair
148       /// @param _penaltyRate The interest rate after maturity date
149       /// @param _isBorrowerWhitelistActive Enables borrower whitelist
150       /// @param _isLenderWhitelistActive Enables lender whitelist
151       constructor(
152           bytes memory _configData,
153           bytes memory _immutables,
154           uint256 _maxLTV,
155           uint256 _liquidationFee,
156           uint256 _maturityDate,
157           uint256 _penaltyRate,
158           bool _isBorrowerWhitelistActive,
159:          bool _isLenderWhitelistActive

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

File: src/contracts/FraxlendPairDeployer.sol

/// @audit Missing: '@param _saltSeed'
/// @audit Missing: '@param _immutables'
/// @audit Missing: '@param _penaltyRate'
183       /// @notice The ```_deployFirst``` function is an internal function with deploys the pair
184       /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
185       /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
186       /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
187       /// @param _maturityDate The maturityDate of the Pair
188       /// @param _isBorrowerWhitelistActive Enables borrower whitelist
189       /// @param _isLenderWhitelistActive Enables lender whitelist
190       /// @return _pairAddress The address to which the Pair was deployed
191       function _deployFirst(
192           bytes32 _saltSeed,
193           bytes memory _configData,
194           bytes memory _immutables,
195           uint256 _maxLTV,
196           uint256 _liquidationFee,
197           uint256 _maturityDate,
198           uint256 _penaltyRate,
199           bool _isBorrowerWhitelistActive,
200           bool _isLenderWhitelistActive
201:      ) private returns (address _pairAddress) {

/// @audit Missing: '@param _penaltyRate'
345       /// @notice The ```deployCustom``` function allows whitelisted users to deploy custom Term Sheets for OTC debt structuring
346       /// @dev Caller must be added to FraxLedWhitelist
347       /// @param _name The name of the Pair
348       /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
349       /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
350       /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
351       /// @param _maturityDate The maturityDate of the Pair
352       /// @param _approvedBorrowers An array of approved borrower addresses
353       /// @param _approvedLenders An array of approved lender addresses
354       /// @return _pairAddress The address to which the Pair was deployed
355       function deployCustom(
356           string memory _name,
357           bytes memory _configData,
358           uint256 _maxLTV,
359           uint256 _liquidationFee,
360           uint256 _maturityDate,
361           uint256 _penaltyRate,
362           address[] memory _approvedBorrowers,
363           address[] memory _approvedLenders
364:      ) external returns (address _pairAddress) {

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L183-L201

File: src/contracts/FraxlendPair.sol

/// @audit Missing: '@return'
181       /// @param _amount Amount of borrow
182       /// @param _roundUp Whether to roundup during division
183:      function toBorrowShares(uint256 _amount, bool _roundUp) external view returns (uint256) {

/// @audit Missing: '@return'
188       /// @param _shares Shares of borrow
189       /// @param _roundUp Whether to roundup during division
190:      function toBorrowAmount(uint256 _shares, bool _roundUp) external view returns (uint256) {

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L181-L183

[N‑12] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

There are 18 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

376       event AddInterest(
377           uint256 _interestEarned,
378           uint256 _rate,
379           uint256 _deltaTime,
380           uint256 _feesAmount,
381           uint256 _feesShare
382:      );

389:      event UpdateRate(uint256 _ratePerSec, uint256 _deltaTime, uint256 _utilizationRate, uint256 _newRatePerSec);

504:      event UpdateExchangeRate(uint256 _rate);

696       event BorrowAsset(
697           address indexed _borrower,
698           address indexed _receiver,
699           uint256 _borrowAmount,
700           uint256 _sharesAdded
701:      );

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(
898           address indexed _borrower,
899           uint256 _collateralForLiquidator,
900           uint256 _sharesToLiquidate,
901           uint256 _amountLiquidatorToRepay,
902           uint256 _sharesToAdjust,
903           uint256 _amountToAdjust
904:      );

1045      event LeveragedPosition(
1046          address indexed _borrower,
1047          address _swapperAddress,
1048          uint256 _borrowAmount,
1049          uint256 _borrowShares,
1050          uint256 _initialCollateralAmount,
1051          uint256 _amountCollateralOut
1052:     );

1143      event RepayAssetWithCollateral(
1144          address indexed _borrower,
1145          address _swapperAddress,
1146          uint256 _collateralToSwap,
1147          uint256 _amountAssetOut,
1148          uint256 _sharesRepaid
1149:     );

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

File: src/contracts/FraxlendPair.sol

200:      event SetTimeLock(address _oldAddress, address _newAddress);

211:      event ChangeFee(uint32 _newFee);

228:      event WithdrawFees(uint128 _shares, address _recipient, uint256 _amountToTransfer);

268:      event SetSwapper(address _swapper, bool _approval);

282:      event SetApprovedLender(address indexed _address, bool _approval);

301:      event SetApprovedBorrower(address indexed _address, bool _approval);

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

File: src/contracts/FraxlendWhitelist.sol

45:       event SetOracleWhitelist(address indexed _address, bool _bool);

60:       event SetRateContractWhitelist(address indexed _address, bool _bool);

75:       event SetFraxlendDeployerWhitelist(address indexed _address, bool _bool);

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

[N‑13] Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 6 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit _interestEarned
/// @audit _feesAmount
/// @audit _feesShare
/// @audit _newRate
393       function addInterest()
394           external
395           nonReentrant
396           returns (
397               uint256 _interestEarned,
398               uint256 _feesAmount,
399               uint256 _feesShare,
400:              uint64 _newRate

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

File: src/contracts/LinearInterestRate.sol

/// @audit _calldata
46:       function getConstants() external pure returns (bytes memory _calldata) {

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

File: src/contracts/VariableInterestRate.sol

/// @audit _calldata
52:       function getConstants() external pure returns (bytes memory _calldata) {

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

[N‑14] Avoid the use of sensitive terms

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

There is 1 instance of this issue:

File: /src/contracts/FraxlendPair.sol

270:     /// @notice The ```setSwapper``` function is called to black or whitelist a given swapper address

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

Summary

Gas Optimizations

Total: 124 instances over 19 issues

Gas Optimizations

[G‑01] String should only be generated once, and saved

There is 1 instance of this issue:

File: /src/contracts/FraxlendPair.sol

81:          return string(abi.encodePacked("FraxlendV1 - ", collateralContract.safeSymbol(), "/", assetContract.safeSymbol()));

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

[G‑02] Remove or replace unused state variables

Saves a storage slot. If the variable is assigned a non-zero value, saves Gsset (20000 gas). If it's assigned a zero value, saves Gsreset (2900 gas). If the variable remains unassigned, there is no gas savings unless the variable is public, in which case the compiler-generated non-payable getter deployment cost is saved. If the state variable is overriding an interface's public function, mark the variable as constant or immutable so that it does not use a storage slot

There is 1 instance of this issue:

File: src/contracts/FraxlendPairCore.sol

51:       string public version = "1.0.0";

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

[G‑03] Multiple address/ID mappings can be combined into a single mapping of an address/ID 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.

There is 1 instance of this issue:

File: src/contracts/FraxlendPairCore.sol

127       mapping(address => uint256) public userCollateralBalance; // amount of collateral each user is backed
128       /// @notice Stores the balance of borrow shares for each user
129:      mapping(address => uint256) public userBorrowShares; // represents the shares held by individuals

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

[G‑04] Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 11 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit _configData
/// @audit _immutables
151       constructor(
152           bytes memory _configData,
153           bytes memory _immutables,
154           uint256 _maxLTV,
155           uint256 _liquidationFee,
156           uint256 _maturityDate,
157           uint256 _penaltyRate,
158           bool _isBorrowerWhitelistActive,
159:          bool _isLenderWhitelistActive

/// @audit _path
1062      function leveragedPosition(
1063          address _swapperAddress,
1064          uint256 _borrowAmount,
1065          uint256 _initialCollateralAmount,
1066          uint256 _amountCollateralOutMin,
1067          address[] memory _path
1068      )
1069          external
1070          isNotPastMaturity
1071          nonReentrant
1072          whenNotPaused
1073          approvedBorrower
1074          isSolvent(msg.sender)
1075:         returns (uint256 _totalCollateralBalance)

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

File: src/contracts/FraxlendPairDeployer.sol

/// @audit _configData
310:      function deploy(bytes memory _configData) external returns (address _pairAddress) {

/// @audit _name
/// @audit _configData
/// @audit _approvedBorrowers
/// @audit _approvedLenders
355       function deployCustom(
356           string memory _name,
357           bytes memory _configData,
358           uint256 _maxLTV,
359           uint256 _liquidationFee,
360           uint256 _maturityDate,
361           uint256 _penaltyRate,
362           address[] memory _approvedBorrowers,
363           address[] memory _approvedLenders
364:      ) external returns (address _pairAddress) {

/// @audit _addresses
398:      function globalPause(address[] memory _addresses) external returns (address[] memory _updatedAddresses) {

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

File: src/contracts/FraxlendPair.sol

/// @audit _configData
/// @audit _immutables
45        constructor(
46            bytes memory _configData,
47            bytes memory _immutables,
48            uint256 _maxLTV,
49            uint256 _liquidationFee,
50            uint256 _maturityDate,
51            uint256 _penaltyRate,
52            bool _isBorrowerWhitelistActive,
53            bool _isLenderWhitelistActive
54        )
55            FraxlendPairCore(
56                _configData,
57                _immutables,
58                _maxLTV,
59                _liquidationFee,
60                _maturityDate,
61                _penaltyRate,
62                _isBorrowerWhitelistActive,
63                _isLenderWhitelistActive
64            )
65            ERC20("", "")
66            Ownable()
67:           Pausable()

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPair.sol#L45-L67

[G‑05] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

There are 16 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

419:          CurrentRateInfo memory _currentRateInfo = currentRateInfo;

426:          VaultAccount memory _totalAsset = totalAsset;

427:          VaultAccount memory _totalBorrow = totalBorrow;

517:          ExchangeRateInfo memory _exchangeRateInfo = exchangeRateInfo;

592:          VaultAccount memory _totalAsset = totalAsset;

611:          VaultAccount memory _totalAsset = totalAsset;

666:          VaultAccount memory _totalAsset = totalAsset;

682:          VaultAccount memory _totalAsset = totalAsset;

708:          VaultAccount memory _totalBorrow = totalBorrow;

884:          VaultAccount memory _totalBorrow = totalBorrow;

926:          VaultAccount memory _totalBorrow = totalBorrow;

965:          VaultAccount memory _totalBorrow = totalBorrow;

1204:         VaultAccount memory _totalBorrow = totalBorrow;

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

File: src/contracts/FraxlendPairDeployer.sol

123:          string[] memory _deployedPairsArray = deployedPairsArray;

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

File: src/contracts/FraxlendPair.sol

236:          VaultAccount memory _totalAsset = totalAsset;

237:          VaultAccount memory _totalBorrow = totalBorrow;

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

[G‑06] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 2 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

/// @audit DEFAULT_MAX_LTV on line 332
/// @audit DEFAULT_LIQ_FEE on line 333
342:          _logDeploy(_name, _pairAddress, _configData, DEFAULT_MAX_LTV, DEFAULT_LIQ_FEE, 0);

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

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

Using the addition operator instead of plus-equals saves 113 gas

There are 2 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

773:          totalCollateral += _collateralAmount;

815:          totalCollateral -= _collateralAmount;

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

[G‑08] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }

There are 3 instances of this issue:

File: src/contracts/LinearInterestRate.sol

/// @audit if-condition on line 86
88:               _newRatePerSec = uint64(_vertexInterest + (((_utilization - _vertexUtilization) * _slope) / UTIL_PREC));

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

File: src/contracts/VariableInterestRate.sol

/// @audit if-condition on line 68
69:               uint256 _deltaUtilization = ((MIN_UTIL - _utilization) * 1e18) / MIN_UTIL;

/// @audit if-condition on line 75
76:               uint256 _deltaUtilization = ((_utilization - MAX_UTIL) * 1e18) / (UTIL_PREC - MAX_UTIL);

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

[G‑09] <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

• storage arrays incur a Gwarmaccess (100 gas)
• memory arrays use MLOAD (3 gas)
• calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 7 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

265:          for (uint256 i = 0; i < _approvedBorrowers.length; ++i) {

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

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

File: src/contracts/FraxlendPair.sol

289:          for (uint256 i = 0; i < _lenders.length; i++) {

308:          for (uint256 i = 0; i < _borrowers.length; i++) {

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

File: src/contracts/FraxlendWhitelist.sol

51:           for (uint256 i = 0; i < _addresses.length; i++) {

66:           for (uint256 i = 0; i < _addresses.length; i++) {

81:           for (uint256 i = 0; i < _addresses.length; i++) {

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

[G‑10] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case 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

There are 8 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

265:          for (uint256 i = 0; i < _approvedBorrowers.length; ++i) {

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

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

File: src/contracts/FraxlendPair.sol

289:          for (uint256 i = 0; i < _lenders.length; i++) {

308:          for (uint256 i = 0; i < _borrowers.length; i++) {

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

File: src/contracts/FraxlendWhitelist.sol

51:           for (uint256 i = 0; i < _addresses.length; i++) {

66:           for (uint256 i = 0; i < _addresses.length; i++) {

81:           for (uint256 i = 0; i < _addresses.length; i++) {

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

File: src/contracts/libraries/SafeERC20.sol

27:               for (i = 0; i < 32 && data[i] != 0; i++) {

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/libraries/SafeERC20.sol#L27

[G‑11] require()/revert() strings longer than 32 bytes cost extra gas

Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas

There are 8 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

205:              require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");

228:              require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");

253:          require(deployedPairsByName[_name] == address(0), "FraxlendPairDeployer: Pair name must be unique");

365:          require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");

366           require(
367               IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS).fraxlendDeployerWhitelist(msg.sender),
368               "FraxlendPairDeployer: Only whitelisted addresses"
369:          );

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

File: src/contracts/LinearInterestRate.sol

57            require(
58                _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT,
59                "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"
60:           );

61            require(
62                _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT,
63                "LinearInterestRate: _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT"
64:           );

65            require(
66                _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0,
67                "LinearInterestRate: _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0"
68:           );

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/LinearInterestRate.sol#L57-L60

[G‑12] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 6 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit initialize(), addInterest(), updateExchangeRate(), borrowAsset(), addCollateral(), removeCollateral(), repayAsset(), liquidate(), liquidateClean(), leveragedPosition(), repayAssetWithCollateral()
46:   abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ownable, Pausable, ReentrancyGuard {

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

File: src/contracts/FraxlendPairDeployer.sol

/// @audit deployedPairsLength(), getAllPairAddresses(), getCustomStatuses(), setCreationCode(), deploy(), deployCustom(), globalPause()
44:   contract FraxlendPairDeployer is Ownable {

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

File: src/contracts/FraxlendPair.sol

/// @audit assetsPerShare(), assetsOf(), getConstants(), toBorrowShares(), toBorrowAmount(), setTimeLock(), changeFee(), withdrawFees(), setSwapper(), setApprovedLenders(), setApprovedBorrowers(), pause(), unpause()
41:   contract FraxlendPair is FraxlendPairCore {

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

File: src/contracts/FraxlendWhitelist.sol

/// @audit setOracleContractWhitelist(), setRateContractWhitelist(), setFraxlendDeployerWhitelist()
30:   contract FraxlendWhitelist is Ownable {

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

File: src/contracts/LinearInterestRate.sol

/// @audit getConstants(), requireValidInitData(), getNewRate()
32:   contract LinearInterestRate is IRateCalculator {

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

File: src/contracts/VariableInterestRate.sol

/// @audit getConstants(), requireValidInitData(), getNewRate()
33:   contract VariableInterestRate is IRateCalculator {

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

[G‑13] 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.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past

There are 9 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

78:       mapping(address => bool) public swappers; // approved swapper addresses

133:      bool public immutable borrowerWhitelistActive;

134:      mapping(address => bool) public approvedBorrowers;

136:      bool public immutable lenderWhitelistActive;

137:      mapping(address => bool) public approvedLenders;

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

File: src/contracts/FraxlendPairDeployer.sol

94:       mapping(address => bool) public deployedPairCustomStatusByAddress;

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

File: src/contracts/FraxlendWhitelist.sol

32:       mapping(address => bool) public oracleContractWhitelist;

35:       mapping(address => bool) public rateContractWhitelist;

38:       mapping(address => bool) public fraxlendDeployerWhitelist;

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

[G‑14] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There are 9 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

130:                  i++;

158:                  i++;

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

File: src/contracts/FraxlendPair.sol

289:          for (uint256 i = 0; i < _lenders.length; i++) {

308:          for (uint256 i = 0; i < _borrowers.length; i++) {

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

File: src/contracts/FraxlendWhitelist.sol

51:           for (uint256 i = 0; i < _addresses.length; i++) {

66:           for (uint256 i = 0; i < _addresses.length; i++) {

81:           for (uint256 i = 0; i < _addresses.length; i++) {

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

File: src/contracts/libraries/SafeERC20.sol

24:                   i++;

27:               for (i = 0; i < 32 && data[i] != 0; i++) {

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/libraries/SafeERC20.sol#L24

[G‑15] Splitting require() statements that use && saves gas

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper by 3 gas

There are 3 instances of this issue:

File: src/contracts/LinearInterestRate.sol

57            require(
58                _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT,
59                "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"
60:           );

61            require(
62                _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT,
63                "LinearInterestRate: _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT"
64:           );

65            require(
66                _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0,
67                "LinearInterestRate: _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0"
68:           );

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/LinearInterestRate.sol#L57-L60

[G‑16] 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.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

There are 13 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

/// @audit uint64 _newRate
421:              _newRate = _currentRateInfo.ratePerSec;

/// @audit uint64 _newRate
448:                  _newRate = uint64(penaltyRate);

/// @audit uint64 _newRate
456:                  _newRate = IRateCalculator(rateContract).getNewRate(_rateData, rateInitCallData);

/// @audit uint128 _amountToAdjust
1005:                     _amountToAdjust = (_totalBorrow.toAmount(_sharesToAdjust, false)).toUint128();

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

File: src/contracts/FraxlendPair.sol

/// @audit uint64 _DEFAULT_INT
175:          _DEFAULT_INT = DEFAULT_INT;

/// @audit uint16 _DEFAULT_PROTOCOL_FEE
176:          _DEFAULT_PROTOCOL_FEE = DEFAULT_PROTOCOL_FEE;

/// @audit uint128 _shares
240:          if (_shares == 0) _shares = uint128(balanceOf(address(this)));

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

File: src/contracts/libraries/SafeERC20.sol

/// @audit uint8 i
27:               for (i = 0; i < 32 && data[i] != 0; i++) {

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/libraries/SafeERC20.sol#L27

File: src/contracts/LinearInterestRate.sol

/// @audit uint64 _newRatePerSec
85:               _newRatePerSec = uint64(_minInterest + ((_utilization * _slope) / UTIL_PREC));

/// @audit uint64 _newRatePerSec
88:               _newRatePerSec = uint64(_vertexInterest + (((_utilization - _vertexUtilization) * _slope) / UTIL_PREC));

/// @audit uint64 _newRatePerSec
90:               _newRatePerSec = uint64(_vertexInterest);

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

File: src/contracts/VariableInterestRate.sol

/// @audit uint64 _newRatePerSec
71:               _newRatePerSec = uint64((_currentRatePerSec * INT_HALF_LIFE) / _decayGrowth);

/// @audit uint64 _newRatePerSec
78:               _newRatePerSec = uint64((_currentRatePerSec * _decayGrowth) / INT_HALF_LIFE);

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

[G‑17] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

There are 8 instances of this issue:

File: src/contracts/FraxlendPairCore.sol

64:       uint256 public immutable oracleNormalization;

67:       uint256 public immutable maxLTV;

70:       uint256 public immutable cleanLiquidationFee;

71:       uint256 public immutable dirtyLiquidationFee;

95:       uint256 public immutable maturityDate;

96:       uint256 public immutable penaltyRate;

133:      bool public immutable borrowerWhitelistActive;

136:      bool public immutable lenderWhitelistActive;

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

[G‑18] 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 hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There are 9 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

205:              require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");

228:              require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");

253:          require(deployedPairsByName[_name] == address(0), "FraxlendPairDeployer: Pair name must be unique");

365:          require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");

366           require(
367               IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS).fraxlendDeployerWhitelist(msg.sender),
368               "FraxlendPairDeployer: Only whitelisted addresses"
369:          );

399:          require(msg.sender == CIRCUIT_BREAKER_ADDRESS, "Circuit Breaker only");

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

File: src/contracts/LinearInterestRate.sol

57            require(
58                _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT,
59                "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"
60:           );

61            require(
62                _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT,
63                "LinearInterestRate: _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT"
64:           );

65            require(
66                _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0,
67                "LinearInterestRate: _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0"
68:           );

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/LinearInterestRate.sol#L57-L60

[G‑19] 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

There are 7 instances of this issue:

File: src/contracts/FraxlendPairDeployer.sol

170:      function setCreationCode(bytes calldata _creationCode) external onlyOwner {

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

File: src/contracts/FraxlendPair.sol

204:      function setTimeLock(address _newAddress) external onlyOwner {

234:      function withdrawFees(uint128 _shares, address _recipient) external onlyOwner returns (uint256 _amountToTransfer) {

274:      function setSwapper(address _swapper, bool _approval) external onlyOwner {

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

File: src/contracts/FraxlendWhitelist.sol

50:       function setOracleContractWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {

65:       function setRateContractWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {

80:       function setFraxlendDeployerWhitelist(address[] calldata _addresses, bool _bool) external onlyOwner {

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