-
Notifications
You must be signed in to change notification settings - Fork 84
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
+1,693
−7
Merged
Add Simplified Harvester #2222
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
ddb5151
Add Simplified Harvester
shahthepro e8e1a35
Merge branch 'master' into shah/simplified-harvester
shahthepro 2656304
Add swap methods
shahthepro a179d4e
Add tests
shahthepro ed48dde
Add comments
shahthepro f0880d8
Fix test
shahthepro 845dda5
Merge branch 'master' into shah/simplified-harvester
shahthepro 54248e1
Let harvester move in funds
shahthepro fc8577d
Prettify
shahthepro 1ac651b
Slither
shahthepro 073a16d
Permissioned `harvest()`
shahthepro adfa75c
Merge branch 'master' into shah/simplified-harvester
shahthepro 2df3f66
Merge branch 'master' into shah/simplified-harvester
shahthepro b6e40aa
Add Event, remove yieldRecipient
shahthepro 12b3114
Add send yield to Dripper bool
shahthepro 71aae0f
Update deployId
shahthepro 04a9fe2
Move event down
shahthepro 7b44a99
Update comment
shahthepro e20e5e0
Update comment
shahthepro 8c368a5
Update error message
shahthepro 6127233
Merge branch 'master' into shah/simplified-harvester
shahthepro f9ce3a1
Fix comments
shahthepro 3f2c37b
[OETHb] Deploy 015 - Harvester (#2264)
shahthepro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,233 @@ | ||
// 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); | ||
event YieldSent(address recipient, uint256 yield, uint256 fee); | ||
|
||
/** | ||
* @notice Verifies that the caller is either Governor or Strategist. | ||
*/ | ||
modifier onlyGovernorOrStrategist() { | ||
require( | ||
msg.sender == vault.strategistAddr() || isGovernor(), | ||
"Caller is not the Strategist or Governor" | ||
); | ||
_; | ||
} | ||
|
||
/** | ||
* @notice Verifies that the caller is either Governor or Strategist. | ||
*/ | ||
modifier onlyGovernorOrStrategistOrOperator() { | ||
require( | ||
msg.sender == operatorAddr || | ||
msg.sender == vault.strategistAddr() || | ||
isGovernor(), | ||
"Caller is not the Operator or Strategist or Governor" | ||
); | ||
_; | ||
} | ||
|
||
constructor( | ||
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); | ||
} | ||
|
||
/** | ||
* @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; | ||
} | ||
|
||
/** | ||
* @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(); | ||
require(strategistAddr != address(0), "Guardian address not set"); | ||
|
||
// Collect all AERO | ||
amoStrategy.collectRewardTokens(); | ||
|
||
uint256 aeroBalance = aero.balanceOf(address(this)); | ||
if (aeroBalance == 0) { | ||
// Do nothing if there's no AERO to transfer | ||
return; | ||
} | ||
|
||
// Transfer everything to Strategist | ||
aero.safeTransfer(strategistAddr, aeroBalance); | ||
} | ||
|
||
/** | ||
* @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 sendYieldToDripper Sends yield to Dripper, if set to true. | ||
* Otherwise, to the Guardian | ||
*/ | ||
function harvestAndSwap( | ||
uint256 aeroToSwap, | ||
uint256 minWETHExpected, | ||
uint256 feeBps, | ||
bool sendYieldToDripper | ||
) external onlyGovernorOrStrategist { | ||
address strategistAddr = vault.strategistAddr(); | ||
require(strategistAddr != address(0), "Guardian address not set"); | ||
|
||
// Yields can either be sent to the Dripper or Strategist | ||
address yieldRecipient = sendYieldToDripper | ||
? vault.dripper() | ||
: strategistAddr; | ||
require(yieldRecipient != address(0), "Yield recipient not set"); | ||
|
||
require(feeBps <= 10000, "Invalid Fee Bps"); | ||
|
||
// Collect all AERO | ||
amoStrategy.collectRewardTokens(); | ||
|
||
uint256 aeroBalance = aero.balanceOf(address(this)); | ||
if (aeroBalance == 0) { | ||
// Do nothing if there's no AERO to transfer/swap | ||
return; | ||
} | ||
|
||
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( | ||
strategistAddr, | ||
address(this), | ||
aeroToSwap - aeroBalance | ||
); | ||
} | ||
|
||
_doSwap(aeroToSwap, minWETHExpected); | ||
|
||
// Figure out AERO left in contract after swap | ||
aeroBalance = aero.balanceOf(address(this)); | ||
} | ||
|
||
// Transfer out any leftover AERO after swap | ||
if (aeroBalance > 0) { | ||
aero.safeTransfer(strategistAddr, aeroBalance); | ||
} | ||
|
||
// 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)); | ||
// 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; | ||
|
||
// Transfer yield, if any | ||
if (yield > 0) { | ||
weth.safeTransfer(yieldRecipient, yield); | ||
shahthepro marked this conversation as resolved.
Show resolved
Hide resolved
shahthepro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// Transfer fee to the Guardian, if any | ||
if (fee > 0) { | ||
weth.safeTransfer(strategistAddr, fee); | ||
} | ||
|
||
emit YieldSent(yieldRecipient, yield, fee); | ||
} | ||
|
||
/** | ||
* @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 { | ||
// Let the swap router move funds | ||
aero.approve(address(swapRouter), aeroToSwap); | ||
|
||
// Do the swap | ||
uint256 wethReceived = swapRouter.exactInputSingle( | ||
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( | ||
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) | ||
external | ||
virtual | ||
onlyGovernor | ||
{ | ||
IERC20(_asset).safeTransfer(governor(), _amount); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
const { deployOnBaseWithGuardian } = require("../../utils/deploy-l2"); | ||
const { | ||
deployWithConfirmation, | ||
withConfirmation, | ||
} = require("../../utils/deploy"); | ||
const addresses = require("../../utils/addresses"); | ||
|
||
module.exports = deployOnBaseWithGuardian( | ||
{ | ||
deployName: "015_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"); | ||
|
||
const cHarvester = await ethers.getContractAt( | ||
"OETHBaseHarvester", | ||
cHarvesterProxy.address | ||
); | ||
|
||
return { | ||
actions: [ | ||
{ | ||
// 1. Set as harvester address on the strategy | ||
contract: cAMOStrategy, | ||
signature: "setHarvesterAddress(address)", | ||
args: [cHarvesterProxy.address], | ||
}, | ||
{ | ||
// 2. Set Operator address | ||
contract: cHarvester, | ||
signature: "setOperatorAddr(address)", | ||
args: [addresses.base.OZRelayerAddress], | ||
}, | ||
], | ||
}; | ||
} | ||
); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
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!