Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Simplified Harvester #2222

Merged
merged 23 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 232 additions & 0 deletions contracts/contracts/harvest/OETHBaseHarvester.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import { Governable } from "../governance/Governable.sol";

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

import { IVault } from "../interfaces/IVault.sol";
import { IStrategy } from "../interfaces/IStrategy.sol";
import { ISwapRouter } from "../interfaces/aerodrome/ISwapRouter.sol";

contract OETHBaseHarvester is Governable {
using SafeERC20 for IERC20;

IVault public immutable vault;
IStrategy public immutable amoStrategy;
IERC20 public immutable aero;
IERC20 public immutable weth;
ISwapRouter public immutable swapRouter;

address public operatorAddr;

// Similar sig to `AbstractHarvester.RewardTokenSwapped` for
// future compatibility with monitoring
event RewardTokenSwapped(
address indexed rewardToken,
address indexed swappedInto,
uint8 swapPlatform,
uint256 amountIn,
uint256 amountOut
);

event OperatorChanged(address oldOperator, address newOperator);

/**
* @notice Verifies that the caller is either Governor or Strategist.
*/
modifier onlyGovernorOrStrategist() {

Check warning on line 39 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L39

Added line #L39 was not covered by tests
require(
msg.sender == vault.strategistAddr() || isGovernor(),
"Caller is not the Strategist or Governor"
);
_;

Check warning on line 44 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L44

Added line #L44 was not covered by tests
}

/**
* @notice Verifies that the caller is either Governor or Strategist.
*/
modifier onlyGovernorOrStrategistOrOperator() {

Check warning on line 50 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L50

Added line #L50 was not covered by tests
require(
msg.sender == operatorAddr ||
msg.sender == vault.strategistAddr() ||
isGovernor(),

Check warning on line 54 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L53-L54

Added lines #L53 - L54 were not covered by tests
"Caller is not the Operator or Strategist or Governor"
);
_;

Check warning on line 57 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L57

Added line #L57 was not covered by tests
}

constructor(

Check warning on line 60 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L60

Added line #L60 was not covered by tests
address _vault,
address _amoStrategy,
address _aero,
address _weth,
address _swapRouter
) {
vault = IVault(_vault);
amoStrategy = IStrategy(_amoStrategy);
aero = IERC20(_aero);
weth = IERC20(_weth);
swapRouter = ISwapRouter(_swapRouter);

Check warning on line 71 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L67-L71

Added lines #L67 - L71 were not covered by tests
}

/**
* @dev Changes the operator address which can call `harvest`
* @param _operatorAddr New operator address
*/
function setOperatorAddr(address _operatorAddr) external onlyGovernor {
emit OperatorChanged(operatorAddr, _operatorAddr);
operatorAddr = _operatorAddr;

Check warning on line 80 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L79-L80

Added lines #L79 - L80 were not covered by tests
}

/**
* @notice Collects AERO from AMO strategy and
* sends it to the Strategist multisig.
* Anyone can call it.
*/
function harvest() external onlyGovernorOrStrategistOrOperator {
address strategistAddr = vault.strategistAddr();

Check warning on line 89 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L89

Added line #L89 was not covered by tests
require(strategistAddr != address(0), "Guardian address not set");

// Collect all AERO
amoStrategy.collectRewardTokens();

Check warning on line 93 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L93

Added line #L93 was not covered by tests

uint256 aeroBalance = aero.balanceOf(address(this));

Check warning on line 95 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L95

Added line #L95 was not covered by tests
if (aeroBalance == 0) {
// Do nothing if there's no AERO to transfer
return;

Check warning on line 98 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L98

Added line #L98 was not covered by tests
}

// Transfer everything to Strategist
aero.safeTransfer(strategistAddr, aeroBalance);

Check warning on line 102 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L102

Added line #L102 was not covered by tests
}

/**
* @notice Harvests AERO from AMO strategy and then swaps some (or all)
* of it into WETH to distribute yield and fee.
* When `feeBps` is set to 10000 (100%), all WETH received is
* sent to strategist.
*
* @param aeroToSwap Amount of AERO to swap
* @param minWETHExpected Min. amount of WETH to expect
* @param feeBps Performance fee bps (Sent to strategist)
* @param yieldRecipient Yield recipient (must be Vault or Dripper)
*/
function harvestAndSwap(

Check warning on line 116 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L116

Added line #L116 was not covered by tests
uint256 aeroToSwap,
uint256 minWETHExpected,
uint256 feeBps,
address yieldRecipient
) external onlyGovernorOrStrategist {
address strategistAddr = vault.strategistAddr();

Check warning on line 122 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L122

Added line #L122 was not covered by tests
require(strategistAddr != address(0), "Guardian address not set");

require(feeBps <= 10000, "Invalid Fee Bps");

// Collect all AERO
amoStrategy.collectRewardTokens();

Check warning on line 128 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L128

Added line #L128 was not covered by tests

uint256 aeroBalance = aero.balanceOf(address(this));

Check warning on line 130 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L130

Added line #L130 was not covered by tests
if (aeroBalance == 0) {
// Do nothing if there's no AERO to transfer/swap
return;

Check warning on line 133 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L133

Added line #L133 was not covered by tests
}

if (aeroToSwap > 0) {
if (aeroBalance < aeroToSwap) {
// Transfer in balance from the multisig as needed
// slither-disable-next-line unchecked-transfer arbitrary-send-erc20
aero.safeTransferFrom(

Check warning on line 140 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L140

Added line #L140 was not covered by tests
Copy link
Contributor

@clement-ux clement-ux Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you added this, but this seems a bit over-engineering to me. I would rather completely remove the public harvest() and keep only the ones that have the restricted call, to prevent front-running.

However, as I don't know what the project requirements are and this doesn't seem to be unsafe, I have no problem keeping it like this.

Has this been considered to create a new role, only for simple harvest, and grant this role to a OZ Relayer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been considered to create a new role, only for simple harvest, and grant this role to a OZ Relayer?

Yep, that's an option. However, I didn't wanna complicate the storage layout. Sooner or later, we want the harvester to inherit from AbstractHarvester like on mainnet.

Thinking of it, I guess one option is to completely discard this contract and proxy when we plan to make that transition instead of trying to upgrade the implementation at that time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a good idea indeed!

strategistAddr,
address(this),
aeroToSwap - aeroBalance
);
}

_doSwap(aeroToSwap, minWETHExpected);

Check warning on line 147 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L147

Added line #L147 was not covered by tests

// Figure out AERO left in contract after swap
aeroBalance = aero.balanceOf(address(this));

Check warning on line 150 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L150

Added line #L150 was not covered by tests
}

// Transfer out any leftover AERO after swap
if (aeroBalance > 0) {
aero.safeTransfer(strategistAddr, aeroBalance);

Check warning on line 155 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L155

Added line #L155 was not covered by tests
}

// Computes using all balance the contract holds,
// not just the WETH received from swap. Use `transferToken`
// if there's any WETH left that needs to be taken out
uint256 availableWETHBalance = weth.balanceOf(address(this));

Check warning on line 161 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L161

Added line #L161 was not covered by tests
// Computation rounds in favor of protocol
uint256 fee = (availableWETHBalance * feeBps) / 10000;
DanielVF marked this conversation as resolved.
Show resolved Hide resolved
uint256 yield = availableWETHBalance - fee;

Check warning on line 164 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L163-L164

Added lines #L163 - L164 were not covered by tests

// Transfer yield to yield recipient if any
if (yield > 0) {
// Yields can only be sent to the Vault or the Dripper.
// There's no address(0) check since Vault will break if there's
// no Dripper address set.
require(
yieldRecipient != address(0) &&
(yieldRecipient == address(vault) ||
yieldRecipient == vault.dripper()),

Check warning on line 174 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L174

Added line #L174 was not covered by tests
"Invalid yield recipient"
);
weth.safeTransfer(yieldRecipient, yield);

Check warning on line 177 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L177

Added line #L177 was not covered by tests
shahthepro marked this conversation as resolved.
Show resolved Hide resolved
shahthepro marked this conversation as resolved.
Show resolved Hide resolved
}

// Transfer fee to Guardian if any
if (fee > 0) {
weth.safeTransfer(strategistAddr, fee);

Check warning on line 182 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L182

Added line #L182 was not covered by tests
}
}

/**
* @notice Swaps AERO to WETH on Aerodrome
* @param aeroToSwap Amount of AERO to swap
* @param minWETHExpected Min. amount of WETH to expect
*/
function _doSwap(uint256 aeroToSwap, uint256 minWETHExpected) internal {

Check warning on line 191 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L191

Added line #L191 was not covered by tests
// Let the swap router move funds
aero.approve(address(swapRouter), aeroToSwap);

Check warning on line 193 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L193

Added line #L193 was not covered by tests

// Do the swap
uint256 wethReceived = swapRouter.exactInputSingle(

Check warning on line 196 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L196

Added line #L196 was not covered by tests
ISwapRouter.ExactInputSingleParams({
tokenIn: address(aero),
tokenOut: address(weth),
tickSpacing: 200, // From AERO/WETH pool contract
recipient: address(this),
deadline: block.timestamp,
amountIn: aeroToSwap,
amountOutMinimum: minWETHExpected,
sqrtPriceLimitX96: 0
})
);

emit RewardTokenSwapped(

Check warning on line 209 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L209

Added line #L209 was not covered by tests
address(aero),
address(weth),
0,
aeroToSwap,
wethReceived
);
}

/**
* @notice Transfer token to governor. Intended for recovering tokens stuck in
* the contract, i.e. mistaken sends.
* Also, allows to transfer any AERO left in the contract.
* @param _asset Address for the asset
* @param _amount Amount of the asset to transfer
*/
function transferToken(address _asset, uint256 _amount)

Check warning on line 225 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L225

Added line #L225 was not covered by tests
external
virtual
onlyGovernor
{
IERC20(_asset).safeTransfer(governor(), _amount);

Check warning on line 230 in contracts/contracts/harvest/OETHBaseHarvester.sol

View check run for this annotation

Codecov / codecov/patch

contracts/contracts/harvest/OETHBaseHarvester.sol#L230

Added line #L230 was not covered by tests
}
}
7 changes: 7 additions & 0 deletions contracts/contracts/proxies/Proxies.sol
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,13 @@ contract MetaMorphoStrategyProxy is InitializeGovernedUpgradeabilityProxy {

}

