LendingPool Code Style Findings

LPL-01C: Increase of Fee Function Usability

Type	Severity	Location
Code Style		LendingPool.sol:L399, L402

Description:

The LendingPool::withdrawReserve function is meant to allow the owner of the contract to claim any fees that have been stored during the contract's lifetime. Within it, an LendingPool::_updatePoolWithInterestsAndTimestamp function invocation is performed that will increase the poolReserves, causing poolReserves within the contract to be permanently unclaimable unless two withdrawReserve executions are performed in sequence in the same transaction.

Example:

Copy
394/**
395  * Withdraw protocol fees from reserves
396  * @param _amount  Amount to withdraw in token decimals
397*/
398function withdrawReserve(uint256 _amount) external nonReentrant onlyOwner {
399  require(_amount <= poolReserves, "Cannot withdraw more than pool reserves");
400
401  // Update pool with accrued interest and latest timestamp
402  _updatePoolWithInterestsAndTimestamp(0);
403
404  poolReserves = poolReserves - _amount;
405
406  IERC20(asset).safeTransfer(treasury, _amount);
407}

Recommendation:

We advise the code to instead omit the require check between _amount and poolReserves entirely. As an alternative, an if clause can be introduced after the LendingPool::_updatePoolWithInterestsAndTimestamp call that checks whether _amount exceeds the poolReserves and in such a case assigns poolReserves to _amount, allowing the owner to specify type(uint256).max as an input to the LendingPool::withdrawReserve function and withdraw all fees available, including the ones that have yet to be accounted for.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The LendingPool::withdrawReserve function was refactored per our recommendation, ensuring that the full amount of fees accrued by the system is claimable by its owner in an intuitive way.

LPL-02C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		LendingPool.sol:L375, L404

Description:

The linked mathematical operations are guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
370function _to18ConversionFactor(address _token) internal view returns (uint256) {
371  require(ERC20(_token).decimals() <= 18, 'decimals are more than 18');
372
373  if (ERC20(_token).decimals() == 18) return 1;
374
375  return 10**(18 - ERC20(_token).decimals());
376}

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statements to be wrapped in unchecked code blocks thereby optimizing their execution cost.

Alleviation (4325253d6d):

The latter of the two referenced statements has been wrapped in an unchecked code block, rendering this exhibit partially alleviated.

Alleviation (7c9b2b09db):

The final remaining code block has been safely wrapped in an unchecked arithmetic block, alleviating this exhibit fully.

LPL-03C: Inefficient Calculation Offsets

Type	Severity	Location
Gas Optimization		LendingPool.sol:L282, L285, L447, L450

Description:

The referenced operations perform both a multiplication and a division with SAFE_MULTIPLIER in the same arithmetic context.

Example:

Copy
282uint256 debt = _repayAmount * SAFE_MULTIPLIER
283                            / borrowerTotalRepayAmount
284                            * borrowers[msg.sender].debt
285                            / SAFE_MULTIPLIER;

Recommendation:

We advise both the multiplication and division to be omitted, instead performing the actual variable-based multiplication prior to the variable-based division in the referenced calculations as the accuracies of those variables cancel each other out.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The calculation was simplified per our recommendation, performing a multiplication prior to the division of same-accuracy numbers that eliminate each other's offset.

LPL-04C: Inefficient Calculation of First Interest

Type	Severity	Location
Gas Optimization		LendingPool.sol:L352-L363

Description:

The first invocation of LendingPool::_pendingInterest will improperly calculate multiple values until it arrives to a zero value due to the inexistent presence of any totalBorrows.

Example:

Copy
335/**
336  * Interest accrual function that calculates accumulated interest from lastUpdatedTimestamp and add to totalBorrows
337  * @param _value Additonal amount of assets being deposited in token decimals
338*/
339function _updatePoolWithInterestsAndTimestamp(uint256 _value) internal {
340  uint256 interest = _pendingInterest(_value);
341  uint256 toReserve = interest * protocolFee / SAFE_MULTIPLIER;
342  poolReserves = poolReserves + toReserve;
343  totalBorrows = totalBorrows + interest;
344  lastUpdatedAt = block.timestamp;
345}
346
347/**
348  * Returns the pending interest that will be accrued to the reserves in the next call
349  * @param _value Newly deposited assets to be subtracted off total available liquidity in token decimals
350  * @return interest  Amount of interest owned in token decimals
351*/
352function _pendingInterest(uint256 _value) internal view returns (uint256) {
353  uint256 timePassed = block.timestamp - lastUpdatedAt;
354  uint256 floating = totalAvailableSupply() == 0 ? 0 : totalAvailableSupply() - _value;
355  uint256 ratePerSec = lendingPoolConfig.interestRatePerSecond(totalBorrows, floating);
356
357  // First division is due to ratePerSec being in 1e18
358  // Second division is due to ratePerSec being in 1e18
359  return ratePerSec * totalBorrows
360                    * timePassed
361                    / SAFE_MULTIPLIER
362                    / SAFE_MULTIPLIER;
363}

Recommendation:

We advise the function to yield 0 immediately if totalBorrows is evaluated as 0, ensuring that the first time LendingPool::_updatePoolWithInterestsAndTimestamp is invoked the call will incur minimal gas to initialize the lastUpdatedAt variable.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The statement advised in the recommendation chapter of the exhibit has been introduced to the LendingPool::_pendingInterest function, optimizing its execution cost significantly during its first query.

LPL-05C: Inefficient Enforcement of EIP-20 Asset Trait

Type	Severity	Location
Gas Optimization		LendingPool.sol:L371

Description:

