Fix bug in convex yield collection (#842)

* Collect convex yield.

* Use CRV as the official reward token

* Unit tests test harvesting coins

* Remove debugging

* Better comment

* More comment tweaks

contracts/contracts/mocks/curve/MockBooster.sol

@@ -23,18 +23,24 @@ contract MockBooster {
     }
 
     address public minter; // this is CVx for the booster on live
-    address public crv; // this is crv token for crv rewards
+    address public crv; // Curve rewards token
+    address public cvx; // Convex rewards token
     mapping(uint256 => PoolInfo) public poolInfo;
 
-    constructor(address _rewardsMinter, address _crv) public {
+    constructor(
+        address _rewardsMinter,
+        address _crv,
+        address _cvx
+    ) public {
         minter = _rewardsMinter;
         crv = _crv;
+        cvx = _cvx;
     }
 
     function setPool(uint256 pid, address _lpToken) external returns (bool) {
         address token = address(new MockDepositToken());
         address rewards = address(
-            new MockRewardPool(pid, token, crv, address(this))
+            new MockRewardPool(pid, token, crv, cvx, address(this))
         );
 
         poolInfo[pid] = PoolInfo({

contracts/contracts/mocks/curve/MockRewardPool.sol

@@ -38,7 +38,8 @@ contract MockRewardPool {
 
     uint256 public pid;
     address public stakingToken;
-    address public rewardToken;
+    address public rewardTokenA;
+    address public rewardTokenB;
     address public operator;
 
     uint256 private _totalSupply;
@@ -47,14 +48,16 @@ contract MockRewardPool {
     mapping(address => uint256) public rewards;
 
     constructor(
-        uint256 pid_,
-        address stakingToken_,
-        address rewardToken_,
-        address operator_
+        uint256 _pid,
+        address _stakingToken,
+        address _rewardTokenA,
+        address _rewardTokenB,
+        address _operator
     ) public {
-        pid = pid_;
-        stakingToken = stakingToken_;
-        rewardToken = rewardToken_;
+        pid = _pid;
+        stakingToken = _stakingToken;
+        rewardTokenA = _rewardTokenA;
+        rewardTokenB = _rewardTokenB;
     }
 
     function totalSupply() public view returns (uint256) {
@@ -65,12 +68,6 @@ contract MockRewardPool {
         return _balances[account];
     }
 
-    // mock function to set the rewards per account
-    function earnRewards(address account, uint256 amount) external {
-        IMintableERC20(rewardToken).mint(amount);
-        rewards[account] += amount;
-    }
-
     function stakeFor(address _for, uint256 _amount) public returns (bool) {
         require(_amount > 0, "RewardPool : Cannot stake 0");
 
@@ -113,12 +110,16 @@ contract MockRewardPool {
         public
         returns (bool)
     {
-        uint256 reward = rewards[_account];
-        if (reward > 0) {
-            rewards[_account] = 0;
-            IERC20(rewardToken).safeTransfer(_account, reward);
-            IDeposit(operator).rewardClaimed(pid, _account, reward);
-        }
+        IMintableERC20(rewardTokenA).mint(2 * 1e18);
+        IERC20(rewardTokenA).transfer(_account, 2 * 1e18);
+
+        IMintableERC20(rewardTokenB).mint(3 * 1e18);
+        IERC20(rewardTokenB).transfer(_account, 3 * 1e18);
+
         return true;
     }
+
+    function getReward() public returns (bool) {
+        getReward(msg.sender, true);
+    }
 }

contracts/contracts/strategies/BaseCurveStrategy.sol

@@ -147,7 +147,11 @@ abstract contract BaseCurveStrategy is InitializableAbstractStrategy {
         (, uint256 gaugePTokens, uint256 totalPTokens) = _getTotalPTokens();
         _lpWithdraw(gaugePTokens);
         // Withdraws are proportional to assets held by 3Pool
-        uint256[3] memory minWithdrawAmounts = [uint256(0), uint256(0), uint256(0)];
+        uint256[3] memory minWithdrawAmounts = [
+            uint256(0),
+            uint256(0),
+            uint256(0)
+        ];
         // Remove liquidity
         ICurvePool threePool = ICurvePool(platformAddress);
         threePool.remove_liquidity(totalPTokens, minWithdrawAmounts);

contracts/contracts/strategies/ConvexStrategy.sol

@@ -24,9 +24,14 @@ contract ConvexStrategy is BaseCurveStrategy {
         uint256 amount
     );
 
+    event CvxRewardTokenAddressUpdated(
+        address _oldAddress,
+        address _newAddress
+    );
+
     address internal cvxDepositorAddress;
     address internal cvxRewardStakerAddress;
-    address internal crvRewardTokenAddress;
+    address internal cvxRewardTokenAddress;
     uint256 internal cvxDepositorPTokenId;
 
     /**
@@ -35,21 +40,21 @@ contract ConvexStrategy is BaseCurveStrategy {
      * well within that abstraction.
      * @param _platformAddress Address of the Curve 3pool
      * @param _vaultAddress Address of the vault
-     * @param _rewardTokenAddress Address of CRVX
-     * @param _crvRewardTokenAddress Address of CRV *yes we get both*
+     * @param _rewardTokenAddress Address of CRV
+     * @param _cvxRewardTokenAddress Address of CVX *yes we get both*
      * @param _assets Addresses of supported assets. MUST be passed in the same
      *                order as returned by coins on the pool contract, i.e.
      *                DAI, USDC, USDT
      * @param _pTokens Platform Token corresponding addresses
      * @param _cvxDepositorAddress Address of the Convex depositor(AKA booster) for this pool
-     * @param _cvxRewardStakerAddress Address of the CRVX rewards staker
+     * @param _cvxRewardStakerAddress Address of the CVX rewards staker
      * @param _cvxDepositorPTokenId Pid of the pool referred to by Depositor and staker
      */
     function initialize(
         address _platformAddress, // 3Pool address
         address _vaultAddress,
-        address _rewardTokenAddress, // CRVX
-        address _crvRewardTokenAddress,
+        address _rewardTokenAddress, // CRV
+        address _cvxRewardTokenAddress, // CVX
         address[] calldata _assets,
         address[] calldata _pTokens,
         address _cvxDepositorAddress,
@@ -62,7 +67,7 @@ contract ConvexStrategy is BaseCurveStrategy {
         cvxDepositorAddress = _cvxDepositorAddress;
         cvxRewardStakerAddress = _cvxRewardStakerAddress;
         cvxDepositorPTokenId = _cvxDepositorPTokenId;
-        crvRewardTokenAddress = _crvRewardTokenAddress;
+        cvxRewardTokenAddress = _cvxRewardTokenAddress;
         pTokenAddress = _pTokens[0];
         super._initialize(
             _platformAddress,
@@ -74,6 +79,21 @@ contract ConvexStrategy is BaseCurveStrategy {
         _approveBase();
     }
 
+    /**
+     * @dev Set the CVX reward token address.
+     * @param _cvxRewardTokenAddress Address of the reward token
+     */
+    function setCvxRewardTokenAddress(address _cvxRewardTokenAddress)
+        external
+        onlyGovernor
+    {
+        emit CvxRewardTokenAddressUpdated(
+            cvxRewardTokenAddress,
+            _cvxRewardTokenAddress
+        );
+        cvxRewardTokenAddress = _cvxRewardTokenAddress;
+    }
+
     function _lpDepositAll() internal override {
         IERC20 pToken = IERC20(pTokenAddress);
         // Deposit with staking
@@ -129,19 +149,20 @@ contract ConvexStrategy is BaseCurveStrategy {
     }
 
     /**
-     * @dev Collect accumulated CRV and send to Vault.
+     * @dev Collect accumulated CRV and CVX and send to Vault.
      */
     function collectRewardToken() external override onlyVault nonReentrant {
-        // Collect is done automatically with withdrawAndUnwrap
-        // Send CVX
-        IERC20 crvxToken = IERC20(rewardTokenAddress);
-        uint256 balance = crvxToken.balanceOf(address(this));
-        emit RewardTokenCollected(vaultAddress, rewardTokenAddress, balance);
-        crvxToken.safeTransfer(vaultAddress, balance);
+        // Collect CRV and CVX
+        IRewardStaking(cvxRewardStakerAddress).getReward();
         // Send CRV
-        IERC20 crvToken = IERC20(crvRewardTokenAddress);
-        balance = crvToken.balanceOf(address(this));
-        emit RewardTokenCollected(vaultAddress, crvRewardTokenAddress, balance);
+        IERC20 crvToken = IERC20(rewardTokenAddress);
+        uint256 balance = crvToken.balanceOf(address(this));
+        emit RewardTokenCollected(vaultAddress, rewardTokenAddress, balance);
         crvToken.safeTransfer(vaultAddress, balance);
+        // Send CVX
+        IERC20 cvxToken = IERC20(cvxRewardTokenAddress);
+        balance = cvxToken.balanceOf(address(this));
+        emit RewardTokenCollected(vaultAddress, cvxRewardTokenAddress, balance);
+        cvxToken.safeTransfer(vaultAddress, balance);
     }
 }

contracts/deploy/000_mock.js

@@ -230,7 +230,7 @@ const deployMocks = async ({ getNamedAccounts, deployments }) => {
 
   await deploy("MockBooster", {
     from: deployerAddr,
-    args: [mockCVX.address, mockCRV.address],
+    args: [mockCVX.address, mockCRV.address, mockCVX.address],
   });
   const mockBooster = await ethers.getContract("MockBooster");
   await mockBooster.setPool(threeCRVPid, threePoolToken.address);
@@ -241,6 +241,7 @@ const deployMocks = async ({ getNamedAccounts, deployments }) => {
       threeCRVPid,
       threePoolToken.address,
       mockCRV.address,
+      mockCVX.address,
       mockCRV.address,
     ],
   });

contracts/deploy/001_core.js

@@ -264,8 +264,8 @@ const deployConvexStrategy = async () => {
       ](
         assetAddresses.ThreePool,
         cVaultProxy.address,
-        assetAddresses.CVX,
         assetAddresses.CRV,
+        assetAddresses.CVX,
         [assetAddresses.DAI, assetAddresses.USDC, assetAddresses.USDT],
         [
           assetAddresses.ThreePoolToken,

contracts/deploy/032_convex_rewards.js

@@ -0,0 +1,68 @@
+const { deploymentWithProposal } = require("../utils/deploy");
+
+module.exports = deploymentWithProposal(
+  { deployName: "032_convex_rewards", forceDeploy: true },
+  async ({
+    assetAddresses,
+    deployWithConfirmation,
+    ethers,
+    getTxOpts,
+    withConfirmation,
+  }) => {
+    const { deployerAddr, governorAddr } = await getNamedAccounts();
+    const sDeployer = await ethers.provider.getSigner(deployerAddr);
+
+    // Current contracts
+    const cVaultProxy = await ethers.getContract("VaultProxy");
+    const cVaultAdmin = await ethers.getContractAt(
+      "VaultAdmin",
+      cVaultProxy.address
+    );
+
+    // Deployer Actions
+    // ----------------
+
+    // 1. Deploy new implementation
+    const dConvexStrategyImpl = await deployWithConfirmation(
+      "ConvexStrategy",
+      undefined,
+      undefined,
+      true // Disable storage slot checking. We are intentionaly renaming a slot.
+    );
+    const cConvexStrategyProxy = await ethers.getContract(
+      "ConvexStrategyProxy"
+    );
+    console.log(cConvexStrategyProxy.address);
+    const cConvexStrategy = await ethers.getContractAt(
+      "ConvexStrategy",
+      cConvexStrategyProxy.address
+    );
+    console.log("ConvexStrategyProxy ", await cConvexStrategyProxy.governor());
+
+    // Governance Actions
+    // ----------------
+    return {
+      name: "Switch to new Convex implementation",
+      actions: [
+        // 1. Upgrade implementation
+        {
+          contract: cConvexStrategyProxy,
+          signature: "upgradeTo(address)",
+          args: [dConvexStrategyImpl.address],
+        },
+        // 2. Use CRV as main rewards token
+        {
+          contract: cConvexStrategy,
+          signature: "setRewardTokenAddress(address)",
+          args: [assetAddresses.CRV],
+        },
+        // 2. Use correct CVX token addresss
+        {
+          contract: cConvexStrategy,
+          signature: "setCvxRewardTokenAddress(address)",
+          args: [assetAddresses.CVX],
+        },
+      ],
+    };
+  }
+);

contracts/test/strategies/convex.js

@@ -22,7 +22,7 @@ describe("Convex Strategy", function () {
     vault,
     governor,
     crv,
-    crvMinter,
+    cvx,
     threePoolToken,
     convexStrategy,
     cvxBooster,
@@ -45,7 +45,7 @@ describe("Convex Strategy", function () {
     ousd = fixture.ousd;
     governor = fixture.governor;
     crv = fixture.crv;
-    crvMinter = fixture.crvMinter;
+    cvx = fixture.cvx;
     threePoolToken = fixture.threePoolToken;
     convexStrategy = fixture.convexStrategy;
     cvxBooster = fixture.cvxBooster;
@@ -139,26 +139,26 @@ describe("Convex Strategy", function () {
 
     it("Should collect reward tokens using collect rewards on all strategies", async () => {
       // Mint of MockCRVMinter mints a fixed 2e18
-      await crvMinter.connect(governor).mint(convexStrategy.address);
       await vault.connect(governor)["harvest()"]();
       await expect(await crv.balanceOf(vault.address)).to.be.equal(
         utils.parseUnits("2", 18)
       );
+      await expect(await cvx.balanceOf(vault.address)).to.be.equal(
+        utils.parseUnits("3", 18)
+      );
     });
 
     it("Should collect reward tokens using collect rewards on a specific strategy", async () => {
-      // Mint of MockCRVMinter mints a fixed 2e18
-      await crvMinter.connect(governor).mint(convexStrategy.address);
       await vault.connect(governor)[
         // eslint-disable-next-line
         "harvest(address)"
       ](convexStrategy.address);
+
       await expect(await crv.balanceOf(vault.address)).to.be.equal(
         utils.parseUnits("2", 18)
       );
-      await crvMinter.connect(governor).mint(convexStrategy.address);
-      await expect(await crv.balanceOf(vault.address)).to.be.equal(
-        utils.parseUnits("2", 18)
+      await expect(await cvx.balanceOf(vault.address)).to.be.equal(
+        utils.parseUnits("3", 18)
       );
     });
 
@@ -174,15 +174,6 @@ describe("Convex Strategy", function () {
       // Make sure Vault has 0 USDT balance
       await expect(vault).has.a.balanceOf("0", usdt);
 
-      // Make sure the Strategy has CRV balance
-      await crvMinter.connect(governor).mint(convexStrategy.address);
-      await expect(
-        await crv.balanceOf(await governor.getAddress())
-      ).to.be.equal("0");
-      await expect(await crv.balanceOf(convexStrategy.address)).to.be.equal(
-        utils.parseUnits("2", 18)
-      );
-
       // Give Uniswap mock some USDT so it can give it back in CRV liquidation
       await usdt
         .connect(anna)

contracts/utils/addresses.js

@@ -47,7 +47,7 @@ addresses.mainnet.ThreePool = "0xbEbc44782C7dB0a1A60Cb6fe97d0b483032FF1C7";
 addresses.mainnet.ThreePoolToken = "0x6c3f90f043a72fa612cbac8115ee7e52bde6e490";
 addresses.mainnet.ThreePoolGauge = "0xbFcF63294aD7105dEa65aA58F8AE5BE2D9d0952A";
 // CVX
-addresses.mainnet.CVX = "0x30D9410ED1D5DA1F6C8391af5338C93ab8d4035C";
+addresses.mainnet.CVX = "0x4e3fbd56cd56c3e72c1403e103b45db9da5b9d2b";
 addresses.mainnet.CRVRewardsPool = "0x689440f2ff927e1f24c72f1087e1faf471ece1c8";
 addresses.mainnet.CVXBooster = "0xF403C135812408BFbE8713b5A23a04b3D48AAE31";
 // Open Oracle