Skip to content

V4 revised #5

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.

Sign up for GitHub

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

Open
OpenOrg-gg wants to merge 2 commits into
base: master
Choose a base branch
from
Open

V4 revised #5

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5bb2d2
Peer review revisions, round 3
Mar 2, 2022
5e82a7f
Peer review revisions, round 4
Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 35 additions & 24 deletions contracts/Strategy2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ contract Strategy is BaseStrategy {
address public constant LooksRareStaking = 0xBcD7254A1D759EFA08eC7c3291B2E85c5dCC12ce;

//$LOOKS token
address public constant LOOKSToken = 0xf4d2888d29D722226FafA5d9B24F9164c092421E;
IERC20 public constant LOOKSToken = IERC20(0xf4d2888d29D722226FafA5d9B24F9164c092421E);

//$WETH as I don't think we get actual ETH
address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
IERC20 public constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);

//hard code univ2 router
IUniswapV2Router02 public constant univ2router = IUniswapV2Router02(0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D);
Expand All @@ -78,6 +78,9 @@ contract Strategy is BaseStrategy {
//max for 256-1 on approve
uint256 public constant max = type(uint256).max;

//Base amount for floor $LOOKS token
uint256 private constant base = 1e18;

constructor(address _vault) public BaseStrategy(_vault) {
// You can set these parameters on deployment to whatever you want
// maxReportDelay = 6300;
Expand Down Expand Up @@ -121,9 +124,8 @@ contract Strategy is BaseStrategy {
//Ensure we're not going to attempt to sub from unneeded balance
uint256 amount = 0;
if(profit > balanceOfWant()){
uint256 amount = profit.sub(balanceOfWant());
amount = profit.sub(balanceOfWant());
}
uint256 base = 1e18;
//Check that our staked amount is greater than 1 token which is min
//This way we do not attempt to withdraw 0 (which reverts)
if(_stakedAmount() >= base){
Expand Down Expand Up @@ -200,16 +202,13 @@ event withdrawPreReportEvent(uint256 _amount);

//If there is no debt outstanding, deposit all looks back into contract and report back profit
emit debtOutstandingEvent(_debtOutstanding);
if(_debtOutstanding == 0){
Copy link
Copy Markdown

@newmickymousse newmickymousse Mar 16, 2022

Choose a reason for hiding this comment

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

uint256 finalProfit = LOOKSprofit.sub(vaultDebt); -> this line can result in a negative value. Should consider possibility loss. I've read your comment and makes sense but should handle that case also.

return(finalProfit,0,0);
if(finalProfit < _debtOutstanding){
Copy link
Copy Markdown

@newmickymousse newmickymousse Mar 16, 2022

Choose a reason for hiding this comment

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

there is a scenario where finalProfit != balanceOfWant() right? Basically this function withdrawProfitOrFloor(finalProfit); doesn't necessarily guarantee that finalProfit == balanceOfWant(). So, checking finalProfit vs debtOutstanding might not be correct, given that you could have more finalProfit than BalanceOfWant, and if finalProfit >= _debtOutstanding and finalProfit < base, you will end up telling the vault that there is want that can be withdraw, and it's not the case.

Copy link
Copy Markdown

@newmickymousse newmickymousse Mar 16, 2022

Choose a reason for hiding this comment

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

In this method I would do the following:

  1. sellRewards if there are any.
  2. calculate profit = estimatedAssets()-uint256 vaultDebt = vault.strategies(address(this)).totalDebt;
  3. call liquidatePosition(profit)
  4. liquidatePosition will return a liquidatedAmount and a loss. Then you can see if you have a profit, a loss and how much to return.

Let me know if this makes sense

_profit = 0;
_debtPayment = balanceOfWant();
_loss = _debtOutstanding.sub(_debtPayment);
} else {
//If there is debt but our balance exceeds the debt
if(balanceOfWant() > _debtOutstanding){
return(finalProfit,0,_debtOutstanding);
} else {
//Otherwise report back the balance all as debt payment
return(finalProfit,0,balanceOfWant());
}
_profit = finalProfit.sub(_debtOutstanding);
_debtPayment = _debtOutstanding;
}
}

Expand Down Expand Up @@ -238,14 +237,14 @@ event toDepositFloorEvent(uint256 _amount);

function firstTimeApprovals() internal {
//check if tokens are approved as needed.
if(IERC20(LOOKSToken).allowance(address(this),LooksRareStaking) == 0){
IERC20(LOOKSToken).approve(LooksRareStaking, max);
if(LOOKSToken.allowance(address(this),LooksRareStaking) == 0){
LOOKSToken.approve(LooksRareStaking, max);
}
if(IERC20(LOOKSToken).allowance(address(this),address(univ2router)) == 0){
IERC20(LOOKSToken).approve(address(univ2router), max);
if(LOOKSToken.allowance(address(this),address(univ2router)) == 0){
LOOKSToken.approve(address(univ2router), max);
}
if(IERC20(WETH).allowance(address(this),address(univ2router)) == 0){
IERC20(WETH).approve(address(univ2router), max);
if(WETH.allowance(address(this),address(univ2router)) == 0){
WETH.approve(address(univ2router), max);
}

}
Expand All @@ -268,7 +267,7 @@ event postWithdrawBalanceEvent(uint256 _amount);
// NOTE: Maintain invariant `_liquidatedAmount + _loss <= _amountNeeded`

//Sanitycheck that amount needed is not 0
if(_amountNeeded != 0){
if(_amountNeeded > 0){
uint256 unusedBalance = balanceOfWant();

if(unusedBalance < _amountNeeded){
Expand Down Expand Up @@ -300,7 +299,7 @@ event postWithdrawBalanceEvent(uint256 _amount);
}

//We now check the total assets amount we have post withdraw.
uint256 totalAssets = want.balanceOf(address(this));
uint256 totalAssets = balanceOfWant();
emit postWithdrawBalanceEvent(totalAssets);

//If the amount we need is more than the total amount of assets we have.
Expand All @@ -320,8 +319,8 @@ event postWithdrawBalanceEvent(uint256 _amount);
function liquidateAllPositions() internal override returns (uint256) {
// TODO: Liquidate all positions and return the amount freed.

//We then withdraw with claim to get the full balance including the latest rewards
looksRareContract.withdrawAll(true);
//Full withdraw excluding rewards incase of reward errors.
looksRareContract.withdrawAll(false);

//We do a check to see if we have any WETH from the last rewards set that isn't converted.
uint256 wethCheck = IERC20(WETH).balanceOf(address(this));
Expand Down Expand Up @@ -352,7 +351,19 @@ event postWithdrawBalanceEvent(uint256 _amount);
function prepareMigration(address _newStrategy) internal override {
// TODO: Transfer any non-`want` tokens to the new strategy
// NOTE: `migrate` will automatically forward all `want` in this strategy to the new one
liquidateAllPositions();

//Full withdraw including rewards as this is not an emergency liquidation
looksRareContract.withdrawAll(false);

//We do a check to see if we have any WETH from the last rewards set that isn't converted.
uint256 wethCheck = IERC20(WETH).balanceOf(address(this));

//If the WETH balance is non-zero we'll initiate a swap.
if(wethCheck != 0){
_sellRewards();
Copy link
Copy Markdown

@newmickymousse newmickymousse Mar 4, 2022

Choose a reason for hiding this comment

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

don't initiate a swap here. Transfer weth to the new strat.

Copy link
Copy Markdown

@newmickymousse newmickymousse Mar 16, 2022

Choose a reason for hiding this comment

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

Did you change this?

}


}

// Override this to add all tokens/tokenized positions this contract manages
Expand Down