Morpho blue #321
Morpho blue #321
Conversation
shriyatyagii
commented
Nov 28, 2023
•
shriyatyagii
commented
- Add Morpho Blue connectors and test script
|
|
|
|
|
|
|
||
| (uint256 _assets, ) = MORPHO_BLUE.supply( | ||
| _marketParams, | ||
| _amt, |
There was a problem hiding this comment.
Should be set to 0 right?
There was a problem hiding this comment.
You mean the _amt value? So I've converted the shares value to assets amount to give the approval to Morpho's contract. That's why sending the converted assets value in the function.
| if (_amt == type(uint256).max) { | ||
| bytes32 _id = id(_marketParams); | ||
| Position memory _pos = MORPHO_BLUE.position(_id, address(this)); | ||
| uint256 _shares = _pos.supplyShares; | ||
|
|
||
| _amt = _toAssetsUp( | ||
| _shares, | ||
| MORPHO_BLUE.market(_id).totalSupplyAssets, | ||
| MORPHO_BLUE.market(_id).totalSupplyShares | ||
| ); | ||
| } |
There was a problem hiding this comment.
when withdrawing max it's advised to use shares instead to make sure no dust is left on the contract
There was a problem hiding this comment.
Got it!
Updated the logics to use shares in case of max amounts. Here
| Position memory _pos = MORPHO_BLUE.position(_id, _onBehalf); | ||
| uint256 _shares = _pos.supplyShares; | ||
|
|
||
| _amt = _toAssetsUp( |
There was a problem hiding this comment.
same comment as for withdraw
| _assetsAmt, | ||
| 0, |
There was a problem hiding this comment.
| _assetsAmt, | |
| 0, | |
| 0, | |
| _shares, |
There was a problem hiding this comment.
Not using the _shares amount as that can be sent as type(uint256).max, so using the converted assets value instead.
Although I think similar to withdraw, assets value might cause dust amount issues here?
| * @param _getId ID to retrieve amt. | ||
| * @param _setId ID stores the amount of tokens repaid. | ||
| */ | ||
| function repayOnBehalfShares( |
There was a problem hiding this comment.
there's no way to repay max similar to the withdraw max feature?
There was a problem hiding this comment.
The max repayment logics are being handled inside the _performEthToWeth conversion functions. It will check the minimum of balance and the debt amount and use that to repay. Logic can be found here.
|
|
||
| _assets = _toAssetsUp( | ||
| _shareAmt, | ||
| MORPHO_BLUE.market(_id).totalSupplyAssets, |
There was a problem hiding this comment.
This does not take into account the interest generated see a similar comment below
There was a problem hiding this comment.
Updated. Same as this
|
|
||
| contract Events { | ||
| event LogSupplyAssets( | ||
| MarketParams marketParams, |
There was a problem hiding this comment.
Wouldn't it be interesting to instead log the market's id and have it indexed?
There was a problem hiding this comment.
Yes! I was calculating id() in the functions only when needed so that's why I didn't add by default. But yes indexing it can be a good option. Updated it here
| enum Mode { | ||
| Collateral, | ||
| Repay, | ||
| Other // Neither collateral nor repay |
There was a problem hiding this comment.
Neither collateral nor repay mode could be named Loan
There was a problem hiding this comment.
Even more specific as it is used only upon supply: Supply
| if (_isEth) { | ||
| convertEthToWeth( |
There was a problem hiding this comment.
The value of _isEth is checked twice: once here and once inside convertEthToWeth
There was a problem hiding this comment.
Good catch! Removed the external check. Here
| /// @notice Returns (`x` * `y`) / `d` rounded up. | ||
| function _mulDivUp( | ||
| uint256 x, | ||
| uint256 y, | ||
| uint256 d | ||
| ) internal pure returns (uint256) { | ||
| return (x * y + (d - 1)) / d; | ||
| } |
There was a problem hiding this comment.
This could use our audited MathLib: https://github.com/morpho-org/morpho-blue/blob/414b67a523dbc72144a5dfed62ac60771f15d98e/src/libraries/MathLib.sol#L31-L34
There was a problem hiding this comment.
This was not needed anymore since I'm using toAssetsUp directly from Morpho Blue's library. Removed it.
| /// @notice Returns the id of the market `marketParams`. | ||
| function id( | ||
| MarketParams memory marketParams | ||
| ) internal pure returns (bytes32 marketParamsId) { | ||
| assembly { | ||
| marketParamsId := keccak256( | ||
| marketParams, | ||
| MARKET_PARAMS_BYTES_LENGTH | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
This could use our audited MarketParamsLib: https://github.com/morpho-org/morpho-blue/blob/414b67a523dbc72144a5dfed62ac60771f15d98e/src/libraries/MarketParamsLib.sol#L15-L20
There was a problem hiding this comment.
Updated! Same reason as this
| uint256 _shareAmt = MORPHO_BLUE.position(_id, _onBehalf).supplyShares; | ||
|
|
||
| _assets = _toAssetsUp( | ||
| _shareAmt, | ||
| MORPHO_BLUE.market(_id).totalSupplyAssets, | ||
| MORPHO_BLUE.market(_id).totalSupplyShares | ||
| ); |
There was a problem hiding this comment.
Interest to be accrued is not taken into account here
Besides, it seems the payback balance would correspond to the amount to be repaid on the borrow side, not the supply side
Moreover, market is called twice on MORPHO which costs a lot of SLOAD so a lot of gas
This flow could be optimized and simplified using our audited MorphoBalancesLib: https://github.com/morpho-org/morpho-blue/blob/414b67a523dbc72144a5dfed62ac60771f15d98e/src/libraries/periphery/MorphoBalancesLib.sol#L28-L62
There was a problem hiding this comment.
Ah yes! Updated logic here
| /// @notice Helper function to find the minimum of two values | ||
| function min(uint256 a, uint256 b) internal pure returns (uint256) { | ||
| return a < b ? a : b; | ||
| } |
There was a problem hiding this comment.
This could use our audited UtilsLib: https://github.com/morpho-org/morpho-blue/blob/414b67a523dbc72144a5dfed62ac60771f15d98e/src/libraries/UtilsLib.sol#L19-L24
There was a problem hiding this comment.
Yes! I had not imported the libraries due to solidity version conflicts with our common files. Using the libraries now.
Code
| _assets = _toAssetsUp( | ||
| _shareAmt, | ||
| MORPHO_BLUE.market(_id).totalSupplyAssets, | ||
| MORPHO_BLUE.market(_id).totalSupplyShares | ||
| ); |
There was a problem hiding this comment.
Interest to be accrued is not taken into account here
Moreover, market is called twice on MORPHO which costs a lot of SLOAD so a lot of gas
This flow could be optimized and simplified using our audited MorphoBalancesLib: https://github.com/morpho-org/morpho-blue/blob/414b67a523dbc72144a5dfed62ac60771f15d98e/src/libraries/periphery/MorphoBalancesLib.sol#L28-L62
There was a problem hiding this comment.
Didn't realize these are the non accrued interest values. I've imported the necessary libraries and updated calculations to use expectedMarketBalances.
Here
| if (_receiver == address(this)) | ||
| convertWethToEth( | ||
| _marketParams.collateralToken == ethAddr, | ||
| TokenInterface(wethAddr), | ||
| _amt | ||
| ); |
There was a problem hiding this comment.
why always converting weth to eth ?
There was a problem hiding this comment.
Conversion is only happening in case the token addresses sent from our UI for loan (or collateral) is ETH (0xeee...). We allow users to use ETH tokens for the WETH markets and do the Weth to Eth conversion internally.
|
Stale pull request message |
