Fewer than expected LP tokens if the pool is imbalanced during vault restoration
The vault restoration function intends to perform a proportional deposit. If the pool is imbalanced due to unexpected circumstances, performing a proportional deposit is not optimal. This results in fewer pool tokens in return due to sub-optimal trade, eventually leading to a loss for the vault shareholder.\\nPer the comment on Line 498, it was understood that the `restoreVault` function intends to deposit the withdrawn tokens back into the pool proportionally.\\n```\\nFile: SingleSidedLPVaultBase.sol\\n    /// @notice Restores withdrawn tokens from emergencyExit back into the vault proportionally.\\n    /// Unlocks the vault after restoration so that normal functionality is restored.\\n    /// @param minPoolClaim slippage limit to prevent front running\\n    function restoreVault(\\n        uint256 minPoolClaim, bytes calldata /* data */\\n    ) external override whenLocked onlyNotionalOwner {\\n        StrategyVaultState memory state = VaultStorage.getStrategyVaultState();\\n\\n        (IERC20[] memory tokens, /* */) = TOKENS();\\n        uint256[] memory amounts = new uint256[](tokens.length);\\n\\n        // All balances held by the vault are assumed to be used to re-enter\\n        // the pool. Since the vault has been locked no other users should have\\n        // been able to enter the pool.\\n        for (uint256 i; i < tokens.length; i++) {\\n            if (address(tokens[i]) == address(POOL_TOKEN())) continue;\\n            amounts[i] = TokenUtils.tokenBalance(address(tokens[i]));\\n        }\\n\\n        // No trades are specified so this joins proportionally using the\\n        // amounts specified.\\n        uint256 poolTokens = _joinPoolAndStake(amounts, minPoolClaim);\\n..SNIP..\\n```\\n\\nThe main reason to join with all the pool's tokens in exact proportions is to minimize the price impact or slippage of the join. If the deposited tokens are imbalanced, they are often swapped internally within the pool, incurring slippage or fees.\\nHowever, the concept of proportional join to minimize slippage does not always hold with the current implementation of the `restoreVault` function.\\nProof-of-Concept\\nAt T0, assume that a pool is perfectly balanced (50%-50%) with 1000 WETH and 1000 stETH.\\nAt T1, an emergency exit is performed, the LP tokens are redeemed for the underlying pool tokens proportionally, and 100 WETH and 100 stETH are redeemed\\nAt T2, certain events happen or due to ongoing issues with the pool (e.g., attacks, bugs, mass withdrawal), the pool becomes imbalanced (30%-70%) with 540 WETH and 1260 stETH.\\nAt T3, the vault re-enters the withdrawn tokens to the pool proportionally with 100 WETH and 100 stETH. Since the pool is already imbalanced, attempting to enter the pool proportionally (50% WETH and 50% stETH) will incur additional slippage and penalties, resulting in fewer LP tokens returned.\\nThis issue affects both Curve and Balancer pools since joining an imbalanced pool will always incur a loss.\\nExplantation of imbalance pool\\nA Curve pool is considered imbalanced when there is an imbalance between the assets within it. For instance, the Curve stETH/ETH pool is considered imbalanced if it has the following reserves:\\nETH: 340,472.34 (31.70%)\\nstETH: 733,655.65 (68.30%)\\nIf a Curve Pool is imbalanced, attempting to perform a proportional join will not give an optimal return (e.g. result in fewer Pool LP tokens received).\\nIn Curve Pool, there are penalties/bonuses when depositing to a pool. The pools are always trying to balance themselves. If a deposit helps the pool to reach that desired balance, a deposit bonus will be given (receive extra tokens). On the other hand, if a deposit deviates from the pool from the desired balance, a deposit penalty will be applied (receive fewer tokens).\\n```\\ndef add_liquidity(amounts: uint256[N_COINS], min_mint_amount: uint256) -> uint256:\\n..SNIP..\\n    if token_supply > 0:\\n        # Only account for fees if we are not the first to deposit\\n        fee: uint256 = self.fee * N_COINS / (4 * (N_COINS - 1))\\n        admin_fee: uint256 = self.admin_fee\\n        for i in range(N_COINS):\\n            ideal_balance: uint256 = D1 * old_balances[i] / D0\\n            difference: uint256 = 0\\n            if ideal_balance > new_balances[i]:\\n                difference = ideal_balance - new_balances[i]\\n            else:\\n                difference = new_balances[i] - ideal_balance\\n            fees[i] = fee * difference / FEE_DENOMINATOR\\n            if admin_fee != 0:\\n                self.admin_balances[i] += fees[i] * admin_fee / FEE_DENOMINATOR\\n            new_balances[i] -= fees[i]\\n        D2 = self.get_D(new_balances, amp)\\n        mint_amount = token_supply * (D2 - D0) / D0\\n    else:\\n        mint_amount = D1  # Take the dust if there was any\\n..SNIP..\\n```\\n\\nFollowing is the mathematical explanation of the penalties/bonuses extracted from Curve's Discord channel:\\nThere is a “natural” amount of D increase that corresponds to a given total deposit amount; when the pool is perfectly balanced, this D increase is optimally achieved by a balanced deposit. Any other deposit proportions for the same total amount will give you less D.\\nHowever, when the pool is imbalanced, a balanced deposit is no longer optimal for the D increase.
Consider providing the callers the option to deposit the reward tokens in a "non-proportional" manner if a pool becomes imbalanced. For instance, the function could allow the caller to swap the withdrawn tokens in external DEXs within the `restoreVault` function to achieve the most optimal proportion to minimize the penalty and slippage when re-entering the pool. This is similar to the approach in the vault's reinvest function.
There is no guarantee that a pool will always be balanced. Historically, there have been multiple instances where the largest curve pool (stETH/ETH) has become imbalanced (Reference #1 and #2).\\nIf the pool is imbalanced due to unexpected circumstances, performing a proportional deposit is not optimal, leading to the deposit resulting in fewer LP tokens than possible due to the deposit penalty or slippage due to internal swap.\\nThe side-effect is that the vault restoration will result in fewer pool tokens in return due to sub-optimal trade, eventually leading to a loss of assets for the vault shareholder.
```\\nFile: SingleSidedLPVaultBase.sol\\n    /// @notice Restores withdrawn tokens from emergencyExit back into the vault proportionally.\\n    /// Unlocks the vault after restoration so that normal functionality is restored.\\n    /// @param minPoolClaim slippage limit to prevent front running\\n    function restoreVault(\\n        uint256 minPoolClaim, bytes calldata /* data */\\n    ) external override whenLocked onlyNotionalOwner {\\n        StrategyVaultState memory state = VaultStorage.getStrategyVaultState();\\n\\n        (IERC20[] memory tokens, /* */) = TOKENS();\\n        uint256[] memory amounts = new uint256[](tokens.length);\\n\\n        // All balances held by the vault are assumed to be used to re-enter\\n        // the pool. Since the vault has been locked no other users should have\\n        // been able to enter the pool.\\n        for (uint256 i; i < tokens.length; i++) {\\n            if (address(tokens[i]) == address(POOL_TOKEN())) continue;\\n            amounts[i] = TokenUtils.tokenBalance(address(tokens[i]));\\n        }\\n\\n        // No trades are specified so this joins proportionally using the\\n        // amounts specified.\\n        uint256 poolTokens = _joinPoolAndStake(amounts, minPoolClaim);\\n..SNIP..\\n```\\n