From bafea9dda2a26b0494b9dd0628b9285d2b4b127c Mon Sep 17 00:00:00 2001 From: Gautham Anant <32277907+gpsanant@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:46:40 -0700 Subject: [PATCH] Revert "feat: dm cleanup (#788)" (#799) This reverts commit c27004ea11943e488af22e0b5f2b4b45ae105550. --- src/contracts/core/AllocationManager.sol | 6 +- src/contracts/core/DelegationManager.sol | 256 ++++++++++-------- .../core/DelegationManagerStorage.sol | 9 +- src/contracts/core/StrategyManager.sol | 81 +++--- src/contracts/core/StrategyManagerStorage.sol | 7 +- .../interfaces/IDelegationManager.sol | 61 ++--- src/contracts/interfaces/IEigenPodManager.sol | 2 +- src/contracts/interfaces/IShareManager.sol | 13 +- src/contracts/interfaces/IStrategyManager.sol | 11 +- src/contracts/libraries/SlashingLib.sol | 255 ++++++++++------- src/contracts/pods/EigenPodManager.sol | 128 +++++---- src/contracts/pods/EigenPodManagerStorage.sol | 7 +- src/contracts/strategies/StrategyBase.sol | 2 +- 13 files changed, 439 insertions(+), 399 deletions(-) diff --git a/src/contracts/core/AllocationManager.sol b/src/contracts/core/AllocationManager.sol index 80d53b450..8748c860b 100644 --- a/src/contracts/core/AllocationManager.sol +++ b/src/contracts/core/AllocationManager.sol @@ -244,11 +244,7 @@ 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]]; - 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())); + totalMagnitudes.push({key: uint32(block.timestamp), value: totalMagnitudes.latest() - slashedMagnitude}); } } diff --git a/src/contracts/core/DelegationManager.sol b/src/contracts/core/DelegationManager.sol index dd47a1f36..559202130 100644 --- a/src/contracts/core/DelegationManager.sol +++ b/src/contracts/core/DelegationManager.sol @@ -43,11 +43,6 @@ contract DelegationManager is _; } - modifier onlyAllocationManager() { - require(msg.sender == address(allocationManager), OnlyAllocationManager()); - _; - } - /** * * INITIALIZING FUNCTIONS @@ -231,9 +226,9 @@ contract DelegationManager is CallerCannotUndelegate() ); - // Gather strategies and shares from the staker. Calculate depositedShares to remove from operator during undelegation + // Gather strategies and shares from the staker. Calculate delegatedShares to remove from operator during undelegation // Undelegation removes ALL currently-active strategies and shares - (IStrategy[] memory strategies, uint256[] memory depositedShares) = getDepositedShares(staker); + (IStrategy[] memory strategies, OwnedShares[] memory ownedShares) = getDelegatableShares(staker); // emit an event if this action was not initiated by the staker themselves if (msg.sender != staker) { @@ -244,7 +239,7 @@ contract DelegationManager is emit StakerUndelegated(staker, operator); delegatedTo[staker] = address(0); - // if no deposited shares, return an empty array, and don't queue a withdrawal + // if no delegatable shares, return an empty array, and don't queue a withdrawal if (strategies.length == 0) { withdrawalRoots = new bytes32[](0); } else { @@ -253,27 +248,23 @@ contract DelegationManager is for (uint256 i = 0; i < strategies.length; i++) { IStrategy[] memory singleStrategy = new IStrategy[](1); - uint256[] memory singleShares = new uint256[](1); + OwnedShares[] memory singleOwnedShare = new OwnedShares[](1); uint64[] memory singleTotalMagnitude = new uint64[](1); singleStrategy[0] = strategies[i]; - // TODO: this part is a bit gross, can we make it better? - singleShares[0] = depositedShares[i].toShares( - stakerScalingFactor[staker][strategies[i]], - totalMagnitudes[i] - ); + singleOwnedShare[0] = ownedShares[i]; singleTotalMagnitude[0] = totalMagnitudes[i]; withdrawalRoots[i] = _removeSharesAndQueueWithdrawal({ staker: staker, operator: operator, strategies: singleStrategy, - sharesToWithdraw: singleShares, + ownedSharesToWithdraw: singleOwnedShare, totalMagnitudes: singleTotalMagnitude }); // all shares and queued withdrawn and no delegated operator // reset staker's depositScalingFactor to default - stakerScalingFactor[staker][strategies[i]].depositScalingFactor = WAD; + stakerScalingFactors[staker][strategies[i]].depositScalingFactor = WAD; } } @@ -310,7 +301,7 @@ contract DelegationManager is staker: msg.sender, operator: operator, strategies: queuedWithdrawalParams[i].strategies, - sharesToWithdraw: queuedWithdrawalParams[i].ownedShares, + ownedSharesToWithdraw: queuedWithdrawalParams[i].ownedShares, totalMagnitudes: totalMagnitudes }); } @@ -359,9 +350,9 @@ contract DelegationManager is * the delegated delegatedShares. The staker's depositScalingFactor is updated here. * @param staker The address to increase the delegated shares for their operator. * @param strategy The strategy in which to increase the delegated shares. - * @param existingDepositShares The number of deposit shares the staker already has in the strategy. This is the shares amount stored in the + * @param existingShares The number of deposit shares the staker already has in the strategy. This is the shares amount stored in the * StrategyManager/EigenPodManager for the staker's shares. - * @param addedDepositShares The number of deposit shares added to the staker's shares in the strategy + * @param addedOwnedShares The number of shares to added to the staker's shares in the strategy * * @dev *If the staker is actively delegated*, then increases the `staker`'s delegated delegatedShares in `strategy`. * Otherwise does nothing. @@ -370,8 +361,8 @@ contract DelegationManager is function increaseDelegatedShares( address staker, IStrategy strategy, - uint256 existingDepositShares, - uint256 addedDepositShares + Shares existingShares, + OwnedShares addedOwnedShares ) external onlyStrategyManagerOrEigenPodManager { // if the staker is delegated to an operator if (isDelegated(staker)) { @@ -383,8 +374,8 @@ contract DelegationManager is operator: operator, staker: staker, strategy: strategy, - existingDepositShares: existingDepositShares, - addedDepositShares: addedDepositShares, + existingShares: existingShares, + addedOwnedShares: addedOwnedShares, totalMagnitude: totalMagnitude }); } @@ -394,7 +385,7 @@ contract DelegationManager is * @notice Decreases a native restaker's delegated share balance in a strategy due to beacon chain slashing. This updates their beaconChainScalingFactor. * Their operator's stakeShares are also updated (if they are delegated). * @param staker The address to increase the delegated stakeShares for their operator. - * @param existingDepositShares The number of shares the staker already has in the EPM. This does not change upon decreasing shares. + * @param existingShares The number of shares the staker already has in the EPM. This does not change upon decreasing shares. * @param proportionOfOldBalance The current pod owner shares proportion of the previous pod owner shares * * @dev *If the staker is actively delegated*, then decreases the `staker`'s delegated stakeShares in `strategy` by `proportionPodBalanceDecrease` proportion. Otherwise does nothing. @@ -402,42 +393,33 @@ contract DelegationManager is */ function decreaseBeaconChainScalingFactor( address staker, - uint256 existingDepositShares, + Shares existingShares, uint64 proportionOfOldBalance ) external onlyEigenPodManager { + DelegatedShares delegatedSharesBefore = + existingShares.toDelegatedShares(stakerScalingFactors[staker][beaconChainETHStrategy]); + // decrease the staker's beaconChainScalingFactor proportionally - address operator = delegatedTo[staker]; - uint64 totalMagnitude = allocationManager.getTotalMagnitude(operator, beaconChainETHStrategy); + // forgefmt: disable-next-item + stakerScalingFactors[staker][beaconChainETHStrategy].decreaseBeaconChainScalingFactor(proportionOfOldBalance); - uint256 sharesBefore = existingDepositShares.toShares(stakerScalingFactor[staker][beaconChainETHStrategy], totalMagnitude); - stakerScalingFactor[staker][beaconChainETHStrategy].decreaseBeaconChainScalingFactor(proportionOfOldBalance); - uint256 sharesAfter = existingDepositShares.toShares(stakerScalingFactor[staker][beaconChainETHStrategy], totalMagnitude); + DelegatedShares delegatedSharesAfter = + existingShares.toDelegatedShares(stakerScalingFactors[staker][beaconChainETHStrategy]); // if the staker is delegated to an operators if (isDelegated(staker)) { + address operator = delegatedTo[staker]; + // subtract strategy shares from delegated scaled shares _decreaseDelegation({ operator: operator, staker: staker, strategy: beaconChainETHStrategy, - // TODO: fix this - operatorSharesToDecrease: sharesBefore - sharesAfter + delegatedShares: delegatedSharesBefore.sub(delegatedSharesAfter) }); } } - /** - * @notice Decreases the operators shares in storage after a slash - * @param operator The operator to decrease shares for - * @param strategy The strategy to decrease shares for - * @param previousMagnitude The magnitude before the slash - * @param newMagnitude The magnitude after the slash - * @dev Callable only by the AllocationManager - */ - function decreaseOperatorShares(address operator, IStrategy strategy, uint64 previousMagnitude, uint64 newMagnitude) external onlyAllocationManager { - operatorShares[operator][strategy] = operatorShares[operator][strategy].decreaseOperatorShares(previousMagnitude, newMagnitude); - } - /** * * INTERNAL FUNCTIONS @@ -510,9 +492,9 @@ contract DelegationManager is delegatedTo[staker] = operator; emit StakerDelegated(staker, operator); - // read staker's deposited shares and strategies to add to operator's shares + // read staker's delegatable shares and strategies to add to operator's delegatedShares // and also update the staker depositScalingFactor for each strategy - (IStrategy[] memory strategies, uint256[] memory depositedShares) = getDepositedShares(staker); + (IStrategy[] memory strategies, OwnedShares[] memory ownedShares) = getDelegatableShares(staker); uint64[] memory totalMagnitudes = allocationManager.getTotalMagnitudes(operator, strategies); for (uint256 i = 0; i < strategies.length; ++i) { @@ -521,8 +503,8 @@ contract DelegationManager is operator: operator, staker: staker, strategy: strategies[i], - existingDepositShares: uint256(0), - addedDepositShares: depositedShares[i], + existingShares: uint256(0).wrapShares(), + addedOwnedShares: ownedShares[i], totalMagnitude: totalMagnitudes[i] }); } @@ -558,10 +540,9 @@ contract DelegationManager is for (uint256 i = 0; i < withdrawal.strategies.length; i++) { IShareManager shareManager = _getShareManager(withdrawal.strategies[i]); - uint256 sharesToWithdraw = withdrawal.scaledSharesToWithdraw[i].scaleSharesForCompleteWithdrawal( - stakerScalingFactor[withdrawal.staker][withdrawal.strategies[i]], - totalMagnitudes[i] - ); + OwnedShares ownedSharesToWithdraw = withdrawal.delegatedShares[i].scaleForCompleteWithdrawal( + stakerScalingFactors[withdrawal.staker][withdrawal.strategies[i]] + ).toOwnedShares(totalMagnitudes[i]); if (receiveAsTokens) { // Withdraws `shares` in `strategy` to `withdrawer`. If the shares are virtual beaconChainETH shares, @@ -571,15 +552,15 @@ contract DelegationManager is staker: withdrawal.staker, strategy: withdrawal.strategies[i], token: tokens[i], - shares: sharesToWithdraw + ownedShares: ownedSharesToWithdraw }); } else { // Award shares back in StrategyManager/EigenPodManager. - shareManager.addShares({ + shareManager.addOwnedShares({ staker: withdrawal.staker, strategy: withdrawal.strategies[i], token: tokens[i], - shares: sharesToWithdraw + ownedShares: ownedSharesToWithdraw }); } } @@ -590,62 +571,58 @@ contract DelegationManager is } /** - * @notice Increases `operator`s depositedShares in `strategy` based on staker's addedDepositShares + * @notice Increases `operator`s delegated delegatedShares in `strategy` based on staker's added shares and operator's totalMagnitude * and increases the staker's depositScalingFactor for the strategy. * @param operator The operator to increase the delegated delegatedShares for * @param staker The staker to increase the depositScalingFactor for * @param strategy The strategy to increase the delegated delegatedShares and the depositScalingFactor for - * @param existingDepositShares The number of deposit shares the staker already has in the strategy. - * @param addedDepositShares The shares added to the staker in the StrategyManager/EigenPodManager + * @param existingShares The number of deposit shares the staker already has in the strategy. + * @param addedOwnedShares The shares added to the staker in the StrategyManager/EigenPodManager * @param totalMagnitude The current total magnitude of the operator for the strategy */ function _increaseDelegation( address operator, address staker, IStrategy strategy, - uint256 existingDepositShares, - uint256 addedDepositShares, + Shares existingShares, + OwnedShares addedOwnedShares, uint64 totalMagnitude ) internal { - //TODO: double check ordering here - operatorShares[operator][strategy] += addedDepositShares.toShares(stakerScalingFactor[staker][strategy], totalMagnitude); - - uint256 newDepositScalingFactor; - if (existingDepositShares == 0) { - newDepositScalingFactor = WAD / totalMagnitude; - } else { - newDepositScalingFactor = SlashingLib.calculateNewDepositScalingFactor( - existingDepositShares, - addedDepositShares, - stakerScalingFactor[staker][strategy], - totalMagnitude - ); - } + _updateDepositScalingFactor({ + staker: staker, + strategy: strategy, + existingShares: existingShares, + addedOwnedShares: addedOwnedShares, + totalMagnitude: totalMagnitude + }); - // update the staker's depositScalingFactor - stakerScalingFactor[staker][strategy].depositScalingFactor = newDepositScalingFactor; + // based on total magnitude, update operators delegatedShares + DelegatedShares delegatedShares = addedOwnedShares.toDelegatedShares(totalMagnitude); + // forgefmt: disable-next-line + operatorDelegatedShares[operator][strategy] = operatorDelegatedShares[operator][strategy].add(delegatedShares); // TODO: What to do about event wrt scaling? - emit OperatorSharesIncreased(operator, staker, strategy, addedDepositShares); + emit OperatorSharesIncreased(operator, staker, strategy, delegatedShares); } /** - * @notice Decreases `operator`s shares in `strategy` based on staker's removed shares and operator's totalMagnitude + * @notice Decreases `operator`s delegated delegatedShares in `strategy` based on staker's removed shares and operator's totalMagnitude * @param operator The operator to decrease the delegated delegated shares for * @param staker The staker to decrease the delegated delegated shares for * @param strategy The strategy to decrease the delegated delegated shares for - * @param operatorSharesToDecrease The delegatedShares to remove from the operator's delegated shares + * @param delegatedShares The delegatedShares to remove from the operator's delegated shares */ function _decreaseDelegation( address operator, address staker, IStrategy strategy, - uint256 operatorSharesToDecrease + DelegatedShares delegatedShares ) internal { - // Decrement operator shares - operatorShares[operator][strategy] -= operatorSharesToDecrease; - - emit OperatorSharesDecreased(operator, staker, strategy, operatorSharesToDecrease); + // based on total magnitude, decrement operator's delegatedShares + operatorDelegatedShares[operator][strategy] = operatorDelegatedShares[operator][strategy].sub(delegatedShares); + + // TODO: What to do about event wrt scaling? + emit OperatorSharesDecreased(operator, staker, strategy, delegatedShares); } /** @@ -657,24 +634,27 @@ contract DelegationManager is address staker, address operator, IStrategy[] memory strategies, - uint256[] memory sharesToWithdraw, + OwnedShares[] memory ownedSharesToWithdraw, uint64[] memory totalMagnitudes ) internal returns (bytes32) { require(staker != address(0), InputAddressZero()); require(strategies.length != 0, InputArrayLengthZero()); - uint256[] memory scaledSharesToWithdraw = new uint256[](strategies.length); + DelegatedShares[] memory delegatedSharesToWithdraw = new DelegatedShares[](strategies.length); // Remove shares from staker and operator // Each of these operations fail if we attempt to remove more shares than exist for (uint256 i = 0; i < strategies.length; ++i) { IShareManager shareManager = _getShareManager(strategies[i]); - - uint256 depositSharesToRemove = sharesToWithdraw[i].toDepositShares(stakerScalingFactor[staker][strategies[i]], totalMagnitudes[i]); - // TODO: maybe have a getter to get shares for all strategies, + + // delegatedShares for staker to place into queueWithdrawal + DelegatedShares delegatedSharesToRemove = ownedSharesToWithdraw[i].toDelegatedShares(totalMagnitudes[i]); + // TODO: should this include beaconChainScalingFactor? + Shares sharesToWithdraw = delegatedSharesToRemove.toShares(stakerScalingFactors[staker][strategies[i]]); + // TODO: maybe have a getter to get shares for all strategies, like getDelegatableShares // check sharesToWithdraw is valid // but for inputted strategies - uint256 depositSharesWithdrawable = shareManager.stakerDepositShares(staker, strategies[i]); - require(depositSharesToRemove <= depositSharesWithdrawable, WithdrawalExeedsMax()); + Shares sharesWithdrawable = shareManager.stakerStrategyShares(staker, strategies[i]); + require(sharesToWithdraw.unwrap() <= sharesWithdrawable.unwrap(), WithdrawalExeedsMax()); // Similar to `isDelegated` logic if (operator != address(0)) { @@ -683,15 +663,16 @@ contract DelegationManager is operator: operator, staker: staker, strategy: strategies[i], - operatorSharesToDecrease: sharesToWithdraw[i] + delegatedShares: delegatedSharesToRemove }); } - scaledSharesToWithdraw[i] = sharesToWithdraw[i].scaleSharesForQueuedWithdrawal(stakerScalingFactor[staker][strategies[i]], totalMagnitudes[i]); + delegatedSharesToWithdraw[i] = + delegatedSharesToRemove.scaleForQueueWithdrawal(stakerScalingFactors[staker][strategies[i]]); // Remove active shares from EigenPodManager/StrategyManager // EigenPodManager: this call will revert if it would reduce the Staker's virtual beacon chain ETH shares below zero // StrategyManager: this call will revert if `sharesToDecrement` exceeds the Staker's current deposit shares in `strategies[i]` - shareManager.removeDepositShares(staker, strategies[i], depositSharesToRemove); + shareManager.removeShares(staker, strategies[i], sharesToWithdraw); } // Create queue entry and increment withdrawal nonce @@ -705,7 +686,7 @@ contract DelegationManager is nonce: nonce, startTimestamp: uint32(block.timestamp), strategies: strategies, - scaledSharesToWithdraw: scaledSharesToWithdraw + delegatedShares: delegatedSharesToWithdraw }); bytes32 withdrawalRoot = calculateWithdrawalRoot(withdrawal); @@ -741,6 +722,48 @@ contract DelegationManager is * */ + /** + * @notice helper to calculate the new depositScalingFactor after adding shares. This is only used + * when a staker is depositing through the StrategyManager or EigenPodManager. A staker's depositScalingFactor + * is only updated when they have new deposits and their shares are being increased. + */ + function _updateDepositScalingFactor( + address staker, + IStrategy strategy, + uint64 totalMagnitude, + Shares existingShares, + OwnedShares addedOwnedShares + ) internal { + uint256 newDepositScalingFactor; + if (existingShares.unwrap() == 0) { + newDepositScalingFactor = WAD / totalMagnitude; + } else { + // otherwise since + // + // newShares + // = existingShares + addedShares + // = existingPrincipalShares.toDelegatedShares(stakerScalingFactors[staker][strategy).toOwnedShares(totalMagnitude) + addedShares + // + // and it also is + // + // newShares + // = newPrincipalShares.toDelegatedShares(stakerScalingFactors[staker][strategy).toOwnedShares(totalMagnitude) + // = newPrincipalShares * newDepositScalingFactor / WAD * beaonChainScalingFactor / WAD * totalMagnitude / WAD + // = (existingPrincipalShares + addedShares) * newDepositScalingFactor / WAD * beaonChainScalingFactor / WAD * totalMagnitude / WAD + // + // we can solve for + // + OwnedShares existingOwnedShares = + existingShares.toDelegatedShares(stakerScalingFactors[staker][strategy]).toOwnedShares(totalMagnitude); + newDepositScalingFactor = existingOwnedShares.add(addedOwnedShares).unwrap().divWad( + existingShares.unwrap() + addedOwnedShares.unwrap() + ).divWad(stakerScalingFactors[staker][strategy].getBeaconChainScalingFactor()).divWad(totalMagnitude); + } + + // update the staker's depositScalingFactor + stakerScalingFactors[staker][strategy].depositScalingFactor = newDepositScalingFactor; + } + function _getShareManager( IStrategy strategy ) internal view returns (IShareManager) { @@ -815,16 +838,22 @@ contract DelegationManager is return _operatorDetails[operator].stakerOptOutWindowBlocks; } + /// @notice a legacy function that returns the total delegated shares for an operator and strategy + function operatorShares(address operator, IStrategy strategy) public view returns (uint256) { + uint64 totalMagnitude = allocationManager.getTotalMagnitude(operator, strategy); + return operatorDelegatedShares[operator][strategy].toOwnedShares(totalMagnitude).unwrap(); + } + /** * @notice Given a staker and a set of strategies, return the shares they can queue for withdrawal. * This value depends on which operator the staker is delegated to. * The shares amount returned is the actual amount of Strategy shares the staker would receive (subject * to each strategy's underlying shares to token ratio). */ - function getWithdrawableShares( + function getDelegatableShares( address staker, IStrategy[] memory strategies - ) public view returns (uint256[] memory withdrawableShares) { + ) public view returns (OwnedShares[] memory ownedShares) { address operator = delegatedTo[staker]; for (uint256 i = 0; i < strategies.length; ++i) { IShareManager shareManager = _getShareManager(strategies[i]); @@ -832,7 +861,7 @@ contract DelegationManager is // 1. read strategy deposit shares // forgefmt: disable-next-item - uint256 depositShares = shareManager.stakerDepositShares(staker, strategies[i]); + Shares shares = shareManager.stakerStrategyShares(staker, strategies[i]); // 2. if the staker is delegated, actual withdrawable shares can be different from what is stored // in the StrategyManager/EigenPodManager because they could have been slashed @@ -840,43 +869,34 @@ contract DelegationManager is uint64 totalMagnitude = allocationManager.getTotalMagnitude(operator, strategies[i]); // forgefmt: disable-next-item - withdrawableShares[i] = depositShares.toShares( - stakerScalingFactor[staker][strategies[i]], - totalMagnitude - ); - } else { - withdrawableShares[i] = depositShares; + ownedShares[i] = shares + .toDelegatedShares(stakerScalingFactors[staker][strategies[i]]) + .toOwnedShares(totalMagnitude); } } - return withdrawableShares; + return ownedShares; } /** - * @notice Returns the number of shares in storage for a staker and all their strategies - * TODO: make cleaner, get rid of assembly + * @notice Returns the number of actively-delegatable shares a staker has across all strategies. + * @dev Returns two empty arrays in the case that the Staker has no actively-delegateable shares. */ - function getDepositedShares( + function getDelegatableShares( address staker - ) public view returns (IStrategy[] memory, uint256[] memory) { + ) public view returns (IStrategy[] memory, OwnedShares[] memory) { // Get a list of all the strategies to check IStrategy[] memory strategies = strategyManager.getStakerStrategyList(staker); - // resize and add beaconChainETH to the end assembly { mstore(strategies, add(mload(strategies), 1)) } strategies[strategies.length - 1] = beaconChainETHStrategy; - uint256[] memory shares = new uint256[](strategies.length); - // Get owned shares for each strategy - for (uint256 i = 0; i < strategies.length; ++i) { - IShareManager shareManager = _getShareManager(strategies[i]); - shares[i] = shareManager.stakerDepositShares(staker, strategies[i]); - } - strategies[strategies.length - 1] = beaconChainETHStrategy; + // get the delegatable shares for each strategy + OwnedShares[] memory shares = getDelegatableShares(staker, strategies); // if the last shares are 0, remove them - if (shares[strategies.length - 1] == 0) { + if (shares[strategies.length - 1].unwrap() == 0) { // resize the arrays assembly { mstore(strategies, sub(mload(strategies), 1)) diff --git a/src/contracts/core/DelegationManagerStorage.sol b/src/contracts/core/DelegationManagerStorage.sol index 78f767c67..f54570e85 100644 --- a/src/contracts/core/DelegationManagerStorage.sol +++ b/src/contracts/core/DelegationManagerStorage.sol @@ -80,12 +80,13 @@ abstract contract DelegationManagerStorage is IDelegationManager { bytes32 internal _DOMAIN_SEPARATOR; /** - * @notice returns the total number of shares of the operator - * @notice Mapping: operator => strategy => total number of shares of the operator - * + * @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. * @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; @@ -143,7 +144,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 stakerScalingFactor; + mapping(address => mapping(IStrategy => StakerScalingFactors)) public stakerScalingFactors; // Construction diff --git a/src/contracts/core/StrategyManager.sol b/src/contracts/core/StrategyManager.sol index 7f50d58e7..7c965647b 100644 --- a/src/contracts/core/StrategyManager.sol +++ b/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 depositedShares The amount of new shares in the `strategy` created as part of the action. + * @return shares 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 (uint256 depositedShares) { - depositedShares = _depositIntoStrategy(msg.sender, strategy, token, amount); + ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (OwnedShares shares) { + shares = _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 depositedShares The amount of new shares in the `strategy` created as part of the action. + * @return shares 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 (uint256 depositedShares) { + ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (OwnedShares shares) { require(expiry >= block.timestamp, SignatureExpired()); // calculate struct hash, then increment `staker`'s nonce uint256 nonce = nonces[staker]; @@ -148,21 +148,20 @@ contract StrategyManager is EIP1271SignatureUtils.checkSignature_EIP1271(staker, digestHash, signature); // deposit the tokens (from the `msg.sender`) and credit the new shares to the `staker` - depositedShares = _depositIntoStrategy(staker, strategy, token, amount); + shares = _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 removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) external onlyDelegationManager { - _removeDepositShares(staker, strategy, depositSharesToRemove); + function removeShares(address staker, IStrategy strategy, Shares shares) external onlyDelegationManager { + _removeShares(staker, strategy, shares); } /// @notice Used by the DelegationManager to award a Staker some shares that have passed through the withdrawal queue - /// @dev Specifically, this function is called when a withdrawal is completed as shares. - function addShares( + function addOwnedShares( address staker, IStrategy strategy, IERC20 token, - uint256 shares + OwnedShares ownedShares ) external onlyDelegationManager { _addOwnedShares(staker, token, strategy, ownedShares); } @@ -174,9 +173,9 @@ contract StrategyManager is address staker, IStrategy strategy, IERC20 token, - uint256 shares + OwnedShares ownedShares ) external onlyDelegationManager { - strategy.withdraw(staker, token, shares); + strategy.withdraw(staker, token, ownedShares.unwrap()); } /** @@ -232,7 +231,7 @@ 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 `stakerDepositShares[staker][strategy]`, and adds `strategy` + * delegated shares are tracked, increases the stored share amount in `stakerStrategyShares[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 { @@ -240,24 +239,16 @@ contract StrategyManager is require(staker != address(0), StakerAddressZero()); require(ownedShares.unwrap() != 0, SharesAmountZero()); - uint256 existingShares = stakerDepositShares[staker][strategy]; + Shares existingShares = stakerStrategyShares[staker][strategy]; - // if they dont have existingShares of this strategy, add it to their strats - if (existingShares == 0) { + // if they dont have existing ownedShares of this strategy, add it to their strats + if (existingShares.unwrap() == 0) { require(stakerStrategyList[staker].length < MAX_STAKER_STRATEGY_LIST_LENGTH, MaxStrategiesExceeded()); stakerStrategyList[staker].push(strategy); } - // 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 - }); + // add the returned ownedShares to their existing shares for this strategy + stakerStrategyShares[staker][strategy] = existingShares.add(ownedShares.unwrap()).wrapShares(); // Increase shares delegated to operator, if needed delegation.increaseDelegatedShares({ @@ -294,33 +285,33 @@ contract StrategyManager is // add the returned shares to the staker's existing shares for this strategy _addOwnedShares(staker, token, strategy, ownedShares); - return shares; + return ownedShares; } /** - * @notice Decreases the shares that `staker` holds in `strategy` by `depositSharesToRemove`. + * @notice Decreases the shares that `staker` holds in `strategy` by `shareAmount`. * @param staker The address to decrement shares from * @param strategy The strategy for which the `staker`'s shares are being decremented - * @param depositSharesToRemove The amount of deposit shares to decrement + * @param shareAmount The amount of 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 _removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) internal returns (bool) { + function _removeShares(address staker, IStrategy strategy, Shares shareAmount) internal returns (bool) { // sanity checks on inputs - require(depositSharesToRemove != 0, SharesAmountZero()); + require(shareAmount.unwrap() != 0, SharesAmountZero()); //check that the user has sufficient shares - uint256 userDepositShares = stakerDepositShares[staker][strategy]; + Shares userShares = stakerStrategyShares[staker][strategy]; + + require(shareAmount.unwrap() <= userShares.unwrap(), SharesAmountTooHigh()); - require(depositSharesToRemove <= userDepositShares, SharesAmountTooHigh()); + userShares = userShares.sub(shareAmount.unwrap()).wrapShares(); - userDepositShares = userDepositShares - depositSharesToRemove; - // subtract the shares from the staker's existing shares for this strategy - stakerDepositShares[staker][strategy] = userDepositShares; + stakerStrategyShares[staker][strategy] = userShares; // if no existing shares, remove the strategy from the staker's dynamic array of strategies - if (userDepositShares == 0) { + if (userShares.unwrap() == 0) { _removeStrategyFromStakerStrategyList(staker, strategy); // return true in the event that the strategy was removed from stakerStrategyList[staker] @@ -366,7 +357,7 @@ contract StrategyManager is // VIEW FUNCTIONS /** - * @notice Get all details on the staker's strategies and shares deposited into + * @notice Get all details on the staker's deposits and corresponding shares * @param staker The staker of interest, whose deposits this function will fetch * @return (staker's strategies, shares in these strategies) */ @@ -374,18 +365,12 @@ contract StrategyManager is address staker ) external view returns (IStrategy[] memory, Shares[] memory) { uint256 strategiesLength = stakerStrategyList[staker].length; - uint256[] memory depositedShares = new uint256[](strategiesLength); + Shares[] memory shares = new Shares[](strategiesLength); for (uint256 i = 0; i < strategiesLength; ++i) { - depositedShares[i] = stakerDepositShares[staker][stakerStrategyList[staker][i]]; + shares[i] = stakerStrategyShares[staker][stakerStrategyList[staker][i]]; } - return (stakerStrategyList[staker], depositedShares); - } - - function getStakerStrategyList( - address staker - ) external view returns (IStrategy[] memory) { - return stakerStrategyList[staker]; + return (stakerStrategyList[staker], shares); } function getStakerStrategyList( diff --git a/src/contracts/core/StrategyManagerStorage.sol b/src/contracts/core/StrategyManagerStorage.sol index b269cf54a..a4d3fa0d4 100644 --- a/src/contracts/core/StrategyManagerStorage.sol +++ b/src/contracts/core/StrategyManagerStorage.sol @@ -59,9 +59,10 @@ 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 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 => Strategy => number of shares which they currently hold + mapping(address => mapping(IStrategy => Shares)) public stakerStrategyShares; + /// @notice Mapping: staker => array of strategies in which they have nonzero shares mapping(address => IStrategy[]) public stakerStrategyList; diff --git a/src/contracts/interfaces/IDelegationManager.sol b/src/contracts/interfaces/IDelegationManager.sol index d5ebcb534..cb8648b37 100644 --- a/src/contracts/interfaces/IDelegationManager.sol +++ b/src/contracts/interfaces/IDelegationManager.sol @@ -20,11 +20,6 @@ interface IDelegationManager is ISignatureUtils { error UnauthorizedCaller(); /// @dev Thrown when msg.sender is not the EigenPodManager error OnlyEigenPodManager(); -<<<<<<< HEAD -======= - /// @dev Throw when msg.sender is not the AllocationManager - error OnlyAllocationManager(); ->>>>>>> c902d747 (feat: dm cleanup) /// Delegation Status @@ -161,29 +156,17 @@ interface IDelegationManager is ISignatureUtils { uint32 startTimestamp; // Array of strategies that the Withdrawal contains IStrategy[] strategies; -<<<<<<< HEAD // Array containing the amount of staker's delegatedShares for withdrawal in each Strategy in the `strategies` array // Note that these shares need to be multiplied by the operator's totalMagnitude at completion to include // slashing occurring during the queue withdrawal delay DelegatedShares[] delegatedShares; -======= - // TODO: Find a better name for this - // Array containing the amount of staker's scaledSharesToWithdraw for withdrawal in each Strategy in the `strategies` array - // Note that these shares need to be multiplied by the operator's totalMagnitude at completion to include - // slashing occurring during the queue withdrawal delay - uint256[] scaledSharesToWithdraw; ->>>>>>> c902d747 (feat: dm cleanup) } struct QueuedWithdrawalParams { // Array of strategies that the QueuedWithdrawal contains IStrategy[] strategies; // Array containing the amount of withdrawable shares for withdrawal in each Strategy in the `strategies` array -<<<<<<< HEAD OwnedShares[] ownedShares; -======= - uint256[] ownedShares; ->>>>>>> c902d747 (feat: dm cleanup) // The address of the withdrawer address withdrawer; } @@ -201,7 +184,6 @@ interface IDelegationManager is ISignatureUtils { event OperatorMetadataURIUpdated(address indexed operator, string metadataURI); /// @notice Emitted whenever an operator's shares are increased for a given strategy. Note that shares is the delta in the operator's shares. -<<<<<<< HEAD event OperatorSharesIncreased( address indexed operator, address staker, IStrategy strategy, DelegatedShares delegatedShares ); @@ -210,12 +192,6 @@ interface IDelegationManager is ISignatureUtils { event OperatorSharesDecreased( address indexed operator, address staker, IStrategy strategy, DelegatedShares delegatedShares ); -======= - event OperatorSharesIncreased(address indexed operator, address staker, IStrategy strategy, uint256 delegatedShares); - - /// @notice Emitted whenever an operator's shares are decreased for a given strategy. Note that shares is the delta in the operator's shares. - event OperatorSharesDecreased(address indexed operator, address staker, IStrategy strategy, uint256 delegatedShares); ->>>>>>> c902d747 (feat: dm cleanup) /// @notice Emitted when @param staker delegates to @param operator. event StakerDelegated(address indexed staker, address indexed operator); @@ -372,22 +348,24 @@ interface IDelegationManager is ISignatureUtils { /** * @notice Increases a staker's delegated share balance in a strategy. Note that before adding to operator shares, - * the delegated delegatedShares. The staker's depositScalingFactor is updated here. + * the delegated shares are scaled according to the operator's total magnitude as part of slashing accounting. + * The staker's depositScalingFactor is updated here. * @param staker The address to increase the delegated shares for their operator. * @param strategy The strategy in which to increase the delegated shares. - * @param existingDepositShares The number of deposit shares the staker already has in the strategy. This is the shares amount stored in the + * @param existingShares The number of deposit shares the staker already has in the strategy. This is the shares amount stored in the * StrategyManager/EigenPodManager for the staker's shares. - * @param addedShares The number of shares added to the staker's shares in the strategy + * @param addedOwnedShares The number of shares to added to the staker's shares in the strategy. This amount will be scaled prior to adding + * to the operator's scaled shares. * - * @dev *If the staker is actively delegated*, then increases the `staker`'s delegated delegatedShares in `strategy`. + * @dev *If the staker is actively delegated*, then increases the `staker`'s delegated scaled shares in `strategy`. * Otherwise does nothing. * @dev Callable only by the StrategyManager or EigenPodManager. */ function increaseDelegatedShares( address staker, IStrategy strategy, - uint256 existingDepositShares, - uint256 addedShares + Shares existingShares, + OwnedShares addedOwnedShares ) external; /** @@ -402,20 +380,10 @@ interface IDelegationManager is ISignatureUtils { */ function decreaseBeaconChainScalingFactor( address staker, - uint256 existingShares, + Shares existingShares, uint64 proportionOfOldBalance ) external; - /** - * @notice Decreases the operators shares in storage after a slash - * @param operator The operator to decrease shares for - * @param strategy The strategy to decrease shares for - * @param previousMagnitude The magnitude before the slash - * @param newMagnitude The magnitude after the slash - * @dev Callable only by the AllocationManager - */ - function decreaseOperatorShares(address operator, IStrategy strategy, uint64 previousMagnitude, uint64 newMagnitude) external; - /** * @notice returns the address of the operator that `staker` is delegated to. * @notice Mapping: staker => operator whom the staker is currently delegated to. @@ -446,6 +414,17 @@ interface IDelegationManager is ISignatureUtils { address operator ) external view returns (uint256); + /** + * @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. + * @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` + */ + function operatorDelegatedShares(address operator, IStrategy strategy) external view returns (DelegatedShares); + /** * @notice Returns 'true' if `staker` *is* actively delegated, and 'false' otherwise. */ diff --git a/src/contracts/interfaces/IEigenPodManager.sol b/src/contracts/interfaces/IEigenPodManager.sol index 30d9a9122..854a9179e 100644 --- a/src/contracts/interfaces/IEigenPodManager.sol +++ b/src/contracts/interfaces/IEigenPodManager.sol @@ -119,7 +119,7 @@ interface IEigenPodManager is IShareManager, IPausable { * Likewise, when a withdrawal is completed, this "deficit" is decreased and the withdrawal amount is decreased; We can think of this * as the withdrawal "paying off the deficit". */ - function podOwnerDepositShares( + function podOwnerShares( address podOwner ) external view returns (int256); diff --git a/src/contracts/interfaces/IShareManager.sol b/src/contracts/interfaces/IShareManager.sol index 8e1c6f447..b9f3eb666 100644 --- a/src/contracts/interfaces/IShareManager.sol +++ b/src/contracts/interfaces/IShareManager.sol @@ -14,20 +14,25 @@ import "./IStrategy.sol"; interface IShareManager { /// @notice Used by the DelegationManager to remove a Staker's shares from a particular strategy when entering the withdrawal queue /// @dev strategy must be beaconChainETH when talking to the EigenPodManager - function removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) external; + function removeShares(address staker, IStrategy strategy, Shares shares) external; /// @notice Used by the DelegationManager to award a Staker some shares that have passed through the withdrawal queue /// @dev strategy must be beaconChainETH when talking to the EigenPodManager /// @dev token is not validated when talking to the EigenPodManager - function addShares(address staker, IStrategy strategy, IERC20 token, uint256 shares) external; + function addOwnedShares(address staker, IStrategy strategy, IERC20 token, OwnedShares ownedShares) external; /// @notice Used by the DelegationManager to convert withdrawn descaled shares to tokens and send them to a staker /// @dev strategy must be beaconChainETH when talking to the EigenPodManager /// @dev token is not validated when talking to the EigenPodManager - function withdrawSharesAsTokens(address staker, IStrategy strategy, IERC20 token, uint256 shares) external; + function withdrawSharesAsTokens( + address staker, + IStrategy strategy, + IERC20 token, + OwnedShares ownedShares + ) external; /// @notice Returns the current shares of `user` in `strategy` /// @dev strategy must be beaconChainETH when talking to the EigenPodManager /// @dev returns 0 if the user has negative shares - function stakerDepositShares(address user, IStrategy strategy) external view returns (uint256 depositShares); + function stakerStrategyShares(address user, IStrategy strategy) external view returns (Shares shares); } diff --git a/src/contracts/interfaces/IStrategyManager.sol b/src/contracts/interfaces/IStrategyManager.sol index 5678f2847..61515f033 100644 --- a/src/contracts/interfaces/IStrategyManager.sol +++ b/src/contracts/interfaces/IStrategyManager.sol @@ -41,7 +41,7 @@ interface IStrategyManager is IShareManager { * @param token Is the token that `staker` deposited. * @param shares Is the number of new shares `staker` has been granted in `strategy`. */ - event Deposit(address staker, IERC20 token, IStrategy strategy, uint256 shares); + event Deposit(address staker, IERC20 token, IStrategy strategy, OwnedShares shares); /// @notice Emitted when the `strategyWhitelister` is changed event StrategyWhitelisterChanged(address previousAddress, address newAddress); @@ -97,7 +97,7 @@ interface IStrategyManager is IShareManager { address staker, uint256 expiry, bytes memory signature - ) external returns (uint256 shares); + ) external returns (OwnedShares shares); /** * @notice Get all details on the staker's deposits and corresponding shares @@ -111,18 +111,11 @@ interface IStrategyManager is IShareManager { address staker ) external view returns (IStrategy[] memory); - function getStakerStrategyList( - address staker - ) external view returns (IStrategy[] memory); - /// @notice Simple getter function that returns `stakerStrategyList[staker].length`. function stakerStrategyListLength( address staker ) external view returns (uint256); - /// @notice Returns the current shares of `user` in `strategy` - function stakerDepositShares(address user, IStrategy strategy) external view returns (uint256 shares); - /** * @notice Owner-only function that adds the provided Strategies to the 'whitelist' of strategies that stakers can deposit into * @param strategiesToWhitelist Strategies that will be added to the `strategyIsWhitelistedForDeposit` mapping (if they aren't in it already) diff --git a/src/contracts/libraries/SlashingLib.sol b/src/contracts/libraries/SlashingLib.sol index 5eb887bc4..e6814d6aa 100644 --- a/src/contracts/libraries/SlashingLib.sol +++ b/src/contracts/libraries/SlashingLib.sol @@ -3,32 +3,59 @@ pragma solidity ^0.8.27; import "@openzeppelin/contracts/utils/math/Math.sol"; -/// @dev the stakerScalingFactor and operatorMagnitude have initial default values to 1e18 as "1" +/// @dev the stakerScalingFactor and totalMagnitude have initial default values to 1e18 as "1" /// to preserve precision with uint256 math. We use `WAD` where these variables are used /// and divide to represent as 1 uint64 constant WAD = 1e18; /* - * There are 2 types of shares: - * 1. depositShares + * There are 3 types of shares: + * 1. ownedShares * - These can be converted to an amount of tokens given a strategy * - by calling `sharesToUnderlying` on the strategy address (they're already tokens * in the case of EigenPods) - * - These live in the storage of EPM and SM strategies - * 2. shares - * - For a staker, this is the amount of shares that they can withdraw - * - For an operator, this is the sum of its staker's withdrawable shares + * - These are comparable between operators and stakers. + * - These live in the storage of StrategyManager strategies: + * - `totalShares` is the total amount of shares delegated to a strategy + * 2. delegatedShares + * - These can be converted to shares given an operator and a strategy + * - by multiplying by the operator's totalMagnitude for the strategy + * - These values automatically update their conversion into tokens + * - when the operator's total magnitude for the strategy is decreased upon slashing + * - These live in the storage of the DelegationManager: + * - `delegatedShares` is the total amount of delegatedShares delegated to an operator for a strategy + * - `withdrawal.delegatedShares` is the amount of delegatedShares in a withdrawal + * 3. shares + * - These can be converted into delegatedShares given a staker and a strategy + * - by multiplying by the staker's depositScalingFactor for the strategy + * - These values automatically update their conversion into tokens + * - when the staker's depositScalingFactor for the strategy is increased upon new deposits + * - or when the staker's operator's total magnitude for the strategy is decreased upon slashing + * - These represent the total amount of shares the staker would have of a strategy if they were never slashed + * - These live in the storage of the StrategyManager/EigenPodManager + * - `stakerStrategyShares` in the SM is the staker's shares that have not been queued for withdrawal in a strategy + * - `podOwnerShares` in the EPM is the staker's shares that have not been queued for withdrawal in the beaconChainETHStrategy * - * Note that `withdrawal.scaledSharesToWithdraw` is scaled for the beaconChainETHStrategy to divide by the beaconChainScalingFactor upon queueing + * Note that `withdrawal.delegatedShares` is scaled for the beaconChainETHStrategy to divide by the beaconChainScalingFactor upon queueing * and multiply by the beaconChainScalingFactor upon withdrawal */ +type OwnedShares is uint256; + +type DelegatedShares is uint256; + +type Shares is uint256; + struct StakerScalingFactors { uint256 depositScalingFactor; // we need to know if the beaconChainScalingFactor is set because it can be set to 0 through 100% slashing bool isBeaconChainScalingFactorSet; uint64 beaconChainScalingFactor; } + +using SlashingLib for OwnedShares global; +using SlashingLib for DelegatedShares global; +using SlashingLib for Shares global; using SlashingLib for StakerScalingFactors global; // TODO: validate order of operations everywhere @@ -36,120 +63,158 @@ library SlashingLib { using Math for uint256; using SlashingLib for uint256; - // WAD MATH + // MATH - function mulWad(uint256 x, uint256 y) internal pure returns (uint256) { - return x.mulDiv(y, WAD); + function add(Shares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() + y; } - function divWad(uint256 x, uint256 y) internal pure returns (uint256) { - return x.mulDiv(WAD, y); + function add(DelegatedShares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() + y; } - // GETTERS + function add(DelegatedShares x, DelegatedShares y) internal pure returns (DelegatedShares) { + return (x.unwrap() + y.unwrap()).wrapDelegated(); + } + function add(OwnedShares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() + y; + } - function getDepositScalingFactor(StakerScalingFactors memory ssf) internal pure returns (uint256) { - return ssf.depositScalingFactor == 0 ? WAD : ssf.depositScalingFactor; + function add(OwnedShares x, OwnedShares y) internal pure returns (OwnedShares) { + return (x.unwrap() + y.unwrap()).wrapOwned(); } - - function getBeaconChainScalingFactor(StakerScalingFactors memory ssf) internal pure returns (uint64) { - return !ssf.isBeaconChainScalingFactorSet && ssf.beaconChainScalingFactor == 0 ? WAD : ssf.beaconChainScalingFactor; + + function sub(Shares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() - y; + } + + function sub(DelegatedShares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() - y; + } + + function sub(DelegatedShares x, DelegatedShares y) internal pure returns (DelegatedShares) { + return (x.unwrap() - y.unwrap()).wrapDelegated(); + } + + function sub(OwnedShares x, uint256 y) internal pure returns (uint256) { + return x.unwrap() - y; + } + + /// @dev beaconChainScalingFactor = 0 -> WAD for all non beaconChainETH strategies + function toShares( + DelegatedShares delegatedShares, + StakerScalingFactors storage ssf + ) internal view returns (Shares) { + return delegatedShares.unwrap().divWad(ssf.getDepositScalingFactor()).divWad(ssf.getBeaconChainScalingFactor()) + .wrapShares(); } + function toDelegatedShares(OwnedShares shares, uint256 magnitude) internal pure returns (DelegatedShares) { + // forgefmt: disable-next-item + return shares + .unwrap() + .divWad(magnitude) + .wrapDelegated(); + } - function scaleSharesForQueuedWithdrawal(uint256 sharesToWithdraw, StakerScalingFactors memory ssf, uint64 operatorMagnitude) internal pure returns (uint256) { - return - sharesToWithdraw - .divWad(uint256(ssf.getBeaconChainScalingFactor())) - .divWad(uint256(operatorMagnitude)); + function toOwnedShares(DelegatedShares delegatedShares, uint256 magnitude) internal view returns (OwnedShares) { + return delegatedShares.unwrap().mulWad(magnitude).wrapOwned(); } - function scaleSharesForCompleteWithdrawal(uint256 scaledSharesToWithdraw, StakerScalingFactors memory ssf, uint64 operatorMagnitude) internal pure returns (uint256) { - return - scaledSharesToWithdraw - .mulWad(uint256(ssf.getBeaconChainScalingFactor())) - .mulWad(uint256(operatorMagnitude)); + function scaleForQueueWithdrawal( + DelegatedShares delegatedShares, + StakerScalingFactors storage ssf + ) internal view returns (DelegatedShares) { + return delegatedShares.unwrap().divWad(ssf.getBeaconChainScalingFactor()).wrapDelegated(); } - function decreaseOperatorShares(uint256 operatorShares, uint64 previousMagnitude, uint64 newMagnitude) internal pure returns (uint256) { - return operatorShares.divWad(previousMagnitude).mulWad(newMagnitude); + function scaleForCompleteWithdrawal( + DelegatedShares delegatedShares, + StakerScalingFactors storage ssf + ) internal view returns (DelegatedShares) { + return delegatedShares.unwrap().mulWad(ssf.getBeaconChainScalingFactor()).wrapDelegated(); } - function decreaseBeaconChainScalingFactor(StakerScalingFactors storage ssf, uint64 proportionOfOldBalance) internal { + function decreaseBeaconChainScalingFactor( + StakerScalingFactors storage ssf, + uint64 proportionOfOldBalance + ) internal { ssf.beaconChainScalingFactor = uint64(uint256(ssf.beaconChainScalingFactor).mulWad(proportionOfOldBalance)); ssf.isBeaconChainScalingFactorSet = true; } - function calculateNewDepositScalingFactor( - uint256 currentDepositShares, - uint256 addedDepositShares, - StakerScalingFactors memory ssf, - uint64 magnitude - ) internal pure returns (uint64) { - // TODO: update equation for beacon chain scaling factor - /** - * Base Equations: - * (1) newShares = currentShares + addedDepositShares - * (2) newOwnedShares = currentOwnedShares + addedDepositShares - * (3) newOwnedShares = newShares * newStakerDepositScalingFactor * newMagnitude - * - * Plugging (3) into (2): - * (4) newShares * newStakerDepositScalingFactor * newMagnitude = currentOwnedShares + addedDepositShares - * - * Solving for newStakerDepositScalingFactor - * (5) newStakerDepositScalingFactor = (ownedShares + addedDepositShares) / (newShares * newMagnitude) - * - * We also know that the magnitude remains constant on deposits, thus - * (6) newMagnitude = oldMagnitude - * - * Plugging in (6) and (1) into (5): - * (7) newStakerDepositScalingFactor = (ownedShares + addedDepositShares) / ((currentShares + addedDepositShares) * oldMagnitude) - * Note that magnitudes must be divided by WAD for precision. Thus, - * - * (8) newStakerDepositScalingFactor = (ownedShares + addedDepositShares) / ((currentShares + addedDepositShares) * oldMagnitude / WAD) - * (9) newStakerDepositScalingFactor = (ownedShares + addedDepositShares) / ((currentShares + addedDepositShares) / WAD) * (oldMagnitude / WAD)) - */ - - // Step 1: Calculate Numerator - uint256 currentShares = currentDepositShares.toShares(ssf, magnitude); - - // Step 2: Compute currentShares + addedShares - uint256 newShares = currentShares + addedDepositShares; - - // Step 3: Calculate newStakerDepositScalingFactor - // Note: We divide by magnitude to preserve - - //TODO: figure out if we only need to do one divWad here - uint256 newStakerDepositScalingFactor = - newShares - .divWad(currentShares + addedDepositShares) - .divWad(magnitude) - .divWad(uint256(ssf.getBeaconChainScalingFactor())); + /// @dev beaconChainScalingFactor = 0 -> WAD for all non beaconChainETH strategies + function toDelegatedShares( + Shares shares, + StakerScalingFactors storage ssf + ) internal view returns (DelegatedShares) { + return shares.unwrap().mulWad(ssf.getDepositScalingFactor()).mulWad(ssf.getBeaconChainScalingFactor()) + .wrapDelegated(); + } + + function toDelegatedShares(Shares shares, uint256 magnitude) internal view returns (DelegatedShares) { + return shares.unwrap().mulWad(magnitude).wrapDelegated(); + } + + // WAD MATH + + function mulWad(uint256 x, uint256 y) internal pure returns (uint256) { + return x.mulDiv(y, WAD); + } - return uint64(newStakerDepositScalingFactor); + function divWad(uint256 x, uint256 y) internal pure returns (uint256) { + return x.mulDiv(WAD, y); } - // CONVERSION + // TYPE CASTING - function toDepositShares( - uint256 shares, - StakerScalingFactors memory ssf, - uint64 magnitude - ) internal pure returns (uint256 depositShares) { - depositShares = - shares - .divWad(ssf.getDepositScalingFactor()) - .divWad(uint256(ssf.getBeaconChainScalingFactor())) - .divWad(uint256(magnitude)); + function unwrap( + Shares x + ) internal pure returns (uint256) { + return Shares.unwrap(x); } - function toShares(uint256 depositShares, StakerScalingFactors memory ssf, uint64 magnitude) internal pure returns (uint256 shares) { - shares = - depositShares - .mulWad(ssf.getDepositScalingFactor()) - .mulWad(uint256(ssf.getBeaconChainScalingFactor())) - .mulWad(uint256(magnitude)); + function unwrap( + DelegatedShares x + ) internal pure returns (uint256) { + return DelegatedShares.unwrap(x); } + function unwrap( + OwnedShares x + ) internal pure returns (uint256) { + return OwnedShares.unwrap(x); + } + + function wrapShares( + uint256 x + ) internal pure returns (Shares) { + return Shares.wrap(x); + } + + function wrapDelegated( + uint256 x + ) internal pure returns (DelegatedShares) { + return DelegatedShares.wrap(x); + } + + function wrapOwned( + uint256 x + ) internal pure returns (OwnedShares) { + return OwnedShares.wrap(x); + } + + function getDepositScalingFactor( + StakerScalingFactors storage ssf + ) internal view returns (uint256) { + return ssf.depositScalingFactor == 0 ? WAD : ssf.depositScalingFactor; + } + + function getBeaconChainScalingFactor( + StakerScalingFactors storage ssf + ) internal view returns (uint64) { + return + !ssf.isBeaconChainScalingFactorSet && ssf.beaconChainScalingFactor == 0 ? WAD : ssf.beaconChainScalingFactor; + } } diff --git a/src/contracts/pods/EigenPodManager.sol b/src/contracts/pods/EigenPodManager.sol index 0220df3ac..8d099ad56 100644 --- a/src/contracts/pods/EigenPodManager.sol +++ b/src/contracts/pods/EigenPodManager.sol @@ -114,14 +114,12 @@ contract EigenPodManager is // the slashing upgrade. Make people complete queued withdrawals before completing any further checkpoints. // the only effects podOwner UX, not AVS UX, since the podOwner already has 0 shares in the DM if they // have a negative shares in EPM. - require(podOwnerDepositShares[podOwner] >= 0, LegacyWithdrawalsNotCompleted()); + require(podOwnerShares[podOwner] >= 0, LegacyWithdrawalsNotCompleted()); if (sharesDelta > 0) { - _addShares(podOwner, uint256(sharesDelta)); - } else if (sharesDelta < 0 && podOwnerDepositShares[podOwner] > 0) { + _addOwnedShares(podOwner, uint256(sharesDelta).wrapOwned()); + } else if (sharesDelta < 0 && podOwnerShares[podOwner] > 0) { delegationManager.decreaseBeaconChainScalingFactor( - podOwner, - uint256(podOwnerDepositShares[podOwner]), - proportionOfOldBalance + podOwner, uint256(podOwnerShares[podOwner]).wrapShares(), proportionOfOldBalance ); } } @@ -129,16 +127,19 @@ contract EigenPodManager is /** * @notice Used by the DelegationManager to remove a pod owner's shares while they're in the withdrawal queue. * Simply decreases the `podOwner`'s shares by `shares`, down to a minimum of zero. - * @dev This function reverts if it would result in `podOwnerDepositShares[podOwner]` being less than zero, i.e. it is forbidden for this function to + * @dev This function reverts if it would result in `podOwnerShares[podOwner]` being less than zero, i.e. it is forbidden for this function to * result in the `podOwner` incurring a "share deficit". This behavior prevents a Staker from queuing a withdrawal which improperly removes excessive * shares from the operator to whom the staker is delegated. + * @dev Reverts if `shares` is not a whole Gwei amount * @dev The delegation manager validates that the podOwner is not address(0) */ - function removeDepositShares(address staker, IStrategy strategy, uint256 depositSharesToRemove) external onlyDelegationManager { + function removeShares(address staker, IStrategy strategy, Shares shares) external onlyDelegationManager { require(strategy == beaconChainETHStrategy, InvalidStrategy()); - int256 updatedShares = podOwnerDepositShares[staker] - int256(depositSharesToRemove); + require(int256(shares.unwrap()) >= 0, SharesNegative()); + require(shares.unwrap() % GWEI_TO_WEI == 0, SharesNotMultipleOfGwei()); + int256 updatedShares = podOwnerShares[staker] - int256(shares.unwrap()); require(updatedShares >= 0, SharesNegative()); - podOwnerDepositShares[staker] = updatedShares; + podOwnerShares[staker] = updatedShares; emit NewTotalShares(staker, updatedShares); } @@ -146,19 +147,19 @@ contract EigenPodManager is /** * @notice Increases the `podOwner`'s shares by `shares`, paying off deficit if possible. * Used by the DelegationManager to award a pod owner shares on exiting the withdrawal queue - * @dev Returns the number of shares added to `podOwnerDepositShares[podOwner]` above zero, which will be less than the `shares` input - * in the event that the podOwner has an existing shares deficit (i.e. `podOwnerDepositShares[podOwner]` starts below zero). - * Also returns existingPodShares prior to adding shares, this is returned as 0 if the existing podOwnerDepositShares is negative + * @dev Returns the number of shares added to `podOwnerShares[podOwner]` above zero, which will be less than the `shares` input + * in the event that the podOwner has an existing shares deficit (i.e. `podOwnerShares[podOwner]` starts below zero). + * Also returns existingPodShares prior to adding shares, this is returned as 0 if the existing podOwnerShares is negative * @dev Reverts if `shares` is not a whole Gwei amount */ - function addShares( + function addOwnedShares( address staker, - IStrategy strategy, - IERC20, - uint256 shares + IStrategy strategy, + IERC20, + OwnedShares shares ) external onlyDelegationManager { require(strategy == beaconChainETHStrategy, InvalidStrategy()); - _addShares(staker, shares); + _addOwnedShares(staker, shares); } /** @@ -166,51 +167,49 @@ contract EigenPodManager is * @dev Prioritizes decreasing the podOwner's share deficit, if they have one * @dev Reverts if `shares` is not a whole Gwei amount * @dev This function assumes that `removeShares` has already been called by the delegationManager, hence why - * we do not need to update the podOwnerDepositShares if `currentpodOwnerDepositShares` is positive + * we do not need to update the podOwnerShares if `currentPodOwnerShares` is positive */ function withdrawSharesAsTokens( - address staker, - IStrategy strategy, - IERC20, - uint256 shares + address staker, + IStrategy strategy, + IERC20, + OwnedShares shares ) external onlyDelegationManager { - require(strategy == beaconChainETHStrategy, InvalidStrategy()); - require(staker != address(0), InputAddressZero()); - - int256 currentDepositShares = podOwnerDepositShares[staker]; - uint256 sharesToWithdraw; - // if there is an existing shares deficit, prioritize decreasing the deficit first - // this is an M2 legacy codepath. TODO: gross - if (currentDepositShares < 0) { - uint256 currentDepositShareDeficit = uint256(-currentDepositShares); - uint256 depositSharesToAdd; - if (shares > currentDepositShareDeficit) { - // get rid of the whole deficit if possible, and pass any remaining shares onto destination - depositSharesToAdd = currentDepositShareDeficit; - sharesToWithdraw = shares - currentDepositShareDeficit; - } else { - // otherwise get rid of as much deficit as possible, and return early, since there is nothing left over to forward on - depositSharesToAdd = shares; - sharesToWithdraw = 0; - } + // require(strategy == beaconChainETHStrategy, InvalidStrategy()); + // require(staker != address(0), InputAddressZero()); + // require(int256(shares.unwrap()) >= 0, SharesNegative()); + // require(shares.unwrap() % GWEI_TO_WEI == 0, SharesNotMultipleOfGwei()); + // int256 currentShares = podOwnerShares[staker]; - int256 updatedShares = currentDepositShares + int256(depositSharesToAdd); - podOwnerDepositShares[staker] = updatedShares; - emit PodSharesUpdated(staker, int256(depositSharesToAdd)); - emit NewTotalShares(staker, updatedShares); - } - if (sharesToWithdraw > 0) { - // Actually withdraw to the destination - ownerToPod[staker].withdrawRestakedBeaconChainETH(staker, sharesToWithdraw); - } + // // if there is an existing shares deficit, prioritize decreasing the deficit first + // if (currentShares < 0) { + // uint256 currentShareDeficit = uint256(-currentShares); + + // if (shares.unwrap() > currentShareDeficit) { + // // get rid of the whole deficit if possible, and pass any remaining shares onto destination + // podOwnerShares[staker] = 0; + // shares = shares.sub(currentShareDeficit).wrapWithdrawable(); + // emit PodSharesUpdated(staker, int256(currentShareDeficit)); + // emit NewTotalShares(staker, 0); + // } else { + // // otherwise get rid of as much deficit as possible, and return early, since there is nothing left over to forward on + // int256 updatedShares = podOwnerShares[staker] + int256(shares.unwrap()); + // podOwnerShares[staker] = updatedShares; + // emit PodSharesUpdated(staker, int256(shares.unwrap())); + // emit NewTotalShares(staker, updatedShares); + // return; + // } + // } + // // Actually withdraw to the destination + // ownerToPod[staker].withdrawRestakedBeaconChainETH(staker, shares.unwrap()); } /// @notice Returns the current shares of `user` in `strategy` /// @dev strategy must be beaconChainETH when talking to the EigenPodManager /// @dev returns 0 if the user has negative shares - function stakerDepositShares(address user, IStrategy strategy) public view returns (uint256 depositShares) { + function stakerStrategyShares(address user, IStrategy strategy) public view returns (Shares shares) { require(strategy == beaconChainETHStrategy, InvalidStrategy()); - return podOwnerDepositShares[user] < 0 ? 0 : uint256(podOwnerDepositShares[user]); + return (podOwnerShares[user] < 0 ? 0 : uint256(podOwnerShares[user])).wrapShares(); } // INTERNAL FUNCTIONS @@ -233,27 +232,24 @@ contract EigenPodManager is return pod; } - function _addShares( - address staker, - uint256 shares - ) internal { + function _addOwnedShares(address staker, OwnedShares ownedShares) internal { require(staker != address(0), InputAddressZero()); - int256 sharesToAdd = int256(shares); - int256 currentDepositShares = podOwnerDepositShares[staker]; - int256 updatedDepositShares = currentDepositShares + sharesToAdd; - podOwnerDepositShares[staker] = updatedDepositShares; + int256 addedOwnedShares = int256(ownedShares.unwrap()); + int256 currentShares = podOwnerShares[staker]; + int256 updatedShares = currentShares + addedOwnedShares; + podOwnerShares[staker] = updatedShares; - emit PodSharesUpdated(staker, sharesToAdd); - emit NewTotalShares(staker, updatedDepositShares); + emit PodSharesUpdated(staker, addedOwnedShares); + emit NewTotalShares(staker, updatedShares); - if (updatedDepositShares > 0) { + if (updatedShares > 0) { delegationManager.increaseDelegatedShares({ staker: staker, strategy: beaconChainETHStrategy, // existing shares from standpoint of the DelegationManager - existingDepositShares: currentDepositShares < 0 ? 0 : uint256(currentDepositShares), - addedShares: shares + existingShares: currentShares < 0 ? Shares.wrap(0) : uint256(currentShares).wrapShares(), + addedOwnedShares: ownedShares }); } } diff --git a/src/contracts/pods/EigenPodManagerStorage.sol b/src/contracts/pods/EigenPodManagerStorage.sol index c9858f85b..60170eff0 100644 --- a/src/contracts/pods/EigenPodManagerStorage.sol +++ b/src/contracts/pods/EigenPodManagerStorage.sol @@ -65,15 +65,14 @@ abstract contract EigenPodManagerStorage is IEigenPodManager { // BEGIN STORAGE VARIABLES ADDED AFTER MAINNET DEPLOYMENT -- DO NOT SUGGEST REORDERING TO CONVENTIONAL ORDER /** - * // TODO: Update this comment - * @notice Mapping from Pod owner owner to the number of deposit shares they have in the virtual beacon chain ETH strategy. - * @dev The deposit share amount can become negative. This is necessary to accommodate the fact that a pod owner's virtual beacon chain ETH shares can + * @notice Mapping from Pod owner owner to the number of shares they have in the virtual beacon chain ETH strategy. + * @dev The share amount can become negative. This is necessary to accommodate the fact that a pod owner's virtual beacon chain ETH shares can * decrease between the pod owner queuing and completing a withdrawal. * When the pod owner's shares would otherwise increase, this "deficit" is decreased first _instead_. * Likewise, when a withdrawal is completed, this "deficit" is decreased and the withdrawal amount is decreased; We can think of this * as the withdrawal "paying off the deficit". */ - mapping(address => int256) public podOwnerDepositShares; + mapping(address => int256) public podOwnerShares; uint64 internal __deprecated_denebForkTimestamp; diff --git a/src/contracts/strategies/StrategyBase.sol b/src/contracts/strategies/StrategyBase.sol index 58ae5d3b2..3907b1583 100644 --- a/src/contracts/strategies/StrategyBase.sol +++ b/src/contracts/strategies/StrategyBase.sol @@ -302,7 +302,7 @@ contract StrategyBase is Initializable, Pausable, IStrategy { function shares( address user ) public view virtual returns (uint256) { - return strategyManager.stakerDepositShares(user, IStrategy(address(this))); + return strategyManager.stakerStrategyShares(user, IStrategy(address(this))).unwrap(); } /// @notice Internal function used to fetch this contract's current balance of `underlyingToken`.