From 0d9e627cc83ae4cef7cb974f2727f86934534bd0 Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 4 Mar 2026 15:45:59 -0600 Subject: [PATCH 1/4] feat: prevent deposit and withdrawal in the same transaction --- src/controller/BaseController.sol | 18 +++++ src/controller/Controller.sol | 35 +++++++++ tests/harness/ControllerHarness.sol | 16 ++++ tests/helper/Multicall.sol | 20 +++++ .../integration/Controller.integration.t.sol | 49 ++++++++++++ tests/unit/controller/Controller.t.sol | 76 +++++++++++++++++++ 6 files changed, 214 insertions(+) create mode 100644 tests/helper/Multicall.sol diff --git a/src/controller/BaseController.sol b/src/controller/BaseController.sol index 8a1a262..9b0c4b8 100644 --- a/src/controller/BaseController.sol +++ b/src/controller/BaseController.sol @@ -31,6 +31,24 @@ import { IGenericShare } from "../interfaces/IGenericShare.sol"; * - Permanent contract malfunction */ abstract contract BaseController is AccessControlUpgradeable, ReentrancyGuardTransientUpgradeable { + // ======================================== + // TRANSIENT + // ======================================== + + /** + * @notice Flag of deposit action in the current transaction + */ + bool internal transient _rebalanceGuardDeposited; + + /** + * @notice Flag of withdraw action in the current transaction + */ + bool internal transient _rebalanceGuardWithdrawn; + + // ======================================== + // PERSISTENT + // ======================================== + /** * @notice Maximum basis points value representing 100% */ diff --git a/src/controller/Controller.sol b/src/controller/Controller.sol index 779c284..a12dfb7 100644 --- a/src/controller/Controller.sol +++ b/src/controller/Controller.sol @@ -70,6 +70,10 @@ contract Controller is * @notice Thrown when caller is not the main vault for an asset */ error Controller_CallerNotMainVault(); + /** + * @notice Thrown when a deposit/mint and withdrawal/redeem are attempted in the same transaction + */ + error Controller_DepositAndWithdrawInSameTx(); /** * @notice Ensures only registered vaults can call the function @@ -96,6 +100,33 @@ contract Controller is require(_vaultFor[_vaultAsset(msg.sender)] == msg.sender, Controller_CallerNotMainVault()); } + /** + * @notice Enum representing actions that trigger the rebalance guard + * @dev Used to prevent deposit/mint and withdrawal/redeem operations in the same transaction + */ + enum RebalanceGuardAction { + Deposit, + Withdraw + } + + /** + * @notice Ensures deposit/mint and withdrawal/redeem operations cannot occur in the same transaction + */ + modifier rebalanceGuard(RebalanceGuardAction action) { + _rebalanceGuard(action); + _; + } + + function _rebalanceGuard(RebalanceGuardAction action) internal { + if (action == RebalanceGuardAction.Deposit) { + require(!_rebalanceGuardWithdrawn, Controller_DepositAndWithdrawInSameTx()); + _rebalanceGuardDeposited = true; + } else if (action == RebalanceGuardAction.Withdraw) { + require(!_rebalanceGuardDeposited, Controller_DepositAndWithdrawInSameTx()); + _rebalanceGuardWithdrawn = true; + } + } + /** * @notice Constructor that disables initializers to prevent direct initialization * @dev Uses OpenZeppelin's initializer pattern for upgradeable contracts @@ -322,6 +353,7 @@ contract Controller is public onlyMainVault notPaused + rebalanceGuard(RebalanceGuardAction.Deposit) returns (uint256 shares) { address vault = msg.sender; @@ -352,6 +384,7 @@ contract Controller is public onlyMainVault notPaused + rebalanceGuard(RebalanceGuardAction.Deposit) returns (uint256 normalizedAssets) { address vault = msg.sender; @@ -384,6 +417,7 @@ contract Controller is public onlyVault notPaused + rebalanceGuard(RebalanceGuardAction.Withdraw) returns (uint256 shares) { address vault = msg.sender; @@ -418,6 +452,7 @@ contract Controller is public onlyVault notPaused + rebalanceGuard(RebalanceGuardAction.Withdraw) returns (uint256 normalizedAssets) { address vault = msg.sender; diff --git a/tests/harness/ControllerHarness.sol b/tests/harness/ControllerHarness.sol index f6878bf..c7251bc 100644 --- a/tests/harness/ControllerHarness.sol +++ b/tests/harness/ControllerHarness.sol @@ -170,6 +170,14 @@ contract ControllerHarness is Controller { return _totalAssetsDeltaToHitProportionality(proportionalityLimit, vaultAssets, totalAssets); } + function exposed_rebalanceGuardDeposited() external view returns (bool) { + return _rebalanceGuardDeposited; + } + + function exposed_rebalanceGuardWithdrawn() external view returns (bool) { + return _rebalanceGuardWithdrawn; + } + // ======================================== // WORKAROUND FUNCTIONS // ======================================== @@ -235,4 +243,12 @@ contract ControllerHarness is Controller { assert(maxSlippage <= MAX_BPS); maxProtocolRebalanceSlippage = uint16(maxSlippage); } + + function workaround_setRebalanceGuardDeposited(bool deposited) public { + _rebalanceGuardDeposited = deposited; + } + + function workaround_setRebalanceGuardWithdrawn(bool withdrawn) public { + _rebalanceGuardWithdrawn = withdrawn; + } } diff --git a/tests/helper/Multicall.sol b/tests/helper/Multicall.sol new file mode 100644 index 0000000..9286406 --- /dev/null +++ b/tests/helper/Multicall.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.29; + +contract Multicall { + struct Call { + address target; + bytes callData; + } + + function aggregate(Call[] calldata calls) public { + for (uint256 i; i < calls.length; ++i) { + (bool success, bytes memory data) = calls[i].target.call(calls[i].callData); + if (!success) { + assembly { + revert(add(data, 0x20), mload(data)) + } + } + } + } +} diff --git a/tests/integration/Controller.integration.t.sol b/tests/integration/Controller.integration.t.sol index 80b61b7..e9b7e69 100644 --- a/tests/integration/Controller.integration.t.sol +++ b/tests/integration/Controller.integration.t.sol @@ -15,6 +15,7 @@ import { MockERC20 } from "../helper/MockERC20.sol"; import { MockPriceFeed } from "../helper/MockPriceFeed.sol"; import { MockStrategy } from "../helper/MockStrategy.sol"; import { MockSwapper } from "../helper/MockSwapper.sol"; +import { Multicall } from "../helper/Multicall.sol"; abstract contract ControllerIntegrationTest is Test { Controller controller; @@ -81,6 +82,7 @@ abstract contract ControllerIntegrationTest is Test { } contract Controller_DepositWithdraw_IntegrationTest is ControllerIntegrationTest { + /// forge-config: default.isolate = true function test_depositWithdraw_whenPriceStable() public { // Deposit vm.startPrank(user); @@ -131,6 +133,7 @@ contract Controller_DepositWithdraw_IntegrationTest is ControllerIntegrationTest assertEq(asset2.balanceOf(address(strategy2)), 50e8); } + /// forge-config: default.isolate = true function test_depositWithdraw_whenPriceVolatile() public { priceFeed1.setPrice(1.1e8); priceFeed2.setPrice(0.9e8); @@ -204,6 +207,7 @@ contract Controller_DepositWithdraw_IntegrationTest is ControllerIntegrationTest } contract Controller_MintRedeem_IntegrationTest is ControllerIntegrationTest { + /// forge-config: default.isolate = true function test_mintRedeem_whenPriceStable() public { // share:asset ratio is 1:1 when price is stable @@ -256,6 +260,7 @@ contract Controller_MintRedeem_IntegrationTest is ControllerIntegrationTest { assertEq(asset2.balanceOf(address(strategy2)), 50e8); } + /// forge-config: default.isolate = true function test_mintRedeem_whenPriceVolatile() public { priceFeed1.setPrice(1.1e8); priceFeed2.setPrice(0.8e8); @@ -332,6 +337,50 @@ contract Controller_MintRedeem_IntegrationTest is ControllerIntegrationTest { } } +contract Controller_RebalanceGuard_IntegrationTest is ControllerIntegrationTest { + /// forge-config: default.isolate = true + function test_preventDepositWithdrawInSameTx() public { + Multicall multicall = new Multicall(); + + Multicall.Call[] memory calls = new Multicall.Call[](3); + calls[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, user, user)); + + vm.startPrank(user); + require(asset1.transfer(address(multicall), 100e6)); + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + multicall.aggregate(calls); + vm.stopPrank(); + } + + /// forge-config: default.isolate = true + function test_preventWithdrawDepositInSameTx() public { + Multicall multicall = new Multicall(); + + Multicall.Call[] memory calls1 = new Multicall.Call[](2); + calls1[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls1[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + + Multicall.Call[] memory calls2 = new Multicall.Call[](2); + calls2[0] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, user, user)); + calls2[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + + vm.startPrank(user); + require(asset1.transfer(address(multicall), 100e6)); + multicall.aggregate(calls1); + gusd.approve(address(multicall), type(uint256).max); + + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + multicall.aggregate(calls2); + vm.stopPrank(); + } +} + contract Controller_YieldDistribution_IntegrationTest is ControllerIntegrationTest { uint256 errDelta = 0.000001e18; diff --git a/tests/unit/controller/Controller.t.sol b/tests/unit/controller/Controller.t.sol index 0107aee..057b14e 100644 --- a/tests/unit/controller/Controller.t.sol +++ b/tests/unit/controller/Controller.t.sol @@ -742,6 +742,25 @@ contract Controller_Deposit_Test is ControllerTest { vm.prank(vault); controller.deposit(1000e18, receiver); } + + function test_shouldSetRebalanceGuardDeposited() public { + _mockVault(vault, asset, 1, feed, 1e8, 8); + + vm.prank(vault); + controller.deposit(1000e18, receiver); + + assertTrue(controller.exposed_rebalanceGuardDeposited()); + } + + function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + _mockVault(vault, asset, 1, feed, 1e8, 8); + + controller.workaround_setRebalanceGuardWithdrawn(true); + + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + vm.prank(vault); + controller.deposit(1000e18, receiver); + } } contract Controller_Mint_Test is ControllerTest { @@ -833,6 +852,25 @@ contract Controller_Mint_Test is ControllerTest { vm.prank(vault); controller.mint(1000e18, receiver); } + + function test_shouldSetRebalanceGuardDeposited() public { + _mockVault(vault, asset, 1, feed, 1e8, 8); + + vm.prank(vault); + controller.mint(1000e18, receiver); + + assertTrue(controller.exposed_rebalanceGuardDeposited()); + } + + function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + _mockVault(vault, asset, 1, feed, 1e8, 8); + + controller.workaround_setRebalanceGuardWithdrawn(true); + + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + vm.prank(vault); + controller.mint(1000e18, receiver); + } } contract Controller_Withdraw_Test is ControllerTest { @@ -977,6 +1015,25 @@ contract Controller_Withdraw_Test is ControllerTest { vm.prank(vault); controller.withdraw(100e18, spender, owner); } + + function test_shouldSetRebalanceGuardDeposited() public { + _mockVault(vault, asset, 100e18, feed, 1e8, 8); + + vm.prank(vault); + controller.withdraw(100e18, spender, owner); + + assertTrue(controller.exposed_rebalanceGuardWithdrawn()); + } + + function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + _mockVault(vault, asset, 100e18, feed, 1e8, 8); + + controller.workaround_setRebalanceGuardDeposited(true); + + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + vm.prank(vault); + controller.withdraw(100e18, spender, owner); + } } contract Controller_Redeem_Test is ControllerTest { @@ -1128,4 +1185,23 @@ contract Controller_Redeem_Test is ControllerTest { vm.prank(vault); controller.redeem(100e18, spender, owner); } + + function test_shouldSetRebalanceGuardDeposited() public { + _mockVault(vault, asset, 100e18, feed, 1e8, 8); + + vm.prank(vault); + controller.redeem(100e18, spender, owner); + + assertTrue(controller.exposed_rebalanceGuardWithdrawn()); + } + + function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + _mockVault(vault, asset, 100e18, feed, 1e8, 8); + + controller.workaround_setRebalanceGuardDeposited(true); + + vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + vm.prank(vault); + controller.redeem(100e18, spender, owner); + } } From ac9c18757891ac2bfd135319f308b446b000d65d Mon Sep 17 00:00:00 2001 From: ashhanai Date: Thu, 5 Mar 2026 12:49:12 -0600 Subject: [PATCH 2/4] feat: implement secondary action fee --- src/controller/AccountingLogic.sol | 10 +- src/controller/BaseController.sol | 14 +- src/controller/ConfigManager.sol | 22 +++ src/controller/Controller.sol | 35 +--- tests/harness/ControllerHarness.sol | 20 ++- tests/helper/Multicall.sol | 4 +- .../integration/Controller.integration.t.sol | 140 ++++++++++++++-- .../controller/Controller.configManager.t.sol | 44 +++++ tests/unit/controller/Controller.t.sol | 154 +++++++++++++++--- 9 files changed, 358 insertions(+), 85 deletions(-) diff --git a/src/controller/AccountingLogic.sol b/src/controller/AccountingLogic.sol index 66f7c9e..95d33aa 100644 --- a/src/controller/AccountingLogic.sol +++ b/src/controller/AccountingLogic.sol @@ -35,7 +35,10 @@ abstract contract AccountingLogic is BaseController, VaultManager { * @return The asset deposit price, limited to ASSET_DEPOSIT_MAX_PRICE */ function assetDepositPrice(address asset) public view returns (uint256) { - return getAssetPrice(asset).min(ASSET_DEPOSIT_MAX_PRICE); + uint256 _secondaryActionFee = _withdrawnInTx ? secondaryActionFee : 0; + // Note: Additional fee is applied to the asset price during deposit/mint if a withdrawal/redemption has already + // occurred in the same transaction + return getAssetPrice(asset).min(ASSET_DEPOSIT_MAX_PRICE).mulDiv(MAX_BPS - _secondaryActionFee, MAX_BPS); } /** @@ -45,7 +48,10 @@ abstract contract AccountingLogic is BaseController, VaultManager { * @return The asset redemption price, at least ASSET_REDEMPTION_MIN_PRICE */ function assetRedemptionPrice(address asset) public view returns (uint256) { - return getAssetPrice(asset).max(ASSET_REDEMPTION_MIN_PRICE); + uint256 _secondaryActionFee = _depositedInTx ? secondaryActionFee : 0; + // Note: Additional fee is applied to the asset price during withdrawal/redemption if a deposit/mint has already + // occurred in the same transaction + return getAssetPrice(asset).max(ASSET_REDEMPTION_MIN_PRICE).mulDiv(MAX_BPS, MAX_BPS - _secondaryActionFee); } /** diff --git a/src/controller/BaseController.sol b/src/controller/BaseController.sol index 9b0c4b8..cc6c2cd 100644 --- a/src/controller/BaseController.sol +++ b/src/controller/BaseController.sol @@ -38,12 +38,12 @@ abstract contract BaseController is AccessControlUpgradeable, ReentrancyGuardTra /** * @notice Flag of deposit action in the current transaction */ - bool internal transient _rebalanceGuardDeposited; + bool internal transient _depositedInTx; /** * @notice Flag of withdraw action in the current transaction */ - bool internal transient _rebalanceGuardWithdrawn; + bool internal transient _withdrawnInTx; // ======================================== // PERSISTENT @@ -166,8 +166,16 @@ abstract contract BaseController is AccessControlUpgradeable, ReentrancyGuardTra */ uint256 public safetyBufferYieldDeduction; + /** + * @notice Additional fee applied to deposits/mints when a withdrawal/redemption has already occurred in the same + * transaction, and vice versa + * @dev The fee is expressed in basis points (bps) and is applied as a percentage of the asset price, increasing the + * cost of the second action in the same transaction to discourage "swapping" protocol collateral assets. + */ + uint16 public secondaryActionFee; + /** * @notice Reserved storage space to allow for layout changes in the future. */ - uint256[50] private __gap; + uint256[49] private __gap; } diff --git a/src/controller/ConfigManager.sol b/src/controller/ConfigManager.sol index fa7848d..bda9c31 100644 --- a/src/controller/ConfigManager.sol +++ b/src/controller/ConfigManager.sol @@ -30,6 +30,10 @@ abstract contract ConfigManager is BaseController { * @notice Emitted when the maximum protocol rebalance slippage is updated */ event MaxProtocolRebalanceSlippageUpdated(uint256 oldMaxSlippage, uint256 newMaxSlippage); + /** + * @notice Emitted when the secondary action fee is updated + */ + event SecondaryActionFeeUpdated(uint256 oldSecondaryActionFee, uint256 newSecondaryActionFee); /** * @notice Thrown when attempting to set the rewards collector to the zero address @@ -43,6 +47,10 @@ abstract contract ConfigManager is BaseController { * @notice Thrown when attempting to set a max slippage that exceeds the maximum allowed */ error Config_InvalidMaxSlippage(); + /** + * @notice Thrown when attempting to set a secondary action fee that exceeds the maximum allowed + */ + error Config_InvalidSecondaryActionFee(); /** * @notice Internal initializer for the ConfigManager contract @@ -103,4 +111,18 @@ abstract contract ConfigManager is BaseController { // forge-lint: disable-next-line(unsafe-typecast) maxProtocolRebalanceSlippage = uint16(newMaxSlippage); } + + /** + * @notice Updates the secondary action fee applied during deposits/withdrawals if a withdrawal/deposit has already + * occurred in the same transaction + * @dev Only addresses with CONFIG_MANAGER_ROLE can call this function + * @param newSecondaryActionFee The new secondary action fee in basis points (e.g., 100 = 1%) + */ + function setSecondaryActionFee(uint256 newSecondaryActionFee) external onlyRole(CONFIG_MANAGER_ROLE) { + require(newSecondaryActionFee < MAX_BPS, Config_InvalidSecondaryActionFee()); + emit SecondaryActionFeeUpdated(secondaryActionFee, newSecondaryActionFee); + // casting to 'uint16' is safe because 'newSecondaryActionFee' is guaranteed to be less than 'MAX_BPS' + // forge-lint: disable-next-line(unsafe-typecast) + secondaryActionFee = uint16(newSecondaryActionFee); + } } diff --git a/src/controller/Controller.sol b/src/controller/Controller.sol index a12dfb7..da66339 100644 --- a/src/controller/Controller.sol +++ b/src/controller/Controller.sol @@ -100,33 +100,6 @@ contract Controller is require(_vaultFor[_vaultAsset(msg.sender)] == msg.sender, Controller_CallerNotMainVault()); } - /** - * @notice Enum representing actions that trigger the rebalance guard - * @dev Used to prevent deposit/mint and withdrawal/redeem operations in the same transaction - */ - enum RebalanceGuardAction { - Deposit, - Withdraw - } - - /** - * @notice Ensures deposit/mint and withdrawal/redeem operations cannot occur in the same transaction - */ - modifier rebalanceGuard(RebalanceGuardAction action) { - _rebalanceGuard(action); - _; - } - - function _rebalanceGuard(RebalanceGuardAction action) internal { - if (action == RebalanceGuardAction.Deposit) { - require(!_rebalanceGuardWithdrawn, Controller_DepositAndWithdrawInSameTx()); - _rebalanceGuardDeposited = true; - } else if (action == RebalanceGuardAction.Withdraw) { - require(!_rebalanceGuardDeposited, Controller_DepositAndWithdrawInSameTx()); - _rebalanceGuardWithdrawn = true; - } - } - /** * @notice Constructor that disables initializers to prevent direct initialization * @dev Uses OpenZeppelin's initializer pattern for upgradeable contracts @@ -353,9 +326,9 @@ contract Controller is public onlyMainVault notPaused - rebalanceGuard(RebalanceGuardAction.Deposit) returns (uint256 shares) { + _depositedInTx = true; address vault = msg.sender; VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: false }); @@ -384,9 +357,9 @@ contract Controller is public onlyMainVault notPaused - rebalanceGuard(RebalanceGuardAction.Deposit) returns (uint256 normalizedAssets) { + _depositedInTx = true; address vault = msg.sender; VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: false }); @@ -417,9 +390,9 @@ contract Controller is public onlyVault notPaused - rebalanceGuard(RebalanceGuardAction.Withdraw) returns (uint256 shares) { + _withdrawnInTx = true; address vault = msg.sender; VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: true }); @@ -452,9 +425,9 @@ contract Controller is public onlyVault notPaused - rebalanceGuard(RebalanceGuardAction.Withdraw) returns (uint256 normalizedAssets) { + _withdrawnInTx = true; address vault = msg.sender; VaultsOverview memory overview = _vaultsOverview({ calculateTotalValue: true }); diff --git a/tests/harness/ControllerHarness.sol b/tests/harness/ControllerHarness.sol index c7251bc..0c0734d 100644 --- a/tests/harness/ControllerHarness.sol +++ b/tests/harness/ControllerHarness.sol @@ -170,12 +170,12 @@ contract ControllerHarness is Controller { return _totalAssetsDeltaToHitProportionality(proportionalityLimit, vaultAssets, totalAssets); } - function exposed_rebalanceGuardDeposited() external view returns (bool) { - return _rebalanceGuardDeposited; + function exposed_depositedInTx() external view returns (bool) { + return _depositedInTx; } - function exposed_rebalanceGuardWithdrawn() external view returns (bool) { - return _rebalanceGuardWithdrawn; + function exposed_withdrawnInTx() external view returns (bool) { + return _withdrawnInTx; } // ======================================== @@ -244,11 +244,15 @@ contract ControllerHarness is Controller { maxProtocolRebalanceSlippage = uint16(maxSlippage); } - function workaround_setRebalanceGuardDeposited(bool deposited) public { - _rebalanceGuardDeposited = deposited; + function workaround_setDepositedInTx(bool deposited) public { + _depositedInTx = deposited; } - function workaround_setRebalanceGuardWithdrawn(bool withdrawn) public { - _rebalanceGuardWithdrawn = withdrawn; + function workaround_setWithdrawnInTx(bool withdrawn) public { + _withdrawnInTx = withdrawn; + } + + function workaround_setSecondaryActionFee(uint16 fee) public { + secondaryActionFee = fee; } } diff --git a/tests/helper/Multicall.sol b/tests/helper/Multicall.sol index 9286406..14eaf07 100644 --- a/tests/helper/Multicall.sol +++ b/tests/helper/Multicall.sol @@ -7,7 +7,8 @@ contract Multicall { bytes callData; } - function aggregate(Call[] calldata calls) public { + function aggregate(Call[] calldata calls) public returns (bytes[] memory returnData) { + returnData = new bytes[](calls.length); for (uint256 i; i < calls.length; ++i) { (bool success, bytes memory data) = calls[i].target.call(calls[i].callData); if (!success) { @@ -15,6 +16,7 @@ contract Multicall { revert(add(data, 0x20), mload(data)) } } + returnData[i] = data; } } } diff --git a/tests/integration/Controller.integration.t.sol b/tests/integration/Controller.integration.t.sol index e9b7e69..02485f7 100644 --- a/tests/integration/Controller.integration.t.sol +++ b/tests/integration/Controller.integration.t.sol @@ -337,28 +337,137 @@ contract Controller_MintRedeem_IntegrationTest is ControllerIntegrationTest { } } -contract Controller_RebalanceGuard_IntegrationTest is ControllerIntegrationTest { +contract Controller_SecondaryActionFee_IntegrationTest is ControllerIntegrationTest { + Multicall multicall = new Multicall(); + + function setUp() public override { + super.setUp(); + + vm.startPrank(user); + asset1.approve(address(multicall), type(uint256).max); + gusd.approve(address(multicall), type(uint256).max); + require(asset1.transfer(address(multicall), 100e6)); + vm.stopPrank(); + } + + /// forge-config: default.isolate = true + function test_depositAndRedeemInSameTx_1() public { + controller.setSecondaryActionFee(100); // 1% + + Multicall.Call[] memory calls = new Multicall.Call[](3); + calls[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.redeem.selector, 100e18, user, user)); + + vm.prank(user); + bytes[] memory results = multicall.aggregate(calls); + + assertEq(abi.decode(results[1], (uint256)), 100e18); + assertEq(abi.decode(results[2], (uint256)), 99e6); + } + + /// forge-config: default.isolate = true + function test_depositAndRedeemInSameTx_20() public { + controller.setSecondaryActionFee(2000); // 20% + + Multicall.Call[] memory calls = new Multicall.Call[](3); + calls[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.redeem.selector, 100e18, user, user)); + + vm.prank(user); + bytes[] memory results = multicall.aggregate(calls); + + assertEq(abi.decode(results[1], (uint256)), 100e18); + assertEq(abi.decode(results[2], (uint256)), 80e6); + } + + /// forge-config: default.isolate = true + function test_depositAndWithdrawInSameTx_20() public { + controller.setSecondaryActionFee(2000); // 20% + + Multicall.Call[] memory calls = new Multicall.Call[](3); + calls[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 50e6, user, user)); + + vm.prank(user); + bytes[] memory results = multicall.aggregate(calls); + + assertEq(abi.decode(results[1], (uint256)), 100e18); + assertEq(abi.decode(results[2], (uint256)), 62.5e18); + } + /// forge-config: default.isolate = true - function test_preventDepositWithdrawInSameTx() public { - Multicall multicall = new Multicall(); + function test_depositAndWithdrawInSameTx_50() public { + controller.setSecondaryActionFee(5000); // 50% Multicall.Call[] memory calls = new Multicall.Call[](3); calls[0] = Multicall.Call( address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) ); calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); - calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, user, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 50e6, user, user)); + + vm.prank(user); + bytes[] memory results = multicall.aggregate(calls); + + assertEq(abi.decode(results[1], (uint256)), 100e18); + assertEq(abi.decode(results[2], (uint256)), 100e18); + } + + /// forge-config: default.isolate = true + function test_depositAndWithdrawInSameTx_99() public { + controller.setSecondaryActionFee(9900); // 99% + + Multicall.Call[] memory calls = new Multicall.Call[](3); + calls[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + calls[2] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 1e6, user, user)); + + vm.prank(user); + bytes[] memory results = multicall.aggregate(calls); + + assertEq(abi.decode(results[1], (uint256)), 100e18); + assertEq(abi.decode(results[2], (uint256)), 100e18); + } + + /// forge-config: default.isolate = true + function test_withdrawAndDepositInSameTx_1() public { + controller.setSecondaryActionFee(100); // 1% + + Multicall.Call[] memory calls1 = new Multicall.Call[](2); + calls1[0] = Multicall.Call( + address(asset1), abi.encodeWithSelector(asset1.approve.selector, address(vault1), type(uint256).max) + ); + calls1[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); + + Multicall.Call[] memory calls2 = new Multicall.Call[](2); + calls2[0] = Multicall.Call( + address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, address(multicall), user) + ); + calls2[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); vm.startPrank(user); - require(asset1.transfer(address(multicall), 100e6)); - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); - multicall.aggregate(calls); + multicall.aggregate(calls1); + bytes[] memory results = multicall.aggregate(calls2); vm.stopPrank(); + + assertEq(abi.decode(results[0], (uint256)), 100e18); + assertEq(abi.decode(results[1], (uint256)), 99e18); } /// forge-config: default.isolate = true - function test_preventWithdrawDepositInSameTx() public { - Multicall multicall = new Multicall(); + function test_withdrawAndDepositInSameTx_20() public { + controller.setSecondaryActionFee(2000); // 20% Multicall.Call[] memory calls1 = new Multicall.Call[](2); calls1[0] = Multicall.Call( @@ -367,17 +476,18 @@ contract Controller_RebalanceGuard_IntegrationTest is ControllerIntegrationTest calls1[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); Multicall.Call[] memory calls2 = new Multicall.Call[](2); - calls2[0] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, user, user)); + calls2[0] = Multicall.Call( + address(vault1), abi.encodeWithSelector(vault1.withdraw.selector, 100e6, address(multicall), user) + ); calls2[1] = Multicall.Call(address(vault1), abi.encodeWithSelector(vault1.deposit.selector, 100e6, user)); vm.startPrank(user); - require(asset1.transfer(address(multicall), 100e6)); multicall.aggregate(calls1); - gusd.approve(address(multicall), type(uint256).max); - - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); - multicall.aggregate(calls2); + bytes[] memory results = multicall.aggregate(calls2); vm.stopPrank(); + + assertEq(abi.decode(results[0], (uint256)), 100e18); + assertEq(abi.decode(results[1], (uint256)), 80e18); } } diff --git a/tests/unit/controller/Controller.configManager.t.sol b/tests/unit/controller/Controller.configManager.t.sol index eb5f0a6..6c46f46 100644 --- a/tests/unit/controller/Controller.configManager.t.sol +++ b/tests/unit/controller/Controller.configManager.t.sol @@ -182,3 +182,47 @@ contract Controller_ConfigManager_SetMaxProtocolRebalanceSlippage_Test is Contro controller.setMaxProtocolRebalanceSlippage(newMaxSlippage); } } + +contract Controller_ConfigManager_SetSecondaryActionFee_Test is Controller_ConfigManager_Test { + function testFuzz_shouldRevert_whenCallerNotManager(address caller) external { + vm.assume(caller != manager); + + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, caller, managerRole) + ); + vm.prank(caller); + controller.setSecondaryActionFee(0); + } + + function testFuzz_shouldRevert_whenInvalidSecondaryActionFee(uint256 fee) external { + fee = bound(fee, controller.MAX_BPS(), type(uint256).max); + + vm.expectRevert(abi.encodeWithSelector(ConfigManager.Config_InvalidSecondaryActionFee.selector)); + vm.prank(manager); + controller.setSecondaryActionFee(fee); + } + + function testFuzz_shouldUpdateSecondaryActionFee(uint256 fee) external { + fee = bound(fee, 0, controller.MAX_BPS() - 1); + + vm.prank(manager); + controller.setSecondaryActionFee(fee); + + assertEq(controller.secondaryActionFee(), fee); + } + + function test_shouldEmit_SecondaryActionFeeUpdated() external { + uint256 oldFee = 200; // 2% + uint256 newFee = 150; // 1.5% + + // casting to 'uint16' is safe because 'oldFee' is guaranteed to be less than 'MAX_BPS' + // forge-lint: disable-next-line(unsafe-typecast) + controller.workaround_setSecondaryActionFee(uint16(oldFee)); + + vm.expectEmit(); + emit ConfigManager.SecondaryActionFeeUpdated(oldFee, newFee); + + vm.prank(manager); + controller.setSecondaryActionFee(newFee); + } +} diff --git a/tests/unit/controller/Controller.t.sol b/tests/unit/controller/Controller.t.sol index 057b14e..7393e9a 100644 --- a/tests/unit/controller/Controller.t.sol +++ b/tests/unit/controller/Controller.t.sol @@ -743,23 +743,48 @@ contract Controller_Deposit_Test is ControllerTest { controller.deposit(1000e18, receiver); } - function test_shouldSetRebalanceGuardDeposited() public { + function test_shouldSetDepositedInTx() public { _mockVault(vault, asset, 1, feed, 1e8, 8); vm.prank(vault); controller.deposit(1000e18, receiver); - assertTrue(controller.exposed_rebalanceGuardDeposited()); + assertTrue(controller.exposed_depositedInTx()); } - function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + function test_shouldApplySecondaryActionFee_whenWithdrawnInTx() public { _mockVault(vault, asset, 1, feed, 1e8, 8); + controller.workaround_setWithdrawnInTx(true); - controller.workaround_setRebalanceGuardWithdrawn(true); + // 0% + controller.workaround_setSecondaryActionFee(0); + vm.prank(vault); + assertEq(controller.deposit(100e18, receiver), 100e18); - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + // 0.1% + controller.workaround_setSecondaryActionFee(10); vm.prank(vault); - controller.deposit(1000e18, receiver); + assertEq(controller.deposit(100e18, receiver), 99.9e18); + + // 1% + controller.workaround_setSecondaryActionFee(100); + vm.prank(vault); + assertEq(controller.deposit(100e18, receiver), 99e18); + + // 10% + controller.workaround_setSecondaryActionFee(1000); + vm.prank(vault); + assertEq(controller.deposit(100e18, receiver), 90e18); + + // 30% + controller.workaround_setSecondaryActionFee(3000); + vm.prank(vault); + assertEq(controller.deposit(100e18, receiver), 70e18); + + // 80% + controller.workaround_setSecondaryActionFee(8000); + vm.prank(vault); + assertEq(controller.deposit(100e18, receiver), 20e18); } } @@ -853,23 +878,48 @@ contract Controller_Mint_Test is ControllerTest { controller.mint(1000e18, receiver); } - function test_shouldSetRebalanceGuardDeposited() public { + function test_shouldSetDepositedInTx() public { _mockVault(vault, asset, 1, feed, 1e8, 8); vm.prank(vault); controller.mint(1000e18, receiver); - assertTrue(controller.exposed_rebalanceGuardDeposited()); + assertTrue(controller.exposed_depositedInTx()); } - function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + function test_shouldApplySecondaryActionFee_whenWithdrawnInTx() public { _mockVault(vault, asset, 1, feed, 1e8, 8); + controller.workaround_setWithdrawnInTx(true); - controller.workaround_setRebalanceGuardWithdrawn(true); + // 0% + controller.workaround_setSecondaryActionFee(0); + vm.prank(vault); + assertEq(controller.mint(100e18, receiver), 100e18); - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + // 0.1% + controller.workaround_setSecondaryActionFee(10); vm.prank(vault); - controller.mint(1000e18, receiver); + assertEq(controller.mint(100e18, receiver), 100_100_100_100_100_100_101); + + // 1% + controller.workaround_setSecondaryActionFee(100); + vm.prank(vault); + assertEq(controller.mint(100e18, receiver), 101_010_101_010_101_010_102); + + // 10% + controller.workaround_setSecondaryActionFee(1000); + vm.prank(vault); + assertEq(controller.mint(100e18, receiver), 111_111_111_111_111_111_112); + + // 30% + controller.workaround_setSecondaryActionFee(3000); + vm.prank(vault); + assertEq(controller.mint(100e18, receiver), 142_857_142_857_142_857_143); + + // 80% + controller.workaround_setSecondaryActionFee(8000); + vm.prank(vault); + assertEq(controller.mint(100e18, receiver), 500e18); } } @@ -1016,23 +1066,50 @@ contract Controller_Withdraw_Test is ControllerTest { controller.withdraw(100e18, spender, owner); } - function test_shouldSetRebalanceGuardDeposited() public { + function test_shouldSetWithdrawnInTx() public { _mockVault(vault, asset, 100e18, feed, 1e8, 8); vm.prank(vault); controller.withdraw(100e18, spender, owner); - assertTrue(controller.exposed_rebalanceGuardWithdrawn()); + assertTrue(controller.exposed_withdrawnInTx()); } - function test_shouldRevert_whenRebalanceGuardWithdrawn() public { - _mockVault(vault, asset, 100e18, feed, 1e8, 8); + function test_shouldApplySecondaryActionFee_whenDepositedInTx() public { + _mockVault(vault, asset, 1000e18, feed, 1e8, 8); + controller.workaround_setDepositedInTx(true); + // workaround, breaks sum(balance) = total supply + vm.mockCall(address(share), abi.encodeWithSelector(IERC20.balanceOf.selector, owner), abi.encode(1000e18)); - controller.workaround_setRebalanceGuardDeposited(true); + // 0% + controller.workaround_setSecondaryActionFee(0); + vm.prank(vault); + assertEq(controller.withdraw(100e18, spender, owner), 100e18); - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + // 0.1% + controller.workaround_setSecondaryActionFee(10); vm.prank(vault); - controller.withdraw(100e18, spender, owner); + assertEq(controller.withdraw(100e18, spender, owner), 100_100_100_100_100_100_100); + + // 1% + controller.workaround_setSecondaryActionFee(100); + vm.prank(vault); + assertEq(controller.withdraw(100e18, spender, owner), 101_010_101_010_101_010_100); + + // 10% + controller.workaround_setSecondaryActionFee(1000); + vm.prank(vault); + assertEq(controller.withdraw(100e18, spender, owner), 111_111_111_111_111_111_100); + + // 30% + controller.workaround_setSecondaryActionFee(3000); + vm.prank(vault); + assertEq(controller.withdraw(100e18, spender, owner), 142_857_142_857_142_857_100); + + // 80% + controller.workaround_setSecondaryActionFee(8000); + vm.prank(vault); + assertEq(controller.withdraw(100e18, spender, owner), 500e18); } } @@ -1186,22 +1263,49 @@ contract Controller_Redeem_Test is ControllerTest { controller.redeem(100e18, spender, owner); } - function test_shouldSetRebalanceGuardDeposited() public { + function test_shouldSetWithdrawnInTx() public { _mockVault(vault, asset, 100e18, feed, 1e8, 8); vm.prank(vault); controller.redeem(100e18, spender, owner); - assertTrue(controller.exposed_rebalanceGuardWithdrawn()); + assertTrue(controller.exposed_withdrawnInTx()); } - function test_shouldRevert_whenRebalanceGuardWithdrawn() public { + function test_shouldApplySecondaryActionFee_whenDepositedInTx() public { _mockVault(vault, asset, 100e18, feed, 1e8, 8); + controller.workaround_setDepositedInTx(true); + // workaround, breaks sum(balance) = total supply + vm.mockCall(address(share), abi.encodeWithSelector(IERC20.balanceOf.selector, owner), abi.encode(1000e18)); - controller.workaround_setRebalanceGuardDeposited(true); + // 0% + controller.workaround_setSecondaryActionFee(0); + vm.prank(vault); + assertEq(controller.redeem(100e18, spender, owner), 100e18); - vm.expectRevert(Controller.Controller_DepositAndWithdrawInSameTx.selector); + // 0.1% + controller.workaround_setSecondaryActionFee(10); vm.prank(vault); - controller.redeem(100e18, spender, owner); + assertEq(controller.redeem(100e18, spender, owner), 99.9e18); + + // 1% + controller.workaround_setSecondaryActionFee(100); + vm.prank(vault); + assertEq(controller.redeem(100e18, spender, owner), 99e18); + + // 10% + controller.workaround_setSecondaryActionFee(1000); + vm.prank(vault); + assertEq(controller.redeem(100e18, spender, owner), 90e18 + 9); // ?? + + // 30% + controller.workaround_setSecondaryActionFee(3000); + vm.prank(vault); + assertEq(controller.redeem(100e18, spender, owner), 70e18 + 21); // ?? + + // 80% + controller.workaround_setSecondaryActionFee(8000); + vm.prank(vault); + assertEq(controller.redeem(100e18, spender, owner), 20e18); } } From 60b51a282dce2f6e0439689b47bd9a9ad1edde3e Mon Sep 17 00:00:00 2001 From: ashhanai Date: Mon, 9 Mar 2026 11:14:28 -0600 Subject: [PATCH 3/4] test: fork test Controller upgrade --- tests/fork/ControllerUpgrade.fork.t.sol | 86 +++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/fork/ControllerUpgrade.fork.t.sol diff --git a/tests/fork/ControllerUpgrade.fork.t.sol b/tests/fork/ControllerUpgrade.fork.t.sol new file mode 100644 index 0000000..7f89654 --- /dev/null +++ b/tests/fork/ControllerUpgrade.fork.t.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.29; + +import { Test } from "forge-std/Test.sol"; + +import { + ITransparentUpgradeableProxy +} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; +import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; + +import { Controller } from "../../src/controller/Controller.sol"; +import { GenericVault, IERC20 } from "../../src/vault/GenericVault.sol"; +import { IChainlinkAggregatorLike } from "../../src/interfaces/IChainlinkAggregatorLike.sol"; + +contract ControllerUpgradeForkTest is Test { + address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; + address constant USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7; + address constant USDS = 0xdC035D45d973E3EC169d2276DDab16f1e407384F; + + address proxyAdminOwner = 0x3794d7f91b3Dd3b338FEe671aC6AA42BEA5e3D17; + ProxyAdmin proxyAdmin = ProxyAdmin(0x55835D50EbD78977a4686264E411f93E8ce75bFb); + Controller controller = Controller(0x3a64D23313E1bEAABa25Ec13149bD8D514C973Ae); + + address user = makeAddr("user"); + + function setUp() public virtual { + vm.createSelectFork("mainnet"); + + deal(user, 1e18); + deal(USDC, user, 1_000_000e6); + } + + function _upgradeController() internal { + Controller newImpl = new Controller(); + + vm.startPrank(proxyAdminOwner); + proxyAdmin.upgradeAndCall(ITransparentUpgradeableProxy(address(controller)), address(newImpl), ""); + vm.stopPrank(); + } + + function test_shouldReturnSameStoredValues() public { + bool origPaused = controller.paused(); + address origShare = controller.share(); + + address origUsdcMainVault = controller.vaultFor(USDC); + address origUsdtMainVault = controller.vaultFor(USDT); + address origUsdsMainVault = controller.vaultFor(USDS); + + (IChainlinkAggregatorLike origUsdcPriceFeed,) = controller.priceFeeds(USDC); + (IChainlinkAggregatorLike origUsdtPriceFeed,) = controller.priceFeeds(USDT); + (IChainlinkAggregatorLike origUsdsPriceFeed,) = controller.priceFeeds(USDS); + + _upgradeController(); + + (IChainlinkAggregatorLike newUsdcPriceFeed,) = controller.priceFeeds(USDC); + (IChainlinkAggregatorLike newUsdtPriceFeed,) = controller.priceFeeds(USDT); + (IChainlinkAggregatorLike newUsdsPriceFeed,) = controller.priceFeeds(USDS); + + assertEq(controller.paused(), origPaused); + assertEq(controller.share(), origShare); + assertEq(controller.vaultFor(USDC), origUsdcMainVault); + assertEq(controller.vaultFor(USDT), origUsdtMainVault); + assertEq(controller.vaultFor(USDS), origUsdsMainVault); + assertEq(address(newUsdcPriceFeed), address(origUsdcPriceFeed)); + assertEq(address(newUsdtPriceFeed), address(origUsdtPriceFeed)); + assertEq(address(newUsdsPriceFeed), address(origUsdsPriceFeed)); + } + + function test_shouldWithdrawSameAfterUpgrade() public { + GenericVault vault = GenericVault(controller.vaultFor(USDC)); + + vm.startPrank(user); + IERC20(USDC).approve(controller.vaultFor(USDC), type(uint256).max); + uint256 shares = vault.deposit(1000e6, user); + vm.stopPrank(); + + uint256 origWithdrawnAmount = vault.previewRedeem(shares); + + _upgradeController(); + + vm.prank(user); + uint256 newWithdrawnAmount = vault.redeem(shares, user, user); + + assertEq(origWithdrawnAmount, newWithdrawnAmount); + } +} From 5faf76de80aea00eef68b8b023fec60111fb2c0f Mon Sep 17 00:00:00 2001 From: ashhanai Date: Wed, 25 Mar 2026 09:17:46 -0600 Subject: [PATCH 4/4] fix: remove unused error --- src/controller/Controller.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/controller/Controller.sol b/src/controller/Controller.sol index da66339..4772c19 100644 --- a/src/controller/Controller.sol +++ b/src/controller/Controller.sol @@ -70,10 +70,6 @@ contract Controller is * @notice Thrown when caller is not the main vault for an asset */ error Controller_CallerNotMainVault(); - /** - * @notice Thrown when a deposit/mint and withdrawal/redeem are attempted in the same transaction - */ - error Controller_DepositAndWithdrawInSameTx(); /** * @notice Ensures only registered vaults can call the function