fix: slippage check in wrong order

Author: 0xTimepunk

Makefile

@@ -121,7 +121,7 @@ build-sizes: ## Builds the project and shows sizes
 
 .PHONY: test-vvv
 test-vvv: ## Runs tests with verbose output
-	forge test --match-test test_rebalanceSinglePosition_singleXChainMultiVaultDeposit --evm-version cancun -vvv
+	forge test --match-contract SuperformRouterPlusTest --evm-version cancun
 
 
 .PHONY: ftest

src/router-plus/SuperformRouterPlus.sol

@@ -469,18 +469,18 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
         /// @dev send SPs to router
         _callSuperformRouter(router_, callData, args.rebalanceFromMsgValue);
 
-        uint256 amountToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;
+        uint256 availableBalanceToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;
 
-        if (amountToDeposit == 0) revert Error.ZERO_AMOUNT();
+        if (availableBalanceToDeposit == 0) revert Error.ZERO_AMOUNT();
 
         if (
-            ENTIRE_SLIPPAGE * amountToDeposit
+            ENTIRE_SLIPPAGE * availableBalanceToDeposit
                 < ((args.expectedAmountToReceivePostRebalanceFrom * (ENTIRE_SLIPPAGE - args.slippage)))
         ) {
             revert Error.VAULT_IMPLEMENTATION_FAILED();
         }
 
-        uint256 amountIn = _validateAndGetAmountIn(rebalanceToCallData, amountToDeposit);
+        uint256 amountIn = _validateAndGetAmountIn(rebalanceToCallData, availableBalanceToDeposit);
 
         _deposit(router_, interimAsset, amountIn, args.rebalanceToMsgValue, rebalanceToCallData);
     }
@@ -651,7 +651,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
 
     function _validateAndGetAmountIn(
         bytes calldata rebalanceToCallData,
-        uint256 amountToDeposit
+        uint256 availableBalanceToDeposit
     )
         internal
         view
@@ -711,12 +711,13 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
             }
         }
 
-        /// @dev amountIn must be artificially off-chain reduced to be less than amountToDeposit otherwise the approval
-        /// @dev to transfer tokens to SuperformRouter won't work
-        if (amountIn > amountToDeposit) revert AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();
+        /// @dev amountIn must be artificially off-chain reduced to be less than availableBalanceToDeposit otherwise the
+        /// @dev approval to transfer tokens to SuperformRouter won't work
+        if (amountIn > availableBalanceToDeposit) revert AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();
 
-        /// @dev check against a GLOBAL_SLIPPAGE to prevent a malicious keeper from sending a low amountIn
-        if (ENTIRE_SLIPPAGE * amountToDeposit < ((amountIn * (ENTIRE_SLIPPAGE - GLOBAL_SLIPPAGE)))) {
+        /// @dev check amountIn against availableBalanceToDeposit (available balance) via a GLOBAL_SLIPPAGE to prevent a
+        /// @dev malicious keeper from sending a low amountIn
+        if (ENTIRE_SLIPPAGE * amountIn < ((availableBalanceToDeposit * (ENTIRE_SLIPPAGE - GLOBAL_SLIPPAGE)))) {
             revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
         }
     }

test/unit/router-plus/SuperformRouterPlus.t.sol

@@ -734,6 +734,78 @@ contract SuperformRouterPlusTest is ProtocolActions {
         assertEq(SuperPositions(SUPER_POSITIONS_SOURCE).balanceOf(deployer, superformId1), 0);
     }
 