/**
* @notice OETHBaseHarvesterProxy delegates calls to a OETHBaseHarvester implementation
*/
contract OETHBaseHarvesterProxy is InitializeGovernedUpgradeabilityProxy {

}

/**
* @notice ARMBuybackProxy delegates calls to Buyback implementation
*/
Expand Down
62 changes: 62 additions & 0 deletions contracts/deploy/base/013_harvester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
const { deployOnBaseWithGuardian } = require("../../utils/deploy-l2");
const {
deployWithConfirmation,
withConfirmation,
} = require("../../utils/deploy");
const addresses = require("../../utils/addresses");

module.exports = deployOnBaseWithGuardian(
{
deployName: "013_harvester",
useTimelock: true,
},
async ({ ethers }) => {
const { deployerAddr } = await getNamedAccounts();
const sDeployer = await ethers.provider.getSigner(deployerAddr);

const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy");
const cAMOStrategyProxy = await ethers.getContract(
"AerodromeAMOStrategyProxy"
);

// Deploy proxy
await deployWithConfirmation("OETHBaseHarvesterProxy");
const cHarvesterProxy = await ethers.getContract("OETHBaseHarvesterProxy");

// Deploy implementation
const dHarvesterImpl = await deployWithConfirmation("OETHBaseHarvester", [
cOETHbVaultProxy.address,
cAMOStrategyProxy.address,
addresses.base.AERO,
addresses.base.WETH,
addresses.base.swapRouter,
]);

const cAMOStrategy = await ethers.getContractAt(
"AerodromeAMOStrategy",
cAMOStrategyProxy.address
);

// prettier-ignore
await withConfirmation(
cHarvesterProxy
.connect(sDeployer)["initialize(address,address,bytes)"](
dHarvesterImpl.address,
addresses.base.timelock,
"0x"
)
);
console.log("Initialized OETHBaseHarvesterProxy");

return {
actions: [
{
// 1. Set as harvester address on the strategy
contract: cAMOStrategy,
signature: "setHarvesterAddress(address)",
args: [cHarvesterProxy.address],
},
],
};
}
);
17 changes: 12 additions & 5 deletions contracts/test/_fixture-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const log = require("../utils/logger")("test:fixtures-arb");

const aeroSwapRouterAbi = require("./abi/aerodromeSwapRouter.json");
const aeroNonfungiblePositionManagerAbi = require("./abi/aerodromeNonfungiblePositionManager.json");
const aerodromeClGaugeAbi = require("./abi/aerodromeClGauge.json");

const MINTER_ROLE =
"0x9f2df0fed2c77648de5860a4cc508cd0818c85b8b8a1ab4ceeef8d981c8956a6";
Expand All @@ -20,8 +19,6 @@ const BURNER_ROLE =

let snapshotId;
const defaultBaseFixture = deployments.createFixture(async () => {
let aerodromeAmoStrategy;

if (!snapshotId && !isFork) {
snapshotId = await nodeSnapshot();
}
Expand Down Expand Up @@ -62,6 +59,7 @@ const defaultBaseFixture = deployments.createFixture(async () => {
oethbVaultProxy.address
);

let aerodromeAmoStrategy, harvester;
if (isFork) {
// Aerodrome AMO Strategy
const aerodromeAmoStrategyProxy = await ethers.getContract(
Expand All @@ -71,6 +69,13 @@ const defaultBaseFixture = deployments.createFixture(async () => {
"AerodromeAMOStrategy",
aerodromeAmoStrategyProxy.address
);

// Harvester
const harvesterProxy = await ethers.getContract("OETHBaseHarvesterProxy");
harvester = await ethers.getContractAt(
"OETHBaseHarvester",
harvesterProxy.address
);
}

// Bridged wOETH
Expand Down Expand Up @@ -132,7 +137,7 @@ const defaultBaseFixture = deployments.createFixture(async () => {
await woeth.connect(governor).grantRole(MINTER_ROLE, minter.address);
await woeth.connect(governor).grantRole(BURNER_ROLE, burner.address);

for (const user of [rafael, nick]) {
for (const user of [rafael, nick, clement]) {
// Mint some bridged WOETH
await woeth.connect(minter).mint(user.address, oethUnits("1"));
await weth.connect(user).deposit({ value: oethUnits("100") });
Expand All @@ -153,7 +158,7 @@ const defaultBaseFixture = deployments.createFixture(async () => {
addresses.base.swapRouter
);
const aeroClGauge = await ethers.getContractAt(
aerodromeClGaugeAbi,
"ICLGauge",
addresses.base.aerodromeOETHbWETHClGauge
);
const aeroNftManager = await ethers.getContractAt(
Expand All @@ -167,11 +172,13 @@ const defaultBaseFixture = deployments.createFixture(async () => {
aeroNftManager,
aeroClGauge,
aero,

// OETHb
oethb,
oethbVault,
wOETHb,
zapper,
harvester,

// Bridged WOETH
woeth,
Expand Down
Loading
Loading