fix: delegate shares

docs/core/accounting/SharesAccounting.md

@@ -162,6 +162,67 @@ See implementation in:
 * [`StrategyManager.depositIntoStrategy`](../../../src/contracts/core/StrategyManager.sol)
 * [`EigenPodManager.recordBeaconChainETHBalanceUpdate`](../../../src/contracts/pods/EigenPodManager.sol)
 
+
+---
+
+### Delegation
+
+Suppose we have an undelegated staker who decides to delegate to an operator.
+We have the following properties that should be preserved.
+
+#### Operator Level
+
+Operator shares should be increased by the amount of delegatable shares the staker has, this is synonymous to their withdrawable shares $a_n$. Therefore,
+
+$$
+op_{n+1} = op_{n} + a_n
+$$
+
+$$
+= op_{n} + s_n k_n l_n m_n
+$$
+
+
+#### Staker Level
+
+withdrawable shares should remain unchanged
+
+$$
+a_{n+1} = a_n
+$$
+
+deposit shares should remain unchanged
+
+$$
+s_{n+1} = s_n
+$$
+
+beaconChainSlashingFactor and maxMagnitude should also remain unchanged. In this case, since the staker is not delegated, then their maxMagnitude should by default be equal to 1.
+
+$$
+l_{n+1} = l_n
+$$
+
+Now the question is what is the new depositScalingFactor equal to?
+
+$$
+a_{n+1} = a_n
+$$
+
+$$
+=> s_{n+1} k_{n+1} l_{n+1} m_{n+1} = s_n k_n l_n m_n
+$$
+
+$$
+=> s_{n} k_{n+1} l_{n} m_{n+1} = s_n k_n l_n m_n
+$$
+
+$$
+=> k_{n+1} = \frac {k_n m_n} { m_{n+1} }
+$$
+
+Notice how the staker variables that update $k_{n+1}$ and $m_{n+1}$ do not affect previously queued withdrawals and shares received upon withdrawal completion. This is because the maxMagnitude that is looked up is dependent on the operator at the time of the queued withdrawal and the $k_n$ is effectively stored in the scaled shares field.
+
 ---
 
 ### Slashing

src/contracts/core/DelegationManager.sol

@@ -241,7 +241,8 @@ contract DelegationManager is
             strategy: strategy,
             prevDepositShares: prevDepositShares,
             addedShares: addedShares,
-            slashingFactor: slashingFactor
+            slashingFactor: slashingFactor,
+            newDelegation: false
         });
     }
 
@@ -340,24 +341,38 @@ contract DelegationManager is
      *          1) new delegations are not paused (PAUSED_NEW_DELEGATION)
      */
     function _delegate(address staker, address operator) internal onlyWhenNotPaused(PAUSED_NEW_DELEGATION) {
+        // read staker's withdrawable shares and strategies to add to operator's shares
+        // also update the staker depositScalingFactor for each strategy
+        (IStrategy[] memory strategies,) = strategyManager.getDeposits(staker);
+        uint256 podOwnerShares = eigenPodManager.stakerDepositShares(staker, beaconChainETHStrategy);
+        if (podOwnerShares != 0) {
+            // Allocate extra space for beaconChainETHStrategy and shares
+            IStrategy[] memory newStrategies = new IStrategy[](strategies.length + 1);
+            newStrategies[strategies.length] = beaconChainETHStrategy;
+
+            // Copy any strategy manager shares to complete array
+            for (uint256 i = 0; i < strategies.length; i++) {
+                newStrategies[i] = strategies[i];
+            }
+            strategies = newStrategies;
+        }
+        (uint256[] memory withdrawableShares,) = getWithdrawableShares(staker, strategies);
+        //Ignoring beaconChainSlashingFactor here because Beacon Chain ETH DSF already reflects BCSF
+        uint64[] memory slashingFactors = allocationManager.getMaxMagnitudes(operator, strategies);
         // record the delegation relation between the staker and operator, and emit an event
         delegatedTo[staker] = operator;
         emit StakerDelegated(staker, operator);
 
-        // read staker's deposited shares and strategies to add to operator's shares
-        // and also update the staker depositScalingFactor for each strategy
-        (IStrategy[] memory strategies, uint256[] memory depositedShares) = getDepositedShares(staker);
-        uint256[] memory slashingFactors = _getSlashingFactors(staker, operator, strategies);
-
         for (uint256 i = 0; i < strategies.length; ++i) {
             // forgefmt: disable-next-item
             _increaseDelegation({
                 operator: operator, 
                 staker: staker, 
                 strategy: strategies[i],
                 prevDepositShares: uint256(0),
-                addedShares: depositedShares[i],
-                slashingFactor: slashingFactors[i]
+                addedShares: withdrawableShares[i],
+                slashingFactor: slashingFactors[i],
+                newDelegation: true
             });
         }
     }