+    function test_revert_AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE() public {
+        vm.startPrank(deployer);
+
+        _directDeposit(superformId1, 1e18);
+
+        ISuperformRouterPlus.RebalanceSinglePositionSyncArgs memory args =
+            _buildRebalanceSinglePositionToOneVaultArgs(deployer);
+
+        SingleVaultSFData memory sfDataRebalanceTo =
+            abi.decode(_parseCallData(args.rebalanceToCallData), (SingleDirectSingleVaultStateReq)).superformData;
+        sfDataRebalanceTo.amount = 1e30;
+
+        args.rebalanceToCallData = abi.encodeCall(
+            IBaseRouter.singleDirectSingleVaultDeposit, SingleDirectSingleVaultStateReq(sfDataRebalanceTo)
+        );
+        SuperPositions(SUPER_POSITIONS_SOURCE).increaseAllowance(ROUTER_PLUS_SOURCE, superformId1, args.sharesToRedeem);
+
+        vm.expectRevert(ISuperformRouterPlus.AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE.selector);
+        SuperformRouterPlus(ROUTER_PLUS_SOURCE).rebalanceSinglePosition{ value: 2 ether }(args);
+    }
+
+    function test_revert_ASSETS_RECEIVED_OUT_OF_SLIPPAGE() public {
+        vm.startPrank(deployer);
+
+        _directDeposit(superformId1, 1e18);
+        _directDeposit(superformId2, 1e6);
+
+        (ISuperformRouterPlus.RebalanceMultiPositionsSyncArgs memory args, uint256 totalAmountToDeposit) =
+            _buildRebalanceTwoPositionsToOneVaultXChainArgs();
+
+        SingleVaultSFData memory sfDataRebalanceTo =
+            abi.decode(_parseCallData(args.rebalanceToCallData), (SingleXChainSingleVaultStateReq)).superformData;
+
+        /// @dev keeper attempting to rug the user by reducing amount in
+        sfDataRebalanceTo.liqRequest.txData = _buildLiqBridgeTxData(
+            LiqBridgeTxDataArgs(
+                1,
+                args.interimAsset,
+                getContract(OP, "DAI"),
+                getContract(OP, "DAI"),
+                getContract(SOURCE_CHAIN, "SuperformRouter"),
+                SOURCE_CHAIN,
+                OP,
+                OP,
+                false,
+                getContract(OP, "CoreStateRegistry"),
+                uint256(OP),
+                totalAmountToDeposit - 5e5,
+                false,
+                0,
+                1,
+                1,
+                1,
+                address(0)
+            ),
+            false
+        );
+
+        args.rebalanceToCallData = abi.encodeCall(
+            IBaseRouter.singleXChainSingleVaultDeposit, SingleXChainSingleVaultStateReq(AMBs, OP, sfDataRebalanceTo)
+        );
+
+        SuperPositions(SUPER_POSITIONS_SOURCE).increaseAllowance(
+            ROUTER_PLUS_SOURCE, superformId1, args.sharesToRedeem[0]
+        );
+        SuperPositions(SUPER_POSITIONS_SOURCE).increaseAllowance(
+            ROUTER_PLUS_SOURCE, superformId2, args.sharesToRedeem[1]
+        );
+        vm.expectRevert(ISuperformRouterPlus.ASSETS_RECEIVED_OUT_OF_SLIPPAGE.selector);
+        SuperformRouterPlus(ROUTER_PLUS_SOURCE).rebalanceMultiPositions{ value: 2 ether }(args);
+    }
+
     function test_rebalanceSinglePosition_singleXChainSingleVaultDepositSelector() public {
         vm.startPrank(deployer);
 
@@ -3189,23 +3261,24 @@ contract SuperformRouterPlusTest is ProtocolActions {
         args.callData = _callDataRebalanceFromTwoVaults(args.interimAsset);
 
         uint256 decimal1 = MockERC20(getContract(SOURCE_CHAIN, "DAI")).decimals();
-        uint256 decimal2 = MockERC20(args.interimAsset).decimals();
+        uint256 decimal2 = MockERC20(getContract(SOURCE_CHAIN, "USDC")).decimals();
+        uint256 decimalInterim = MockERC20(args.interimAsset).decimals();
         uint256 previewRedeemAmount1 = IBaseForm(superform1).previewRedeemFrom(args.sharesToRedeem[0]);
 
         uint256 expectedAmountToReceivePostRebalanceFrom1;
-        if (decimal1 > decimal2) {
-            expectedAmountToReceivePostRebalanceFrom1 = previewRedeemAmount1 / (10 ** (decimal1 - decimal2));
+        if (decimal1 > decimalInterim) {
+            expectedAmountToReceivePostRebalanceFrom1 = previewRedeemAmount1 / (10 ** (decimal1 - decimalInterim));
         } else {
-            expectedAmountToReceivePostRebalanceFrom1 = previewRedeemAmount1 * 10 ** (decimal2 - decimal1);
+            expectedAmountToReceivePostRebalanceFrom1 = previewRedeemAmount1 * 10 ** (decimalInterim - decimal1);
         }
 
         uint256 previewRedeemAmount2 = IBaseForm(superform2).previewRedeemFrom(args.sharesToRedeem[1]);
 
         uint256 expectedAmountToReceivePostRebalanceFrom2;
-        if (decimal1 > decimal2) {
-            expectedAmountToReceivePostRebalanceFrom2 = previewRedeemAmount2 / (10 ** (decimal1 - decimal2));
+        if (decimal2 > decimalInterim) {
+            expectedAmountToReceivePostRebalanceFrom2 = previewRedeemAmount2 / (10 ** (decimal2 - decimalInterim));
         } else {
-            expectedAmountToReceivePostRebalanceFrom2 = previewRedeemAmount2 * 10 ** (decimal2 - decimal1);
+            expectedAmountToReceivePostRebalanceFrom2 = previewRedeemAmount2 * 10 ** (decimalInterim - decimal2);
         }
 
         totalAmountToDeposit = expectedAmountToReceivePostRebalanceFrom1 + expectedAmountToReceivePostRebalanceFrom2;