build: updates and features (#6)

* build: updates and features

* fix: fix

* fix: lint

* fix: migration fixes

* test: rewards

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();
     }
 }

contracts/Strategy.sol

@@ -22,21 +22,21 @@ 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);
     UniswapAnchoredViewI public constant PRICE_FEED =
         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;
     }
 
     /**

contracts/interfaces/IVault.sol

@@ -25,4 +25,6 @@ interface IVault {
         address strategy,
         uint256 target_debt
     ) external returns (uint256);
+
+    function tend_strategy(address strategy) external;
 }

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,28 +64,22 @@ 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
 
     # 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

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