From 2b6df9c0c71113426de45a7b68d46f91d658dd44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pr=C3=A9vost?= <998369+prevostc@users.noreply.github.com> Date: Fri, 19 Jan 2024 00:38:06 +0100 Subject: [PATCH 1/3] Add BeefyHarvestLensV2 --- contracts/src/BeefyHarvestLensV2.sol | 63 ++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 contracts/src/BeefyHarvestLensV2.sol diff --git a/contracts/src/BeefyHarvestLensV2.sol b/contracts/src/BeefyHarvestLensV2.sol new file mode 100644 index 0000000..3e77539 --- /dev/null +++ b/contracts/src/BeefyHarvestLensV2.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "./interfaces/beefy/IStrategyV7.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +struct LensResult { + uint256 callReward; + uint256 lastHarvest; + uint256 gasUsed; + uint256 blockNumber; + bool paused; + bool success; + bytes harvestResult; +} + +error CallRewardTooLow(uint256 callReward, uint256 minCallReward); + +// Simulate a harvest while recieving a call reward. Return callReward amount and whether or not it was a success. +contract BeefyHarvestLens { + using SafeERC20 for IERC20; + + // Simulate harvest calling callStatic/simulateContract for return results. + // this method will hide any harvest errors and it is not recommended to use it to do the harvesting + // only the simulation using callStatic/simulateContract is recommended + function harvest(IStrategyV7 _strategy, IERC20 _rewardToken, uint256 _minCallReward) + external + returns (LensResult memory res) + { + res.blockNumber = block.number; + res.paused = _strategy.paused(); + + // some strategies don't have lastHarvest + try _strategy.lastHarvest() returns (uint256 _lastHarvest) { + res.lastHarvest = _lastHarvest; + } catch {} + + if (!res.paused) { + uint256 rewardsBefore = IERC20(_rewardToken).balanceOf(address(this)); + uint256 gasBefore = gasleft(); + (bool _success, bytes memory _harvestResult) = + address(_strategy).call(abi.encodeWithSignature("harvest(address)", address(this))); + uint256 gasAfter = gasleft(); + res.success = _success; + res.harvestResult = _harvestResult; + + if (res.success) { + res.callReward = IERC20(_rewardToken).balanceOf(address(this)) - rewardsBefore; + if (res.callReward < _minCallReward) { + revert CallRewardTooLow({callReward: res.callReward, minCallReward: _minCallReward}); + } + + res.gasUsed = gasBefore - gasAfter; + + // protection in case someone actually uses this contract to harvest despite our warnings + if (res.callReward > 0) { + _rewardToken.safeTransfer(msg.sender, res.callReward); + } + } + } + } +} From 22946721f73bdb951bacc792f9ec6459463ed396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pr=C3=A9vost?= <998369+prevostc@users.noreply.github.com> Date: Fri, 19 Jan 2024 00:44:48 +0100 Subject: [PATCH 2/3] Add safer harvest on lens --- contracts/src/BeefyHarvestLens.sol | 11 ++++- contracts/src/BeefyHarvestLensV2.sol | 63 --------------------------- contracts/test/BeefyHarvestLens.t.sol | 24 +++++++--- 3 files changed, 27 insertions(+), 71 deletions(-) delete mode 100644 contracts/src/BeefyHarvestLensV2.sol diff --git a/contracts/src/BeefyHarvestLens.sol b/contracts/src/BeefyHarvestLens.sol index 6977345..3e77539 100644 --- a/contracts/src/BeefyHarvestLens.sol +++ b/contracts/src/BeefyHarvestLens.sol @@ -15,6 +15,8 @@ struct LensResult { bytes harvestResult; } +error CallRewardTooLow(uint256 callReward, uint256 minCallReward); + // Simulate a harvest while recieving a call reward. Return callReward amount and whether or not it was a success. contract BeefyHarvestLens { using SafeERC20 for IERC20; @@ -22,7 +24,10 @@ contract BeefyHarvestLens { // Simulate harvest calling callStatic/simulateContract for return results. // this method will hide any harvest errors and it is not recommended to use it to do the harvesting // only the simulation using callStatic/simulateContract is recommended - function harvest(IStrategyV7 _strategy, IERC20 _rewardToken) external returns (LensResult memory res) { + function harvest(IStrategyV7 _strategy, IERC20 _rewardToken, uint256 _minCallReward) + external + returns (LensResult memory res) + { res.blockNumber = block.number; res.paused = _strategy.paused(); @@ -42,6 +47,10 @@ contract BeefyHarvestLens { if (res.success) { res.callReward = IERC20(_rewardToken).balanceOf(address(this)) - rewardsBefore; + if (res.callReward < _minCallReward) { + revert CallRewardTooLow({callReward: res.callReward, minCallReward: _minCallReward}); + } + res.gasUsed = gasBefore - gasAfter; // protection in case someone actually uses this contract to harvest despite our warnings diff --git a/contracts/src/BeefyHarvestLensV2.sol b/contracts/src/BeefyHarvestLensV2.sol deleted file mode 100644 index 3e77539..0000000 --- a/contracts/src/BeefyHarvestLensV2.sol +++ /dev/null @@ -1,63 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "./interfaces/beefy/IStrategyV7.sol"; -import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; - -struct LensResult { - uint256 callReward; - uint256 lastHarvest; - uint256 gasUsed; - uint256 blockNumber; - bool paused; - bool success; - bytes harvestResult; -} - -error CallRewardTooLow(uint256 callReward, uint256 minCallReward); - -// Simulate a harvest while recieving a call reward. Return callReward amount and whether or not it was a success. -contract BeefyHarvestLens { - using SafeERC20 for IERC20; - - // Simulate harvest calling callStatic/simulateContract for return results. - // this method will hide any harvest errors and it is not recommended to use it to do the harvesting - // only the simulation using callStatic/simulateContract is recommended - function harvest(IStrategyV7 _strategy, IERC20 _rewardToken, uint256 _minCallReward) - external - returns (LensResult memory res) - { - res.blockNumber = block.number; - res.paused = _strategy.paused(); - - // some strategies don't have lastHarvest - try _strategy.lastHarvest() returns (uint256 _lastHarvest) { - res.lastHarvest = _lastHarvest; - } catch {} - - if (!res.paused) { - uint256 rewardsBefore = IERC20(_rewardToken).balanceOf(address(this)); - uint256 gasBefore = gasleft(); - (bool _success, bytes memory _harvestResult) = - address(_strategy).call(abi.encodeWithSignature("harvest(address)", address(this))); - uint256 gasAfter = gasleft(); - res.success = _success; - res.harvestResult = _harvestResult; - - if (res.success) { - res.callReward = IERC20(_rewardToken).balanceOf(address(this)) - rewardsBefore; - if (res.callReward < _minCallReward) { - revert CallRewardTooLow({callReward: res.callReward, minCallReward: _minCallReward}); - } - - res.gasUsed = gasBefore - gasAfter; - - // protection in case someone actually uses this contract to harvest despite our warnings - if (res.callReward > 0) { - _rewardToken.safeTransfer(msg.sender, res.callReward); - } - } - } - } -} diff --git a/contracts/test/BeefyHarvestLens.t.sol b/contracts/test/BeefyHarvestLens.t.sol index 8486bec..70e672a 100644 --- a/contracts/test/BeefyHarvestLens.t.sol +++ b/contracts/test/BeefyHarvestLens.t.sol @@ -53,7 +53,7 @@ contract BeefyHarvestLensTest is Test { revertOnHarvest = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 0); assertEq(res.success, false); @@ -67,7 +67,7 @@ contract BeefyHarvestLensTest is Test { function test_normal_harvest() public { (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -84,7 +84,7 @@ contract BeefyHarvestLensTest is Test { harvestRewards = 1 ether; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 1 ether); assertEq(res.success, true); @@ -101,7 +101,7 @@ contract BeefyHarvestLensTest is Test { pausedMock = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 0); assertEq(res.success, false); @@ -117,7 +117,7 @@ contract BeefyHarvestLensTest is Test { lastHarvestMock = 98765; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -134,7 +134,7 @@ contract BeefyHarvestLensTest is Test { harvestRewards = 0; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 0); assertEq(res.success, true); @@ -151,7 +151,7 @@ contract BeefyHarvestLensTest is Test { revertOnLastHarvest = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken); + LensResult memory res = lens.harvest(strat, rewardToken, 0); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -163,4 +163,14 @@ contract BeefyHarvestLensTest is Test { assertEq(res.harvestResult.length, 0); assertEq(rewardToken.balanceOf(address(this)), 987654); } + + function test_lens_reverts_when_call_reward_is_too_low() public { + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); + + vm.expectRevert(abi.encodeWithSelector(CallRewardTooLow.selector, 987654, 987655)); + + LensResult memory res = lens.harvest(strat, rewardToken, 987655); + + assertEq(res.callReward, 0); + } } From 0f907905e20d743474d1ad2436f46982996deb36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pr=C3=A9vost?= <998369+prevostc@users.noreply.github.com> Date: Fri, 19 Jan 2024 01:06:41 +0100 Subject: [PATCH 3/3] Split safe harvest and simulation to avoid masking harvest errors --- contracts/src/BeefyHarvestLens.sol | 37 +++++++++++--- contracts/test/BeefyHarvestLens.t.sol | 72 ++++++++++++++++++++------- 2 files changed, 83 insertions(+), 26 deletions(-) diff --git a/contracts/src/BeefyHarvestLens.sol b/contracts/src/BeefyHarvestLens.sol index 3e77539..9524d7c 100644 --- a/contracts/src/BeefyHarvestLens.sol +++ b/contracts/src/BeefyHarvestLens.sol @@ -16,6 +16,8 @@ struct LensResult { } error CallRewardTooLow(uint256 callReward, uint256 minCallReward); +error StrategyPaused(); +error MinCallRewardTooLow(uint256 minCallReward); // Simulate a harvest while recieving a call reward. Return callReward amount and whether or not it was a success. contract BeefyHarvestLens { @@ -24,10 +26,7 @@ contract BeefyHarvestLens { // Simulate harvest calling callStatic/simulateContract for return results. // this method will hide any harvest errors and it is not recommended to use it to do the harvesting // only the simulation using callStatic/simulateContract is recommended - function harvest(IStrategyV7 _strategy, IERC20 _rewardToken, uint256 _minCallReward) - external - returns (LensResult memory res) - { + function harvestSimulation(IStrategyV7 _strategy, IERC20 _rewardToken) external returns (LensResult memory res) { res.blockNumber = block.number; res.paused = _strategy.paused(); @@ -47,10 +46,6 @@ contract BeefyHarvestLens { if (res.success) { res.callReward = IERC20(_rewardToken).balanceOf(address(this)) - rewardsBefore; - if (res.callReward < _minCallReward) { - revert CallRewardTooLow({callReward: res.callReward, minCallReward: _minCallReward}); - } - res.gasUsed = gasBefore - gasAfter; // protection in case someone actually uses this contract to harvest despite our warnings @@ -60,4 +55,30 @@ contract BeefyHarvestLens { } } } + + function safeHarvest(IStrategyV7 _strategy, IERC20 _rewardToken, uint256 _minCallReward) + external + returns (uint256) + { + if (_strategy.paused()) { + revert StrategyPaused(); + } + + if (_minCallReward == 0) { + revert MinCallRewardTooLow({minCallReward: _minCallReward}); + } + + uint256 rewardsBefore = IERC20(_rewardToken).balanceOf(address(this)); + IStrategyV7(_strategy).harvest(address(this)); + + // ensure we are not getting sandwiched by a flash loan + uint256 callReward = IERC20(_rewardToken).balanceOf(address(this)) - rewardsBefore; + if (callReward < _minCallReward) { + revert CallRewardTooLow({callReward: callReward, minCallReward: _minCallReward}); + } + + _rewardToken.safeTransfer(msg.sender, callReward); + + return callReward; + } } diff --git a/contracts/test/BeefyHarvestLens.t.sol b/contracts/test/BeefyHarvestLens.t.sol index 70e672a..5440b6c 100644 --- a/contracts/test/BeefyHarvestLens.t.sol +++ b/contracts/test/BeefyHarvestLens.t.sol @@ -49,11 +49,11 @@ contract BeefyHarvestLensTest is Test { lens = new BeefyHarvestLens(); } - function test_lens_do_not_throw_when_harvest_reverts() public { + function test_lens_simulation_do_not_throw_when_harvest_reverts() public { revertOnHarvest = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 0); assertEq(res.success, false); @@ -65,9 +65,9 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 0); } - function test_normal_harvest() public { + function test_lens_simulation_can_simulate_harvest() public { (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -80,11 +80,11 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 987654); } - function test_lens_returns_call_rewards() public { + function test_lens_simulation_returns_call_rewards() public { harvestRewards = 1 ether; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 1 ether); assertEq(res.success, true); @@ -97,11 +97,11 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 1 ether); } - function test_lens_returns_paused() public { + function test_lens_simulation_returns_paused() public { pausedMock = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 0); assertEq(res.success, false); @@ -113,11 +113,11 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 0); } - function test_lens_returns_last_harvest() public { + function test_lens_simulation_returns_last_harvest() public { lastHarvestMock = 98765; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -130,11 +130,11 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 987654); } - function test_lens_success_when_call_reward_is_zero() public { + function test_lens_simulation_success_when_call_reward_is_zero() public { harvestRewards = 0; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 0); assertEq(res.success, true); @@ -147,11 +147,11 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 0); } - function test_lens_do_not_crash_when_last_harvest_isnt_defined() public { + function test_lens_simulation_do_not_crash_when_last_harvest_isnt_defined() public { revertOnLastHarvest = true; (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - LensResult memory res = lens.harvest(strat, rewardToken, 0); + LensResult memory res = lens.harvestSimulation(strat, rewardToken); assertEq(res.callReward, 987654); assertEq(res.success, true); @@ -164,13 +164,49 @@ contract BeefyHarvestLensTest is Test { assertEq(rewardToken.balanceOf(address(this)), 987654); } - function test_lens_reverts_when_call_reward_is_too_low() public { + function test_lens_safe_transfer_do_throw_when_harvest_reverts() public { + revertOnHarvest = true; + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); - vm.expectRevert(abi.encodeWithSelector(CallRewardTooLow.selector, 987654, 987655)); + vm.expectRevert("revertOnHarvest"); - LensResult memory res = lens.harvest(strat, rewardToken, 987655); + lens.safeHarvest(strat, rewardToken, 1000); + } - assertEq(res.callReward, 0); + function test_lens_safe_transfer_can_simulate_harvest() public { + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); + uint256 callReward = lens.safeHarvest(strat, rewardToken, 1000); + + assertEq(callReward, 987654); + assertEq(rewardToken.balanceOf(address(this)), 987654); + } + + function test_lens_safe_transfer_returns_call_rewards() public { + harvestRewards = 1 ether; + + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); + uint256 callReward = lens.safeHarvest(strat, rewardToken, 1000); + + assertEq(callReward, 1 ether); + assertEq(rewardToken.balanceOf(address(this)), 1 ether); + } + + function test_lens_safe_harvest_fails_when_expected_call_rewards_is_zero() public { + harvestRewards = 0; + + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); + + vm.expectRevert(abi.encodeWithSelector(MinCallRewardTooLow.selector, 0)); + + lens.safeHarvest(strat, rewardToken, 0); + } + + function test_lens_safe_harvest_reverts_when_call_reward_is_too_low() public { + (IStrategyV7 strat, BeefyHarvestLens lens) = _helper_create_contracts(); + + vm.expectRevert(abi.encodeWithSelector(CallRewardTooLow.selector, 987654, 987655)); + + lens.safeHarvest(strat, rewardToken, 987655); } }