diff --git a/src/controller/RebalancingManager.sol b/src/controller/RebalancingManager.sol index ca2bf33..24995ab 100644 --- a/src/controller/RebalancingManager.sol +++ b/src/controller/RebalancingManager.sol @@ -157,9 +157,6 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { require(originalShareSupply == _share.totalSupply(), Rebalance_ShareSupplyChanged()); require(IERC20(toAsset).balanceOf(toVault) - originalToVaultBalance == toAmount, Rebalance_IncorrectToAmount()); - - IControlledVault(toVault).controllerDeposit(toAmount); - // Individual swap slippage protection require(toAmount >= minToAmount, Rebalance_SlippageTooHigh()); // Check overall protocol backing value to ensure slippage is within acceptable range @@ -174,5 +171,7 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { } else { require(_safetyBuffer(newBackingValue) > 0, Rebalance_SlippageTooHigh()); } + + IControlledVault(toVault).controllerDeposit(toAmount); } } diff --git a/src/controller/RewardsManager.sol b/src/controller/RewardsManager.sol index a5b6417..c4b13d7 100644 --- a/src/controller/RewardsManager.sol +++ b/src/controller/RewardsManager.sol @@ -47,6 +47,14 @@ abstract contract RewardsManager is BaseController, VaultManager { * @notice Thrown when the received assets are below the minimum expected amount */ error Reward_SlippageTooHigh(); + /** + * @notice Thrown when the total share supply changes during a sell rewards operation + */ + error Reward_ShareSupplyChanged(); + /** + * @notice Thrown when the actual received amount does not match the expected amount after a swap + */ + error Reward_IncorrectToAmount(); /** * @notice Initializes the RewardsManager contract @@ -60,15 +68,15 @@ abstract contract RewardsManager is BaseController, VaultManager { * @notice Sells reward tokens from a vault and converts them to vault assets * @dev Withdraws reward tokens from the vault, swaps them for vault assets using the swapper, * and deposits the received assets back into the vault - * @param vault The address of the vault containing the reward tokens * @param rewardAsset The address of the reward token to sell + * @param vault The address of the vault containing the reward tokens * @param minAmountOut The minimum amount of vault assets expected from the swap * @param swapperData Additional data required by the swapper for the token swap * @return assets The amount of vault assets received from selling the rewards */ function sellRewards( - address vault, address rewardAsset, + address vault, uint256 minAmountOut, bytes calldata swapperData ) @@ -79,25 +87,33 @@ abstract contract RewardsManager is BaseController, VaultManager { { (uint256 rewards, address vaultAsset) = _rewards(vault, rewardAsset); + uint256 originalShareSupply = _share.totalSupply(); + uint256 originalVaultBalance = IERC20(vaultAsset).balanceOf(vault); + IControlledVault(vault).controllerWithdraw(rewardAsset, rewards, address(_swapper)); assets = _swapper.swap(rewardAsset, rewards, vaultAsset, minAmountOut, vault, swapperData); - IControlledVault(vault).controllerDeposit(assets); + // Note: Check swap amount before deposit because balanceOf returns the amount of unallocated assets + + require(originalShareSupply == _share.totalSupply(), Reward_ShareSupplyChanged()); + require(IERC20(vaultAsset).balanceOf(vault) - originalVaultBalance == assets, Reward_IncorrectToAmount()); require(assets >= minAmountOut, Reward_SlippageTooHigh()); + IControlledVault(vault).controllerDeposit(assets); + emit RewardsSold(vault, rewardAsset, rewards, assets); } /** * @notice Claims reward tokens from a vault and sends them to the rewards collector * @dev Withdraws all available reward tokens from the vault and transfers them to the rewards collector - * @param vault The address of the vault containing the reward tokens * @param rewardAsset The address of the reward token to claim + * @param vault The address of the vault containing the reward tokens * @return rewards The amount of reward tokens claimed and transferred */ function claimRewards( - address vault, - address rewardAsset + address rewardAsset, + address vault ) external nonReentrant diff --git a/tests/integration/Controller.integration.t.sol b/tests/integration/Controller.integration.t.sol index b9efcaa..80b61b7 100644 --- a/tests/integration/Controller.integration.t.sol +++ b/tests/integration/Controller.integration.t.sol @@ -477,7 +477,7 @@ contract Controller_Rewards_IntegrationTest is ControllerIntegrationTest { deal(address(asset1), address(swapper), swapAmount); swapper.setAmountOut(swapAmount); - controller.sellRewards(address(vault1), address(reward), 0, ""); + controller.sellRewards(address(reward), address(vault1), 0, ""); assertEq(vault1.totalAssets(), 100e6 + swapAmount); assertEq(strategy1.totalAssets(), 100e6 + swapAmount); @@ -489,7 +489,7 @@ contract Controller_Rewards_IntegrationTest is ControllerIntegrationTest { deal(address(reward), address(vault1), 10e18); - controller.claimRewards(address(vault1), address(reward)); + controller.claimRewards(address(reward), address(vault1)); assertEq(reward.balanceOf(rewardsCollector), 10e18); } diff --git a/tests/unit/controller/Controller.rewardsManager.t.sol b/tests/unit/controller/Controller.rewardsManager.t.sol index 6199e7b..7c55aed 100644 --- a/tests/unit/controller/Controller.rewardsManager.t.sol +++ b/tests/unit/controller/Controller.rewardsManager.t.sol @@ -39,12 +39,21 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager uint256 minAmountOut = 1; bytes swapperData = "some swapper data"; uint256 swapAmount = 50 ether; + bytes[] balances; + uint256 shareSupply = 1800e18; function setUp() public virtual override { super.setUp(); vm.mockCall(vault, abi.encodeWithSelector(IControlledVault.controllerDeposit.selector), ""); vm.mockCall(address(swapper), abi.encodeWithSelector(ISwapper.swap.selector), abi.encode(swapAmount)); + + vm.mockCall(address(share), abi.encodeWithSelector(IERC20.totalSupply.selector), abi.encode(shareSupply)); + + balances = new bytes[](2); + balances[0] = abi.encode(0); + balances[1] = abi.encode(swapAmount); + vm.mockCalls(vaultAsset, abi.encodeWithSelector(IERC20.balanceOf.selector, vault), balances); } function testFuzz_shouldRevert_whenCallerNotManager(address caller) external { @@ -54,13 +63,13 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, caller, managerRole) ); vm.prank(caller); - controller.sellRewards(vault, rewardAsset, minAmountOut, swapperData); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); } function test_shouldRevert_whenInvalidVault() external { vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_InvalidVault.selector)); vm.prank(manager); - controller.sellRewards(makeAddr("invalidVault"), rewardAsset, minAmountOut, swapperData); + controller.sellRewards(rewardAsset, makeAddr("invalidVault"), minAmountOut, swapperData); } function testFuzz_shouldRevert_whenRewardAssetNotApproved(address _rewardAsset) external { @@ -69,7 +78,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_NotRewardAsset.selector)); vm.prank(manager); - controller.sellRewards(vault, _rewardAsset, minAmountOut, swapperData); + controller.sellRewards(_rewardAsset, vault, minAmountOut, swapperData); } function test_shouldRevert_whenSameAssets() external { @@ -77,7 +86,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_SameAssets.selector)); vm.prank(manager); - controller.sellRewards(vault, vaultAsset, minAmountOut, swapperData); + controller.sellRewards(vaultAsset, vault, minAmountOut, swapperData); } function test_shouldRevert_whenZeroRewards() external { @@ -85,7 +94,31 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_ZeroRewards.selector)); vm.prank(manager); - controller.sellRewards(vault, rewardAsset, minAmountOut, swapperData); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); + } + + function testFuzz_shouldRevert_whenShareSupplyChanged(uint256 newShareSupply) external { + vm.assume(newShareSupply != shareSupply); + + bytes[] memory supplies = new bytes[](2); + supplies[0] = abi.encode(shareSupply); + supplies[1] = abi.encode(newShareSupply); + vm.mockCalls(address(share), abi.encodeWithSelector(IERC20.totalSupply.selector), supplies); + + vm.expectRevert(RewardsManager.Reward_ShareSupplyChanged.selector); + vm.prank(manager); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); + } + + function testFuzz_shouldRevert_whenSwapperReturnValueNotMatchingBalanceChange(uint256 _swapAmount) public { + vm.assume(_swapAmount != swapAmount); + + balances[1] = abi.encode(_swapAmount); + vm.mockCalls(vaultAsset, abi.encodeWithSelector(IERC20.balanceOf.selector, vault), balances); + + vm.expectRevert(RewardsManager.Reward_IncorrectToAmount.selector); + vm.prank(manager); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); } function test_shouldSellRewards() external { @@ -102,7 +135,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectCall(vault, abi.encodeWithSelector(IControlledVault.controllerDeposit.selector, swapAmount)); vm.prank(manager); - uint256 assets = controller.sellRewards(vault, rewardAsset, minAmountOut, swapperData); + uint256 assets = controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); assertEq(assets, swapAmount); } @@ -111,7 +144,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_SlippageTooHigh.selector)); vm.prank(manager); - controller.sellRewards(vault, rewardAsset, _minAmountOut, swapperData); + controller.sellRewards(rewardAsset, vault, _minAmountOut, swapperData); } function test_shouldEmit_RewardsSold() external { @@ -119,7 +152,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager emit RewardsManager.RewardsSold(vault, rewardAsset, rewards, swapAmount); vm.prank(manager); - controller.sellRewards(vault, rewardAsset, minAmountOut, swapperData); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); } function test_shouldRevert_whenReentrant() external { @@ -137,7 +170,7 @@ contract Controller_RewardsManager_SellRewards_Test is Controller_RewardsManager vm.expectRevert(ReentrancyGuardTransientUpgradeable.ReentrancyGuardReentrantCall.selector); vm.prank(manager); - controller.sellRewards(vault, rewardAsset, minAmountOut, swapperData); + controller.sellRewards(rewardAsset, vault, minAmountOut, swapperData); } } @@ -149,13 +182,13 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, caller, managerRole) ); vm.prank(caller); - controller.claimRewards(vault, rewardAsset); + controller.claimRewards(rewardAsset, vault); } function test_shouldRevert_whenInvalidVault() external { vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_InvalidVault.selector)); vm.prank(manager); - controller.claimRewards(makeAddr("invalidVault"), rewardAsset); + controller.claimRewards(rewardAsset, makeAddr("invalidVault")); } function testFuzz_shouldRevert_whenRewardAssetNotApproved(address _rewardAsset) external { @@ -164,7 +197,7 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_NotRewardAsset.selector)); vm.prank(manager); - controller.claimRewards(vault, _rewardAsset); + controller.claimRewards(_rewardAsset, vault); } function test_shouldRevert_whenSameAssets() external { @@ -172,7 +205,7 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_SameAssets.selector)); vm.prank(manager); - controller.claimRewards(vault, vaultAsset); + controller.claimRewards(vaultAsset, vault); } function test_shouldRevert_whenZeroRewards() external { @@ -180,7 +213,7 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_ZeroRewards.selector)); vm.prank(manager); - controller.claimRewards(vault, rewardAsset); + controller.claimRewards(rewardAsset, vault); } function test_shouldClaimRewards() external { @@ -190,7 +223,7 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage ); vm.prank(manager); - uint256 claimedRewards = controller.claimRewards(vault, rewardAsset); + uint256 claimedRewards = controller.claimRewards(rewardAsset, vault); assertEq(claimedRewards, rewards); } @@ -199,7 +232,7 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage emit RewardsManager.RewardsClaimed(vault, rewardAsset, rewardsCollector, rewards); vm.prank(manager); - controller.claimRewards(vault, rewardAsset); + controller.claimRewards(rewardAsset, vault); } function test_shouldRevert_whenReentrant() external { @@ -214,6 +247,6 @@ contract Controller_RewardsManager_ClaimRewards_Test is Controller_RewardsManage vm.expectRevert(ReentrancyGuardTransientUpgradeable.ReentrancyGuardReentrantCall.selector); vm.prank(manager); - controller.claimRewards(vault, rewardAsset); + controller.claimRewards(rewardAsset, vault); } } diff --git a/tests/unit/controller/Controller.vaultManager.t.sol b/tests/unit/controller/Controller.vaultManager.t.sol index 1336bac..fbbfee2 100644 --- a/tests/unit/controller/Controller.vaultManager.t.sol +++ b/tests/unit/controller/Controller.vaultManager.t.sol @@ -747,8 +747,9 @@ contract Controller_VaultManager_CheckVaultsProportionalities_Test is Controller ) public { - maxP1 = uint16(bound(maxP1, 0, 10_000 - 1)); - maxP2 = uint16(bound(maxP2, 0, 12_000 - (maxP1 > 2000 ? maxP1 : 2000))); + maxP1 = uint16(bound(maxP1, 0, 10_000)); + maxP2 = uint16(bound(maxP2, 0, 10_000)); + vm.assume(maxP1 + maxP2 < 12_000); controller.workaround_addVault(vault[0]); controller.workaround_addVault(vault[1]);