The LendingPool::_to18ConversionFactor function mandates that the decimals of the _token address are at most 18 whilst the _token evaluated (asset) can never change in the lifetime of the LendingPool contract.

Example:

Copy
365/**
366  * Conversion factor for tokens with less than 1e18 to return in 1e18
367  * @param _token  Asset address
368  * @return conversionFactor  Amount of decimals for conversion to 1e18
369*/
370function _to18ConversionFactor(address _token) internal view returns (uint256) {
371  require(ERC20(_token).decimals() <= 18, 'decimals are more than 18');
372
373  if (ERC20(_token).decimals() == 18) return 1;
374
375  return 10**(18 - ERC20(_token).decimals());
376}

Recommendation:

We advise the decimals value to be mandated as at most equal to 18 in the constructor of the contract instead, optimizing all LendingPool::_to18ConversionFactor function executions and preventing the creation of inoperable LendingPool implementations.

Alleviation (4325253d6d):

The exhibit appears to have been remediated in PR#156, however, the latest state of the codebase does not reflect this change. As the decimal check is removed entirely, this has been converted to a vulnerability that needs to be remediated.

Alleviation (7c9b2b09db):

The original recommendation of the exhibit has been applied to the codebase, relocating the decimals validation check to the contract's constructor thus addressing this exhibit.

LPL-06C: Inefficient Event Emission

Type	Severity	Location
Gas Optimization		LendingPool.sol:L388-L389, L391

Description:

The referenced event emission is suboptimal as it loads the protocolFee into memory prior to emitting it in the ProtocolFeeUpdated event.

Example:

Copy
380/**
381  * Update protocol fee
382  * @param _protocolFee  Fee percentage in 1e18
383*/
384function updateProtocolFee(uint256 _protocolFee) external onlyOwner {
385  // Update pool with accrued interest and latest timestamp
386  _updatePoolWithInterestsAndTimestamp(0);
387
388  uint256 previousProtocolFee = protocolFee;
389  protocolFee = _protocolFee;
390
391  emit ProtocolFeeUpdated(msg.sender, previousProtocolFee, protocolFee);
392}

Recommendation:

We advise the LendingPool::updateProtocolFee function to be updated to instead emit the ProtocolFeeUpdated event immediately after _updatePoolWithInterestsAndTimestamp with the arguments being msg.sender, protocolFee, and _protocolFee; rendering the usage of a local variable unnecessary.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The event's emission has been optimized per our recommendation, no longer requiring an interim local variable and thus optimizing its gas cost.

LPL-07C: Inefficient Recalculation of Formulas

Type	Severity	Location
Gas Optimization		LendingPool.sol:L116, L124, L127, L144, L147, L270, L272-L273, L279, L354, L371, L373, L375, L435, L437-L438, L444

Description:

The referenced function invocations are performed repetitively in the same function context, incurring a significant gas overhead redundantly.

Example:

Copy
111/**
112  * Returns the the borrow utilization rate of the pool
113  * @return utilizationRate   Ratio of borrows to total liquidity in 1e18
114*/
115function utilizationRate() public view returns (uint256){
116  return (totalValue() == 0) ? 0 : totalBorrows * SAFE_MULTIPLIER / totalValue();
117}

Recommendation:

We advise them to be invoked only once, storing their result to a local variable that is consequently utilized wherever needed.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

All referenced repetitive invocations have been optimized to the greatest extent possible, utilizing either local or immutable variables that represent the repetitive data point required calculated only once.

LPL-08C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		LendingPool.sol:L258-L259, L292-L293

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
257// Update borrower state
258borrowers[msg.sender].debt = borrowers[msg.sender].debt + debt;
259borrowers[msg.sender].lastUpdatedAt = block.timestamp;

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The referenced mapping lookups have been optimized as advised, greatly reducing the gas cost of the referenced statements.

LPL-09C: Repetitive Value Literal

Type	Severity	Location
Code Style		LendingPool.sol:L371, L373, L375

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

Copy
371require(ERC20(_token).decimals() <= 18, 'decimals are more than 18');

Recommendation:

We advise it to be set to a constant variable instead optimizing the legibility of the codebase.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The referenced literal is no longer utilized multiple times within the codebase rendering this exhibit inapplicable.

LPL-10C: Variable Mutability Specifiers (Immutable)

Type	Severity	Location
Gas Optimization		LendingPool.sol:L85-L86, L88-L89

Description:

The linked variables are assigned to only once during the contract's constructor.

Example:

Copy
68/**
69  * @param _lendingPoolConfig  Contract for Lending Pool Configuration
70  * @param _name  Name for ibToken for this lending pool, e.g. Interest Bearing AVAX
71  * @param _symbol  Symbol for ibToken for this lending pool, e.g. ibAVAX
72  * @param _asset  Contract address for underlying ERC20 asset
73  * @param _isNativeAsset  Boolean for whether this lending pool accepts the native asset (e.g. AVAX)
74  * @param _protocolFee  Protocol fee in 1e18
75*/
76constructor(
77  string memory _name,
78  string memory _symbol,
79  address _asset,
80  bool _isNativeAsset,
81  uint256 _protocolFee,
82  ILendingPoolConfig _lendingPoolConfig,
83  address _treasury
84  ) ERC20(_name, _symbol) {
85    asset = _asset;
86    isNativeAsset = _isNativeAsset;
87    protocolFee = _protocolFee;
88    lendingPoolConfig = _lendingPoolConfig;
89    treasury = _treasury;
90}

Recommendation:

We advise them to be set as immutable greatly optimizing their read-access gas cost.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

All variables have been set as immutable per our recommendation, greatly reducing their read-access gas cost.