search

mil0xeth / Strategy-Maker-lev-wstETH

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

Strategy-Maker-lev-wstETH #1

Open mil0xeth opened 2 years ago

mil0xeth commented 2 years ago

Protocol name: MakerDAO (Oasis App)

Strategy name: Strategy-Maker-lev-wstETH

Brief Strategy Description:

Claiming a strategy Due Diligence Review Protocol Due Diligence and Path to Prod: tba

Once a strategy's due diligence has been approved by the team, the strategist can begin codifying it for code review Strategy Review

Detailed Strategy Description:

Repo: https://github.com/mil0xeth/Strategy-Maker-lev-wstETH Commit (hash): 0ad347f Tag:

Scope: contracts/Strategy.sol,contracts/libraries/MakerDaiDelegateLib.sol

Additional References or Repo (Link to underlying protocols or relevant documents): Previous Oasis/MakerDAO References

Review Ongoing By:

Review Completed By:

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L718

anyone can enter here

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L735

anyone can enter here too

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L426

There is no handling of loss situation here. What happens if the system is in a loss?

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/libraries/MakerDaiDelegateLib.sol#L638

should there be an else here for the routerroute route?

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L544

There is a lot going on before here. is it safe to treat anyamount freed below the requested as a loss?

flashfish0x commented 2 years ago

What do you do when the steth/eth peg is broken but you need to repay? from my reading it looks like the strategy will revert and won't be able to repay its debt?

flashfish0x commented 2 years ago

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/libraries/MakerDaiDelegateLib.sol#L699

you always do the max amount regardless of price impact. this means you will get rekt by arbitrage on large swaps. you need to be super careful with these swaps

flashfish0x commented 2 years ago

not sure how best to solve it. but there is a contradiction of risk handling with this type of strategy.

On the one side you need to be able to get out immediately when the price changes so that you dont get liquidated.

On the other hand you dont want to do bad or large price impact trades. You also dont want to be open to manipulation

mil0xeth commented 2 years ago

Thanks for the great comments! Based on your input, I'm implementing Maker flashloans instead of using DYDX flashloans (and AAVE as a fallback). I've now removed AAVE and will replace DYDX with Maker.

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L718 anyone can enter here

fixed

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L735

anyone can enter here too

removed AAVE (see above)

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L426

There is no handling of loss situation here. What happens if the system is in a loss?

Actually, the previous line determines if the strategy is in a net profit and if so, will liquidate that profit together with the _debtOustanding. If the strategy is not in a net profit, the _profit is 0 and the _loss is determined through the resulting realized loss by liquidatePosition(_debtOutstanding) here (the loss report to the vault follows below that): https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L428-L438

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/libraries/MakerDaiDelegateLib.sol#L638

should there be an else here for the routerroute route?

This route follows the DAI flashloan to repay the DAI debt and determines if it's more efficient to repay the DAI flashloan debt with the unlocked collateral directly through uniswap OR if it's worth it to first unwrap the wstETH to stETH, then sell the stETH on Curve for ETH to THEN sell the ETH to DAI on uniswap. The if-statement envelops the unwrapping and Curve selling steps and changes the swapping in-token for uniswap, so an else-statement is not necessary. Do you think this is unnecessary, as in all the tests I've done curve has shown to be the more profitable route? For gas efficiency sake, I could remove this functionality.

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/Strategy.sol#L544

There is a lot going on before here. is it safe to treat anyamount freed below the requested as a loss?

The strategy is basically trying to free up want token from all the channels possible in order of ease. A user withdrawal is always fully honoured, there is no amount withheld, but a potential loss of the strategy's net worth in-between depositing and withdrawing will be given as a loss to the user.

What do you do when the steth/eth peg is broken but you need to repay? from my reading it looks like the strategy will revert and won't be able to repay its debt?

What part of the code are you referring to? In the function liquidatePosition, the collateralization ratio calculation uses the function "_getYieldBearingUSDPrice()" to retrieve Maker's spot collateral pricing for wstETH, so adjustments or repayments of the collateralization ratio are only dependent on wstETH market price and not ETH's, so a peg breaking does not matter here. For added robustness, we can add OSMedianizer support to see Maker's future price as well (c.f. https://forum.makerdao.com/t/mip10c9-sp6-whitelist-yearn-finance-on-ethusd-oracle/3773) During a withdrawal, the amount to be freed for a requested amount of WETH (want token of the vault) in terms of wstETH is calculated in DAI as a percentage of the total net worth of the strategy. So even if the peg is broken and the strategy is at a loss or in profit, withdrawing will thus give the appropriate amount of withdrawn ETH according to the percent of initial deposit as a function of total TVL of the vault at time of deposit.

https://github.com/mil0xeth/Strategy-Maker-lev-wstETH/blob/5bb5f86b0c1284fdc8213199333ba2e38752774e/contracts/libraries/MakerDaiDelegateLib.sol#L699

you always do the max amount regardless of price impact. this means you will get rekt by arbitrage on large swaps. you need to be super careful with these swaps

I'll introduce a maxSingleTrade implementation in addition to the slippage to make sure that not more than a maximum amount of WETH is requested at a time.

not sure how best to solve it. but there is a contradiction of risk handling with this type of strategy.

On the one side you need to be able to get out immediately when the price changes so that you dont get liquidated.

On the other hand you dont want to do bad or large price impact trades. You also dont want to be open to manipulation

The goal is to not have an aggressive collateralization ratio, such that we are never forced into fast and big debt repayments. Macro market changes can be met with gradual debt repayments well ahead of the liquidation threshold.

Manipulation is definitely an important consideration: wavey and I have discussed quite in-depth ideas of manipulation and it seems to me that the strategy always calculating a net position for its total assets (effectively subtracting the debt from the collateral), which is always up to date, does thus not allow for any withdrawal-before-rebalancing/debt-repayment manipulation.

vany365 commented 2 years ago

Description:

Commit (hash):

Tag:

Repo: https://github.com/mil0xeth/Strategy-Maker-lev-wstETH

Scope: (List link to files)

Due Diligence: (attach link to due diligence document, see template here)

Additional References or Repo (Link to underlying protocols or relevant documents):

Deployed Contract (etherscan link):

Review Ongoing By:

Review Completed By: