code-423n4 / 2022-05-sturdy-findings

7 stars 3 forks source link

Upgraded Q -> M from 135 [1654503715861] #172

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #135 as Medium risk. The relevant finding follows:

HickupHH3 commented 2 years ago

At YieldManager.sol, distributeYield() function, an expensive loop is used by incrementing state_variable in a loop incurs a lot of gas because of expensive SSTOREs, which might lead to an out-of-gas and revert

function distributeYield(uint256 _offset, uint256 _count) external onlyAdmin {
    // 1. convert from asset to exchange token via uniswap
    for (uint256 i = 0; i < _count; i++) {
      address asset = _assetsList[_offset + i];
      require(asset != address(0), Errors.UL_INVALID_INDEX);
      uint256 _amount = IERC20Detailed(asset).balanceOf(address(this));
      _convertAssetToExchangeToken(asset, _amount);
    }
    uint256 exchangedAmount = IERC20Detailed(_exchangeToken).balanceOf(address(this));

    // 2. convert from exchange token to other stable assets via curve swap
    AssetYield[] memory assetYields = _getAssetYields(exchangedAmount);
    for (uint256 i = 0; i < assetYields.length; i++) {
      if (assetYields[i].amount > 0) {
        uint256 _amount = _convertToStableCoin(assetYields[i].asset, assetYields[i].amount);
        // 3. deposit Yield to pool for suppliers
        _depositYield(assetYields[i].asset, _amount);
      }
    }
  }
HickupHH3 commented 2 years ago

I initially wanted to mark this as a duplicate of #70, but I realise the reason given was different and it's for distributeYield() which has offset and count. Closing this instead, marking as invalid.