From c15080be67977f161b8834decaccc781d91461f2 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 26 Nov 2025 12:55:22 -0300 Subject: [PATCH 1/5] feat: add additional safety checks to rebalancing --- src/controller/RebalancingManager.sol | 36 +++++++++++++------ .../Controller.rebalancingManager.t.sol | 33 +++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/controller/RebalancingManager.sol b/src/controller/RebalancingManager.sol index 6735fba0..ca2bf333 100644 --- a/src/controller/RebalancingManager.sol +++ b/src/controller/RebalancingManager.sol @@ -47,6 +47,14 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { * @notice Thrown when the received amount is less than the minimum expected amount */ error Rebalance_SlippageTooHigh(); + /** + * @notice Thrown when the total share supply changes during a rebalance operation + */ + error Rebalance_ShareSupplyChanged(); + /** + * @notice Thrown when the actual received amount does not match the expected amount after a swap + */ + error Rebalance_IncorrectToAmount(); /** * @notice Internal initializer for the RebalancingManager contract @@ -83,8 +91,8 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { address toAsset = IControlledVault(toVault).asset(); uint256 toAmount = fromAsset == toAsset - ? _rebalanceSameAssets(fromVault, toVault, fromAsset, fromAmount) - : _rebalanceDiffAssets(fromVault, toVault, fromAsset, toAsset, fromAmount, minToAmount, swapperData); + ? _rebalanceSameAssets(fromVault, fromAsset, fromAmount, toVault) + : _rebalanceDiffAssets(fromVault, fromAsset, fromAmount, toVault, toAsset, minToAmount, swapperData); emit Rebalanced(fromVault, toVault, fromAmount, toAmount); } @@ -92,15 +100,16 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { /** * @dev Internal function to rebalance the same asset type between two vaults * @param fromVault The address of the vault to withdraw assets from - * @param toVault The address of the vault to deposit assets to + * @param asset The address of the asset being rebalanced * @param amount The amount of assets to rebalance between vaults + * @param toVault The address of the vault to deposit assets to * @return toAmount The actual amount of assets that were successfully rebalanced */ function _rebalanceSameAssets( address fromVault, - address toVault, address asset, - uint256 amount + uint256 amount, + address toVault ) internal returns (uint256 toAmount) @@ -117,31 +126,38 @@ abstract contract RebalancingManager is BaseController, AccountingLogic { * - Protocol-wide backing value slippage protection * - Safety buffer validation to ensure losses don't exceed acceptable limits * @param fromVault The address of the vault to withdraw assets from - * @param toVault The address of the vault to deposit assets to * @param fromAsset The address of the asset being withdrawn from the source vault - * @param toAsset The address of the asset being deposited to the destination vault * @param fromAmount The amount of assets to withdraw from the source vault + * @param toVault The address of the vault to deposit assets to + * @param toAsset The address of the asset being deposited to the destination vault * @param minToAmount The minimum amount of destination assets expected (slippage protection) * @param swapperData Additional data passed to the swapper for asset conversion * @return toAmount The actual amount of destination assets received and deposited */ function _rebalanceDiffAssets( address fromVault, - address toVault, address fromAsset, - address toAsset, uint256 fromAmount, + address toVault, + address toAsset, uint256 minToAmount, bytes calldata swapperData ) internal returns (uint256 toAmount) { - // Store original backing value for slippage calculations uint256 originalBackingValue = backingAssetsValue(); + uint256 originalShareSupply = _share.totalSupply(); + uint256 originalToVaultBalance = IERC20(toAsset).balanceOf(toVault); IControlledVault(fromVault).controllerWithdraw(fromAsset, fromAmount, address(_swapper)); toAmount = _swapper.swap(fromAsset, fromAmount, toAsset, minToAmount, toVault, swapperData); + + // Note: Check swap amount before deposit because balanceOf returns the amount of unallocated assets + + require(originalShareSupply == _share.totalSupply(), Rebalance_ShareSupplyChanged()); + require(IERC20(toAsset).balanceOf(toVault) - originalToVaultBalance == toAmount, Rebalance_IncorrectToAmount()); + IControlledVault(toVault).controllerDeposit(toAmount); // Individual swap slippage protection diff --git a/tests/unit/controller/Controller.rebalancingManager.t.sol b/tests/unit/controller/Controller.rebalancingManager.t.sol index caa0d378..1e988c6b 100644 --- a/tests/unit/controller/Controller.rebalancingManager.t.sol +++ b/tests/unit/controller/Controller.rebalancingManager.t.sol @@ -35,6 +35,7 @@ contract Controller_RebalancingManager_Rebalance_Test is Controller_RebalancingM uint256 toAmount = 100e18; uint256 minToAmount = 10e18; bytes swapperData; + bytes[] balances; function _mockVaultAsset(address vault, address asset) internal { vm.mockCall(vault, abi.encodeWithSelector(IControlledVault.asset.selector), abi.encode(asset)); @@ -50,6 +51,11 @@ contract Controller_RebalancingManager_Rebalance_Test is Controller_RebalancingM vm.mockCall(fromAsset, abi.encodeWithSelector(IERC20.transfer.selector), abi.encode(true)); vm.mockCall(toAsset, abi.encodeWithSelector(IERC20.transfer.selector), abi.encode(true)); + balances = new bytes[](2); + balances[0] = abi.encode(0); + balances[1] = abi.encode(toAmount); + vm.mockCalls(toAsset, abi.encodeWithSelector(IERC20.balanceOf.selector, toVault), balances); + vm.mockCall(address(share), abi.encodeWithSelector(IERC20.totalSupply.selector), abi.encode(1800e18)); } @@ -137,7 +143,34 @@ contract Controller_RebalancingManager_Rebalance_Test is Controller_RebalancingM controller.rebalance(fromVault, fromAmount, toVault, minToAmount, swapperData); } + function testFuzz_shouldRevert_whenShareSupplyChanges(uint256 _newSupply) public { + vm.assume(_newSupply != 1800e18); + + bytes[] memory supplies = new bytes[](2); + supplies[0] = abi.encode(1800e18); + supplies[1] = abi.encode(_newSupply); + + vm.mockCalls(address(share), abi.encodeWithSelector(IERC20.totalSupply.selector), supplies); + + vm.prank(manager); + vm.expectRevert(RebalancingManager.Rebalance_ShareSupplyChanged.selector); + controller.rebalance(fromVault, fromAmount, toVault, minToAmount, swapperData); + } + + function testFuzz_shouldRevert_whenSwapperReturnValueNotMatchingBalanceChange(uint256 _toAmount) public { + vm.assume(_toAmount != toAmount); + + balances[1] = abi.encode(_toAmount); + vm.mockCalls(toAsset, abi.encodeWithSelector(IERC20.balanceOf.selector, toVault), balances); + + vm.prank(manager); + vm.expectRevert(RebalancingManager.Rebalance_IncorrectToAmount.selector); + controller.rebalance(fromVault, fromAmount, toVault, minToAmount, swapperData); + } + function test_shouldRevert_whenToAmountLessThanMinToAmount() public { + balances[1] = abi.encode(minToAmount - 1); + vm.mockCalls(toAsset, abi.encodeWithSelector(IERC20.balanceOf.selector, toVault), balances); vm.mockCall(address(swapper), abi.encodeWithSelector(ISwapper.swap.selector), abi.encode(minToAmount - 1)); vm.prank(manager); From cee9c13fa3a59cf983d9696dcaadbc2e6be1c627 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 26 Nov 2025 11:21:47 -0300 Subject: [PATCH 2/5] chore: use foundry v1.4.3 in CI --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4706a99d..7b9ee278 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,8 @@ jobs: - name: "Install Foundry" uses: "foundry-rs/foundry-toolchain@v1" + with: + version: "v1.4.3" - name: "Install dependencies" run: "forge install" @@ -38,6 +40,8 @@ jobs: - name: "Install Foundry" uses: "foundry-rs/foundry-toolchain@v1" + with: + version: "v1.4.3" - name: "Install dependencies" run: "forge install" From 6f0b4ca405b57475cfcfe8758c48a626cad148c9 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 26 Nov 2025 17:24:34 -0300 Subject: [PATCH 3/5] feat: allow any executor to perform 1inch swap --- src/periphery/swapper/OneInchSwapper.sol | 31 +---------- tests/fork/OneInchSwapper.fork.t.sol | 7 +-- .../periphery/swapper/OneInchSwapper.t.sol | 55 +------------------ 3 files changed, 4 insertions(+), 89 deletions(-) diff --git a/src/periphery/swapper/OneInchSwapper.sol b/src/periphery/swapper/OneInchSwapper.sol index b8568b5a..6853ef53 100644 --- a/src/periphery/swapper/OneInchSwapper.sol +++ b/src/periphery/swapper/OneInchSwapper.sol @@ -2,7 +2,6 @@ pragma solidity 0.8.29; import { SafeERC20, IERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import { Ownable2Step, Ownable } from "@openzeppelin/contracts/access/Ownable2Step.sol"; import { ISwapper } from "../../interfaces/ISwapper.sol"; import { IOneInchAggregationRouterLike } from "../../interfaces/IOneInchAggregationRouterLike.sol"; @@ -12,7 +11,7 @@ import { IOneInchAggregationRouterLike } from "../../interfaces/IOneInchAggregat * @notice A swapper implementation that integrates with 1inch Aggregation Router for token swaps * @dev Implements the ISwapper interface to provide token swapping functionality through 1inch protocol */ -contract OneInchSwapper is Ownable2Step, ISwapper { +contract OneInchSwapper is ISwapper { using SafeERC20 for IERC20; /** @@ -20,16 +19,6 @@ contract OneInchSwapper is Ownable2Step, ISwapper { */ IOneInchAggregationRouterLike public immutable oneInchRouter; - /** - * @notice This mapping tracks which addresses are permitted to execute 1inch swap operations - */ - mapping(address executor => bool) public allowedExecutors; - - /** - * @notice Emitted when an executor's authorization status is updated - */ - event ExecutorAuthorizationUpdated(address indexed executor, bool allowed); - /** * @notice Thrown when a zero address is provided where a valid address is required */ @@ -50,10 +39,6 @@ contract OneInchSwapper is Ownable2Step, ISwapper { * @notice Thrown when the swap does not use the entire input amount */ error PartialFill(); - /** - * @notice Thrown when an unauthorized executor attempts to perform a swap - */ - error UnauthorizedExecutor(); /** * @notice Thrown when the swap description parameters do not match the expected values */ @@ -63,7 +48,7 @@ contract OneInchSwapper is Ownable2Step, ISwapper { * @notice Initializes the OneInchSwapper with the 1inch router address * @param _oneInchRouter The address of the 1inch Aggregation Router contract */ - constructor(address owner, IOneInchAggregationRouterLike _oneInchRouter) Ownable(owner) { + constructor(IOneInchAggregationRouterLike _oneInchRouter) { oneInchRouter = _oneInchRouter; } @@ -100,7 +85,6 @@ contract OneInchSwapper is Ownable2Step, ISwapper { (address executor, IOneInchAggregationRouterLike.SwapDescription memory desc, bytes memory swapData) = abi.decode(swapperParams[4:], (address, IOneInchAggregationRouterLike.SwapDescription, bytes)); - require(allowedExecutors[executor], UnauthorizedExecutor()); require(desc.srcToken == assetIn, InvalidSwapDescription()); require(desc.dstToken == assetOut, InvalidSwapDescription()); require(desc.amount == amountIn, InvalidSwapDescription()); @@ -117,15 +101,4 @@ contract OneInchSwapper is Ownable2Step, ISwapper { return amountOut; } - - /** - * @dev Updates the allowance status for an executor address - * @param executor The address to update the allowance for - * @param allowed True to allow the address to be used as executor of the swap, false to disallow - */ - function setAllowedExecutor(address executor, bool allowed) external onlyOwner { - require(executor != address(0), ZeroAddress()); - allowedExecutors[executor] = allowed; - emit ExecutorAuthorizationUpdated(executor, allowed); - } } diff --git a/tests/fork/OneInchSwapper.fork.t.sol b/tests/fork/OneInchSwapper.fork.t.sol index f4bf813f..ec6259ee 100644 --- a/tests/fork/OneInchSwapper.fork.t.sol +++ b/tests/fork/OneInchSwapper.fork.t.sol @@ -8,7 +8,6 @@ import { OneInchSwapper, IOneInchAggregationRouterLike, IERC20 } from "../../src abstract contract OneInchSwapperForkTest is Test { IOneInchAggregationRouterLike constant SWAP_ROUTER = IOneInchAggregationRouterLike(0x111111125421cA6dc452d289314280a0f8842A65); - address constant EXECUTOR = 0x5141B82f5fFDa4c6fE1E372978F1C5427640a190; IERC20 constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48); IERC20 constant USDT = IERC20(0xdAC17F958D2ee523a2206206994597C13D831ec7); @@ -16,16 +15,12 @@ abstract contract OneInchSwapperForkTest is Test { OneInchSwapper swapper; - address owner = makeAddr("owner"); address user = makeAddr("user"); function setUp() public virtual { vm.createSelectFork("mainnet"); - swapper = new OneInchSwapper(owner, SWAP_ROUTER); - - vm.prank(owner); - swapper.setAllowedExecutor(EXECUTOR, true); + swapper = new OneInchSwapper(SWAP_ROUTER); } } diff --git a/tests/unit/periphery/swapper/OneInchSwapper.t.sol b/tests/unit/periphery/swapper/OneInchSwapper.t.sol index db5b6541..3a4e7e91 100644 --- a/tests/unit/periphery/swapper/OneInchSwapper.t.sol +++ b/tests/unit/periphery/swapper/OneInchSwapper.t.sol @@ -13,7 +13,6 @@ import { abstract contract OneInchSwapperTest is Test { OneInchSwapper swapper; - address owner = makeAddr("owner"); address router = makeAddr("router"); address assetIn = makeAddr("assetIn"); address assetOut = makeAddr("assetOut"); @@ -32,7 +31,7 @@ abstract contract OneInchSwapperTest is Test { } function setUp() public virtual { - swapper = new OneInchSwapper(owner, IOneInchAggregationRouterLike(router)); + swapper = new OneInchSwapper(IOneInchAggregationRouterLike(router)); vm.mockCall(assetIn, abi.encodeWithSelector(IERC20.approve.selector), abi.encode(true)); vm.mockCall( @@ -53,12 +52,6 @@ abstract contract OneInchSwapperTest is Test { } contract OneInchSwapper_Swap_Test is OneInchSwapperTest { - function setUp() public override { - super.setUp(); - vm.prank(owner); - swapper.setAllowedExecutor(executor, true); - } - function test_shouldRevert_whenAssetInIsZeroAddress() public { vm.expectRevert(OneInchSwapper.ZeroAddress.selector); swapper.swap(address(0), amountIn, assetOut, minAmountOut, recipient, swapperParams); @@ -84,14 +77,6 @@ contract OneInchSwapper_Swap_Test is OneInchSwapperTest { swapper.swap(assetIn, amountIn, assetOut, 0, recipient, swapperParams); } - function test_shouldRevert_whenExecutorIsNotAllowed() public { - executor = makeAddr("notAllowedExecutor"); - swapperParams = _encodeSwapperParams(); - - vm.expectRevert(OneInchSwapper.UnauthorizedExecutor.selector); - swapper.swap(assetIn, amountIn, assetOut, minAmountOut, recipient, swapperParams); - } - function test_shouldRevert_whenSrcTokenDoesNotMatchAssetIn() public { desc.srcToken = makeAddr("differentAssetIn"); swapperParams = _encodeSwapperParams(); @@ -178,41 +163,3 @@ contract OneInchSwapper_Swap_Test is OneInchSwapperTest { assertEq(result, amountOut); } } - -contract OneInchSwapper_SetAllowedExecutor_Test is OneInchSwapperTest { - function test_shouldRevert_whenNotOwner() public { - vm.expectRevert(); - vm.prank(makeAddr("notOwner")); - swapper.setAllowedExecutor(executor, true); - } - - function test_shouldRevert_whenExecutorIsZeroAddress() public { - vm.expectRevert(OneInchSwapper.ZeroAddress.selector); - vm.prank(owner); - swapper.setAllowedExecutor(address(0), true); - } - - function test_shouldSetAllowedExecutor() public { - vm.prank(owner); - swapper.setAllowedExecutor(executor, true); - assertTrue(swapper.allowedExecutors(executor)); - - vm.prank(owner); - swapper.setAllowedExecutor(executor, false); - assertFalse(swapper.allowedExecutors(executor)); - } - - function test_shouldEmit_ExecutorAuthorizationUpdated() public { - vm.expectEmit(); - emit OneInchSwapper.ExecutorAuthorizationUpdated(executor, true); - - vm.prank(owner); - swapper.setAllowedExecutor(executor, true); - - vm.expectEmit(); - emit OneInchSwapper.ExecutorAuthorizationUpdated(executor, false); - - vm.prank(owner); - swapper.setAllowedExecutor(executor, false); - } -} From ba96f8d24bdc990f18660022273c9a95afeb4719 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 26 Nov 2025 17:27:16 -0300 Subject: [PATCH 4/5] chore: update deploy script --- .env.sepolia.example | 1 - script/Deploy.s.sol | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.env.sepolia.example b/.env.sepolia.example index 8a54b010..54d2794e 100644 --- a/.env.sepolia.example +++ b/.env.sepolia.example @@ -35,7 +35,6 @@ CONTROLLER_REWARDS_COLLECTOR=0xFCFc06C6E1862Ee8e482359A15d8F137b8a072a1 # Controller operators (acting addresses during deployment) # ----------------------------------------------------------------------------- CONTROLLER_PROXY_ADMIN=0xFCFc06C6E1862Ee8e482359A15d8F137b8a072a1 -CONTROLLER_SWAPPER_OWNER=0xFCFc06C6E1862Ee8e482359A15d8F137b8a072a1 CONTROLLER_VAULT_OWNER=0xFCFc06C6E1862Ee8e482359A15d8F137b8a072a1 # ----------------------------------------------------------------------------- diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index bc80c4aa..d83c5598 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -40,7 +40,6 @@ contract Deploy is BaseScript { struct ControllerOperators { address proxyAdmin; - address swapperOwner; address vaultOwner; } @@ -101,9 +100,7 @@ contract Deploy is BaseScript { ); deployment.gunit = new GenericUnit(address(deployment.controller), "Generic Unit USD", "GU_USD"); - deployment.swapper = new OneInchSwapper( - controllerOperators.swapperOwner, IOneInchAggregationRouterLike(externalAddresses.oneInchRouter) - ); + deployment.swapper = new OneInchSwapper(IOneInchAggregationRouterLike(externalAddresses.oneInchRouter)); deployment.controller .initialize( @@ -209,7 +206,6 @@ contract Deploy is BaseScript { console2.log("-- Controller Operators --"); console2.log("Proxy admin:", controllerOperators.proxyAdmin); - console2.log("Swapper owner:", controllerOperators.swapperOwner); console2.log("Vault owner:", controllerOperators.vaultOwner); console2.log("-- Controller Entities --"); @@ -252,7 +248,6 @@ contract Deploy is BaseScript { function _controllerOperators() internal view returns (ControllerOperators memory operators) { operators = ControllerOperators({ proxyAdmin: vm.envAddress("CONTROLLER_PROXY_ADMIN"), - swapperOwner: vm.envAddress("CONTROLLER_SWAPPER_OWNER"), vaultOwner: vm.envAddress("CONTROLLER_VAULT_OWNER") }); } From c3d6cc285bf9dacad1c6cabf9e5d3568687a7471 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 26 Nov 2025 17:30:43 -0300 Subject: [PATCH 5/5] style: run formatter --- script/Deploy.s.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index d83c5598..370e9ed8 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -247,8 +247,7 @@ contract Deploy is BaseScript { function _controllerOperators() internal view returns (ControllerOperators memory operators) { operators = ControllerOperators({ - proxyAdmin: vm.envAddress("CONTROLLER_PROXY_ADMIN"), - vaultOwner: vm.envAddress("CONTROLLER_VAULT_OWNER") + proxyAdmin: vm.envAddress("CONTROLLER_PROXY_ADMIN"), vaultOwner: vm.envAddress("CONTROLLER_VAULT_OWNER") }); }