LendingPoolImplementation Manual Review Findings

LPI-01M: Potentially Incorrect Reimbursement Calculation

Type	Severity	Location
Mathematical Operations		LendingPoolImplementation.sol:L234-L236

Description:

The linked calculation of the owed reimbursement in case of a mint ratio that is greater than the current one appears incorrect in the last division it performs.

Example:

Copy
233// Reimburse the borrower
234uint256 diffToReimburse = (userReport.colAmount *
235    ((newPool.mintRatio() - mintRatio) / 1e18)) /
236    (newPool.mintRatio() / 1e18);

Recommendation:

We advise this calculation to be thoroughly documented and simplified as it currently suffers from mathematical truncation and overall may be invalid.

Alleviation:

The code was simplified to eliminate the 1e18 offsets and now legibly evaluates the percentage increase in relation to the new value. As a result, we consider the calculation concisely defined and thus addressing this exhibit.

LPI-02M: Ill-Advised Toggle Pattern

Type	Severity	Location
Language Specific		LendingPoolImplementation.sol:L379-L382

Description:

The linked function conforms to a "toggle" pattern whereby a value is switched between two states on each invocation of the function.

Example:

Copy
379function flipBorrow() external {
380    onlyOwner();
381    disabledBorrow = disabledBorrow == 1 ? 0 : 1;
382}

Recommendation:

This type of function is highly discouraged in a blockchain deployment as transaction ordering, pending transactions and other similar blockchain-specific caveats can cause repeat invocations of the same function to occur thereby leading to an undesirable final state. We advise the function to be adjusted to a setter function instead preventing this form of ambiguity from ever arising.

Alleviation:

The toggle function has been adjusted to a setter function instead, thereby alleviating this exhibit in full.

LPI-03M: Inefficient & Restrictive Decimal Normalizations

Type	Severity	Location
Mathematical Operations		LendingPoolImplementation.sol:L297-L299, L350-L353

Description:

The linked decimal normalizations performed are inefficient as they do not account for decimal similarity and are restrictive as a lending or a collateral token with high decimals will cause the operations to overflow.

Impact:

In the current system, it is possible to create a debt that can never be repaid depending on the decimals of the system as the multiplication for the debt creation would succeed (small number of decimals on the lending token) but fail on repayment (high number of decimals on the collateral token), a potentially destructive scenario for the system.

Example:

Copy
345function _computePayoutAmount(uint256 _colDepositAmount, uint256 _mintRatio)
346    private
347    view
348    returns (uint256)
349{
350    return
351        (_colDepositAmount * _mintRatio * (10**lendToken.decimals())) /
352        (10**colToken.decimals()) /
353        1e18;
354}

Recommendation:

We advise the decimal delta to be calculated efficiently by performing a single multiplication or division depending on the difference of decimals between the two assets.

Alleviation:

The Vendor Finance team provided us with an extensive document detailing what scenarios are in play for bad debt creation and addressing the concerns laid out in this exhibit. Based on the new material and internal review performed by the Vendor Finance team and in conjunction with our own analysis, we conclude that the decimal normalizations are being performed as adequately as possible maintaining the highest degree of fairness achievable.

LPI-04M: Truncation of Collateral / Lending Conversion

Type	Severity	Location
Mathematical Operations		LendingPoolImplementation.sol:L297-L299, L351-L353

Description:

The lending token and collateral token amounts are converted interchangeably via multiplications and divisions that can ultimately truncate and thus lead to an unintended loss / gain for the borrowers.

Impact:

Mathematical truncations in a borrowing protocol can be especially harmful as they may cause a debt to never be cleared or a debt to be created with surplus units borrowed.

Example:

Copy
297uint256 colReturnAmount = (repayRemainder * 1e18 * (10**colToken.decimals())) /
298                            (10**lendToken.decimals()) /
299                            mintRatio;

Recommendation:

We advise mathematical truncations to be accounted for by firstly ensuring they are one-way by applying the decimal optimization identified in a separate exhibit and then either preventing them from occurring altogether via require checks or accounting for them using a carry-over mechanism.

Alleviation:

The Vendor Finance team provided us with an extensive document detailing what scenarios are in play for bad debt creation and addressing the concerns laid out in this exhibit. Based on the new material and internal review performed by the Vendor Finance team and in conjunction with our own analysis, we conclude that the division calculations are being performed as adequately as possible maintaining the highest degree of fairness achievable.

LPI-05M: Improper Validation of Price

Type	Severity	Location
Logical Fault		LendingPoolImplementation.sol:L364

Description:

The isValidPrice function will yield true for an asset that has not been registered with the feed registry which is incorrect.

Impact:

Assets that do not contain any oracle will be marked as having a "valid" price.

Example:

Copy
356///@notice Check if col price is below mint ratio
357function isValidPrice() public view returns (bool) {
358    if (address(priceFeed) == address(0)) revert OracleNotSet();
359    int256 priceLend = priceFeed.getPriceUSD(address(lendToken));
360    int256 priceCol = priceFeed.getPriceUSD(address(colToken));
361    if (priceLend != -1 && priceCol != -1) {
362        return (priceCol > ((int256(mintRatio) * priceLend) / 1e18));
363    }
364    return true;
365}

Recommendation:

We advise the function to return false if any of the prices of the assets is equivalent to -1.

Alleviation:

The function which has been relocated to the VendorUtils contract now properly yields false if the price is not defined within the price feeds thereby alleviating this exhibit.