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

fix: delegate shares #1045

Merged
merged 51 commits into from
Feb 13, 2025
Merged

Conversation

eigenmikem
Copy link
Contributor

@eigenmikem eigenmikem commented Jan 22, 2025

Motivation:

Fixes an issue where stakers delegating Beacon Chain ETH from slashed Eigen Pods were able to delegate more shares than they should. Specifically, operators now are delegated a staker's withdrawableShares rather than their depositShares.

Modifications:

  • Changed accounting logic on delegation in DelegationManger.sol
  • DepositScalingFactor now resets when a staker withdraws all their shares, whether through undelegation, redelegation, or a simple withdrawal
  • Changes in StrategyManager.sol, IShareManager.sol, SlashingLib.sol, and EigenPodManager.sol to accommodate new accounting
  • New test files and changes to others to reflect new accounting and invariants
  • Updated docs/SharesAccounting.md

Result:

System is now robust to stakers with arbitrary EigenPod states

@eigenmikem eigenmikem changed the base branch from dev to slashing-magnitudes-fixes January 22, 2025 22:12
wadealexc

This comment was marked as outdated.

uint64 stakerBeaconChainSlashing = eigenPodManager.beaconChainSlashingFactor(staker);

DepositScalingFactor memory dsf = _depositScalingFactor[staker][strategies[i]];
withdrawableShares[i] = dsf.calcWithdrawable(withdrawableShares[i], stakerBeaconChainSlashing);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: since undelegated, staker maxMagnitude is 0 so their slashingFactor only needs to use the BCSF for calculating withdrawable.

Copy link
Collaborator

@wadealexc wadealexc left a comment

Choose a reason for hiding this comment

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

Nice work. There's test cleanup to be done in followup PRs, but this looks great otherwise.

@0xClandestine
Copy link
Member

0xClandestine commented Feb 13, 2025

flaky ci fixed upstream

NOTE: commented out here

@0xClandestine 0xClandestine merged commit f7413d9 into slashing-magnitudes-fixes Feb 13, 2025
11 checks passed
@0xClandestine 0xClandestine deleted the delegate-shares-fix branch February 13, 2025 22:45
ypatil12 added a commit that referenced this pull request Feb 19, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
ypatil12 added a commit that referenced this pull request Feb 20, 2025
**Motivation:**

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <michael@Michaels-MacBook-Pro.local>
Co-authored-by: Michael Sun <michaelsun97@gmail.com>
Co-authored-by: wadealexc <pragma-services@proton.me>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚖️ Audit Fix Audit-related fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants