From 8fe5bee5c5acada4b564340f9967ddf7ef972ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Fri, 20 Dec 2024 15:43:46 +0100 Subject: [PATCH 01/25] feat: Implement simple harvester. --- .../contracts/harvest/OETHHarvesterSimple.sol | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 contracts/contracts/harvest/OETHHarvesterSimple.sol diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol new file mode 100644 index 0000000000..a733854742 --- /dev/null +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { Governable } from "../governance/Governable.sol"; +import { IStrategy } from "../interfaces/IStrategy.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +contract OETHHarvesterSimple is Governable { + //////////////////////////////////////////////////// + /// --- STORAGE + //////////////////////////////////////////////////// + address public operator; + address public strategy; + address public strategist; + + //////////////////////////////////////////////////// + /// --- EVENTS + //////////////////////////////////////////////////// + event Harvested(address token, uint256 amount); + event OperatorSet(address operator); + event StrategySet(address strategy); + event StrategistSet(address strategist); + + //////////////////////////////////////////////////// + /// --- MODIFIERS + //////////////////////////////////////////////////// + modifier onlyOperator() { + require( + msg.sender == operator || isGovernor(), + "Only Operator or Governor" + ); + _; + } + + //////////////////////////////////////////////////// + /// --- CONSTRUCTOR + //////////////////////////////////////////////////// + constructor( + address _governor, + address _operator, + address _strategist, + address _strategy + ) { + operator = _operator; + strategy = _strategy; + strategist = _strategist; + _setGovernor(_governor); + } + + //////////////////////////////////////////////////// + /// --- MUTATIVE FUNCTIONS + //////////////////////////////////////////////////// + function harvestAndTransfer() external onlyOperator { + require(strategy != address(0), "Strategy not set"); + require(strategist != address(0), "Strategist not set"); + + IStrategy(strategy).collectRewardTokens(); + + address[] memory rewardTokens = IStrategy(strategy) + .getRewardTokenAddresses(); + for (uint256 i = 0; i < rewardTokens.length; i++) { + uint256 balance = IERC20(rewardTokens[i]).balanceOf(address(this)); + if (balance > 0) { + IERC20(rewardTokens[i]).transfer(strategist, balance); + emit Harvested(rewardTokens[i], balance); + } + } + } + + //////////////////////////////////////////////////// + /// --- GOVERNANCE + //////////////////////////////////////////////////// + function setOperator(address _operator) external onlyGovernor { + operator = _operator; + emit OperatorSet(_operator); + } + + function setStrategy(address _strategy) external onlyGovernor { + strategy = _strategy; + emit StrategySet(_strategy); + } + + function setStrategist(address _strategist) external onlyGovernor { + strategist = _strategist; + emit StrategistSet(_strategist); + } +} From e14774ab0d52e69532a65a8e3e7d4156c32fac27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 10:26:20 +0100 Subject: [PATCH 02/25] feat: support multiple strategies. --- .../contracts/harvest/OETHHarvesterSimple.sol | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index a733854742..dc77866769 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -10,16 +10,16 @@ contract OETHHarvesterSimple is Governable { /// --- STORAGE //////////////////////////////////////////////////// address public operator; - address public strategy; address public strategist; + mapping(address => bool) public isAuthorized; //////////////////////////////////////////////////// /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); event OperatorSet(address operator); - event StrategySet(address strategy); event StrategistSet(address strategist); + event StrategyStatusSet(address strategy, bool status); //////////////////////////////////////////////////// /// --- MODIFIERS @@ -38,11 +38,10 @@ contract OETHHarvesterSimple is Governable { constructor( address _governor, address _operator, - address _strategist, - address _strategy + address _strategist ) { + require(_strategist != address(0), "Invalid strategist"); operator = _operator; - strategy = _strategy; strategist = _strategist; _setGovernor(_governor); } @@ -50,17 +49,34 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// /// --- MUTATIVE FUNCTIONS //////////////////////////////////////////////////// - function harvestAndTransfer() external onlyOperator { - require(strategy != address(0), "Strategy not set"); - require(strategist != address(0), "Strategist not set"); + function harvestAndTransfer(address _strategy) external onlyOperator { + _harvestAndTransfer(_strategy); + } + + function harvestAndTransfer(address[] calldata _strategies) + external + onlyOperator + { + for (uint256 i = 0; i < _strategies.length; i++) { + _harvestAndTransfer(_strategies[i]); + } + } + + function _harvestAndTransfer(address _strategy) internal { + // Ensure strategy is authorized + require(isAuthorized[_strategy], "Not authorized"); - IStrategy(strategy).collectRewardTokens(); + // Harvest rewards + IStrategy(_strategy).collectRewardTokens(); - address[] memory rewardTokens = IStrategy(strategy) + // Cache reward tokens + address[] memory rewardTokens = IStrategy(_strategy) .getRewardTokenAddresses(); for (uint256 i = 0; i < rewardTokens.length; i++) { + // Cache balance uint256 balance = IERC20(rewardTokens[i]).balanceOf(address(this)); if (balance > 0) { + // Transfer to strategist IERC20(rewardTokens[i]).transfer(strategist, balance); emit Harvested(rewardTokens[i], balance); } @@ -75,12 +91,16 @@ contract OETHHarvesterSimple is Governable { emit OperatorSet(_operator); } - function setStrategy(address _strategy) external onlyGovernor { - strategy = _strategy; - emit StrategySet(_strategy); + function setStrategyStatus(address _strategy, bool _status) + external + onlyGovernor + { + isAuthorized[_strategy] = _status; + emit StrategyStatusSet(_strategy, _status); } function setStrategist(address _strategist) external onlyGovernor { + require(_strategist != address(0), "Invalid strategist"); strategist = _strategist; emit StrategistSet(_strategist); } From 16513f492d6cc1d21e3d738787f0eddfaefec880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 12:30:29 +0100 Subject: [PATCH 03/25] fix: adjust error message. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index dc77866769..8873cf88ce 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -64,7 +64,7 @@ contract OETHHarvesterSimple is Governable { function _harvestAndTransfer(address _strategy) internal { // Ensure strategy is authorized - require(isAuthorized[_strategy], "Not authorized"); + require(isAuthorized[_strategy], "Strategy not authorized"); // Harvest rewards IStrategy(_strategy).collectRewardTokens(); From 128c288aa518e7e671781b6aebd168643215fd8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 12:31:58 +0100 Subject: [PATCH 04/25] feat: add test for simple harvester. --- contracts/test/_fixture.js | 5 + .../simple-harvester.mainnet.fork-test.js | 120 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 contracts/test/harvest/simple-harvester.mainnet.fork-test.js diff --git a/contracts/test/_fixture.js b/contracts/test/_fixture.js index 51a81a387d..cff8ed6b72 100644 --- a/contracts/test/_fixture.js +++ b/contracts/test/_fixture.js @@ -336,6 +336,10 @@ const defaultFixture = deployments.createFixture(async () => { morphoGauntletPrimeUSDTStrategyProxy.address ); + const simpleOETHHarvester = isFork + ? await ethers.getContract("OETHHarvesterSimple") + : undefined; + let usdt, dai, tusd, @@ -805,6 +809,7 @@ const defaultFixture = deployments.createFixture(async () => { morphoGauntletPrimeUSDCVault, morphoGauntletPrimeUSDTStrategy, morphoGauntletPrimeUSDTVault, + simpleOETHHarvester, // Flux strategy fluxStrategy, diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js new file mode 100644 index 0000000000..100510a554 --- /dev/null +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -0,0 +1,120 @@ +const { expect } = require("chai"); + +const addresses = require("../../utils/addresses"); +const { isCI } = require("../helpers"); + +const { loadDefaultFixture } = require("../_fixture"); + +describe("ForkTest: CurvePoolBooster", function () { + this.timeout(0); + + // Retry up to 3 times on CI + this.retries(isCI ? 3 : 0); + + let fixture; + beforeEach(async () => { + fixture = await loadDefaultFixture(); + }); + + it("Should have correct parameters", async () => { + const { simpleOETHHarvester } = fixture; + const { deployerAddr } = await getNamedAccounts(); + + expect(await simpleOETHHarvester.governor()).to.be.equal( + addresses.mainnet.Timelock + ); + expect(await simpleOETHHarvester.operator()).to.be.equal(deployerAddr); + expect(await simpleOETHHarvester.strategist()).to.be.equal(deployerAddr); + }); + + it("Should Set Strategy status", async () => { + const { simpleOETHHarvester } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + expect( + await simpleOETHHarvester.isAuthorized( + addresses.mainnet.ConvexOETHAMOStrategy + ) + ).to.be.equal(false); + await simpleOETHHarvester + .connect(timelock) + .setStrategyStatus(addresses.mainnet.ConvexOETHAMOStrategy, true); + expect( + await simpleOETHHarvester.isAuthorized( + addresses.mainnet.ConvexOETHAMOStrategy + ) + ).to.be.equal(true); + }); + + it("Should Set operator", async () => { + const { simpleOETHHarvester, josh } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + expect(await simpleOETHHarvester.operator()).not.to.equal(josh.address); + await simpleOETHHarvester.connect(timelock).setOperator(josh.address); + expect(await simpleOETHHarvester.operator()).to.equal(josh.address); + }); + + it("Should Set strategist", async () => { + const { simpleOETHHarvester, josh } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + expect(await simpleOETHHarvester.strategist()).not.to.equal(josh.address); + await simpleOETHHarvester.connect(timelock).setStrategist(josh.address); + expect(await simpleOETHHarvester.strategist()).to.equal(josh.address); + }); + + it("Should Harvest and transfer rewards", async () => { + const { simpleOETHHarvester, convexEthMetaStrategy, crv } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + const strategist = await simpleOETHHarvester.strategist(); + + const balanceBeforeCRV = await crv.balanceOf(strategist); + await convexEthMetaStrategy + .connect(timelock) + .setHarvesterAddress(simpleOETHHarvester.address); + await simpleOETHHarvester + .connect(timelock) + .setStrategyStatus(convexEthMetaStrategy.address, true); + await simpleOETHHarvester + .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); + + const balanceAfterCRV = await crv.balanceOf(strategist); + expect(balanceAfterCRV).to.be.gt(balanceBeforeCRV); + }); + + it("Should revert if strategy is not authorized", async () => { + const { simpleOETHHarvester, convexEthMetaStrategy } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + await expect( + simpleOETHHarvester + .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) + ).to.be.revertedWith("Strategy not authorized"); + }); + + it("Should revert if caller is not operator", async () => { + const { simpleOETHHarvester, convexEthMetaStrategy, josh } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + await simpleOETHHarvester + .connect(timelock) + .setStrategyStatus(convexEthMetaStrategy.address, true); + await expect( + simpleOETHHarvester + .connect(josh)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) + ).to.be.revertedWith("Only Operator or Governor"); + }); +}); From cb7814595aa69ac8d3d3927879168b0fa3ee41c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 12:50:45 +0100 Subject: [PATCH 05/25] feat: WIP add deployment script for simple harvester. --- .../deploy/mainnet/114_simple_harvester.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 contracts/deploy/mainnet/114_simple_harvester.js diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js new file mode 100644 index 0000000000..533fc3f90f --- /dev/null +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -0,0 +1,39 @@ +const addresses = require("../../utils/addresses"); +const { deploymentWithGovernanceProposal } = require("../../utils/deploy"); + +module.exports = deploymentWithGovernanceProposal( + { + deployName: "114_simple_harvester", + forceDeploy: false, + // forceSkip: true, + reduceQueueTime: true, + deployerIsProposer: false, + // proposalId: "", + }, + async ({ deployWithConfirmation }) => { + const { deployerAddr } = await getNamedAccounts(); + + // 1. Deploy contract + const dOETHHarvesterSimple = await deployWithConfirmation( + "OETHHarvesterSimple", + [addresses.mainnet.Timelock, deployerAddr, deployerAddr] + ); + const cOETHHarvesterSimple = await ethers.getContract("OETHHarvesterSimple", dOETHHarvesterSimple.address); + + // Get AMO contract + const cAMO = await ethers.getContract("ConvexEthMetaStrategy", addresses.mainnet.ConvexOETHAMOStrategy); + + // Governance Actions + // ---------------- + return { + name: "Change harvester in OETH AMO", + actions: [ + { + contract: cAMO, + signature: "setHarvesterAddress(address)", + args: [cOETHHarvesterSimple.address], + }, + ], + }; + } +); From 0208ddd630a40b313a736c17fff9d9894022f804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 12:59:09 +0100 Subject: [PATCH 06/25] fix: adjust deployment for simple harvester. --- contracts/deploy/mainnet/114_simple_harvester.js | 4 ++-- contracts/test/harvest/simple-harvester.mainnet.fork-test.js | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js index 533fc3f90f..6aede51017 100644 --- a/contracts/deploy/mainnet/114_simple_harvester.js +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -18,10 +18,10 @@ module.exports = deploymentWithGovernanceProposal( "OETHHarvesterSimple", [addresses.mainnet.Timelock, deployerAddr, deployerAddr] ); - const cOETHHarvesterSimple = await ethers.getContract("OETHHarvesterSimple", dOETHHarvesterSimple.address); + const cOETHHarvesterSimple = await ethers.getContractAt("OETHHarvesterSimple", dOETHHarvesterSimple.address); // Get AMO contract - const cAMO = await ethers.getContract("ConvexEthMetaStrategy", addresses.mainnet.ConvexOETHAMOStrategy); + const cAMO = await ethers.getContractAt("ConvexEthMetaStrategy", addresses.mainnet.ConvexOETHAMOStrategy); // Governance Actions // ---------------- diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index 100510a554..9f5df15589 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -78,9 +78,6 @@ describe("ForkTest: CurvePoolBooster", function () { const strategist = await simpleOETHHarvester.strategist(); const balanceBeforeCRV = await crv.balanceOf(strategist); - await convexEthMetaStrategy - .connect(timelock) - .setHarvesterAddress(simpleOETHHarvester.address); await simpleOETHHarvester .connect(timelock) .setStrategyStatus(convexEthMetaStrategy.address, true); From fe7596c4269281aca761f2f5517a6436d8505b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 23 Dec 2024 14:16:19 +0100 Subject: [PATCH 07/25] doc: add comment on deployment file. --- contracts/deploy/mainnet/114_simple_harvester.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js index 6aede51017..73ba1e967c 100644 --- a/contracts/deploy/mainnet/114_simple_harvester.js +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -16,7 +16,7 @@ module.exports = deploymentWithGovernanceProposal( // 1. Deploy contract const dOETHHarvesterSimple = await deployWithConfirmation( "OETHHarvesterSimple", - [addresses.mainnet.Timelock, deployerAddr, deployerAddr] + [addresses.mainnet.Timelock, deployerAddr, deployerAddr] // Need to adjust governor, strategist and operator ); const cOETHHarvesterSimple = await ethers.getContractAt("OETHHarvesterSimple", dOETHHarvesterSimple.address); From 814281121a9a27699feb7e9882fe024411eb59b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 10:29:51 +0100 Subject: [PATCH 08/25] prettier deployment file. --- contracts/deploy/mainnet/114_simple_harvester.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js index 73ba1e967c..ef218cd905 100644 --- a/contracts/deploy/mainnet/114_simple_harvester.js +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -16,12 +16,18 @@ module.exports = deploymentWithGovernanceProposal( // 1. Deploy contract const dOETHHarvesterSimple = await deployWithConfirmation( "OETHHarvesterSimple", - [addresses.mainnet.Timelock, deployerAddr, deployerAddr] // Need to adjust governor, strategist and operator + [addresses.mainnet.Timelock, deployerAddr, deployerAddr] // Need to adjust governor, strategist and operator + ); + const cOETHHarvesterSimple = await ethers.getContractAt( + "OETHHarvesterSimple", + dOETHHarvesterSimple.address ); - const cOETHHarvesterSimple = await ethers.getContractAt("OETHHarvesterSimple", dOETHHarvesterSimple.address); // Get AMO contract - const cAMO = await ethers.getContractAt("ConvexEthMetaStrategy", addresses.mainnet.ConvexOETHAMOStrategy); + const cAMO = await ethers.getContractAt( + "ConvexEthMetaStrategy", + addresses.mainnet.ConvexOETHAMOStrategy + ); // Governance Actions // ---------------- From 0f22981681b4b2a78258509aba44a87899d59622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 10:35:49 +0100 Subject: [PATCH 09/25] add prettier-ignore. --- contracts/test/harvest/simple-harvester.mainnet.fork-test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index 9f5df15589..2b9048d312 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -81,8 +81,9 @@ describe("ForkTest: CurvePoolBooster", function () { await simpleOETHHarvester .connect(timelock) .setStrategyStatus(convexEthMetaStrategy.address, true); + // prettier-ignore await simpleOETHHarvester - .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); + .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); const balanceAfterCRV = await crv.balanceOf(strategist); expect(balanceAfterCRV).to.be.gt(balanceBeforeCRV); @@ -95,6 +96,7 @@ describe("ForkTest: CurvePoolBooster", function () { ); await expect( + // prettier-ignore simpleOETHHarvester .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) ).to.be.revertedWith("Strategy not authorized"); @@ -110,6 +112,7 @@ describe("ForkTest: CurvePoolBooster", function () { .connect(timelock) .setStrategyStatus(convexEthMetaStrategy.address, true); await expect( + // prettier-ignore simpleOETHHarvester .connect(josh)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) ).to.be.revertedWith("Only Operator or Governor"); From 500b6d351ab3eacc6138745b2473cc12187b3ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 14:01:49 +0100 Subject: [PATCH 10/25] feat: remove operator role. --- .../contracts/harvest/OETHHarvesterSimple.sol | 32 ++----------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 8873cf88ce..c583b8e991 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -9,7 +9,6 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// /// --- STORAGE //////////////////////////////////////////////////// - address public operator; address public strategist; mapping(address => bool) public isAuthorized; @@ -17,31 +16,14 @@ contract OETHHarvesterSimple is Governable { /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); - event OperatorSet(address operator); event StrategistSet(address strategist); event StrategyStatusSet(address strategy, bool status); - //////////////////////////////////////////////////// - /// --- MODIFIERS - //////////////////////////////////////////////////// - modifier onlyOperator() { - require( - msg.sender == operator || isGovernor(), - "Only Operator or Governor" - ); - _; - } - //////////////////////////////////////////////////// /// --- CONSTRUCTOR //////////////////////////////////////////////////// - constructor( - address _governor, - address _operator, - address _strategist - ) { + constructor(address _governor, address _strategist) { require(_strategist != address(0), "Invalid strategist"); - operator = _operator; strategist = _strategist; _setGovernor(_governor); } @@ -49,14 +31,11 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// /// --- MUTATIVE FUNCTIONS //////////////////////////////////////////////////// - function harvestAndTransfer(address _strategy) external onlyOperator { + function harvestAndTransfer(address _strategy) external { _harvestAndTransfer(_strategy); } - function harvestAndTransfer(address[] calldata _strategies) - external - onlyOperator - { + function harvestAndTransfer(address[] calldata _strategies) external { for (uint256 i = 0; i < _strategies.length; i++) { _harvestAndTransfer(_strategies[i]); } @@ -86,11 +65,6 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// /// --- GOVERNANCE //////////////////////////////////////////////////// - function setOperator(address _operator) external onlyGovernor { - operator = _operator; - emit OperatorSet(_operator); - } - function setStrategyStatus(address _strategy, bool _status) external onlyGovernor From 23a885f542f3dcc22d9a661d269ff382aa5cf19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 14:06:15 +0100 Subject: [PATCH 11/25] fix: `setStrategyStatus` can be called by strategist. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index c583b8e991..70c64fed6c 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -19,6 +19,14 @@ contract OETHHarvesterSimple is Governable { event StrategistSet(address strategist); event StrategyStatusSet(address strategy, bool status); + //////////////////////////////////////////////////// + /// --- MODIFIERS + //////////////////////////////////////////////////// + modifier onlyStrategist() { + require(msg.sender == strategist || isGovernor(), "Not strategist"); + _; + } + //////////////////////////////////////////////////// /// --- CONSTRUCTOR //////////////////////////////////////////////////// @@ -67,7 +75,7 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// function setStrategyStatus(address _strategy, bool _status) external - onlyGovernor + onlyStrategist { isAuthorized[_strategy] = _status; emit StrategyStatusSet(_strategy, _status); From 5446bde0221c8fca5f6792203e9a1602fa0e7293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 14:11:59 +0100 Subject: [PATCH 12/25] fix: change wording for supporting strategies. --- .../contracts/harvest/OETHHarvesterSimple.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 70c64fed6c..a74adb5b7c 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -10,14 +10,14 @@ contract OETHHarvesterSimple is Governable { /// --- STORAGE //////////////////////////////////////////////////// address public strategist; - mapping(address => bool) public isAuthorized; + mapping(address => bool) public supportedStrategies; //////////////////////////////////////////////////// /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); event StrategistSet(address strategist); - event StrategyStatusSet(address strategy, bool status); + event SupportedStrategyUpdate(address strategy, bool status); //////////////////////////////////////////////////// /// --- MODIFIERS @@ -50,8 +50,8 @@ contract OETHHarvesterSimple is Governable { } function _harvestAndTransfer(address _strategy) internal { - // Ensure strategy is authorized - require(isAuthorized[_strategy], "Strategy not authorized"); + // Ensure strategy is supported + require(supportedStrategies[_strategy], "Strategy not supported"); // Harvest rewards IStrategy(_strategy).collectRewardTokens(); @@ -73,12 +73,12 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// /// --- GOVERNANCE //////////////////////////////////////////////////// - function setStrategyStatus(address _strategy, bool _status) + function setSupportedStrategy(address _strategy, bool _isSupported) external onlyStrategist { - isAuthorized[_strategy] = _status; - emit StrategyStatusSet(_strategy, _status); + supportedStrategies[_strategy] = _isSupported; + emit SupportedStrategyUpdate(_strategy, _isSupported); } function setStrategist(address _strategist) external onlyGovernor { From b79578b4f0b02d6fcfb99beede7c18b761086c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 14:16:49 +0100 Subject: [PATCH 13/25] feat: prevent supporting address 0. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index a74adb5b7c..07715d7de7 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -77,6 +77,8 @@ contract OETHHarvesterSimple is Governable { external onlyStrategist { + require(_strategy != address(0), "Invalid strategy"); + supportedStrategies[_strategy] = _isSupported; emit SupportedStrategyUpdate(_strategy, _isSupported); } From 00515f3892cdea448e201052e5e07d889c9bad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 15:17:52 +0100 Subject: [PATCH 14/25] fix: change event for changing strategist. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 07715d7de7..68feaa992f 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -16,7 +16,7 @@ contract OETHHarvesterSimple is Governable { /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); - event StrategistSet(address strategist); + event StrategistChanged(address strategist); event SupportedStrategyUpdate(address strategy, bool status); //////////////////////////////////////////////////// @@ -86,6 +86,6 @@ contract OETHHarvesterSimple is Governable { function setStrategist(address _strategist) external onlyGovernor { require(_strategist != address(0), "Invalid strategist"); strategist = _strategist; - emit StrategistSet(_strategist); + emit StrategistChanged(_strategist); } } From 8427f2685cde59034ede567de3bcb88c00d1f171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 15:22:51 +0100 Subject: [PATCH 15/25] prettier. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 1 - contracts/deploy/mainnet/114_simple_harvester.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 68feaa992f..2029ad77ab 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -78,7 +78,6 @@ contract OETHHarvesterSimple is Governable { onlyStrategist { require(_strategy != address(0), "Invalid strategy"); - supportedStrategies[_strategy] = _isSupported; emit SupportedStrategyUpdate(_strategy, _isSupported); } diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js index ef218cd905..d1ea4b418e 100644 --- a/contracts/deploy/mainnet/114_simple_harvester.js +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -16,7 +16,7 @@ module.exports = deploymentWithGovernanceProposal( // 1. Deploy contract const dOETHHarvesterSimple = await deployWithConfirmation( "OETHHarvesterSimple", - [addresses.mainnet.Timelock, deployerAddr, deployerAddr] // Need to adjust governor, strategist and operator + [addresses.mainnet.Timelock, deployerAddr] // Need to adjust governor and strategist ); const cOETHHarvesterSimple = await ethers.getContractAt( "OETHHarvesterSimple", From 23a5a50d73b6d693184109af77847e8c21c7e7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 30 Dec 2024 15:22:59 +0100 Subject: [PATCH 16/25] fix tests. --- .../simple-harvester.mainnet.fork-test.js | 37 ++++++------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index 2b9048d312..8bcfc3426c 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -23,42 +23,30 @@ describe("ForkTest: CurvePoolBooster", function () { expect(await simpleOETHHarvester.governor()).to.be.equal( addresses.mainnet.Timelock ); - expect(await simpleOETHHarvester.operator()).to.be.equal(deployerAddr); expect(await simpleOETHHarvester.strategist()).to.be.equal(deployerAddr); }); - it("Should Set Strategy status", async () => { + it("Should support Strategy", async () => { const { simpleOETHHarvester } = fixture; const timelock = await ethers.provider.getSigner( addresses.mainnet.Timelock ); expect( - await simpleOETHHarvester.isAuthorized( + await simpleOETHHarvester.supportedStrategies( addresses.mainnet.ConvexOETHAMOStrategy ) ).to.be.equal(false); await simpleOETHHarvester .connect(timelock) - .setStrategyStatus(addresses.mainnet.ConvexOETHAMOStrategy, true); + .setSupportedStrategy(addresses.mainnet.ConvexOETHAMOStrategy, true); expect( - await simpleOETHHarvester.isAuthorized( + await simpleOETHHarvester.supportedStrategies( addresses.mainnet.ConvexOETHAMOStrategy ) ).to.be.equal(true); }); - it("Should Set operator", async () => { - const { simpleOETHHarvester, josh } = fixture; - const timelock = await ethers.provider.getSigner( - addresses.mainnet.Timelock - ); - - expect(await simpleOETHHarvester.operator()).not.to.equal(josh.address); - await simpleOETHHarvester.connect(timelock).setOperator(josh.address); - expect(await simpleOETHHarvester.operator()).to.equal(josh.address); - }); - it("Should Set strategist", async () => { const { simpleOETHHarvester, josh } = fixture; const timelock = await ethers.provider.getSigner( @@ -80,10 +68,10 @@ describe("ForkTest: CurvePoolBooster", function () { const balanceBeforeCRV = await crv.balanceOf(strategist); await simpleOETHHarvester .connect(timelock) - .setStrategyStatus(convexEthMetaStrategy.address, true); + .setSupportedStrategy(convexEthMetaStrategy.address, true); // prettier-ignore await simpleOETHHarvester - .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); + .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); const balanceAfterCRV = await crv.balanceOf(strategist); expect(balanceAfterCRV).to.be.gt(balanceBeforeCRV); @@ -99,22 +87,19 @@ describe("ForkTest: CurvePoolBooster", function () { // prettier-ignore simpleOETHHarvester .connect(timelock)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) - ).to.be.revertedWith("Strategy not authorized"); + ).to.be.revertedWith("Strategy not supported"); }); - it("Should revert if caller is not operator", async () => { - const { simpleOETHHarvester, convexEthMetaStrategy, josh } = fixture; + it("Should revert if strategy is address 0", async () => { + const { simpleOETHHarvester } = fixture; const timelock = await ethers.provider.getSigner( addresses.mainnet.Timelock ); - await simpleOETHHarvester - .connect(timelock) - .setStrategyStatus(convexEthMetaStrategy.address, true); await expect( // prettier-ignore simpleOETHHarvester - .connect(josh)["harvestAndTransfer(address)"](convexEthMetaStrategy.address) - ).to.be.revertedWith("Only Operator or Governor"); + .connect(timelock).setSupportedStrategy(addresses.zero, true) + ).to.be.revertedWith("Invalid strategy"); }); }); From 9e968fdd6e9173e3f22a02a9d89ccfccfb53f12f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Tue, 31 Dec 2024 08:46:00 +0100 Subject: [PATCH 17/25] fix: change event name. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 2029ad77ab..0a27bae94b 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -16,8 +16,8 @@ contract OETHHarvesterSimple is Governable { /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); - event StrategistChanged(address strategist); - event SupportedStrategyUpdate(address strategy, bool status); + event StrategistUpdated(address strategist); + event SupportedStrategyUpdated(address strategy, bool status); //////////////////////////////////////////////////// /// --- MODIFIERS @@ -79,12 +79,12 @@ contract OETHHarvesterSimple is Governable { { require(_strategy != address(0), "Invalid strategy"); supportedStrategies[_strategy] = _isSupported; - emit SupportedStrategyUpdate(_strategy, _isSupported); + emit SupportedStrategyUpdated(_strategy, _isSupported); } function setStrategist(address _strategist) external onlyGovernor { require(_strategist != address(0), "Invalid strategist"); strategist = _strategist; - emit StrategistChanged(_strategist); + emit StrategistUpdated(_strategist); } } From 00766a9838c67a6742a7470e93fe1dd0178db50b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Thu, 2 Jan 2025 19:08:23 +0100 Subject: [PATCH 18/25] feat: set strategist in internal method. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 0a27bae94b..0a1bac82c8 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -31,8 +31,7 @@ contract OETHHarvesterSimple is Governable { /// --- CONSTRUCTOR //////////////////////////////////////////////////// constructor(address _governor, address _strategist) { - require(_strategist != address(0), "Invalid strategist"); - strategist = _strategist; + _setStrategist(_strategist); _setGovernor(_governor); } @@ -82,9 +81,13 @@ contract OETHHarvesterSimple is Governable { emit SupportedStrategyUpdated(_strategy, _isSupported); } - function setStrategist(address _strategist) external onlyGovernor { + function _setStrategist(address _strategist) internal { require(_strategist != address(0), "Invalid strategist"); strategist = _strategist; emit StrategistUpdated(_strategist); } + + function setStrategist(address _strategist) external onlyGovernor { + _setStrategist(_strategist); + } } From 534ce82b6b1adc56e019447d2ab1edd9254838b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Thu, 2 Jan 2025 20:29:14 +0100 Subject: [PATCH 19/25] fix: use `changeGov` instead of `setGov` in constructor. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index 0a1bac82c8..f173cba9cd 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -32,7 +32,7 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// constructor(address _governor, address _strategist) { _setStrategist(_strategist); - _setGovernor(_governor); + _changeGovernor(_governor); } //////////////////////////////////////////////////// From 639e7aeb82f62add2003cf57a3fbe437cf362de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Sat, 4 Jan 2025 22:26:41 +0100 Subject: [PATCH 20/25] feat: use `Strategizable` instead of `Governable`. --- .../contracts/harvest/OETHHarvesterSimple.sol | 30 ++++--------------- .../simple-harvester.mainnet.fork-test.js | 14 +++++---- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index f173cba9cd..b953720bc3 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -1,37 +1,27 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import { Governable } from "../governance/Governable.sol"; +import { Strategizable } from "../governance/Strategizable.sol"; import { IStrategy } from "../interfaces/IStrategy.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -contract OETHHarvesterSimple is Governable { +contract OETHHarvesterSimple is Strategizable { //////////////////////////////////////////////////// /// --- STORAGE //////////////////////////////////////////////////// - address public strategist; mapping(address => bool) public supportedStrategies; //////////////////////////////////////////////////// /// --- EVENTS //////////////////////////////////////////////////// event Harvested(address token, uint256 amount); - event StrategistUpdated(address strategist); event SupportedStrategyUpdated(address strategy, bool status); - //////////////////////////////////////////////////// - /// --- MODIFIERS - //////////////////////////////////////////////////// - modifier onlyStrategist() { - require(msg.sender == strategist || isGovernor(), "Not strategist"); - _; - } - //////////////////////////////////////////////////// /// --- CONSTRUCTOR //////////////////////////////////////////////////// constructor(address _governor, address _strategist) { - _setStrategist(_strategist); + _setStrategistAddr(_strategist); _changeGovernor(_governor); } @@ -63,7 +53,7 @@ contract OETHHarvesterSimple is Governable { uint256 balance = IERC20(rewardTokens[i]).balanceOf(address(this)); if (balance > 0) { // Transfer to strategist - IERC20(rewardTokens[i]).transfer(strategist, balance); + IERC20(rewardTokens[i]).transfer(strategistAddr, balance); emit Harvested(rewardTokens[i], balance); } } @@ -74,20 +64,10 @@ contract OETHHarvesterSimple is Governable { //////////////////////////////////////////////////// function setSupportedStrategy(address _strategy, bool _isSupported) external - onlyStrategist + onlyGovernorOrStrategist { require(_strategy != address(0), "Invalid strategy"); supportedStrategies[_strategy] = _isSupported; emit SupportedStrategyUpdated(_strategy, _isSupported); } - - function _setStrategist(address _strategist) internal { - require(_strategist != address(0), "Invalid strategist"); - strategist = _strategist; - emit StrategistUpdated(_strategist); - } - - function setStrategist(address _strategist) external onlyGovernor { - _setStrategist(_strategist); - } } diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index 8bcfc3426c..daadef2e19 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -23,7 +23,9 @@ describe("ForkTest: CurvePoolBooster", function () { expect(await simpleOETHHarvester.governor()).to.be.equal( addresses.mainnet.Timelock ); - expect(await simpleOETHHarvester.strategist()).to.be.equal(deployerAddr); + expect(await simpleOETHHarvester.strategistAddr()).to.be.equal( + deployerAddr + ); }); it("Should support Strategy", async () => { @@ -53,9 +55,11 @@ describe("ForkTest: CurvePoolBooster", function () { addresses.mainnet.Timelock ); - expect(await simpleOETHHarvester.strategist()).not.to.equal(josh.address); - await simpleOETHHarvester.connect(timelock).setStrategist(josh.address); - expect(await simpleOETHHarvester.strategist()).to.equal(josh.address); + expect(await simpleOETHHarvester.strategistAddr()).not.to.equal( + josh.address + ); + await simpleOETHHarvester.connect(timelock).setStrategistAddr(josh.address); + expect(await simpleOETHHarvester.strategistAddr()).to.equal(josh.address); }); it("Should Harvest and transfer rewards", async () => { @@ -63,7 +67,7 @@ describe("ForkTest: CurvePoolBooster", function () { const timelock = await ethers.provider.getSigner( addresses.mainnet.Timelock ); - const strategist = await simpleOETHHarvester.strategist(); + const strategist = await simpleOETHHarvester.strategistAddr(); const balanceBeforeCRV = await crv.balanceOf(strategist); await simpleOETHHarvester From afaf9e580e7f25fbfdb14278f94c47228578d449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Sat, 4 Jan 2025 22:31:57 +0100 Subject: [PATCH 21/25] feat: add rescue token. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index b953720bc3..b3ac5b2257 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -4,8 +4,11 @@ pragma solidity ^0.8.0; import { Strategizable } from "../governance/Strategizable.sol"; import { IStrategy } from "../interfaces/IStrategy.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; contract OETHHarvesterSimple is Strategizable { + using SafeERC20 for IERC20; + //////////////////////////////////////////////////// /// --- STORAGE //////////////////////////////////////////////////// @@ -53,7 +56,7 @@ contract OETHHarvesterSimple is Strategizable { uint256 balance = IERC20(rewardTokens[i]).balanceOf(address(this)); if (balance > 0) { // Transfer to strategist - IERC20(rewardTokens[i]).transfer(strategistAddr, balance); + IERC20(rewardTokens[i]).safeTransfer(strategistAddr, balance); emit Harvested(rewardTokens[i], balance); } } @@ -70,4 +73,11 @@ contract OETHHarvesterSimple is Strategizable { supportedStrategies[_strategy] = _isSupported; emit SupportedStrategyUpdated(_strategy, _isSupported); } + + function transferToken(address _asset, uint256 _amount) + external + onlyGovernorOrStrategist + { + IERC20(_asset).safeTransfer(strategistAddr, _amount); + } } From 9eefde240b257df22ab56b3c74d24efcc2ee8fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Sat, 4 Jan 2025 22:58:47 +0100 Subject: [PATCH 22/25] test: add more tests. --- .../simple-harvester.mainnet.fork-test.js | 94 ++++++++++++++++++- 1 file changed, 92 insertions(+), 2 deletions(-) diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index daadef2e19..9deef6eb4c 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -2,6 +2,7 @@ const { expect } = require("chai"); const addresses = require("../../utils/addresses"); const { isCI } = require("../helpers"); +const { setERC20TokenBalance } = require("../_fund"); const { loadDefaultFixture } = require("../_fixture"); @@ -28,7 +29,7 @@ describe("ForkTest: CurvePoolBooster", function () { ); }); - it("Should support Strategy", async () => { + it("Should support Strategy as governor", async () => { const { simpleOETHHarvester } = fixture; const timelock = await ethers.provider.getSigner( addresses.mainnet.Timelock @@ -49,6 +50,49 @@ describe("ForkTest: CurvePoolBooster", function () { ).to.be.equal(true); }); + it("Should support Strategy as strategist", async () => { + const { simpleOETHHarvester } = fixture; + const strategist = await ethers.provider.getSigner( + await simpleOETHHarvester.strategistAddr() + ); + + expect( + await simpleOETHHarvester.supportedStrategies( + addresses.mainnet.ConvexOETHAMOStrategy + ) + ).to.be.equal(false); + await simpleOETHHarvester + .connect(strategist) + .setSupportedStrategy(addresses.mainnet.ConvexOETHAMOStrategy, true); + expect( + await simpleOETHHarvester.supportedStrategies( + addresses.mainnet.ConvexOETHAMOStrategy + ) + ).to.be.equal(true); + }); + + it("Should revert if support strategy is not governor or strategist", async () => { + const { simpleOETHHarvester, josh } = fixture; + + await expect( + // prettier-ignore + simpleOETHHarvester + .connect(josh) + .setSupportedStrategy(addresses.mainnet.ConvexOETHAMOStrategy, true) + ).to.be.revertedWith("Caller is not the Strategist or Governor"); + }); + + it("Should revert if when setting strategist is not governor", async () => { + const { simpleOETHHarvester, josh } = fixture; + + await expect( + // prettier-ignore + simpleOETHHarvester + .connect(josh) + .setStrategistAddr(josh.address) + ).to.be.revertedWith("Caller is not the Governor"); + }); + it("Should Set strategist", async () => { const { simpleOETHHarvester, josh } = fixture; const timelock = await ethers.provider.getSigner( @@ -62,7 +106,24 @@ describe("ForkTest: CurvePoolBooster", function () { expect(await simpleOETHHarvester.strategistAddr()).to.equal(josh.address); }); - it("Should Harvest and transfer rewards", async () => { + it("Should Harvest and transfer rewards as strategist", async () => { + const { simpleOETHHarvester, convexEthMetaStrategy, crv } = fixture; + const strategistAddress = await simpleOETHHarvester.strategistAddr(); + const strategist = await ethers.provider.getSigner(strategistAddress); + + const balanceBeforeCRV = await crv.balanceOf(strategistAddress); + await simpleOETHHarvester + .connect(strategist) + .setSupportedStrategy(convexEthMetaStrategy.address, true); + // prettier-ignore + await simpleOETHHarvester + .connect(strategist)["harvestAndTransfer(address)"](convexEthMetaStrategy.address); + + const balanceAfterCRV = await crv.balanceOf(strategistAddress); + expect(balanceAfterCRV).to.be.gt(balanceBeforeCRV); + }); + + it("Should Harvest and transfer rewards as governor", async () => { const { simpleOETHHarvester, convexEthMetaStrategy, crv } = fixture; const timelock = await ethers.provider.getSigner( addresses.mainnet.Timelock @@ -106,4 +167,33 @@ describe("ForkTest: CurvePoolBooster", function () { .connect(timelock).setSupportedStrategy(addresses.zero, true) ).to.be.revertedWith("Invalid strategy"); }); + + it("Should test to rescue tokens as governor", async () => { + const { simpleOETHHarvester, crv } = fixture; + const timelock = await ethers.provider.getSigner( + addresses.mainnet.Timelock + ); + + await setERC20TokenBalance(simpleOETHHarvester.address, crv, "1000"); + const balanceBeforeCRV = await crv.balanceOf(simpleOETHHarvester.address); + await simpleOETHHarvester + .connect(timelock) + .transferToken(crv.address, "1000"); + const balanceAfterCRV = await crv.balanceOf(simpleOETHHarvester.address); + expect(balanceAfterCRV).to.be.lt(balanceBeforeCRV); + }); + + it("Should test to rescue tokens as strategist", async () => { + const { simpleOETHHarvester, crv } = fixture; + const strategistAddress = await simpleOETHHarvester.strategistAddr(); + const strategist = await ethers.provider.getSigner(strategistAddress); + + await setERC20TokenBalance(simpleOETHHarvester.address, crv, "1000"); + const balanceBeforeCRV = await crv.balanceOf(simpleOETHHarvester.address); + await simpleOETHHarvester + .connect(strategist) + .transferToken(crv.address, "1000"); + const balanceAfterCRV = await crv.balanceOf(simpleOETHHarvester.address); + expect(balanceAfterCRV).to.be.lt(balanceBeforeCRV); + }); }); From 0e8ab277eb269cbbe30175dc2cdb0084b1d5dc83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Sun, 5 Jan 2025 11:32:47 +0100 Subject: [PATCH 23/25] feat: add strategy address in event. --- contracts/contracts/harvest/OETHHarvesterSimple.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/harvest/OETHHarvesterSimple.sol b/contracts/contracts/harvest/OETHHarvesterSimple.sol index b3ac5b2257..23c6ecd983 100644 --- a/contracts/contracts/harvest/OETHHarvesterSimple.sol +++ b/contracts/contracts/harvest/OETHHarvesterSimple.sol @@ -17,7 +17,7 @@ contract OETHHarvesterSimple is Strategizable { //////////////////////////////////////////////////// /// --- EVENTS //////////////////////////////////////////////////// - event Harvested(address token, uint256 amount); + event Harvested(address strategy, address token, uint256 amount); event SupportedStrategyUpdated(address strategy, bool status); //////////////////////////////////////////////////// @@ -57,7 +57,7 @@ contract OETHHarvesterSimple is Strategizable { if (balance > 0) { // Transfer to strategist IERC20(rewardTokens[i]).safeTransfer(strategistAddr, balance); - emit Harvested(rewardTokens[i], balance); + emit Harvested(_strategy, rewardTokens[i], balance); } } } From 543378d98a5c5a1812b19044608d1523694110ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 6 Jan 2025 13:26:41 +0100 Subject: [PATCH 24/25] fix: use correct address for strategist at deployment. --- contracts/deploy/mainnet/114_simple_harvester.js | 6 ++++-- .../test/harvest/simple-harvester.mainnet.fork-test.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/contracts/deploy/mainnet/114_simple_harvester.js b/contracts/deploy/mainnet/114_simple_harvester.js index d1ea4b418e..f4d40821de 100644 --- a/contracts/deploy/mainnet/114_simple_harvester.js +++ b/contracts/deploy/mainnet/114_simple_harvester.js @@ -11,13 +11,15 @@ module.exports = deploymentWithGovernanceProposal( // proposalId: "", }, async ({ deployWithConfirmation }) => { - const { deployerAddr } = await getNamedAccounts(); + const { strategistAddr } = await getNamedAccounts(); // 1. Deploy contract const dOETHHarvesterSimple = await deployWithConfirmation( "OETHHarvesterSimple", - [addresses.mainnet.Timelock, deployerAddr] // Need to adjust governor and strategist + [addresses.mainnet.Timelock, strategistAddr] ); + + console.log("strategistAddr: ", strategistAddr); const cOETHHarvesterSimple = await ethers.getContractAt( "OETHHarvesterSimple", dOETHHarvesterSimple.address diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index 9deef6eb4c..f4be38cb42 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -19,13 +19,15 @@ describe("ForkTest: CurvePoolBooster", function () { it("Should have correct parameters", async () => { const { simpleOETHHarvester } = fixture; - const { deployerAddr } = await getNamedAccounts(); + const { strategistAddr } = await getNamedAccounts(); expect(await simpleOETHHarvester.governor()).to.be.equal( addresses.mainnet.Timelock ); + + console.log("strategistAddr: ", strategistAddr); expect(await simpleOETHHarvester.strategistAddr()).to.be.equal( - deployerAddr + strategistAddr ); }); From f0559ad2376d971c2d293a162379699b3c2eced1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment?= Date: Mon, 6 Jan 2025 14:52:39 +0100 Subject: [PATCH 25/25] fix: remove console.log --- contracts/test/harvest/simple-harvester.mainnet.fork-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js index f4be38cb42..d6b52e0732 100644 --- a/contracts/test/harvest/simple-harvester.mainnet.fork-test.js +++ b/contracts/test/harvest/simple-harvester.mainnet.fork-test.js @@ -25,7 +25,6 @@ describe("ForkTest: CurvePoolBooster", function () { addresses.mainnet.Timelock ); - console.log("strategistAddr: ", strategistAddr); expect(await simpleOETHHarvester.strategistAddr()).to.be.equal( strategistAddr );