-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(RewardsStreamerMP): introduce leave()
function
#77
Conversation
@@ -47,7 +47,8 @@ rule accountCanOnlyLeaveInEmergencyMode(method f) { | |||
f@withrevert(e, args); | |||
bool isReverted = lastReverted; | |||
|
|||
assert !isReverted => isViewFunction(f) || | |||
assert !isReverted => f.selector == sig:streamer.leave().selector || | |||
isViewFunction(f) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave()
will not revert but should still be protected by a rule that ensures that, if leave()
was called and the accounting was updated, the stake manager was not trusted by the caller (vault).
If it was trusted and the vault still manages to leave the system, this is a bug. Vault should properly unstake in that case.
revert StakeVault__StakeManagerImplementationNotTrusted(); | ||
} | ||
_; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modifier prevents users to stake/unstake/lock etc as soon as an upgrade on the stake manager has happened.
We basically force them to either leave or accept the upgrade by explicitly trusting the new stake manager (will be done in a future commit).
bool success = STAKING_TOKEN.transferFrom(_source, address(this), _amount); | ||
if (!success) { | ||
revert StakeVault__StakingFailed(); | ||
} | ||
|
||
stakeManager.stake(_amount, _seconds); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I just moved the call up in the function to adhere to CEI pattern
|
||
function _stakeManagerImplementationTrusted() internal pure virtual returns (bool) { | ||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will change in future commits, once #73 lands. Because then we can check if the stake manager's implementation address has changed (at which point the manager is no longer trusted unless explicitly trusted)
f5109a9
to
ff84621
Compare
This function allows users to `leave()` the system if they can't or don't want to trust the stake manager. This is the case when the owner of the stake manager performs an upgrade (upgradeability lands in future changes). In case of such an upgrade, the stake manager will be marked as not trusted which prevents the user from staking, unstaking, locking etc. The user can then either explicitly trust stake manager (will happen in future changes) to enable the vault's functionality again, or, `leave()` the system at which point it will try to perform a benign `leave()` operation and then move the funds out of the vault. Closes #66
18fe9d7
to
7ad3428
Compare
Closing this one in favor of #78 |
This function allows users to
leave()
the system if they can't or don't want to trust the stake manager. This is the case when the owner of the stake manager performs an upgrade (upgradeability lands in future changes).In case of such an upgrade, the stake manager will be marked as not trusted which prevents the user from staking, unstaking, locking etc.
The user can then either explicitly trust stake manager (will happen in future changes) to enable the vault's functionality again, or,
leave()
the system at which point it will try to perform a benignleave()
operation and then move the funds out of the vault.Closes #66
Checklist
Ensure you completed all of the steps below before submitting your pull request:
pnpm adorno
?pnpm verify
?