feat: dm cleanup (#788)

Co-authored-by: wadealexc <[email protected]>

src/contracts/core/AllocationManager.sol

@@ -244,7 +244,11 @@ contract AllocationManager is
             // 2. update totalMagnitude, get total magnitude and subtract slashedMagnitude
             // this will be reflected in the conversion of delegatedShares to shares in the DM
             Snapshots.History storage totalMagnitudes = _totalMagnitudeUpdate[operator][strategies[i]];
-            totalMagnitudes.push({key: uint32(block.timestamp), value: totalMagnitudes.latest() - slashedMagnitude});
+            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()));
         }
     }
 

src/contracts/core/DelegationManagerStorage.sol

@@ -80,13 +80,12 @@ abstract contract DelegationManagerStorage is IDelegationManager {
     bytes32 internal _DOMAIN_SEPARATOR;
 
     /**
-     * @notice returns the total number of delegatedShares (i.e. shares divided by the `operator`'s
-     * totalMagnitude) in `strategy` that are delegated to `operator`.
-     * @notice Mapping: operator => strategy => total number of delegatedShares in the strategy delegated to the operator.
+     * @notice returns the total number of shares of the operator
+     * @notice Mapping: operator => strategy => total number of shares of the operator
+     * 
      * @dev By design, the following invariant should hold for each Strategy:
      * (operator's delegatedShares in delegation manager) = sum (delegatedShares above zero of all stakers delegated to operator)
      * = sum (delegateable delegatedShares of all stakers delegated to the operator)
-     * @dev FKA `operatorShares`
      */
     mapping(address => mapping(IStrategy => DelegatedShares)) public operatorDelegatedShares;
 
@@ -144,7 +143,7 @@ abstract contract DelegationManagerStorage is IDelegationManager {
     ///       beacon chain scaling factor used to calculate the staker's withdrawable shares in the strategy.
     ///    )
     /// Note that we don't need the beaconChainScalingFactor for non beaconChainETHStrategy strategies, but it's nicer syntactically to keep it.
-    mapping(address => mapping(IStrategy => StakerScalingFactors)) public stakerScalingFactors;
+    mapping(address => mapping(IStrategy => StakerScalingFactors)) public stakerScalingFactor;
 
     // Construction
 

src/contracts/core/StrategyManager.sol

@@ -86,7 +86,7 @@ contract StrategyManager is
      * @param strategy is the specified strategy where deposit is to be made,
      * @param token is the denomination in which the deposit is to be made,
      * @param amount is the amount of token to be deposited in the strategy by the staker
-     * @return shares The amount of new shares in the `strategy` created as part of the action.
+     * @return depositedShares The amount of new shares in the `strategy` created as part of the action.
      * @dev The `msg.sender` must have previously approved this contract to transfer at least `amount` of `token` on their behalf.
      *
      * WARNING: Depositing tokens that allow reentrancy (eg. ERC-777) into a strategy is not recommended.  This can lead to attack vectors
@@ -96,8 +96,8 @@ contract StrategyManager is
         IStrategy strategy,
         IERC20 token,
         uint256 amount
-    ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (OwnedShares shares) {
-        shares = _depositIntoStrategy(msg.sender, strategy, token, amount);
+    ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 depositedShares) {
+        depositedShares = _depositIntoStrategy(msg.sender, strategy, token, amount);
     }
 
     /**
@@ -112,7 +112,7 @@ contract StrategyManager is
      * @param expiry the timestamp at which the signature expires
      * @param signature is a valid signature from the `staker`. either an ECDSA signature if the `staker` is an EOA, or data to forward
      * following EIP-1271 if the `staker` is a contract
-     * @return shares The amount of new shares in the `strategy` created as part of the action.
+     * @return depositedShares The amount of new shares in the `strategy` created as part of the action.
      * @dev The `msg.sender` must have previously approved this contract to transfer at least `amount` of `token` on their behalf.
      * @dev A signature is required for this function to eliminate the possibility of griefing attacks, specifically those
      * targeting stakers who may be attempting to undelegate.
@@ -127,7 +127,7 @@ contract StrategyManager is
         address staker,
         uint256 expiry,
         bytes memory signature
-    ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (OwnedShares shares) {
+    ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 depositedShares) {
         require(expiry >= block.timestamp, SignatureExpired());
         // calculate struct hash, then increment `staker`'s nonce
         uint256 nonce = nonces[staker];
@@ -148,20 +148,21 @@ contract StrategyManager is
         EIP1271SignatureUtils.checkSignature_EIP1271(staker, digestHash, signature);
 
         // deposit the tokens (from the `msg.sender`) and credit the new shares to the `staker`
-        shares = _depositIntoStrategy(staker, strategy, token, amount);
+        depositedShares = _depositIntoStrategy(staker, strategy, token, amount);
     }
 
     /// @notice Used by the DelegationManager to remove a Staker's shares from a particular strategy when entering the withdrawal queue
-    function removeShares(address staker, IStrategy strategy, Shares shares) external onlyDelegationManager {
-        _removeShares(staker, strategy, shares);
+    function removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) external onlyDelegationManager {
+        _removeDepositShares(staker, strategy, depositSharesToRemove);
     }
 
     /// @notice Used by the DelegationManager to award a Staker some shares that have passed through the withdrawal queue
-    function addOwnedShares(
+    /// @dev    Specifically, this function is called when a withdrawal is completed as shares. 
+    function addShares(
         address staker,
         IStrategy strategy,
         IERC20 token,
-        OwnedShares ownedShares
+        uint256 shares
     ) external onlyDelegationManager {
         _addOwnedShares(staker, token, strategy, ownedShares);
     }
@@ -173,9 +174,9 @@ contract StrategyManager is
         address staker,
         IStrategy strategy,
         IERC20 token,
-        OwnedShares ownedShares
+        uint256 shares
     ) external onlyDelegationManager {
-        strategy.withdraw(staker, token, ownedShares.unwrap());
+        strategy.withdraw(staker, token, shares);
     }
 
     /**
@@ -231,24 +232,32 @@ contract StrategyManager is
      * @param strategy The Strategy in which the `staker` is receiving shares
      * @param ownedShares The amount of ownedShares to grant to the `staker`
      * @dev In particular, this function calls `delegation.increaseDelegatedShares(staker, strategy, shares)` to ensure that all
-     * delegated shares are tracked, increases the stored share amount in `stakerStrategyShares[staker][strategy]`, and adds `strategy`
+     * delegated shares are tracked, increases the stored share amount in `stakerDepositShares[staker][strategy]`, and adds `strategy`
      * to the `staker`'s list of strategies, if it is not in the list already.
      */
     function _addOwnedShares(address staker, IERC20 token, IStrategy strategy, OwnedShares ownedShares) internal {
         // sanity checks on inputs
         require(staker != address(0), StakerAddressZero());
         require(ownedShares.unwrap() != 0, SharesAmountZero());
 
-        Shares existingShares = stakerStrategyShares[staker][strategy];
+        uint256 existingShares = stakerDepositShares[staker][strategy];
 
-        // if they dont have existing ownedShares of this strategy, add it to their strats
-        if (existingShares.unwrap() == 0) {
+        // if they dont have existingShares of this strategy, add it to their strats
+        if (existingShares == 0) {
             require(stakerStrategyList[staker].length < MAX_STAKER_STRATEGY_LIST_LENGTH, MaxStrategiesExceeded());
             stakerStrategyList[staker].push(strategy);
         }
 
-        // add the returned ownedShares to their existing shares for this strategy
-        stakerStrategyShares[staker][strategy] = existingShares.add(ownedShares.unwrap()).wrapShares();
+        // add the returned depositedShares to their existing shares for this strategy
+        stakerDepositShares[staker][strategy] = existingShares + shares;
+
+        // Increase shares delegated to operator, if needed
+        delegation.increaseDelegatedShares({
+            staker: staker,
+            strategy: strategy,
+            existingDepositShares: existingShares,
+            addedShares: shares
+        });
 
         // Increase shares delegated to operator, if needed
         delegation.increaseDelegatedShares({
@@ -285,33 +294,33 @@ contract StrategyManager is
         // add the returned shares to the staker's existing shares for this strategy
         _addOwnedShares(staker, token, strategy, ownedShares);
 
-        return ownedShares;
+        return shares;
     }
 
     /**
-     * @notice Decreases the shares that `staker` holds in `strategy` by `shareAmount`.
+     * @notice Decreases the shares that `staker` holds in `strategy` by `depositSharesToRemove`.
      * @param staker The address to decrement shares from
      * @param strategy The strategy for which the `staker`'s shares are being decremented
-     * @param shareAmount The amount of shares to decrement
+     * @param depositSharesToRemove The amount of deposit shares to decrement
      * @dev If the amount of shares represents all of the staker`s shares in said strategy,
      * then the strategy is removed from stakerStrategyList[staker] and 'true' is returned. Otherwise 'false' is returned.
      */
-    function _removeShares(address staker, IStrategy strategy, Shares shareAmount) internal returns (bool) {
+    function _removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) internal returns (bool) {
         // sanity checks on inputs
-        require(shareAmount.unwrap() != 0, SharesAmountZero());
+        require(depositSharesToRemove != 0, SharesAmountZero());
 
         //check that the user has sufficient shares
-        Shares userShares = stakerStrategyShares[staker][strategy];
-
-        require(shareAmount.unwrap() <= userShares.unwrap(), SharesAmountTooHigh());
+        uint256 userDepositShares = stakerDepositShares[staker][strategy];
 
-        userShares = userShares.sub(shareAmount.unwrap()).wrapShares();
+        require(depositSharesToRemove <= userDepositShares, SharesAmountTooHigh());
 
+        userDepositShares = userDepositShares - depositSharesToRemove;
+
         // subtract the shares from the staker's existing shares for this strategy
-        stakerStrategyShares[staker][strategy] = userShares;
+        stakerDepositShares[staker][strategy] = userDepositShares;
 
         // if no existing shares, remove the strategy from the staker's dynamic array of strategies
-        if (userShares.unwrap() == 0) {
+        if (userDepositShares == 0) {
             _removeStrategyFromStakerStrategyList(staker, strategy);
 
             // return true in the event that the strategy was removed from stakerStrategyList[staker]
@@ -357,20 +366,26 @@ contract StrategyManager is
     // VIEW FUNCTIONS
 
     /**
-     * @notice Get all details on the staker's deposits and corresponding shares
+     * @notice Get all details on the staker's strategies and shares deposited into
      * @param staker The staker of interest, whose deposits this function will fetch
      * @return (staker's strategies, shares in these strategies)
      */
     function getDeposits(
         address staker
     ) external view returns (IStrategy[] memory, Shares[] memory) {
         uint256 strategiesLength = stakerStrategyList[staker].length;
-        Shares[] memory shares = new Shares[](strategiesLength);
+        uint256[] memory depositedShares = new uint256[](strategiesLength);
 
         for (uint256 i = 0; i < strategiesLength; ++i) {
-            shares[i] = stakerStrategyShares[staker][stakerStrategyList[staker][i]];
+            depositedShares[i] = stakerDepositShares[staker][stakerStrategyList[staker][i]];
         }
-        return (stakerStrategyList[staker], shares);
+        return (stakerStrategyList[staker], depositedShares);
+    }
+
+    function getStakerStrategyList(
+        address staker
+    ) external view returns (IStrategy[] memory) {
+        return stakerStrategyList[staker];
     }
 
     function getStakerStrategyList(

src/contracts/core/StrategyManagerStorage.sol

@@ -59,10 +59,9 @@ abstract contract StrategyManagerStorage is IStrategyManager {
      * This variable was migrated to the DelegationManager instead.
      */
     uint256 private __deprecated_withdrawalDelayBlocks;
-
-    /// @notice Mapping: staker => Strategy => number of shares which they currently hold
-    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;
     /// @notice Mapping: staker => array of strategies in which they have nonzero shares
     mapping(address => IStrategy[]) public stakerStrategyList;