Refactor: use morpho utils #90
Conversation
Rubilmax
requested review from
MerlinEgalite,
pakim249CAL,
MathisGD,
Jean-Grimal and
makcandrov
src/Blue.sol
Outdated
| @@ -243,7 +245,7 @@ contract Blue { | |||
| if (marketTotalSupply != 0) { | |||
| uint marketTotalBorrow = totalBorrow[id]; | |||
| uint borrowRate = market.irm.borrowRate(market); | |||
| uint accruedInterests = marketTotalBorrow.wMul(borrowRate).wMul(block.timestamp - lastUpdate[id]); | |||
| uint accruedInterests = marketTotalBorrow.wadMulDown(borrowRate * (block.timestamp - lastUpdate[id])); | |||
There was a problem hiding this comment.
Borrow rate would typically be 0.001% per second, in WAD. So we should simply multiply the borrow rate by the seconds elapsed, to get a WAD-based % of interests to accrue, that we WAD-multiply with the total borrow to get the quantity of interests accrued.
src/libraries/MathLib.sol
Outdated
There was a problem hiding this comment.
I'm against using WadRayMath from morpho-utils, as is at least. We don't need Ray maths, and some other things from it. It would prefer to copy only the used functions (strict minimum) for this particular repo.
There was a problem hiding this comment.
plus, I like the naming wDiv/wMul (rounding to add) as there is no ambiguity in this code
There was a problem hiding this comment.
You can rename function from a lib instead of copy pasting
| @@ -111,7 +111,7 @@ contract Blue { | |||
| supplyShare[id][msg.sender] = WAD; | |||
| totalSupplyShares[id] = WAD; | |||
| } else { | |||
| uint256 shares = amount.wMul(totalSupplyShares[id]).wDiv(totalSupply[id]); | |||
| uint256 shares = amount.wadDivDown(totalSupply[id]).wadMulDown(totalSupplyShares[id]); | |||
There was a problem hiding this comment.
I'm wondering... isn't this basically equivalent to amount.mulDivDown(totalSupplyShares, totalSupply)?
There was a problem hiding this comment.
There was a problem hiding this comment.
why did you change the order @Rubilmax ?
There was a problem hiding this comment.
@MerlinEgalite discussed here: #90 (comment)
| @@ -111,7 +111,7 @@ contract Blue { | |||
| supplyShare[id][msg.sender] = WAD; | |||
| totalSupplyShares[id] = WAD; | |||
| } else { | |||
| uint256 shares = amount.wMul(totalSupplyShares[id]).wDiv(totalSupply[id]); | |||
| uint256 shares = amount.wadDivDown(totalSupply[id]).wadMulDown(totalSupplyShares[id]); | |||
There was a problem hiding this comment.
why did you change the order @Rubilmax ?
src/libraries/MathLib.sol
Outdated
There was a problem hiding this comment.
You can rename function from a lib instead of copy pasting
WadRayMath#71This PR is a draft because tests need to be updated with regard to rounding directions. I just want to make sure we're on the same page on this before.
@MathisGD @QGarchery please check rounding directions