search

sturdyfi / sturdy-yearn-strategy

GNU Affero General Public License v3.0
0 stars 44 forks source link

Sturdy Strategy Review #1

Open newmickymousse opened 2 years ago

newmickymousse commented 2 years ago

Claiming a strategy Protocol name: Sturdy

Brief Strategy Description: Deposits stablecoins (USDC, USDT, or DAI) to Sturdy's lending pool

Due Diligence Review Protocol Due Diligence and Path to Prod: here

Once a strategy's due diligence has been approved by the team, the strategist can begin codifying it for code review Strategy Review Description: Deposits stablecoins (USDC, USDT, or DAI) to Sturdy's lending pool. Sturdy is a two-sided money market, whose primary differentiator is that collateral is staked, with the yields from staking being used to improve rates for lenders. Yields are paid out automatically in-kind, requiring no additional logic to harvest. There are no fees to deposit or withdraw from Sturdy.

Commit (hash): fe8fad5307a218f6964393952e2b3f91ea15e3b7

Tag:

Scope: contracts/Strategy.sol

Additional References or Repo (Link to underlying protocols or relevant documents): docs.sturdy.finance

Deployed Contract (etherscan link):

Review Ongoing By:

Review Completed By:

newmickymousse commented 2 years ago
  1. This could be implemented as a gen-lender plug-in instead of a whole new strategy. That allows us to simplify management as well as having a simpler strategy. Please have a look at this repository
  2. Make name variable with a parameter on a constructor. The strategy should be named something like: StrategySturdyUSDC, StrategySturdyDAI, StrategySturdyUSDT.
  3. https://github.com/sturdyfi/sturdy-yearn-strategy/blob/fe8fad5307a218f6964393952e2b3f91ea15e3b7/src/Strategy.sol#L51 To simplify the want.balanceOf(this) that is used in the whole contract, we usually create a public method called balanceOfWant() because it is also useful to check how much want is in the strategy.
  4. https://github.com/sturdyfi/sturdy-yearn-strategy/blob/fe8fad5307a218f6964393952e2b3f91ea15e3b7/src/Strategy.sol#L66-L67 This means that when _debtOutstanding == 0 we are returning profit=0, loss=0.
  5. https://github.com/sturdyfi/sturdy-yearn-strategy/blob/fe8fad5307a218f6964393952e2b3f91ea15e3b7/src/Strategy.sol#L192 _removePosition() should receive a parameter with the amount to remove. We should calculate the exact amount to remove, to avoid an unnecessary re-deposit.
  6. If you do a gen-lender plugin you will inherit a clone method. https://github.com/flashfish0x/yearnV2-generic-lender-strat/blob/f55c63ef3feaa03fb61e262ed4954db3e7f1cddc/contracts/GenericLender/GenericLenderBase.sol#L61-L76 If not, please add a similar one.
sforman2000 commented 2 years ago

Thanks @newmickymousse -- regarding implementing this as a gen-lender plug-in, we don't believe this to be possible since we can't get APR on chain.

iris112 commented 2 years ago

@newmickymousse, @sforman2000 I fixed the issue 2,3,4,5 and issue 1 and 6 is not suitable for us because we can't get the APR on chain. Please review and let me know.

sforman2000 commented 2 years ago

@newmickymousse we were able to find a way to get APR on chain, you can find the gen-lender plugin here: https://github.com/sturdyfi/yearnV2-generic-lender-strat