-
Notifications
You must be signed in to change notification settings - Fork 24
If OWR is used with rebase tokens and there's a negative rebase, principal can be lost #79
Description
The OptimisticWithdrawalRecipient is deployed with a specific token immutably set on the clone. It is presumed that that token will usually be ETH, but it can also be an ERC20 to account for future integrations with tokenized versions of ETH.
In the event that one of these integrations used a rebasing version of ETH (like stETH), the architecture would need to be set up as follows:
OptimisticWithdrawalRecipient => rewards to something like LidoSplit.sol => Split Wallet
In this case, the OWR would need to be able to handle rebasing tokens.
In the event that rebasing tokens are used, there is the risk that slashing or inactivity leads to a period with a negative rebase. In this case, the following chain of events could happen:
distribute(PULL)is called, settingfundsPendingWithdrawal == balance- rebasing causes the balance to decrease slightly
distribute(PULL)is called again, so whenfundsToBeDistributed = balance - fundsPendingWithdrawalis calculated in an unchecked block, it ends up being neartype(uint256).max- since this is more than
16 ether, the firstamountOfPrincipalStake - _claimedPrincipalFundswill be allocated to the principal recipient, and the rest to the reward recipient - we check that
endingDistributedFunds <= type(uint128).max, but unfortunately this check misses the issue, because onlyfundsToBeDistributedunderflows, notendingDistributedFunds _claimedPrincipalFundsis set toamountOfPrincipalStake, so all future claims will go to the reward recipient- the
pullBalancesfor both recipients will be set higher than the balance of the contract, and so will be unusable
In this situation, the only way for the principal to get their funds back would be for the full amountOfPrincipalStake to hit the contract at once, and for them to call withdraw() before anyone called distribute(PUSH). If anyone was to be able to call distribute(PUSH) before them, all principal would be sent to the reward recipient instead.
Recommendation
Similar to #74, I would recommend removing the ability for the OptimisticWithdrawalRecipient to accept non-ETH tokens.
Otherwise, I would recommend two changes for redundant safety:
-
Do not allow the OWR to be used with rebasing tokens.
-
Move the
_fundsToBeDistributed = _endingDistributedFunds - _startingDistributedFunds;out of the unchecked block. The case where_endingDistributedFundsunderflows is already handled by a later check, so this one change should be sufficient to prevent any risk of this issue.