Skip to content

Conversation

@shahthepro
Copy link
Collaborator

@shahthepro shahthepro commented Dec 20, 2022

Adds Morpho Aave strategy. Morpho Aave doesn't distribute any reward tokens, so the rewards related functions don't do anything right now.

The deploy script uses Aave v2 tokens and have tested it on fork.

Uses migration ID 046 instead of 045 (since that's used in #1205 for LUSD).


If you made a contract change, make sure to complete the checklist below before merging it in master.

Refer to our documentation for more details about contract security best practices.

Contract change checklist:

  • Code reviewed by 2 reviewers.
  • Copy & paste code review security checklist below this checklist.
  • Unit tests pass
  • Slither tests pass with no warning
  • Echidna tests pass if PR includes changes to OUSD contract (not automated, run manually on local)

Copy link
Member

@sparrowDom sparrowDom left a comment

Choose a reason for hiding this comment

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

Great job @shahthepro. Seems solid!

I got 1 small consideration whether we should refactor the supportsAsset, _getCTokenFor into something compound and avve strategies might use. Since it seems they currently differ only in the names of the contracts.

Otherwise looks solid 💪

Copy link
Contributor

@MerlinEgalite MerlinEgalite left a comment

Choose a reason for hiding this comment

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

Left some comments, but LGTM overall nice work! 🎉

Comment on lines 13 to 14
import { StableMath } from "../utils/StableMath.sol";
import "../utils/Helpers.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's used

Copy link
Contributor

Choose a reason for hiding this comment

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

@shahthepro. We should go ahead and remove this line if it's not used. Will keep the codebase clean.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, will remove and redeploy the contracts today

@sparrowDom sparrowDom merged commit 5885f8e into master Jan 4, 2023
@sparrowDom sparrowDom deleted the shah/morpho-aave branch January 4, 2023 20:32
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.

5 participants