feat: stake cap - #860
Conversation
afdfb97 to
f03ba6d
Compare
f03ba6d to
e38e47d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f43ab09068
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25a79842d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| currentStakeByOperatorId = new uint256[](operatorsCount); | ||
|
|
||
| IMetaRegistry metaRegistry = ICuratedModule(address(this)).META_REGISTRY(); | ||
| uint256 stakeCap = metaRegistry.maximumStakeCapPerNodeOperator(); |
There was a problem hiding this comment.
Maximum and cap have similar meaning
| uint256 stakeCap = metaRegistry.maximumStakeCapPerNodeOperator(); | |
| uint256 stakeCap = metaRegistry.maximumStakePerNodeOperator(); |
| event GroupWeightsRefreshed(uint256 indexed groupId); | ||
| event OperatorMetadataSet(uint256 indexed nodeOperatorId, OperatorMetadata metadata); | ||
| event NodeOperatorEffectiveWeightChanged(uint256 indexed nodeOperatorId, uint256 oldWeight, uint256 newWeight); | ||
| event MaximumStakeCapPerNodeOperatorSet(uint256 previousCap, uint256 newCap); |
There was a problem hiding this comment.
'per node operator' sounds like it's related to a specific node operator, and I don't get why we need Maximum if it's a cap already, so I'd suggest just StakeCapSet here and update the method names as well.
| if (newCount > 0) { | ||
| uint256 weight = _metaRegistry().getNodeOperatorWeight(nodeOperatorId); | ||
| IMetaRegistry metaRegistry = _metaRegistry(); | ||
| uint256 weight = metaRegistry.getNodeOperatorWeight(nodeOperatorId); | ||
| if (weight == 0) newCount = 0; | ||
|
|
||
| if (newCount > 0) { | ||
| uint256 cap = metaRegistry.maximumStakeCapPerNodeOperator(); | ||
| uint256 currentStake = StakeTracker.getOperatorBalance(_baseStorage(), nodeOperatorId); | ||
| uint256 capCapacity; | ||
| if (currentStake < cap) { | ||
| capCapacity = (cap - currentStake) / ValidatorBalanceLimits.MIN_ACTIVATION_BALANCE; | ||
| } | ||
| if (newCount > capCapacity) newCount = capCapacity; | ||
| } | ||
| } |
There was a problem hiding this comment.
It feels like it can be a separate function, might be a bit cleaner then.
| uint256 currentStake = StakeTracker.getOperatorBalance(_baseStorage(), nodeOperatorId); | ||
| uint256 capCapacity; | ||
| if (currentStake < cap) { | ||
| capCapacity = (cap - currentStake) / ValidatorBalanceLimits.MIN_ACTIVATION_BALANCE; |
There was a problem hiding this comment.
I guess it should be MAX_EB instead? But the granularity might be off and we need ceil div.
| } | ||
|
|
||
| function _setMaximumStakeCapPerNodeOperator(uint256 newCap) internal { | ||
| if (newCap == 0 || newCap % 1 ether != 0) revert InvalidStakeCap(); |
There was a problem hiding this comment.
Let's make it a multiple of MEB.
| } | ||
|
|
||
| /// @inheritdoc IBaseModule | ||
| function reportValidatorBalance( |
There was a problem hiding this comment.
It might be tricky when the method is gone, we might have some unaccounted diff <2 ether. I think it's another argument to consider validator count approach suggested below.
|
|
||
| if (newCount > 0) { | ||
| uint256 cap = metaRegistry.maximumStakeCapPerNodeOperator(); | ||
| uint256 currentStake = StakeTracker.getOperatorBalance(_baseStorage(), nodeOperatorId); |
There was a problem hiding this comment.
I think we can compare validators instead. Like cap / MEB - deposited. And we would not need to update depositable validators every time the operator balance changes.
Description
Stake cap per Node Operator (initial deposit, Top-Up)
Checklist
just coverage)