Skip to content
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

DM Cleanup #788

Merged
merged 1 commit into from
Oct 4, 2024
Merged

DM Cleanup #788

merged 1 commit into from
Oct 4, 2024

Conversation

ypatil12
Copy link
Collaborator

@ypatil12 ypatil12 commented Oct 2, 2024

This PR updates the storage of the delegationManager to store the operator's magnitude as part of its shares. Thus, we essentially store $$b_n$$ in this doc

In doing so, we only need two types of shares

  1. DepositShares: shares that are deposited in strategyManager/EPM
  2. Shares: A concept native to DM. For a staker, this is withdrawable shares as we need to take into account scaling factors. For an operator, this is the sum of its stakers withdrawable shares. This lives in storage for the operator.

What I'm looking for feedback on:

  • How do we feel about removing custom types and going for depositShares and shares everywhere
  • Naming of depositShares/shares
  • Any issues with updating operatorShares on slashOperator?

Note: this only compiles for SM/DM, not EPM.

TODOs:

  • Fix EPM compilation
  • Clean up naming upon feedback
  • Clean up refs to SlashingLib functions (add params when calling)
  • Update functions for beacon chain slashing & helper function for avs slashing

mapping(address => mapping(IStrategy => Shares)) public stakerStrategyShares;
/// @notice Mapping: staker => Strategy => number of shares which they have deposited. All of these shares
/// may not be withdrawable if the staker has delegated to an operator that has been slashed.
mapping(address => mapping(IStrategy => uint256)) public stakerDepositShares;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ok to break this interface?

Copy link
Contributor

@gpsanant gpsanant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few comments and todos

@gpsanant gpsanant force-pushed the yash/dm-sm-slashing-refactor branch from 8f0e116 to 01fed89 Compare October 4, 2024 04:59
@ypatil12 ypatil12 changed the title DRAFT: DM Cleanup DM Cleanup Oct 4, 2024
@wadealexc wadealexc force-pushed the slashing-magnitudes branch 2 times, most recently from f62d94e to 069c669 Compare October 4, 2024 15:06
Comment on lines +247 to +251
uint64 totalMagnitudeBeforeSlashing = uint64(totalMagnitudes.latest());
totalMagnitudes.push({key: uint32(block.timestamp), value: totalMagnitudeBeforeSlashing - slashedMagnitude});

// 3. Decrease operators shares in the DM
delegation.decreaseOperatorShares(operator, strategies[i], totalMagnitudeBeforeSlashing, uint64(totalMagnitudes.latest()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we calc and reuse rather than reading from state?

uint64 totalMagnitudeAfterSlashing = totalMagnitudeBeforeSlashing - slashedMagnitude

@gpsanant
Copy link
Contributor

gpsanant commented Oct 4, 2024

Will get naming feedback later

@gpsanant gpsanant merged commit c27004e into slashing-magnitudes Oct 4, 2024
5 of 12 checks passed
@gpsanant gpsanant deleted the yash/dm-sm-slashing-refactor branch October 4, 2024 22:14
gpsanant added a commit that referenced this pull request Oct 4, 2024
@gpsanant gpsanant mentioned this pull request Oct 4, 2024
gpsanant added a commit that referenced this pull request Oct 4, 2024
@gpsanant gpsanant restored the yash/dm-sm-slashing-refactor branch October 4, 2024 22:47
@gpsanant gpsanant mentioned this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants