Deposited funds are locked in inactive vaults

Summary

Users cannot request withdrawal of their funds from inactive vaults: if a vault receives 0 allocations from gamers, only previous withdrawal requests can be process and new requests cannot be made.

Vulnerability Detail

During rebalancing, if a vault receives 0 allocations from gamers, it becomes inactive. MainVault doesn't allow calling these functions on an inactive vault:

deposit, which makes sense because the vault doesn't generate yield and users shouldn't be allowed to deposit funds in such vaults;
withdraw, which also makes sense because the vault holds no funds, so immediate withdrawals are not possible;
withdrawalRequest, which seems wrong because this doesn't allow users to withdraw funds after next rebalancing.

Since inactive vaults don't receive funds during rebalancing, it expected that immediate withdrawals via the withdraw function are disabled (inactive vaults don't hold funds, thus funds cannot be withdrawn immediately). However, disallowing withdrawalRequest seems like a mistake: users who have deposited their funds into an inactive vault and who are still holding shares of the vault might decide, after seeing that the vault haven't received allocations during last rebalancing (e.g. protocols on other networks might generate higher yields and gamers might want to direct all allocations to those protocols), to withdraw their funds. withdrawalRequest allows them to request withdrawal after next rebalancing, which is a crucial feature of the protocol.

Impact

In the worst case scenario, user funds can be locked in an inactive vault for a prolonged period of time. E.g. if the vault is deployed in a network where yield generating protocols produce lower APY than protocols on other networks. In this scenario, gamers will be willing to allocate to more profitable protocols, but depositors of the inactive vault will be forced to keep their funds locked in the vault until on-chain conditions change (which are not controlled by users, e.g. users cannot force a protocol to have a higher APY).

Code Snippet

When XChainController receives allocations, it disables vault that have 0 allocations and that haven't received new allocations: https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/XChainController.sol#L229-L235
Withdrawals cannot be requested in inactive vaults due to the onlyWhenVaultIsOn modifier: https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L149-L151
withdrawAllowance can only withdraw funds when there's a withdrawal request: https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L166-L169
Tool used

Manual Review

Recommendation

Consider allowing users call withdrawalRequest to request withdrawal in inactive vault. During rebalancing, inactive vaults with positive withdrawalRequest should receive enough funds for all requests to be processed.