Liquid Staking

The owner of the contract can call `PriorityPool::setWithdrawalPool` to change the `withdrawalPool` address. This function works as intended when called for the first time to set the `withdrawalPool`. However, issues arise when it is called subsequently. The function includes the following conditional statement:

​

if (address(withdrawalPool) != address(0)) {

}

​

This checks if the `withdrawalPool` address has been previously set (i.e., not `address(0)`), and if so, it sets the token and LST approvals to 0 for the existing `withdrawalPool`. This implies that the owner can change the `withdrawalPool` from a non-zero address to a new address.

​

Since the `PriorityPool` contract interacts closely with the `withdrawalPool` contract for various functionalities (such as `deposit` and `withdraw` methods) and involves the transfer of protocol tokens (including staking asset tokens and LST tokens), changing the `withdrawalPool` address without considering the state of the previous `withdrawalPool` can lead to token locks and the loss of user funds that were queued for withdrawal.

​

Additionally, in the `withdrawalPool` contract, the withdrawal functionality is protected by the `onlyPriorityPool` modifier. There is also no function available to change the `withdrawalPool` address within the `withdrawalPool` contract itself. As a result, the only way to withdraw tokens is by using the address set as the `withdrawalPool` in the `PriorityPool` contract. Therefore, a mechanism to migrate queued withdrawals and locked tokens to the new `withdrawalPool` is necessary.

​

In the `setWithdrawalPool` function, if the `withdrawalPool` address is not `address(0)` (meaning it has been previously set), the approvals for tokens are reset to 0 for the current `withdrawalPool`. However, there is no mechanism to migrate the locked tokens or queued withdrawals from the old `withdrawalPool` to the new one.

​

function setWithdrawalPool(address _withdrawalPool) external onlyOwner {

+ // migrate the tokens and qeueud withdrawals of previous withdrawPool contract to the new contract.

}

​

This issue can cause queued user withdrawals and the corresponding LST tokens transferred to the old `withdrawalPool` to become locked. Since key functions in the `withdrawalPool` contract are protected by the `onlyPriorityPool` modifier, rolling back to the previous `withdrawalPool` address does not solve the problem. In such a case, new funds may be locked in the new `withdrawalPool`, resulting in two separate `withdrawalPool` contracts, each with locked funds.

​

Manual review.

​

The solution depends on the requirements of the development team. One possible fix is to revert the `setWithdrawalPool` function if the `withdrawalPool` address has already been set. However, given that the team seems to need this functionality (as indicated by the `if` statement that checks if the `withdrawalPool` is already set), another solution would be to implement a mechanism to migrate tokens and queued withdrawals from the old `withdrawalPool` to the new one.

​