Skip to content

Commit

Permalink
fix: slippage check in wrong order
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xTimepunk
0xTimepunk committed Oct 30, 2024
1 parent 0d7ea69 commit 86e76c5
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 18 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 11 additions & 10 deletions src/router-plus/SuperformRouterPlus.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -651,7 +651,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {

function _validateAndGetAmountIn(
bytes calldata rebalanceToCallData,
uint256 amountToDeposit
uint256 availableBalanceToDeposit
)
internal
view
Expand Down Expand Up @@ -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();
}
}
Expand Down
87 changes: 80 additions & 7 deletions test/unit/router-plus/SuperformRouterPlus.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 86e76c5

Please sign in to comment.