[VAULTS]: Granular permissions for operations in Dashboard/Delegation #929
Conversation
| /** | ||
| * @notice Transfers ownership of the staking vault to a new owner. | ||
| * @param _newOwner Address of the new owner. | ||
| */ | ||
| function transferStVaultOwnership(address _newOwner) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| _transferStVaultOwnership(_newOwner); | ||
| function transferOwnership(address _newOwner) external virtual { |
There was a problem hiding this comment.
StVault was there for a reason. Without it, you may think that Dashboard has some ownership, which can be transferred.
There was a problem hiding this comment.
I thought we rejected the "StVault" term
| function transferStVaultOwnership(address _newOwner) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| _transferStVaultOwnership(_newOwner); | ||
| function transferOwnership(address _newOwner) external virtual { | ||
| _authTransferOwnership(); |
There was a problem hiding this comment.
Why not a modifier from the start?
There was a problem hiding this comment.
Because then, we'll have to reimplement the function with a different modifier. In this current impl, we only need to override the auth function.
| } | ||
|
|
||
| /** | ||
| * @notice Disconnects the staking vault from the vault hub. | ||
| */ | ||
| function voluntaryDisconnect() external payable virtual onlyRole(DEFAULT_ADMIN_ROLE) fundAndProceed { | ||
| _voluntaryDisconnect(); | ||
| function voluntaryDisconnect() external payable virtual onlyRole(VOLUNTARY_DISCONNECT_ROLE) fundAndProceed { |
There was a problem hiding this comment.
Do we need a virtual here and generally everywhere?
There was a problem hiding this comment.
Nope, will remove it.
| @@ -290,7 +365,7 @@ contract Dashboard is AccessControlEnumerable { | |||
| function mintWstETH( | |||
| address _recipient, | |||
| uint256 _tokens | |||
| ) external payable virtual onlyRole(DEFAULT_ADMIN_ROLE) fundAndProceed { | |||
| ) external payable virtual onlyRole(MINT_ROLE) fundAndProceed { | |||
There was a problem hiding this comment.
Here we break permission model. Mint role can fund, because of fundAndProceed
There was a problem hiding this comment.
Let's get rid of fundAndProceed maybe?
| @@ -400,7 +465,7 @@ contract Dashboard is AccessControlEnumerable { | |||
| * @notice Rebalances the vault by transferring ether | |||
| * @param _ether Amount of ether to rebalance | |||
| */ | |||
| function rebalanceVault(uint256 _ether) external payable virtual onlyRole(DEFAULT_ADMIN_ROLE) fundAndProceed { | |||
| function rebalanceVault(uint256 _ether) external payable virtual onlyRole(REBALANCE_ROLE) fundAndProceed { | |||
This PR introduces granular permissions for each operation in Dashboard/Delegation to allow flexible role setups.