refactor: pull unrelated operations out and condense library method u…

…sage

script/tasks/complete_withdrawal_from_strategy.s.sol

@@ -67,7 +67,7 @@ contract CompleteWithdrawFromStrategy is Script, Test {
 
         // Get scaled shares for the given amount
         uint256[] memory scaledShares = new uint256[](1);
-        scaledShares[0] = SlashingLib.scaleSharesForQueuedWithdrawal({
+        scaledShares[0] = SlashingLib.scaleForQueueWithdrawal({
             sharesToWithdraw: sharesToWithdraw,
             slashingFactor: slashingFactor
         });

src/contracts/core/DelegationManager.sol

@@ -349,13 +349,18 @@ contract DelegationManager is
         require(newMaxMagnitude < prevMaxMagnitude, MaxMagnitudeCantIncrease());
 
         /// forgefmt: disable-next-item
-        (uint256 sharesToDecrement, uint256 sharesToBurn) = SlashingLib.calcSlashedAmount({
+        uint256 sharesToDecrement = SlashingLib.calcSlashedAmount({
             operatorShares: operatorShares[operator][strategy],
-            slashableSharesInQueue: _getSlashableSharesInQueue(operator, strategy, newMaxMagnitude),
             prevMaxMagnitude: prevMaxMagnitude,
             newMaxMagnitude: newMaxMagnitude
         });
 
+        // While `sharesToDecrement` describes the amount we should directly remove from the operator's delegated
+        // shares, `sharesToBurn` also includes any shares that have been queued for withdrawal and are still
+        // slashable given the withdrawal delay.
+        uint256 sharesToBurn = sharesToDecrement + _getSlashableSharesInQueue(operator, strategy, newMaxMagnitude);
+
+        // Remove shares from operator
         _decreaseDelegation({
             operator: operator,
             staker: address(0), // we treat this as a decrease for the zero address staker
@@ -493,7 +498,7 @@ contract DelegationManager is
             IShareManager shareManager = _getShareManager(withdrawal.strategies[i]);
 
             // Calculate how much slashing to apply, as well as shares to withdraw
-            uint256 sharesToWithdraw = SlashingLib.scaleSharesForCompleteWithdrawal({
+            uint256 sharesToWithdraw = SlashingLib.scaleForCompleteWithdrawal({
                 scaledShares: withdrawal.scaledShares[i],
                 slashingFactor: prevSlashingFactors[i]
             });
@@ -641,7 +646,7 @@ contract DelegationManager is
             sharesToWithdraw[i] = dsf.calcWithdrawable(depositSharesToWithdraw[i], slashingFactors[i]);
 
             // Apply slashing. If the staker or operator has been fully slashed, this will return 0
-            scaledShares[i] = SlashingLib.scaleSharesForQueuedWithdrawal({
+            scaledShares[i] = SlashingLib.scaleForQueueWithdrawal({
                 sharesToWithdraw: sharesToWithdraw[i],
                 slashingFactor: slashingFactors[i]
             });
@@ -757,15 +762,21 @@ contract DelegationManager is
         IStrategy strategy,
         uint64 maxMagnitude
     ) internal view returns (uint256) {
-        uint256 currCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].latest();
-        // Note: this will simply return 0 if no history exists, same for latest() as well
-        uint256 pastCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].upperLookup({
+        // Fetch the cumulative scaled shares sitting in the withdrawal queue both now and before
+        // the withdrawal delay.
+        uint256 curCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].latest();
+        uint256 prevCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].upperLookup({
             key: uint32(block.number) - MIN_WITHDRAWAL_DELAY_BLOCKS
         });
-        uint256 slashableShareInQueue =
-            SlashingLib.scaleSharesForBurning(pastCumulativeScaledShares, currCumulativeScaledShares, maxMagnitude);
 
-        return slashableShareInQueue;
+        // The difference between these values represents the number of scaled shares that entered the
+        // withdrawal queue less than `MIN_WITHDRAWAL_DELAY_BLOCKS` ago. These shares are still slashable,
+        // so we use them to calculate the number of slashable shares in the withdrawal queue.
+        uint256 slashableScaledShares = curCumulativeScaledShares - prevCumulativeScaledShares;
+
+        // Note: for this method, `maxMagnitude` is used as the `slashingFactor`, as the beacon chain ETH strategy
+        // does not support burning. (TODO)
+        return SlashingLib.scaleForCompleteWithdrawal(slashableScaledShares, maxMagnitude);
     }
 
     /// @dev Add to the cumulative withdrawn scaled shares from an operator for a given strategy
@@ -939,7 +950,7 @@ contract DelegationManager is
             uint256[] memory slashingFactors = _getSlashingFactors(staker, operator, withdrawals[i].strategies);
 
             for (uint256 j; j < withdrawals[i].strategies.length; ++j) {
-                shares[i][j] = SlashingLib.scaleSharesForCompleteWithdrawal({
+                shares[i][j] = SlashingLib.scaleForCompleteWithdrawal({
                     scaledShares: withdrawals[i].scaledShares[j],
                     slashingFactor: slashingFactors[i]
                 });

src/contracts/libraries/SlashingLib.sol

@@ -62,7 +62,7 @@ library SlashingLib {
         return dsf._scalingFactor == 0 ? WAD : dsf._scalingFactor;
     }
 
-    function scaleSharesForQueuedWithdrawal(
+    function scaleForQueueWithdrawal(
         uint256 sharesToWithdraw,
         uint256 slashingFactor
     ) internal pure returns (uint256) {
@@ -73,26 +73,10 @@ library SlashingLib {
         return sharesToWithdraw.divWad(slashingFactor);
     }
 
-    function scaleSharesForCompleteWithdrawal(
-        uint256 scaledShares,
-        uint256 slashingFactor
-    ) internal pure returns (uint256) {
+    function scaleForCompleteWithdrawal(uint256 scaledShares, uint256 slashingFactor) internal pure returns (uint256) {
         return scaledShares.mulWad(slashingFactor);
     }
 
-    /**
-     * @notice Scales the share difference between two cumulative scaled shares. This is used
-     * to read the total slashable/burnable shares that are queued for withdrawal from an operator.
-     * This is because shares are still slashable for the duration they are sitting in the withdrawal queue.
-     */
-    function scaleSharesForBurning(
-        uint256 pastCumulativeScaledShares,
-        uint256 currCumulativeScaledShares,
-        uint64 maxMagnitude
-    ) internal pure returns (uint256) {
-        return (currCumulativeScaledShares - pastCumulativeScaledShares).mulWad(maxMagnitude);
-    }
-
     function update(
         DepositScalingFactor storage dsf,
         uint256 existingDepositShares,
@@ -156,12 +140,10 @@ library SlashingLib {
 
     function calcSlashedAmount(
         uint256 operatorShares,
-        uint256 slashableSharesInQueue,
         uint256 prevMaxMagnitude,
         uint256 newMaxMagnitude
-    ) internal pure returns (uint256 sharesToDecrement, uint256 sharesToBurn) {
-        // round up mulDiv so we don't round up sharesToDecrement and overslash
-        sharesToDecrement = operatorShares - operatorShares.mulDiv(newMaxMagnitude, prevMaxMagnitude, Math.Rounding.Up);
-        sharesToBurn = sharesToDecrement + slashableSharesInQueue;
+    ) internal pure returns (uint256) {
+        // round up mulDiv so we don't overslash
+        return operatorShares - operatorShares.mulDiv(newMaxMagnitude, prevMaxMagnitude, Math.Rounding.Up);
     }
 }

src/test/mocks/DelegationManagerMock.sol

@@ -28,13 +28,11 @@ contract DelegationManagerMock is Test {
     function burnOperatorShares(
         address operator,
         IStrategy strategy,
-        uint256 slashableSharesInQueue,
         uint64 prevMaxMagnitude,
         uint64 newMaxMagnitude
     ) external {
-        (uint256 amountSlashed, /*uint256 amountBurned*/) = SlashingLib.calcSlashedAmount({
+        uint256 amountSlashed = SlashingLib.calcSlashedAmount({
             operatorShares: operatorShares[operator][strategy],
-            slashableSharesInQueue: slashableSharesInQueue,
             prevMaxMagnitude: prevMaxMagnitude,
             newMaxMagnitude: newMaxMagnitude
         });

src/test/unit/DelegationUnit.t.sol

@@ -381,7 +381,7 @@ contract DelegationManagerUnitTests is EigenLayerUnitTestSetup, IDelegationManag
         DepositScalingFactor memory _dsf = DepositScalingFactor(delegationManager.depositScalingFactor(staker, strategy));
         uint256 sharesToWithdraw = _dsf.calcWithdrawable(depositSharesToWithdraw, slashingFactor);
 
-        uint256 scaledShares = SlashingLib.scaleSharesForQueuedWithdrawal({
+        uint256 scaledShares = SlashingLib.scaleForQueueWithdrawal({
             sharesToWithdraw: sharesToWithdraw,
             slashingFactor: slashingFactor
         });