Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set asset as collateral #569

Merged
merged 45 commits into from
Mar 16, 2023
Merged

Set asset as collateral #569

merged 45 commits into from
Mar 16, 2023

Conversation

MerlinEgalite
Copy link
Contributor

@MerlinEgalite MerlinEgalite commented Feb 22, 2023

Pull Request

Descrition

This PR covers the edge case related to the LTV = 0 described here among other issues.

What the PR is doing:

  • implements a "hot fix" that allows the DAO to set as not collateral on both Morpho and Aave (first Morpho, then Aave, otherwise it reverts). The Aave part is only possible if Morpho's aggregated HF will be > 1 after the switch. The "hot fix" is needed to avoid breaking the whole Protocol when an asset is LTV=0 on Aave.
  • after applying the "hot fix" users that still own the asset as collateral and have not applied the proper actions will incur into all the side effects of owning an LTV=0 asset on Morpho that are different compared to LT=0 on Aave. In this case the user's HF and LTV will be lowered (the asset is not counted at all as collateral) and they could get liquidated, could not be able to borrow (this is the same as on Aave, user's LTV is lowered) and could not be able to withdraw.
  • the PR does not include a detailed explanation of how Morpho plans to handle the edge case of an LTV=0 asset or soon to become LTV=0 asset and what happens to users that do not follow the procedure (see point above) or what users should do before Morpho apply the "hot fix"

We are working on complementary remediations are not contract related such as:

  • Aave governance monitoring
  • Communication channel with BGD
  • Banner or pop ups in the frontend to warn users
  • Communications texts prepare in advanded to send to Discord, Twitter, etc.

Issue(s) fixed

This pull request:

src/MorphoSetters.sol Show resolved Hide resolved
@MerlinEgalite MerlinEgalite marked this pull request as ready for review February 23, 2023 15:59
src/MorphoSetters.sol Outdated Show resolved Hide resolved
src/MorphoSetters.sol Outdated Show resolved Hide resolved
src/MorphoSetters.sol Outdated Show resolved Hide resolved
src/PositionsManagerInternal.sol Show resolved Hide resolved
src/PositionsManagerInternal.sol Outdated Show resolved Hide resolved
@MerlinEgalite
Copy link
Contributor Author

MerlinEgalite commented Feb 23, 2023

I'm trying to fix the CI but this is quite long 🫠

Tristan22400
Tristan22400 previously approved these changes Feb 24, 2023
Rubilmax
Rubilmax previously approved these changes Mar 15, 2023
src/MorphoSetters.sol Outdated Show resolved Hide resolved
@MerlinEgalite MerlinEgalite dismissed stale reviews from Rubilmax and pakim249CAL via a57a7c0 March 15, 2023 17:35
Copy link
Contributor

@MathisGD MathisGD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect
Waiting for #680 to approve

QGarchery
QGarchery previously approved these changes Mar 15, 2023
pakim249CAL
pakim249CAL previously approved these changes Mar 15, 2023
Rubilmax
Rubilmax previously approved these changes Mar 16, 2023
Tristan22400
Tristan22400 previously approved these changes Mar 16, 2023
src/MorphoSetters.sol Show resolved Hide resolved
@MerlinEgalite MerlinEgalite merged commit 178ae65 into dev Mar 16, 2023
@MerlinEgalite MerlinEgalite deleted the feat/asset-as-collateral branch March 16, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We must implement setAssetAsCollateral function to handle ltv to 0 Wrong check for asset liquidity data
7 participants