fix(exchangeRate): add 1:1 exchange rate

[PierrickGT]

Issue: code-423n4/2023-07-pooltogether-findings#143

[asselstine]

Now that there is no historic _lastRecordedExchangeRate, it doesn't feel like this function serves a strong purpose, and it's doubling the number of mulDiv that convertToShares and convertToAssets have to do.

I imagine this function could be refactored into a separate function that checks available assets:

function _collateral() {
    uint256 _depositedAssets = _totalSupply();
    uint256 _withdrawableAssets = _yieldVault.maxWithdraw(address(this));

    // If the Vault is collateralized, users can only withdraw the amount of underlying assets they deposited.
    // An exchange rate of 1 is returned to exclude the amount of yield generated by the YieldVault.
    if (_withdrawableAssets >= _depositedAssets) {
      return _depositedAssets;
    }
    return _withdrawableAssets;
}

Then in your convertToShares and convertToAssets you're only doing 1 mulDiv:

function _convertToShares(
    uint256 _assets,
    Math.Rounding _rounding
  ) internal view virtual override returns (uint256) {
    uint totalSupplyShares = _totalSupply();
    return
      (_assets == 0 || totalSupplyShares == 0)
        ? _assets
        : _assets.mulDiv(totalSupplyShares, _collateral(), _rounding);
  }

and vice versa for assets.

This reduces the mulDiv, and makes the exchange rate clearer I think (less logic).

[PierrickGT]

But then once the Vault becomes undercollateralized, any yield that has not been claimed won't be withdrawable since we wouldn't include it in the exchange rate calculation.

[asselstine]

Yes we would; the _collateral() would be equal to _withdrawableAssets because it's undercollateralized.

[PierrickGT]

Added the _collateral() function in this commit: 81fe36a