Convex frxETH/WETH strategy

[naddison36]

This implementation of the Convex frxETH/WETH strategy uses Curve solidity libraries to abstract whether the Curve pool has two or three coins.

The other implementation clones BaseCurveStrategy to BaseTwoAssetCurveStrategy and ConvexStrategy to ConvexTwoAssetStrategy. https://github.com/OriginProtocol/origin-dollar/pull/1842

Changes Summary

• Removed unused imports from the Harvester contracts
• I've moved the curve mocks into a mocks/curve folder
• Curve pool and gauge mocks now burn the LP token on remove liquidity instead of transferring it back to the pool
• Added calc_token_amount and fee functions to the abstract Curve Metapool. Also added remove_liquidity_imbalance with receiver override.
• Changes to other strategies to fit strategy behaviour
  • Aave - added Withdrawal event to withdrawAll
  • Compound - added Withdrawal event to withdrawAll
  • Generalized 4626 - only emit Withdrawal event if there was a withdraw amount in withdrawAll
• OSUD AMO and 3Pool strategies updated to new BaseConvexStrategy
• Moved Curve interfaces in the strategies folder into a curve subfolder
• Add new Curve pool libraries that abstract the number of coins in a pool
• deployWithConfirmation in utils/deploy.js now supports linked libraries
• Moved replaceContractAt and hardhatSetBalance from utils/deploy.js to utils/hardhat.js
• Added reusable unit tests for governable, harvester and strategy behaviours
• Added ConvexFrxEthWethStrategyProxy that delegates to ConvexStrategy and is linked to the CurveTwoCoinLib library
• Added 078_convex_frax_strategy deploy script for the new Convex strategy that uses the frxETH/WETH Curve pool

Design decisions

https://www.notion.so/originprotocol/Design-of-frxETH-WETH-Strategy-7cf0ddf4f14746a2a433b15cd7aa67a5

Contracts

The new strategy is in green

Value Flow

Tests

Unit tests

yarn test ./test/strategies/convexFrxEthWeth.js

Fork tests

yarn test:fork ./test/strategies/convex-frxeth.fork-test.js

Deploy

The contracts/deploy/078_convex_frax_strategy.js script will deploy the new strategy and propose the required governance acrtions.

Review

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.
• [x] Unit tests pass
• [x] Slither tests pass with no warning
• [ ] Echidna tests pass if PR includes changes to OUSD contract (not automated, run manually on local)

[github-actions[bot]]

	Warnings
:warning:	:eyes: This PR needs at least 2 reviewers

Generated by :no_entry_sign: dangerJS against f98accd5ee6c2f3b68eae7425ed3dc0037290125

[codecov[bot]]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (0572191) 59.60% compared to head (f98accd) 73.29%. Report is 2 commits behind head on master.

:exclamation: Current head f98accd differs from pull request most recent head d65c9c5. Consider uploading reports for the commit d65c9c5 to get more accurate results

Files	Patch %	Lines
...racts/strategies/ConvexGeneralizedMetaStrategy.sol	20.00%	4 Missing :warning:
...ntracts/contracts/strategies/BaseCurveStrategy.sol	97.95%	2 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1846       +/-   ##
===========================================
+ Coverage   59.60%   73.29%   +13.68%     
===========================================
  Files          55       59        +4     
  Lines        2820     2887       +67     
  Branches      728      748       +20     
===========================================
+ Hits         1681     2116      +435     
+ Misses       1136      768      -368     
  Partials        3        3               

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[naddison36]

This is a huge PR. On a quick look, the changes look good to me. Will do a bit of tests and review the code in-depth next

One thing about the already deployed strategies is that I don't think we would be upgrading them anytime soon (since some of them are inactive now).

yes, the main reason to change the old, unused strategies was to get consistency across the strategies so the behaviour tests would work. We don't need to upgrade them, especially the ones no longer used.

[naddison36]

Closing as frxETH is no longer part of OETH