Skip to content

Add rewards swap slippage protection #180

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.

Sign up for GitHub

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

Merged
carlosgj94 merged 1 commit into from
Dec 11, 2025
Merged

Add rewards swap slippage protection #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions src/controller/RebalancingManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -174,5 +171,7 @@ abstract contract RebalancingManager is BaseController, AccountingLogic {
} else {
require(_safetyBuffer(newBackingValue) > 0, Rebalance_SlippageTooHigh());
}

IControlledVault(toVault).controllerDeposit(toAmount);
}
}
28 changes: 22 additions & 6 deletions src/controller/RewardsManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
)
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/Controller.integration.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
67 changes: 50 additions & 17 deletions tests/unit/controller/Controller.rewardsManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -69,23 +78,47 @@ 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 {
controller.workaround_setRewardAsset(vaultAsset, true);

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 {
vm.mockCall(rewardAsset, abi.encodeWithSelector(IERC20.balanceOf.selector), abi.encode(0));

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 {
Expand All @@ -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);
}

Expand All @@ -111,15 +144,15 @@ 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 {
vm.expectEmit();
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 {
Expand All @@ -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);
}
}

Expand All @@ -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 {
Expand All @@ -164,23 +197,23 @@ 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 {
controller.workaround_setRewardAsset(vaultAsset, true);

vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_SameAssets.selector));
vm.prank(manager);
controller.claimRewards(vault, vaultAsset);
controller.claimRewards(vaultAsset, vault);
}

function test_shouldRevert_whenZeroRewards() external {
vm.mockCall(rewardAsset, abi.encodeWithSelector(IERC20.balanceOf.selector), abi.encode(0));

vm.expectRevert(abi.encodeWithSelector(RewardsManager.Reward_ZeroRewards.selector));
vm.prank(manager);
controller.claimRewards(vault, rewardAsset);
controller.claimRewards(rewardAsset, vault);
}

function test_shouldClaimRewards() external {
Expand All @@ -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);
}

Expand All @@ -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 {
Expand All @@ -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);
}
}
5 changes: 3 additions & 2 deletions tests/unit/controller/Controller.vaultManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down