From 6d40b6eda56884f5a5c67f00790071b8df8e6d3f Mon Sep 17 00:00:00 2001 From: "pata.eth" Date: Sun, 22 May 2022 20:27:12 -0400 Subject: [PATCH] updates following review from strategists and security on https://github.com/dudesahn/StrategyCurveTemplate/pull/8 --- contracts/StrategyCurveGeist.sol | 127 ++++++++++++++----------------- contracts/interfaces/curve.sol | 120 ++--------------------------- contracts/interfaces/yearn.sol | 43 ----------- tests/test_migration.py | 51 +++++++++++-- tests/test_odds_and_ends.py | 14 ++-- 5 files changed, 117 insertions(+), 238 deletions(-) delete mode 100644 contracts/interfaces/yearn.sol diff --git a/contracts/StrategyCurveGeist.sol b/contracts/StrategyCurveGeist.sol index 063499f..dbf6d5d 100644 --- a/contracts/StrategyCurveGeist.sol +++ b/contracts/StrategyCurveGeist.sol @@ -9,32 +9,9 @@ import "@openzeppelin/contracts/utils/Address.sol"; import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; import "@openzeppelin/contracts/math/Math.sol"; -import "./interfaces/curve.sol"; -import "./interfaces/yearn.sol"; -import {IUniswapV2Router02} from "./interfaces/uniswap.sol"; -import { - BaseStrategy, - StrategyParams -} from "@yearnvaults/contracts/BaseStrategy.sol"; - -interface IBaseFee { - function isCurrentBaseFeeAcceptable() external view returns (bool); -} - -interface IUniV3 { - struct ExactInputParams { - bytes path; - address recipient; - uint256 deadline; - uint256 amountIn; - uint256 amountOutMinimum; - } - - function exactInput(ExactInputParams calldata params) - external - payable - returns (uint256 amountOut); -} +import { IGauge, IGaugeFactory, ICurveFi } from "./interfaces/curve.sol"; +import { IUniswapV2Router02 } from "./interfaces/uniswap.sol"; +import { BaseStrategy, StrategyParams } from "@yearnvaults/contracts/BaseStrategy.sol"; abstract contract StrategyCurveBase is BaseStrategy { using SafeERC20 for IERC20; @@ -46,10 +23,10 @@ abstract contract StrategyCurveBase is BaseStrategy { // Curve stuff IGauge public constant gauge = - IGauge(0xF7b9c402c4D6c2eDbA04a7a515b53D11B1E9b2cc); // Curve gauge contract, most are tokenized, held by strategy + IGauge(0xF7b9c402c4D6c2eDbA04a7a515b53D11B1E9b2cc); // Curve gauge contract, most are tokenized, held by strategy IGaugeFactory public constant gaugeFactory = - IGaugeFactory(0xabC000d88f23Bb45525E447528DBF656A9D55bf5); + IGaugeFactory(0xabC000d88f23Bb45525E447528DBF656A9D55bf5); // keepCRV stuff uint256 public keepCRV; // the percentage of CRV we re-lock for boost (in basis points) @@ -133,7 +110,19 @@ abstract contract StrategyCurveBase is BaseStrategy { return balanceOfWant(); } + function claimRewards() internal { + // Claims any pending CRV + // + // Mints claimable CRV from the factory gauge. Reward tokens are sent to `msg.sender` + gaugeFactory.mint(address(gauge)); + + // harvest third-party rewards from the gauge, if any + gauge.claim_rewards(); + } + function prepareMigration(address _newStrategy) internal override { + // Withdraw LP tokens from the gauge. The transfer to the new strategy is done + // by migrate() in BaseStrategy.sol uint256 _stakedBal = stakedBalance(); if (_stakedBal > 0) { gauge.withdraw(_stakedBal); @@ -204,10 +193,8 @@ contract StrategyCurveGeist is StrategyCurveBase { address spirit = 0x16327E3FbDaCA3bcF7E38F5Af2599D2DDc33aE52; want.approve(address(gauge), type(uint256).max); crv.approve(spooky, type(uint256).max); - wftm.approve(spooky, type(uint256).max); geist.approve(spooky, type(uint256).max); crv.approve(spirit, type(uint256).max); - wftm.approve(spirit, type(uint256).max); geist.approve(spirit, type(uint256).max); // set our strategy's name @@ -218,13 +205,35 @@ contract StrategyCurveGeist is StrategyCurveBase { usdc.approve(address(curve), type(uint256).max); fusdt.safeApprove(address(curve), type(uint256).max); - // start off with fusdt - targetToken = address(fusdt); + // start off with usdc, determined by amount of bonus paid by the LP, if any + targetToken = address(usdc); } /* ========== MUTATIVE FUNCTIONS ========== */ // these will likely change across different wants. + function claimAndTransferRewards(address _targetAdress) + external + onlyVaultManagers + { + require(_targetAdress != address(0)); + + // Claim any pending rewards and transfer CRV balance to the new strategy + claimRewards(); + + uint256 crvBalance = crv.balanceOf(address(this)); + + if (crvBalance > 0) { + crv.safeTransfer(_targetAdress, crvBalance); + } + + uint256 geistBalance = geist.balanceOf(address(this)); + + if (geistBalance > 0) { + geist.safeTransfer(_targetAdress, geistBalance); + } + } + function prepareReturn(uint256 _debtOutstanding) internal override @@ -234,17 +243,14 @@ contract StrategyCurveGeist is StrategyCurveBase { uint256 _debtPayment ) { - // Mint CRV from the gauge factory. CRV is then transfered to the strategy (msg.sender) - gaugeFactory.mint(address(gauge)); + // Claim and get a fresh snapshot of the strategy's CRV and GEIST balance + claimRewards(); uint256 crvBalance = crv.balanceOf(address(this)); - // harvest third-party rewards from the gauge, if any - gauge.claim_rewards(); - - uint256 wftmBalance = wftm.balanceOf(address(this)); uint256 geistBalance = geist.balanceOf(address(this)); - // if we claimed any CRV, then sell it + + // Sell CRV if we have any if (crvBalance > 0) { // keep some of our CRV to increase our boost uint256 sendToVoter = crvBalance.mul(keepCRV).div(FEE_DENOMINATOR); @@ -260,12 +266,8 @@ contract StrategyCurveGeist is StrategyCurveBase { _sellToken(address(crv), crvBalance); } } - // sell WFTM if we have any - if (wftmBalance > 0) { - _sellToken(address(wftm), wftmBalance); - } - // sell the rest of our CRV + // Sell GEIST if we have any if (geistBalance > 0) { _sellToken(address(geist), geistBalance); } @@ -316,32 +318,19 @@ contract StrategyCurveGeist is StrategyCurveBase { forceHarvestTriggerOnce = false; } - // Sells our CRV, WFTM, or GEIST for our target token + // Sells our CRV or GEIST for our target token function _sellToken(address token, uint256 _amount) internal { - if (token == address(wftm)) { - address[] memory tokenPath = new address[](2); - tokenPath[0] = address(wftm); - tokenPath[1] = address(targetToken); - IUniswapV2Router02(router).swapExactTokensForTokens( - _amount, - uint256(0), - tokenPath, - address(this), - block.timestamp - ); - } else { - address[] memory tokenPath = new address[](3); - tokenPath[0] = address(token); - tokenPath[1] = address(wftm); - tokenPath[2] = address(targetToken); - IUniswapV2Router02(router).swapExactTokensForTokens( - _amount, - uint256(0), - tokenPath, - address(this), - block.timestamp - ); - } + address[] memory tokenPath = new address[](3); + tokenPath[0] = address(token); + tokenPath[1] = address(wftm); + tokenPath[2] = address(targetToken); + IUniswapV2Router02(router).swapExactTokensForTokens( + _amount, + uint256(0), + tokenPath, + address(this), + block.timestamp + ); } /* ========== KEEP3RS ========== */ diff --git a/contracts/interfaces/curve.sol b/contracts/interfaces/curve.sol index c3a1b19..5a49c06 100644 --- a/contracts/interfaces/curve.sol +++ b/contracts/interfaces/curve.sol @@ -2,135 +2,31 @@ pragma solidity 0.6.12; pragma experimental ABIEncoderV2; -import {IERC20} from "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; - interface IGauge { function deposit(uint256) external; function balanceOf(address) external view returns (uint256); + function withdraw(uint256) external; + function claim_rewards() external; - function withdraw(uint256) external; + // CRV + function claimable_tokens(address) external returns (uint256); + + // Third-party tokens only + function claimable_reward(address, address) external view returns (uint256); } interface IGaugeFactory { - function mint(address _gauge) external; + function mint(address _gauge) external; } interface ICurveFi { - function get_virtual_price() external view returns (uint256); - - function add_liquidity( - // EURt - uint256[2] calldata amounts, - uint256 min_mint_amount - ) external payable; - - function add_liquidity( - // Compound, sAave - uint256[2] calldata amounts, - uint256 min_mint_amount, - bool _use_underlying - ) external payable returns (uint256); - function add_liquidity( // Iron Bank, Aave uint256[3] calldata amounts, uint256 min_mint_amount, bool _use_underlying ) external payable returns (uint256); - - function add_liquidity( - // 3Crv Metapools - address pool, - uint256[4] calldata amounts, - uint256 min_mint_amount - ) external; - - function add_liquidity( - // Y and yBUSD - uint256[4] calldata amounts, - uint256 min_mint_amount, - bool _use_underlying - ) external payable returns (uint256); - - function add_liquidity( - // 3pool - uint256[3] calldata amounts, - uint256 min_mint_amount - ) external payable; - - function add_liquidity( - // sUSD - uint256[4] calldata amounts, - uint256 min_mint_amount - ) external payable; - - function remove_liquidity_imbalance( - uint256[2] calldata amounts, - uint256 max_burn_amount - ) external; - - function remove_liquidity(uint256 _amount, uint256[2] calldata amounts) - external; - - function remove_liquidity_one_coin( - uint256 _token_amount, - int128 i, - uint256 min_amount - ) external; - - function exchange( - int128 from, - int128 to, - uint256 _from_amount, - uint256 _min_to_amount - ) external; - - function balances(uint256) external view returns (uint256); - - function get_dy( - int128 from, - int128 to, - uint256 _from_amount - ) external view returns (uint256); - - // EURt - function calc_token_amount(uint256[2] calldata _amounts, bool _is_deposit) - external - view - returns (uint256); - - // 3Crv Metapools - function calc_token_amount( - address _pool, - uint256[4] calldata _amounts, - bool _is_deposit - ) external view returns (uint256); - - // sUSD, Y pool, etc - function calc_token_amount(uint256[4] calldata _amounts, bool _is_deposit) - external - view - returns (uint256); - - // 3pool, Iron Bank, etc - function calc_token_amount(uint256[3] calldata _amounts, bool _is_deposit) - external - view - returns (uint256); - - function calc_withdraw_one_coin(uint256 amount, int128 i) - external - view - returns (uint256); -} - -interface ICrvV3 is IERC20 { - function minter() external view returns (address); -} - -interface IMinter { - function mint(address) external; } diff --git a/contracts/interfaces/yearn.sol b/contracts/interfaces/yearn.sol deleted file mode 100644 index fa8104e..0000000 --- a/contracts/interfaces/yearn.sol +++ /dev/null @@ -1,43 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0 -pragma solidity 0.6.12; -pragma experimental ABIEncoderV2; - -import {IERC20} from "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; - -interface ICurveStrategyProxy { - function proxy() external returns (address); - - function balanceOf(address _gauge) external view returns (uint256); - - function deposit(address _gauge, address _token) external; - - function withdraw( - address _gauge, - address _token, - uint256 _amount - ) external returns (uint256); - - function withdrawAll(address _gauge, address _token) - external - returns (uint256); - - function harvest(address _gauge) external; - - function lock() external; - - function approveStrategy(address) external; - - function revokeStrategy(address) external; - - function claimRewards(address _gauge, address _token) external; -} - -interface IVoter { - function execute( - address to, - uint256 value, - bytes calldata data - ) external returns (bool, bytes memory); - - function increaseAmount(uint256) external; -} diff --git a/tests/test_migration.py b/tests/test_migration.py index a2f6a60..bf2e695 100644 --- a/tests/test_migration.py +++ b/tests/test_migration.py @@ -20,6 +20,8 @@ def test_migration( pool, strategy_name, gauge, + crv, + geist, ): ## deposit to the vault after approving @@ -47,11 +49,46 @@ def test_migration( chain.sleep(86400) chain.mine(1) + claimable_crv_tokens = gauge.claimable_tokens.call(strategy) + crv_balance_old_strat = crv.balanceOf(strategy) + + # assert claimable_crv_tokens > 0, "No CRV to be claimed" + + claimable_geist = gauge.claimable_reward(strategy, geist) + geist_balance_old_strat = geist.balanceOf(strategy) + + # assert claimable_geist > 0, "No GEIST tokens to be claimed" + # migrate our old strategy vault.migrateStrategy(strategy, new_strategy, {"from": gov}) new_strategy.setHealthCheck(healthCheck, {"from": gov}) new_strategy.setDoHealthCheck(True, {"from": gov}) + with brownie.reverts("!authorized"): + strategy.claimAndTransferRewards(new_strategy, {"from": whale}) + + with brownie.reverts(): + strategy.claimAndTransferRewards(brownie.ZERO_ADDRESS, {"from": gov}) + + strategy.claimAndTransferRewards(new_strategy, {"from": gov}) + + migrated_crv = crv.balanceOf(new_strategy) + migrated_geist = geist.balanceOf(new_strategy) + + assert migrated_crv >= ( + claimable_crv_tokens + crv_balance_old_strat + ), "CRV not transferred as expected" + + claimable_crv = gauge.claimable_tokens.call(strategy) + assert claimable_crv == 0, "Left CRV behind" + + assert migrated_geist >= ( + claimable_geist + geist_balance_old_strat + ), "GEIST not transferred as expected" + + claimable_geist = gauge.claimable_reward(strategy, geist) + assert claimable_geist == 0, "Left GEIST behind" + # assert that our old strategy is empty updated_total_old = strategy.estimatedTotalAssets() assert updated_total_old == 0 @@ -66,8 +103,8 @@ def test_migration( new_strat_balance, total_old, abs_tol=5 ) - startingVault = vault.totalAssets() - print("\nVault starting assets with new strategy: ", startingVault) + starting_vault_assets = vault.totalAssets() + print("\nVault starting assets with new strategy: ", starting_vault_assets) # simulate one day of earnings chain.sleep(86400) @@ -77,8 +114,8 @@ def test_migration( new_strategy.harvest({"from": gov}) vaultAssets_2 = vault.totalAssets() # confirm we made money, or at least that we have about the same - assert vaultAssets_2 >= startingVault or math.isclose( - vaultAssets_2, startingVault, abs_tol=5 + assert vaultAssets_2 >= starting_vault_assets or math.isclose( + vaultAssets_2, starting_vault_assets, abs_tol=5 ) print("\nAssets after 1 day harvest: ", vaultAssets_2) @@ -121,8 +158,8 @@ def test_migration_from_real_strat( # confirm that at least the same amount of assets were moved to the new strat assert new_strat_balance >= total_old - startingVault = vaultDeployed.totalAssets() - print("\nVault starting assets with new strategy: ", startingVault) + starting_vault_assets = vaultDeployed.totalAssets() + print("\nVault starting assets with new strategy: ", starting_vault_assets) # simulate one day of earnings chain.sleep(86400) @@ -133,4 +170,4 @@ def test_migration_from_real_strat( vaultAssets_2 = vaultDeployed.totalAssets() # confirm we made money, or remained the same - assert vaultAssets_2 >= startingVault + assert vaultAssets_2 >= starting_vault_assets diff --git a/tests/test_odds_and_ends.py b/tests/test_odds_and_ends.py index e63167e..ca97251 100644 --- a/tests/test_odds_and_ends.py +++ b/tests/test_odds_and_ends.py @@ -75,8 +75,8 @@ def test_odds_and_ends( new_strat_balance = new_strategy.estimatedTotalAssets() assert new_strat_balance >= total_old - startingVault = vault.totalAssets() - print("\nVault starting assets with new strategy: ", startingVault) + starting_vault_assets = vault.totalAssets() + print("\nVault starting assets with new strategy: ", starting_vault_assets) # simulate one day of earnings chain.sleep(86400) @@ -85,7 +85,7 @@ def test_odds_and_ends( # Test out our migrated strategy, confirm we're making a profit new_strategy.harvest({"from": gov}) vaultAssets_2 = vault.totalAssets() - assert vaultAssets_2 >= startingVault + assert vaultAssets_2 >= starting_vault_assets print("\nAssets after 1 day harvest: ", vaultAssets_2) # check our oracle @@ -201,8 +201,8 @@ def test_odds_and_ends_migration( new_strat_balance, total_old, abs_tol=5 ) - startingVault = vault.totalAssets() - print("\nVault starting assets with new strategy: ", startingVault) + starting_vault_assets = vault.totalAssets() + print("\nVault starting assets with new strategy: ", starting_vault_assets) # simulate one day of earnings chain.sleep(86400) @@ -216,8 +216,8 @@ def test_odds_and_ends_migration( new_strategy.harvest({"from": gov}) vaultAssets_2 = vault.totalAssets() # confirm we made money, or at least that we have about the same - assert vaultAssets_2 >= startingVault or math.isclose( - vaultAssets_2, startingVault, abs_tol=5 + assert vaultAssets_2 >= starting_vault_assets or math.isclose( + vaultAssets_2, starting_vault_assets, abs_tol=5 ) print("\nAssets after 1 day harvest: ", vaultAssets_2)