Platform: Code4rena
Start Date: 12/08/2022
Pot Size: $50,000 USDC
Total HM: 15
Participants: 120
Period: 5 days
Judge: Justin Goro
Total Solo HM: 6
Id: 153
League: ETH
Rank: 60/120
Findings: 2
Award: $67.01
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0x1f8b
Also found by: 0x52, 0xA5DF, 0xDjango, 0xNazgul, 0xNineDec, 0xSmartContract, 0xmatt, 0xsolstars, Aymen0909, Bnke0x0, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, EthLedger, Funen, IllIllI, JC, Junnon, Lambda, LeoS, MiloTruck, Noah3o6, PaludoX0, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, SaharAP, Sm4rty, SooYa, The_GUILD, TomJ, Waze, Yiko, _Adam, __141345__, a12jmx, ak1, asutorufos, auditor0517, ayeslick, ballx, beelzebufo, berndartmueller, bin2chen, brgltd, c3phas, cRat1st0s, cccz, cryptonue, cryptphi, d3e4, delfin454000, dipp, djxploit, durianSausage, dy, erictee, fatherOfBlocks, gogo, gzeon, hyh, ignacio, kyteg, ladboy233, medikko, mics, minhquanym, oyc_109, pfapostol, rbserver, reassor, ret2basic, robee, sach1r0, simon135, sryysryy, tabish, yac, yash90, zzzitron
45.8358 USDC - $45.84
@param
statementsFraxlendPairCore.sol: L143-160
/// @notice The ```constructor``` function is called on deployment /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData) /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision) /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision) /// @param _maturityDate The maturityDate date of the Pair /// @param _penaltyRate The interest rate after maturity date /// @param _isBorrowerWhitelistActive Enables borrower whitelist /// @param _isLenderWhitelistActive Enables lender whitelist constructor( bytes memory _configData, bytes memory _immutables, uint256 _maxLTV, uint256 _liquidationFee, uint256 _maturityDate, uint256 _penaltyRate, bool _isBorrowerWhitelistActive, bool _isLenderWhitelistActive ) {
@param
statement is missing for _immutables
FraxlendPairCore.sol: L791-800
/// @notice The ```RemoveCollateral``` event is emitted when collateral is removed from a borrower's position /// @param _sender The account from which funds are transferred /// @param _collateralAmount The amount of Collateral Token to be transferred /// @param _receiver The address to which Collateral Tokens will be transferred event RemoveCollateral( address indexed _sender, uint256 _collateralAmount, address indexed _receiver, address indexed _borrower );
@param
statement is missing for _borrower
FraxlendPairCore.sol: L892-904
/// @notice The ```Liquidate``` event is emitted when a liquidation occurs /// @param _borrower The borrower account for which the liquidation occured /// @param _collateralForLiquidator The amount of Collateral Token transferred to the liquidator /// @param _sharesToLiquidate The number of Borrow Shares the liquidator repaid on behalf of the borrower /// @param _sharesToAdjust The number of Borrow Shares that were adjusted on liabilites and assets (a writeoff) event Liquidate( address indexed _borrower, uint256 _collateralForLiquidator, uint256 _sharesToLiquidate, uint256 _amountLiquidatorToRepay, uint256 _sharesToAdjust, uint256 _amountToAdjust );
@param
statements missing for _amountLiquidatorToRepay
and _amountToAdjust
FraxlendPairDeployer.sol: L183-201
/// @notice The ```_deployFirst``` function is an internal function with deploys the pair /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData) /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision) /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision) /// @param _maturityDate The maturityDate of the Pair /// @param _isBorrowerWhitelistActive Enables borrower whitelist /// @param _isLenderWhitelistActive Enables lender whitelist /// @return _pairAddress The address to which the Pair was deployed function _deployFirst( bytes32 _saltSeed, bytes memory _configData, bytes memory _immutables, uint256 _maxLTV, uint256 _liquidationFee, uint256 _maturityDate, uint256 _penaltyRate, bool _isBorrowerWhitelistActive, bool _isLenderWhitelistActive ) private returns (address _pairAddress) {
@param
statements missing for _saltSeed
, _immutables
and _penaltyRate
FraxlendPairDeployer.sol: L345-364
/// @notice The ```deployCustom``` function allows whitelisted users to deploy custom Term Sheets for OTC debt structuring /// @dev Caller must be added to FraxLedWhitelist /// @param _name The name of the Pair /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData) /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision) /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision) /// @param _maturityDate The maturityDate of the Pair /// @param _approvedBorrowers An array of approved borrower addresses /// @param _approvedLenders An array of approved lender addresses /// @return _pairAddress The address to which the Pair was deployed function deployCustom( string memory _name, bytes memory _configData, uint256 _maxLTV, uint256 _liquidationFee, uint256 _maturityDate, uint256 _penaltyRate, address[] memory _approvedBorrowers, address[] memory _approvedLenders ) external returns (address _pairAddress) {
@param
statement is missing for _penaltyRate
FraxlendPairCore.sol: L1020-1022
// NOTE: reverts if _collateralForLiquidator > userCollateralBalance // Collateral is removed on behalf of borrower and sent to liquidator // NOTE: reverts if _collateralForLiquidator > userCollateralBalance
Remove duplicate comment line
/// @notice A Contract for calulcating interest rates as a function of utilization and time
Change calulcating
to calculating
/// @notice The ```setOracleContractWhitelist``` function sets a given address to true/false for use as oralce
Change oralce
to Oracle
// Set approved lenders whitlist active
Change whitlist
to whitelist
/// @param _borrower The borrower account for which the liquidation occured
Change occured
to occurred
/// @param _sharesToAdjust The number of Borrow Shares that were adjusted on liabilites and assets (a writeoff)
Change liabilites
to liabilities
// Note: Ensure this memory stuct will be passed to _repayAsset for write to state
Change stuct
to struct
/// @param _borrowAmount The amount of Asset Token to be borrowed to be borrowed
Remove repeated phrase to be borrowed
The same typo (black list
) occurs in both lines below:
/// @dev Cannot black list self
Change black list
to blacklist
in both cases
The same typo (whos
) occurs in both lines below:
/// @param _lenders The addresses whos status will be set
Change whos
to whose
The same typo (approcal
) occurs in both lines below:
/// @param _approval The approcal status
Change approcal
to approval
FraxlendPairDeployer.sol: L168
/// @dev splits the data if necessary to accomodate creation code that is slightly larger than 24kb
Change accomodate
to accommodate
In theory, comments over 79 characters should wrap using multi-line comment syntax. Even if somewhat longer comments are acceptable, there are cases where very long comments interfere with readability. Below are five instances of extra-long comments whose readability could be improved by wrapping, as shown:
/// @notice The ```addInterest``` function is a public implementation of _addInterest and allows 3rd parties to trigger interest accrual
Suggestion:
/// @notice The ```addInterest``` function is a public implementation of _addInterest /// and allows 3rd parties to trigger interest accrual.
/// @notice The ```_addInterest``` function is invoked prior to every external function and is used to accrue interest and update interest rate
Suggestion:
/// @notice The ```_addInterest``` function is invoked prior to every external function /// and is used to accrue interest and update interest rate.
FraxlendPairCore.sol: L616-618
/// @notice The ```_redeem``` function is an internal implementation which allows a Lender to pull their Asset Tokens out of the Pair /// @dev Caller must invoke ```ERC20.approve``` on the Asset Token contract prior to calling function /// @param _totalAsset An in-memory VaultAccount struct which holds the total amount of Asset Tokens and the total number of Asset Shares (fTokens)
Suggestion:
/// @notice The ```_redeem``` function is an internal implementation /// which allows a Lender to pull their Asset Tokens out of the Pair. /// @dev Caller must invoke ```ERC20.approve``` on the Asset Token contract prior to calling function /// @param _totalAsset An in-memory VaultAccount struct which holds the total amount of Asset Tokens /// and the total number of Asset Shares (fTokens).
FraxlendPairCore.sol: L982-983
// Because interest accrues every block, _liquidationAmountInCollateralUnits from a few lines up is an ever increasing value // This means that leftoverCollateral can occasionally go negative by a few hundred wei (cleanLiqFee premium covers this for liquidator)
Suggestion:
// Because interest accrues every block, _liquidationAmountInCollateralUnits // from a few lines up is an ever increasing value. // This means that leftoverCollateral can occasionally go negative // by a few hundred wei (cleanLiqFee premium covers this for liquidator).
FraxlendPairCore.sol: L1151-1156
/// @notice The ```repayAssetWithCollateral``` function allows a borrower to repay their debt using existing collateral in contract /// @param _swapperAddress The address of the whitelisted swapper to use for token swaps /// @param _collateralToSwap The amount of Collateral Tokens to swap for Asset Tokens /// @param _amountAssetOutMin The minimum amount of Asset Tokens to receive during the swap /// @param _path An array containing the addresses of ERC20 tokens to swap. Adheres to UniV2 style path params. /// @return _amountAssetOut The amount of Asset Tokens received for the Collateral Tokens, the amount the borrowers account was credited
Suggestion:
/// @notice The ```repayAssetWithCollateral``` function allows a borrower /// to repay their debt using existing collateral in contract. /// @param _swapperAddress The address of the whitelisted swapper to use for token swaps /// @param _collateralToSwap The amount of Collateral Tokens to swap for Asset Tokens /// @param _amountAssetOutMin The minimum amount of Asset Tokens to receive during the swap /// @param _path An array containing the addresses of ERC20 tokens to swap — /// adheres to UniV2 style path params. /// @return _amountAssetOut The amount of Asset Tokens received for the Collateral Tokens, /// the amount the borrowers account was credited.
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0xA5DF, 0xDjango, 0xNazgul, 0xSmartContract, 0xackermann, 0xbepresent, 0xc0ffEE, 0xkatana, 2997ms, Amithuddar, Aymen0909, Bnke0x0, Chinmay, Chom, CodingNameKiki, Deivitto, Diraco, Dravee, ElKu, EthLedger, Fitraldys, Funen, IgnacioB, JC, Junnon, Lambda, LeoS, Metatron, MiloTruck, Noah3o6, NoamYakov, PaludoX0, Randyyy, ReyAdmirado, Rohan16, Rolezn, Ruhum, SaharAP, Sm4rty, SooYa, TomJ, Tomio, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, asutorufos, ballx, brgltd, c3phas, cRat1st0s, carlitox477, chrisdior4, d3e4, delfin454000, dharma09, djxploit, durianSausage, erictee, fatherOfBlocks, find_a_bug, flyx, francoHacker, gerdusx, gogo, gzeon, hakerbaya, ignacio, jag, kyteg, ladboy233, ltyu, m_Rassska, medikko, mics, mrpathfindr, newfork01, nxrblsrpr, oyc_109, pfapostol, rbserver, reassor, ret2basic, robee, sach1r0, saian, simon135, sryysryy, zeesaw
21.1706 USDC - $21.17
Require
revert string is too longThe revert strings below should be shortened to 32 characters or fewer to save gas, or else consider using custom error codes (which could save even more gas)
LinearInterestRate.sol: L57-60
require( _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT, "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"
LinearInterestRate.sol: L61-64
require( _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT, "LinearInterestRate: _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT" );
LinearInterestRate.sol: L65-68
require( _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0, "LinearInterestRate: _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0" );
FraxlendPairDeployer.sol: L205
require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");
FraxlendPairDeployer.sol: L228
require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");
FraxlendPairDeployer.sol: L365
require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");
FraxlendPairDeployer.sol: L366-369
require( IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS).fraxlendDeployerWhitelist(msg.sender), "FraxlendPairDeployer: Only whitelisted addresses" );
require
functionSplitting into separate require()
statements saves gas
LinearInterestRate.sol: L57-60
require( _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT, "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"
Recommendation:
require(_minInterest < MAX_INT, "Error message"); require(_minInterest <= _vertexInterest, "Error message"); require(_minInterest >= MIN_INT, "Error message");
Similarly for the additional instances of &&
within require
functions referenced below:
LinearInterestRate.sol: L61-64
LinearInterestRate.sol: L65-68
for
loopSince calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop (as in fact occurs in some of the for
loops in Fraxlend
)
for (uint256 i = 0; i < _addresses.length; i++) { oracleContractWhitelist[_addresses[i]] = _bool; emit SetOracleWhitelist(_addresses[i], _bool); }
Suggestion:
uint256 totalAddressesLength = _addresses.length; for (uint256 i = 0; i < totalAddressesLength; i++) { oracleContractWhitelist[_addresses[i]] = _bool; emit SetOracleWhitelist(_addresses[i], _bool); }
Similarly for the six additional for
loops referenced below:
FraxlendPairCore.sol: L265-267
FraxlendPairCore.sol: L270-272
++i
instead of i++
to increase count in a for
loopSince use of i++
(or equivalent counter) costs more gas, it is better to use ++i
for (i = 0; i < 32 && data[i] != 0; i++) { bytesArray[i] = data[i]; }
Suggestion:
for (i = 0; i < 32 && data[i] != 0; ++i) { bytesArray[i] = data[i]; }
Similarly for the seven for
loops referenced below:
FraxlendPairDeployer.sol: L127-132
FraxlendPairDeployer.sol: L152-160
for
loopUnderflow/overflow checks are made every time ++i
(or i++
or equivalent counter) is called. Such checks are unnecessary since i
is already limited. Therefore, use unchecked {++i}
/unchecked {i++}
instead
for (uint256 i = 0; i < _addresses.length; i++) { oracleContractWhitelist[_addresses[i]] = _bool; emit SetOracleWhitelist(_addresses[i], _bool); }
Recommendation
uint256 totalAddressesLength = _addresses.length; for (uint256 i = 0; i < totalAddressesLength;) { oracleContractWhitelist[_addresses[i]] = _bool; emit SetOracleWhitelist(_addresses[i], _bool); unchecked { ++i; } }
Similarly for the seven following for
loops:
FraxlendPairCore.sol: L265-267
FraxlendPairCore.sol: L270-272
Note that while, for presentation purposes, I have been addressing the for
loop-related gas savings opportunities separately, they should be combined. I've included the other two types of gas saving that were described in previous sections in the recommendation above. Also, the for
loops in the contest are inconsistent, so that the lists of loops for each gas saving type presented above are not identical.