Code maturity

• The multiple liquidity pool contains unused internal functions _calculateLendingModuleCollateral(...) and _getTotalCollateral(...). Consider removing these functions for the benefit of code clarity and gas savings on contract deployment.

• Consider improving UX by changing the visibility of the following functions from internal to public: _calculateMint(...) and _calculateRedeem(...). This would allow users to easily determine the number of synthetic tokens their collateral would mint or the number of collateral units their synthetic tokens would redeem. Currently this can only be done through a series of calls that includes (1) interacting with the multiple liquidity manager's collateralTokenDecimals() and feePercentage() functions and (2) finding and calling the correct implementer of the ISynthereumPriceFeed for retrieving price data.

• To minimize potential lost gas from user input errors in the multiple liquidity pool's liquidate(...) function, add a check for non-zero liquidation tokens at the beginning of liquidate(...) as follows:

function liquidate(address _lp, uint256 _numSynthTokens)
  external
  override
  nonReentrant
  returns (uint256)
{
  require(_numSynthTokens > 0, 'No synthetic tokens to liquidate');
  ...

Currently a similar check exists within the internal function call _updateAndLiquidate(...) deep within the function body, which results in increased gas fees on transaction reversion.

Lending modules & return units

The withdraw/deposit functions in the lending modules return three values as shown below:

function withdraw(...)
  external
  override
  returns (
    uint256 totalInterest,
    uint256 tokensOut,
    uint256 tokensTransferred
  )
{
	...
    uint256 netWithdrawal =
      IERC20(poolData.collateral).balanceOf(recipient) - initialBalance;

    tokensOut = aTokensAmount;
    tokensTransferred = netWithdrawal;
	...
}

When these return values are passed to the MultiLpLiquidityPool, they are used in conjuction with other variables that are in collateral units and not interest-bearing tokens (IBS). In the current lending module (AaveV3), the types of the returned values are not specified as either collateral units or IBS units. Currently such interactions are harmless due to the 1:1 nature of the AaveV3 tokens; however, neither in the documentation of ILendingModule or in the lending module itself (AaveV3) is it noted that the return values are to be in collateral units.

For now this observation has no impact on the project; however, future developers who are not aware of this fact, who may return the values tokensOut and others like it in IBS may inadvertently cause reward inflation, which might result in incorrect rewards/losses.

We recommend either adding an explicit call of interestToCollateralToken/collateralToInterestToken whenever dealing with these return values to signal their type or documenting their types before usage and in ILendingModule.ReturnValues.