BalancedVault.sol: loss of funds + global settlement flywheel / user settlement flywheels getting out of sync
When an epoch has become "stale", the `BalancedVault` will treat any new deposits and redemptions in this epoch as "pending". This means they won't get processed by the global settlement flywheel in the next epoch but one epoch later than that.\\nDue to the fact that anyone can push a pending deposit or redemption of a user further ahead by making an arbitrarily small deposit in the "intermediate epoch" (i.e. the epoch between when the user creates the pending deposit / redemption and the epoch when it is scheduled to be processed by the global settlement flywheel), the user can experience a DOS.\\nWorse than that, by pushing the pending deposit / pending redemption further ahead, the global settlement flywheel and the user settlement flywheel get out of sync.\\nAlso users can experience a loss of funds.\\nSo far so good. The global settlement flywheel and the user settlement flywheel are in sync and will process the pending deposit in epoch `3`.\\nNow here's the issue. A malicious user2 or user1 unknowingly (depending on the specific scenario) calls `deposit` for user1 again in the current epoch `2` once it has become `stale` (it's possible to `deposit` an arbitrarily small amount). By doing so we set `_pendingEpochs[user1] = context.epoch + 1 = 3`, thereby pushing the processing of the `deposit` in the user settlement flywheel one epoch ahead.\\nIt's important to understand that the initial deposit will still be processed in epoch `3` in the global settlement flywheel, it's just being pushed ahead in the user settlement flywheel.\\nThereby the global settlement flywheel and user settlement flywheel are out of sync now.\\nAn example for a loss of funds that can occur as a result of this issue is when the PnL from epoch `3` to epoch `4` is positive. Thereby the user1 will get less shares than he is entitled to.\\nSimilarly it is possible to push pending redemptions ahead, thereby the `_totalUnclaimed` amount would be increased by an amount that is different from the amount that `_unclaimed[account]` is increased by.\\nComing back to the case with the pending deposit, I wrote a test that you can add to BalancedVaultMulti.test.ts:\\n```\\nit('pending deposit pushed by 1 epoch causing shares difference', async () => {\\n      const smallDeposit = utils.parseEther('1000')\\n      const smallestDeposit = utils.parseEther('0.000001')\\n\\n      await updateOracleEth() // epoch now stale\\n      // make a pending deposit\\n      await vault.connect(user).deposit(smallDeposit, user.address)\\n      await updateOracleBtc()\\n      await vault.sync()\\n\\n      await updateOracleEth() // epoch now stale\\n      /* \\n      user2 deposits for user1, thereby pushing the pending deposit ahead and causing the \\n      global settlement flywheel and user settlement flywheel to get out of sync\\n      */\\n      await vault.connect(user2).deposit(smallestDeposit, user.address)\\n      await updateOracleBtc()\\n      await vault.sync()\\n\\n      await updateOracle()\\n      // pending deposit for user1 is now processed in the user settlement flywheel\\n      await vault.syncAccount(user.address)\\n\\n      const totalSupply = await vault.totalSupply()\\n      const balanceUser1 = await vault.balanceOf(user.address)\\n      const balanceUser2 = await vault.balanceOf(user2.address)\\n\\n      /*\\n      totalSupply is bigger than the amount of shares of both users together\\n      this is because user1 loses out on some shares that he is entitled to\\n      -> loss of funds\\n      */\\n      console.log(totalSupply);\\n      console.log(balanceUser1.add(balanceUser2));\\n\\n})\\n```\\n\\nThe impact that is generated by having one pending deposit that is off by one epoch is small. However over time this would evolve into a chaotic situation, where the state of the Vault is significantly corrupted.
My recommendation is to implement a queue for pending deposits / pending redemptions of a user. Pending deposits / redemptions can then be processed independently (without new pending deposits / redemptions affecting when existing ones are processed).\\nPossibly there is a simpler solution which might involve restricting the ability to make deposits to the user himself and only allowing one pending deposit / redemption to exist at a time.\\nThe solution to implement depends on how flexible the sponsor wants the deposit / redemption functionality to be.
The biggest impact comes from the global settlement flywheel and user settlement flywheel getting out of sync. As shown above, this can lead to a direct loss of funds for the user (e.g. the amount of shares he gets for a deposit are calculated with the wrong context).\\nApart from the direct impact for a single user, there is a subtler impact which can be more severe in the long term. Important invariants are violated:\\nSum of user balances is equal to the total supply\\nSum of unclaimed user assets is equal to total unclaimed assets\\nThereby the impact is not limited to a single user but affects the calculations for all users.\\nLess important but still noteworthy is that users that deposit into the Vault are partially exposed to PnL in the underlying products. The Vault does not employ a fully delta-neutral strategy. Therefore by experiencing a larger delay until the pending deposit / redemption is processed, users incur the risk of negative PnL.
```\\nit('pending deposit pushed by 1 epoch causing shares difference', async () => {\\n      const smallDeposit = utils.parseEther('1000')\\n      const smallestDeposit = utils.parseEther('0.000001')\\n\\n      await updateOracleEth() // epoch now stale\\n      // make a pending deposit\\n      await vault.connect(user).deposit(smallDeposit, user.address)\\n      await updateOracleBtc()\\n      await vault.sync()\\n\\n      await updateOracleEth() // epoch now stale\\n      /* \\n      user2 deposits for user1, thereby pushing the pending deposit ahead and causing the \\n      global settlement flywheel and user settlement flywheel to get out of sync\\n      */\\n      await vault.connect(user2).deposit(smallestDeposit, user.address)\\n      await updateOracleBtc()\\n      await vault.sync()\\n\\n      await updateOracle()\\n      // pending deposit for user1 is now processed in the user settlement flywheel\\n      await vault.syncAccount(user.address)\\n\\n      const totalSupply = await vault.totalSupply()\\n      const balanceUser1 = await vault.balanceOf(user.address)\\n      const balanceUser2 = await vault.balanceOf(user2.address)\\n\\n      /*\\n      totalSupply is bigger than the amount of shares of both users together\\n      this is because user1 loses out on some shares that he is entitled to\\n      -> loss of funds\\n      */\\n      console.log(totalSupply);\\n      console.log(balanceUser1.add(balanceUser2));\\n\\n})\\n```\\n