feat(RewardsStreamerMP): introduce emergency mode and ability to leave #72
Conversation
|
The ideal logic would be: Stake Manager contains a flag which is read by Stake Vault. When this flag is true, Stake Vault just allows withdraw and Stake Manager wont accept any iteractions anymore. |
0x-r4bbit
force-pushed
the
feat/emergency-exit
branch
2 times, most recently
from
d5888dd
to
2f4b5a0
Compare
This adds a new emergency mode that can be enabled by the owner of the system. When in emergency mode, stakers or `StakeVault`s can leave the system immediately. Closes #66
0x-r4bbit
force-pushed
the
feat/emergency-exit
branch
from
2f4b5a0
to
9e1dcaf
Compare
|
@3esmit So, the reason I've done it this way (StakeManager sets flag, and reverts accordingly in functions where necessary), is because the logic for preventing interaction with the manager should stay the same no matter what stakevault you interact with. For example, if we upgrade StakeVault to a different version, we'd otherwise have to ensure that it reverts in functions where it should revert. If we keep the revert logic in the StakeManager, that's one thing less to keep in mind. -- Are there any other good reasons to have StakeVault explicitly check emergency mode first, before interacting? Happy to make the necessary changes in that case, but right now I don't see how that would improve the state. |
|
I've updated this PR with a certora rule that ensures only view functions, ownable functions, trusted codehash functions and |
| @@ -132,6 +135,27 @@ contract RewardsStreamerMP is IStakeManager, TrustedCodehashAccess, ReentrancyGu | |||
| account.accountRewardIndex = rewardIndex; | |||
| } | |||
|
|
|||
| function leave() external onlyTrustedCodehash nonReentrant { | |||
There was a problem hiding this comment.
emergency mode should not have any complex logic. what happens if there is a problem, such as underflow, inside of _updateGlobalState() or _updateAccountMP()?
Emergency exit suppousedly should not process any storage or anything, just allow withdraws.
| uint256 aliceInitialRewardBalance = rewardToken.balanceOf(vaults[alice]); | ||
|
|
||
| streamer.enableEmergencyMode(); | ||
|
|
There was a problem hiding this comment.
To properly test emergency mode, cause the code to make a division by zero in the logic, or whatever unexpected fail.
There would be no reason in having an emergency mode for expected cases, that wouldnt be called emergency mode.
|
|
||
| uint256 accountRewards = calculateAccountRewards(msg.sender); | ||
| if (accountRewards > 0) { | ||
| distributeRewards(msg.sender, accountRewards); |
There was a problem hiding this comment.
In contrast with emergency exits on real world, they are usually used when there is a fire in the building. In that case, you don't go grab all your stuff before leave, you just run for your life directly to the exit.
| Account storage account = accounts[msg.sender]; | ||
|
|
||
| if (account.stakedBalance > 0) { | ||
| _updateGlobalState(); |
There was a problem hiding this comment.
Why need to update global state if its not going being used anymore?
This adds a new emergency mode that can be enabled by the owner of the system. When in emergency mode, stakers or
StakeVaults can leave the system immediately.Closes #66
Checklist
Ensure you completed all of the steps below before submitting your pull request:
pnpm adorno?pnpm verify?