diff --git a/contracts/BaseStrategy.sol b/contracts/BaseStrategy.sol index a1ca399..0f0f422 100644 --- a/contracts/BaseStrategy.sol +++ b/contracts/BaseStrategy.sol @@ -3,14 +3,28 @@ pragma solidity 0.8.14; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "./interfaces/IVault.sol"; +interface IBaseFee { + function isCurrentBaseFeeAcceptable() external view returns (bool); +} + abstract contract BaseStrategy { - address public vault; + address public immutable vault; address public immutable asset; string public name; + modifier onlyVault() { + _onlyVault(); + _; + } + + function _onlyVault() internal { + require(msg.sender == vault, "not vault"); + } + constructor(address _vault, string memory _name) { vault = _vault; name = _name; @@ -20,7 +34,11 @@ abstract contract BaseStrategy { function maxDeposit( address receiver ) public view virtual returns (uint256 maxAssets) { - maxAssets = type(uint256).max; + if (receiver == vault) { + maxAssets = type(uint256).max; + } else { + return 0; + } } function convertToAssets(uint256 shares) public view returns (uint256) { @@ -47,11 +65,9 @@ abstract contract BaseStrategy { function deposit( uint256 assets, address receiver - ) public returns (uint256) { - require(msg.sender == vault && msg.sender == receiver, "not owner"); - + ) public onlyVault returns (uint256) { // transfer and invest - IERC20(asset).transferFrom(vault, address(this), assets); + IERC20(asset).transferFrom(msg.sender, address(this), assets); _invest(); return assets; } @@ -60,32 +76,51 @@ abstract contract BaseStrategy { return _maxWithdraw(owner); } - function _maxWithdraw( - address owner - ) internal view virtual returns (uint256 withdraw_amount) {} + function tend() external onlyVault { + _tend(); + } + + function tendTrigger() external view returns (bool) { + return _tendTrigger(); + } + + function migrate(address _newStrategy) external onlyVault { + _migrate(_newStrategy); + } function withdraw( uint256 amount, address receiver, address owner - ) public returns (uint256) { - require(msg.sender == owner, "not owner"); - require(amount <= maxWithdraw(owner), "withdraw more than max"); + ) public onlyVault returns (uint256) { + require(amount <= _maxWithdraw(msg.sender), "withdraw more than max"); - uint256 amount_withdrawn = _withdraw(amount, receiver, owner); - IERC20(asset).transfer(receiver, amount_withdrawn); - return amount_withdrawn; + uint256 amountWithdrawn = _withdraw(amount); + IERC20(asset).transfer(msg.sender, amountWithdrawn); + return amountWithdrawn; } - function _withdraw( - uint256 amount, - address receiver, + function _maxWithdraw( address owner - ) internal virtual returns (uint256 withdraw_amount) {} + ) internal view virtual returns (uint256 withdrawAmount); + + function _withdraw( + uint256 amount + ) internal virtual returns (uint256 withdrawnAmount); + + function _invest() internal virtual; + + function _totalAssets() internal view virtual returns (uint256); + + function _tend() internal virtual {} + + function _tendTrigger() internal view virtual returns (bool) {} - function _invest() internal virtual {} + function _migrate(address) internal virtual; - function _totalAssets() internal view virtual returns (uint256) { - return IERC20(asset).balanceOf(address(this)); + function isBaseFeeAcceptable() internal view returns (bool) { + return + IBaseFee(0xb5e1CAcB567d98faaDB60a1fD4820720141f064F) + .isCurrentBaseFeeAcceptable(); } } diff --git a/contracts/Strategy.sol b/contracts/Strategy.sol index 48fae2b..b8fd01a 100644 --- a/contracts/Strategy.sol +++ b/contracts/Strategy.sol @@ -22,13 +22,13 @@ contract Strategy is BaseStrategy, Ownable { ISwapRouter(0xE592427A0AEce92De3Edee1F18E0157C05861564); //Fees for the V3 pools if the supply is incentivized uint24 public compToEthFee; - uint24 public ethToWantFee; + uint24 public ethToAssetFee; // eth blocks are mined every 12s -> 3600 * 24 * 365 / 12 = 2_628_000 uint256 private constant BLOCKS_PER_YEAR = 2_628_000; - address public constant COMP = + address internal constant COMP = address(0xc00e94Cb662C3520282E6f5717214004A7f26888); - address public constant WETH = + address internal constant WETH = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); ComptrollerI public constant COMPTROLLER = ComptrollerI(0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B); @@ -36,7 +36,7 @@ contract Strategy is BaseStrategy, Ownable { UniswapAnchoredViewI(0x65c816077C29b557BEE980ae3cC2dCE80204A0C5); uint256 public minCompToSell = 1 ether; - uint256 public minCompToClaim = 1 ether; + uint256 public minCompToClaim = 10 ether; uint256 public dustThreshold = 1; address public tradeFactory; @@ -60,8 +60,12 @@ contract Strategy is BaseStrategy, Ownable { address owner ) internal view override returns (uint256) { // TODO: may not be accurate due to unaccrued balance in cToken - return - Math.min(IERC20(asset).balanceOf(address(cToken)), _totalAssets()); + if (owner == vault) { + // return total value we have even if illiquid so the vault doesnt assess incorrect unrealized losses + return _totalAssets(); + } else { + return 0; + } } function _freeFunds( @@ -88,11 +92,7 @@ contract Strategy is BaseStrategy, Ownable { } } - function _withdraw( - uint256 amount, - address receiver, - address owner - ) internal override returns (uint256) { + function _withdraw(uint256 amount) internal override returns (uint256) { return _freeFunds(amount); } @@ -109,6 +109,10 @@ contract Strategy is BaseStrategy, Ownable { function _withdrawFromCompound(uint256 _amount) internal { if (_amount > dustThreshold) { + _amount = Math.min( + _amount, + IERC20(asset).balanceOf(address(cToken)) + ); require( cToken.redeemUnderlying(_amount) == 0, "cToken: redeemUnderlying fail" @@ -216,12 +220,19 @@ contract Strategy is BaseStrategy, Ownable { return (cToken.balanceOf(address(this)) * deltaIndex) / 1e36; } - function harvest() external onlyOwner { - if (getRewardsPending() > minCompToClaim) { - _claimRewards(); - } + function _tendTrigger() internal view override returns (bool) { + if (!isBaseFeeAcceptable()) return false; + if ( + getRewardsPending() + IERC20(COMP).balanceOf(address(this)) > + minCompToClaim + ) return true; + } - if (tradeFactory == address(0) && compToEthFee != 0) { + // can be called by either owner or the vault + function _tend() internal override { + _claimRewards(); + + if (tradeFactory == address(0) && ethToAssetFee != 0) { _disposeOfComp(); } @@ -254,8 +265,8 @@ contract Strategy is BaseStrategy, Ownable { COMP, // comp-ETH compToEthFee, WETH, // ETH-want - ethToWantFee, - IVault(vault).asset() + ethToAssetFee, + asset ); // Proceeds from Comp are not subject to minExpectedSwapPercentage @@ -272,14 +283,32 @@ contract Strategy is BaseStrategy, Ownable { } } + function _migrate(address _newStrategy) internal override { + uint256 balanceUnderlying = cToken.balanceOfUnderlying(address(this)); + + // first try and withdraw the full balance + cToken.redeemUnderlying(balanceUnderlying); + + // send whatever tokens we have to new strategy + uint256 looseAsset = balanceOfAsset(); + if (looseAsset > 0) { + IERC20(asset).transfer(_newStrategy, looseAsset); + } + + uint256 cTokenBalance = balanceOfCToken(); + if (cTokenBalance > 0) { + cToken.transfer(_newStrategy, cTokenBalance); + } + } + //These will default to 0. //Will need to be manually set if want is incentized before any harvests function setUniFees( uint24 _compToEth, - uint24 _ethToWant + uint24 _ethToAsset ) external onlyOwner { compToEthFee = _compToEth; - ethToWantFee = _ethToWant; + ethToAssetFee = _ethToAsset; } /** diff --git a/contracts/interfaces/IVault.sol b/contracts/interfaces/IVault.sol index 931fbf7..7bfd65f 100644 --- a/contracts/interfaces/IVault.sol +++ b/contracts/interfaces/IVault.sol @@ -25,4 +25,6 @@ interface IVault { address strategy, uint256 target_debt ) external returns (uint256); + + function tend_strategy(address strategy) external; } diff --git a/tests/test_reward.py b/tests/test_reward.py index 2ea3f2d..877d57e 100644 --- a/tests/test_reward.py +++ b/tests/test_reward.py @@ -1,6 +1,6 @@ from ape import chain import pytest -from utils.constants import MAX_INT +from utils.constants import MAX_INT, ROLES def test_rewards_selling( @@ -21,15 +21,15 @@ def test_rewards_selling( before_bal = strategy.totalAssets() - reward = 2 * 10 ** comp.decimals() + reward = 11 * 10 ** comp.decimals() comp.transfer(strategy, reward, sender=comp_whale) assert comp.balanceOf(strategy) == reward # Set uni fees strategy.setUniFees(3000, 500, sender=strategist) - # harvest function should still work and will swap rewards any rewards - strategy.harvest(sender=strategist) + # tend function should still work and will swap rewards any rewards + strategy.tend(sender=vault) # rewards should be sold assert strategy.totalAssets() > before_bal @@ -64,19 +64,13 @@ def test_rewards_pending( new_debt = amount provide_strategy_with_debt(gov, strategy, vault, new_debt) - # Don't sell rewards nor claim - strategy.setRewardStuff(MAX_INT, MAX_INT, sender=strategist) - - strategy.harvest(sender=strategist) - # Take some time for rewards to accrue chain.mine(3600 * 24 * 10) - # Somebody deposits to trigger to rewards calculation - asset.approve(vault.address, amount, sender=asset_whale) + # Somebody deposits to trigger rewards calculation ctoken.mint(amount, sender=asset_whale) - # rewards should be sold + # rewards should be pending buy not claimed rewards_pending = strategy.getRewardsPending() assert rewards_pending > 0 assert comp.balanceOf(strategy) == 0 @@ -84,8 +78,8 @@ def test_rewards_pending( # Don't sell rewards but claim all strategy.setRewardStuff(MAX_INT, 1, sender=strategist) - # harvest function should still work and will swap rewards any rewards - strategy.harvest(sender=strategist) + # tend function should still work and will not swap any rewards + strategy.tend(sender=vault) assert comp.balanceOf(strategy) >= rewards_pending assert comp.balanceOf(strategy) < rewards_pending * 1.1 diff --git a/tests/test_strategy.py b/tests/test_strategy.py index 3cc816a..fed8ae5 100644 --- a/tests/test_strategy.py +++ b/tests/test_strategy.py @@ -1,6 +1,6 @@ from ape import reverts import pytest -from utils.constants import REL_ERROR, MAX_INT, BLOCKS_PER_YEAR +from utils.constants import REL_ERROR, MAX_INT, BLOCKS_PER_YEAR, ROLES def test_strategy_constructor(asset, vault, strategy): @@ -111,10 +111,11 @@ def test_balance_of(create_vault_and_strategy, gov, amount, provide_strategy_wit def test_deposit_no_vault__reverts(create_vault_and_strategy, gov, amount, user): vault, strategy = create_vault_and_strategy(gov, amount) - with reverts("not owner"): + with reverts("not vault"): strategy.deposit(100, user, sender=user) - with reverts("not owner"): + # will revert due to no approval + with reverts(): strategy.deposit(100, user, sender=vault) @@ -169,13 +170,7 @@ def test_max_withdraw_no_liquidity( user, asset.balanceOf(ctoken) - 10 ** vault.decimals(), sender=ctoken ) - assert strategy.maxWithdraw(vault) == 10 ** vault.decimals() - - -def test_withdraw_no_owner__reverts(create_vault_and_strategy, gov, amount, user): - vault, strategy = create_vault_and_strategy(gov, amount) - with reverts("not owner"): - strategy.withdraw(100, user, user, sender=vault) + assert strategy.maxWithdraw(vault) == strategy.totalAssets() def test_withdraw_above_max__reverts(create_vault_and_strategy, gov, amount, user): @@ -245,7 +240,7 @@ def test_withdraw_low_liquidity( assert new_debt - 10 ** vault.decimals() > strategy.balanceOfCToken() max_withdraw = strategy.maxWithdraw(vault) - assert max_withdraw == 10 ** vault.decimals() + assert max_withdraw == strategy.totalAssets() tx = strategy.withdraw(max_withdraw, vault, vault, sender=vault) # all is in cToken because underlying asset is drained @@ -301,7 +296,7 @@ def test_apr( assert current_real_apr + rewards_apr > strategy.aprAfterDebtChange(int(1e12)) -def test_harvest( +def test_tend( asset, ctoken, user, @@ -317,8 +312,8 @@ def test_harvest( before_bal = strategy.totalAssets() - # harvest function should still work and not revert without any rewards - strategy.harvest(sender=strategist) + # tend function should still work and not revert without any rewards + strategy.tend(sender=vault) stored_balance = strategy.balanceOfCToken() # this will trigger to recalculating the exchange rate used for cToken diff --git a/tests/test_yswap.py b/tests/test_yswap.py index 2e56cb3..9b4f679 100644 --- a/tests/test_yswap.py +++ b/tests/test_yswap.py @@ -1,6 +1,6 @@ from ape import reverts import pytest -from utils.constants import ZERO_ADDRESS +from utils.constants import ZERO_ADDRESS, ROLES def test_reward_yswap( @@ -27,7 +27,7 @@ def test_reward_yswap( before_bal = strategy.totalAssets() - reward = 2 * 10 ** comp.decimals() + reward = 11 * 10 ** comp.decimals() comp.transfer(strategy, reward, sender=comp_whale) assert comp.balanceOf(strategy) == reward @@ -38,13 +38,13 @@ def test_reward_yswap( strategy.setTradeFactory(trade_factory.address, sender=strategist) assert strategy.tradeFactory() == trade_factory.address - # harvest function will not sell COMP because yswap is set - strategy.harvest(sender=strategist) - assert comp.balanceOf(strategy) == reward + # tend function will not sell COMP because yswap is set + strategy.tend(sender=vault) + assert comp.balanceOf(strategy) >= reward token_in = comp token_out = asset - amount_in = reward + amount_in = comp.balanceOf(strategy) asyncTradeExecutionDetails = [ strategy.address, token_in.address, diff --git a/tests/utils/constants.py b/tests/utils/constants.py index 3723b6b..aa9b0de 100644 --- a/tests/utils/constants.py +++ b/tests/utils/constants.py @@ -15,3 +15,4 @@ class ROLES(IntFlag): DEBT_MANAGER = 2 EMERGENCY_MANAGER = 4 ACCOUNTING_MANAGER = 8 + KEEPER = 16