Upgraded Q -> 2 from #42 [1684786437801]

[c4-judge]

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

While the "division before multiplication" issues described in M-01 have been mitigated in the proposed changeset, there are other cases which should be addressed too.

Technical Details In SafEth::stake the calculation of preDepositPrice (now present in the function approxPrice()) multiplies underlyingValue by 1e18, but underlyingValue is first divided by 1e18: https://github.com/asymmetryfinance/smart-contracts/blob/fixMath/contracts/SafEth/SafEth.sol#L356-L370

function approxPrice() public view returns (uint256) { uint256 safEthTotalSupply = totalSupply(); uint256 underlyingValue = 0; uint256 count = derivativeCount;

for (uint256 i = 0; i < count; i++) {
    if (!derivatives[i].enabled) continue;
    IDerivative derivative = derivatives[i].derivative;
    underlyingValue +=
        (derivative.ethPerDerivative() * derivative.balance()) /
        1e18;
}
if (safEthTotalSupply == 0 || underlyingValue == 0) return 1e18;
return (1e18 * underlyingValue) / safEthTotalSupply;

} Also in SafEth::stake, something similar happens with totalStakeValueEth, as this is the sum of derivativeReceivedEthValue that is divided by 1e18, but then totalStakeValueEth is multiplied by 1e18 in the calculation of mintAmount: https://github.com/asymmetryfinance/smart-contracts/blob/fixMath/contracts/SafEth/SafEth.sol#L97-L114

uint256 totalStakeValueEth = 0; // total amount of derivatives staked by user in eth for (uint256 i = 0; i < count; i++) { if (!derivatives[i].enabled) continue; uint256 weight = derivatives[i].weight; if (weight == 0) continue; IDerivative derivative = derivatives[i].derivative; uint256 ethAmount = (msg.value * weight) / totalWeight;

if (ethAmount > 0) {
    // This is slightly less than ethAmount because slippage
    uint256 depositAmount = derivative.deposit{value: ethAmount}();
    uint256 derivativeReceivedEthValue = (derivative
        .ethPerDerivative() * depositAmount) / 1e18;
    totalStakeValueEth += derivativeReceivedEthValue;
}

} // mintedAmount represents a percentage of the total assets in the system mintedAmount = (totalStakeValueEth * 1e18) / preDepositPrice; In Reth::deposit, rethPerEth is calculated as (1e36) / ethPerDerivative() but then rethPerEth is used in the minOut calculation that ends dividing again by 1e36. The expression can be simplified to avoid any division before multiplication. The same applies to idealOut: https://github.com/asymmetryfinance/smart-contracts/blob/fixMath/contracts/SafEth/derivatives/Reth.sol#L148-L151

uint256 rethPerEth = (1e36) / ethPerDerivative(); uint256 minOut = ((rethPerEth msg.value) (1e18 - maxSlippage)) / 1e36; uint256 idealOut = (rethPerEth * msg.value) / 1e18; Recommendation Fix these other cases of "division before multiplication". In most of these, the expressions can be simplified as there is a division that is later multiplied by the same number.