@@ -592,7 +607,8 @@ contract DelegationManager is
                     strategy: withdrawal.strategies[i],
                     prevDepositShares: prevDepositShares,
                     addedShares: addedShares,
-                    slashingFactor: newSlashingFactors[i]
+                    slashingFactor: newSlashingFactors[i],
+                    newDelegation: false
                 });
             }
         }
@@ -607,24 +623,31 @@ contract DelegationManager is
      * @param prevDepositShares The number of delegated deposit shares the staker had in the strategy prior to the increase
      * @param addedShares The shares added to the staker in the StrategyManager/EigenPodManager
      * @param slashingFactor The current slashing factor for the staker/operator/strategy
+     * @param newDelegation Boolean flag which signifies whether this is a new delegation
      */
     function _increaseDelegation(
         address operator,
         address staker,
         IStrategy strategy,
         uint256 prevDepositShares,
         uint256 addedShares,
-        uint256 slashingFactor
+        uint256 slashingFactor,
+        bool newDelegation
     ) internal {
         // Ensure that the operator has not been fully slashed for a strategy
         // and that the staker has not been fully slashed if it is the beaconChainStrategy
         // This is to prevent a divWad by 0 when updating the depositScalingFactor
         require(slashingFactor != 0, FullySlashed());
 
         // Update the staker's depositScalingFactor. This only results in an update
-        // if the slashing factor has changed for this strategy.
+        // if the slashing factor has changed for this strategy. Dsf update formula
+        // is different in the on delegation case.
         DepositScalingFactor storage dsf = _depositScalingFactor[staker][strategy];
-        dsf.update(prevDepositShares, addedShares, slashingFactor);
+        if (newDelegation) {
+            dsf.updateNewDelegation(slashingFactor);
+        } else {
+            dsf.update(prevDepositShares, addedShares, slashingFactor);
+        }
         emit DepositScalingFactorUpdated(staker, strategy, dsf.scalingFactor());
 
         // If the staker is delegated to an operator, update the operator's shares

src/contracts/libraries/SlashingLib.sol

@@ -136,6 +136,15 @@ library SlashingLib {
         dsf._scalingFactor = newDepositScalingFactor;
     }
 
+    /**
+     * @notice In the case of a new delegation, slashingFactor is just maxMagnitude, because the
+     * BCSF is accounted for in _delegate by adjusting the shares added to the operator
+     */
+    function updateNewDelegation(DepositScalingFactor storage dsf, uint256 slashingFactor) internal {
+        //On delegation, we use the old deposit scaling factor in case that it is non-WAD
+        dsf._scalingFactor = dsf.scalingFactor().divWad(slashingFactor);
+    }
+
     // CONVERSION
 
     function calcWithdrawable(

src/test/integration/IntegrationBase.t.sol

@@ -159,6 +159,22 @@ abstract contract IntegrationBase is IntegrationDeployer {
 
         return result;
     }
+
+    /// @dev Choose a random subset of validators (selects AT LEAST ONE but NOT ALL)
+    function _chooseSubset(uint40[] memory validators) internal returns (uint40[] memory) {
+        require(validators.length >= 2, "Need at least 2 validators to choose subset"); 
+
+        uint40[] memory result = new uint40[](validators.length);
+        uint newLen;
+
+        uint rand = _randUint({ min: 1, max: validators.length ** 2 });
+        for (uint i = 0; i < validators.length; i++) {
+            if (rand >> i & 1 == 1) {
+                result[newLen] = validators[i];
+                newLen++;
+            }
+        }
+    }
 
     function _getTokenName(IERC20 token) internal view returns (string memory) {
         if (token == NATIVE_ETH) {
@@ -697,7 +713,7 @@ abstract contract IntegrationBase is IntegrationDeployer {
         }
     }
 
-    /// @dev Check that the staker's withdrawable shares have decreased by `removedShares`
+    /// @dev Check that the staker's withdrawable shares have increased by `addedShares`
     function assert_Snap_Added_Staker_WithdrawableShares(
         User staker, 
         IStrategy[] memory strategies, 

src/test/integration/IntegrationChecks.t.sol

@@ -230,7 +230,7 @@ contract IntegrationCheckUtils is IntegrationBase {
     ) internal {
         /// Undelegate from an operator
         //
-        // ... check that the staker is undelegated, all strategies from which the staker is deposited are unqeuued,
+        // ... check that the staker is undelegated, all strategies from which the staker is deposited are unqueued,
         //     that the returned root matches the hashes for each strategy and share amounts, and that the staker
         //     and operator have reduced shares
         assertFalse(delegationManager.isDelegated(address(staker)),
@@ -249,6 +249,35 @@ contract IntegrationCheckUtils is IntegrationBase {
             "check_QueuedWithdrawal_State: failed to remove staker withdrawable shares");
     }
 
+    function check_Redelegate_State(
+        User staker, 
+        User operator,
+        IDelegationManagerTypes.Withdrawal[] memory withdrawals,
+        bytes32[] memory withdrawalRoots,
+        IStrategy[] memory strategies,
+        uint[] memory shares 
+    ) internal {
+        /// Redelegate to a new operator
+        //
+        // ... check that the staker is delegated to new operator, all strategies from which the staker is deposited are unqueued,
+        //     that the returned root matches the hashes for each strategy and share amounts, and that the staker
+        //     and operator have reduced shares
+        assertTrue(delegationManager.isDelegated(address(staker)),
+            "check_Undelegate_State: staker should not be delegated");
+        assert_ValidWithdrawalHashes(withdrawals, withdrawalRoots,
+            "check_Undelegate_State: calculated withdrawl should match returned root");
+        assert_AllWithdrawalsPending(withdrawalRoots,
+            "check_Undelegate_State: stakers withdrawal should now be pending");
+        assert_Snap_Added_QueuedWithdrawals(staker, withdrawals,
+            "check_Undelegate_State: staker should have increased nonce by withdrawals.length");
+        assert_Snap_Removed_OperatorShares(operator, strategies, shares,
+            "check_Undelegate_State: failed to remove operator shares");
+        assert_Snap_Removed_Staker_DepositShares(staker, strategies, shares,
+            "check_Undelegate_State: failed to remove staker shares");
+        assert_Snap_Removed_Staker_WithdrawableShares(staker, strategies, shares,
+            "check_QueuedWithdrawal_State: failed to remove staker withdrawable shares");
+    }
+
     /**
      * @notice Overloaded function to check the state after a withdrawal as tokens, accepting a non-user type for the operator.
      * @param staker The staker who completed the withdrawal.
@@ -328,6 +357,28 @@ contract IntegrationCheckUtils is IntegrationBase {
         assert_Snap_Unchanged_StrategyShares(strategies, "strategies should have total shares unchanged");
     }
 
+    function check_Withdrawal_AsShares_Redelegated_State(
+        User staker,
+        User operator,
+        User newOperator,
+        IDelegationManagerTypes.Withdrawal memory withdrawal,
+        IStrategy[] memory strategies,
+        uint[] memory shares
+    ) internal {
+        /// Complete withdrawal(s):
+        // The staker will complete the withdrawal as shares
+        // 
+        // ... check that the withdrawal is not pending, that the token balances of the staker and operator are unchanged,
+        //     that the withdrawer received the expected shares, and that that the total shares of each o
+        //     strategy withdrawn remains unchanged 
+        assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
+        assert_Snap_Unchanged_TokenBalances(staker, "staker should not have any change in underlying token balances");
+        assert_Snap_Unchanged_TokenBalances(operator, "operator should not have any change in underlying token balances");
+        assert_Snap_Added_Staker_DepositShares(staker, strategies, shares, "staker should have received expected shares");
+        assert_Snap_Unchanged_OperatorShares(operator, "operator should have shares unchanged");
+        assert_Snap_Unchanged_StrategyShares(strategies, "strategies should have total shares unchanged");
+    }
+
     /*******************************************************************************
                                  ALLOCATION MANAGER CHECKS
     *******************************************************************************/