From 23dabf79dd16925df50297f17fe925a1f7286f30 Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Sat, 7 Sep 2024 12:33:30 -0400 Subject: [PATCH] feat: refactor to staker scaling factors --- src/contracts/core/AVSDirectory.sol | 107 +++- src/contracts/core/DelegationManager.sol | 554 ++++++++++++------ .../core/DelegationManagerStorage.sol | 8 +- src/contracts/core/StrategyManager.sol | 144 ++--- src/contracts/core/StrategyManagerStorage.sol | 8 +- src/contracts/interfaces/IAVSDirectory.sol | 49 +- .../interfaces/IDelegationManager.sol | 76 +-- src/contracts/interfaces/IEigenPodManager.sol | 13 +- src/contracts/interfaces/IStrategyManager.sol | 20 +- src/contracts/libraries/ShareScalingLib.sol | 121 ---- src/contracts/libraries/SlashingConstants.sol | 18 + src/contracts/pods/EigenPodManager.sol | 137 ++--- src/contracts/pods/EigenPodManagerStorage.sol | 8 +- src/contracts/strategies/StrategyBase.sol | 2 +- src/test/EigenLayerTestHelper.t.sol | 2 +- src/test/harnesses/EigenPodManagerWrapper.sol | 4 +- src/test/integration/IntegrationBase.t.sol | 8 +- src/test/integration/users/User.t.sol | 2 +- src/test/mocks/AVSDirectoryMock.sol | 22 +- src/test/mocks/DelegationManagerMock.sol | 37 +- src/test/mocks/EigenPodManagerMock.sol | 6 +- src/test/mocks/StrategyManagerMock.sol | 7 +- src/test/unit/EigenPodManagerUnit.t.sol | 68 +-- src/test/unit/StrategyManagerUnit.t.sol | 126 ++-- 24 files changed, 833 insertions(+), 714 deletions(-) delete mode 100644 src/contracts/libraries/ShareScalingLib.sol create mode 100644 src/contracts/libraries/SlashingConstants.sol diff --git a/src/contracts/core/AVSDirectory.sol b/src/contracts/core/AVSDirectory.sol index 4b1d930b16..06a92ae35b 100644 --- a/src/contracts/core/AVSDirectory.sol +++ b/src/contracts/core/AVSDirectory.sol @@ -7,7 +7,7 @@ import "@openzeppelin-upgrades/contracts/security/ReentrancyGuardUpgradeable.sol import "../permissions/Pausable.sol"; import "../libraries/EIP1271SignatureUtils.sol"; -import "../libraries/ShareScalingLib.sol"; +import "../libraries/SlashingConstants.sol"; import "./AVSDirectoryStorage.sol"; contract AVSDirectory is @@ -29,9 +29,6 @@ contract AVSDirectory is /// @dev BIPS factor for slashable bips uint256 internal constant BIPS_FACTOR = 10_000; - /// @dev Delay before deallocations are completable and can be added back into freeMagnitude - uint32 public constant DEALLOCATION_DELAY = 17.5 days; - /// @dev Maximum number of pending updates that can be queued for allocations/deallocations uint256 public constant MAX_PENDING_UPDATES = 1; @@ -295,7 +292,7 @@ contract AVSDirectory is // effect timestamp for allocations to take effect. This is configurable by operators uint32 effectTimestamp = uint32(block.timestamp) + details.allocationDelay; // completable timestamp for deallocations - uint32 completableTimestamp = uint32(block.timestamp) + DEALLOCATION_DELAY; + uint32 completableTimestamp = uint32(block.timestamp) + SlashingConstants.DEALLOCATION_DELAY; for (uint256 i = 0; i < allocations.length; ++i) { // 1. For the given (operator,strategy) clear all pending free magnitude for the strategy and update freeMagnitude @@ -382,18 +379,16 @@ contract AVSDirectory is uint64 currentMagnitude = uint64( _magnitudeUpdate[operator][strategies[i]][operatorSetKey].upperLookupRecent(uint32(block.timestamp)) ); - // TODO: if we don't continue here we get into weird "total/free magnitude" not initialized cases. Is this ok? - if (currentMagnitude == 0) { - continue; + // slash nonzero current magnitude + if (currentMagnitude != 0) { + /// TODO: add wrapping library for rounding up for slashing accounting + slashedMagnitude = uint64(uint256(bipsToSlash) * uint256(currentMagnitude) / BIPS_FACTOR); + + _magnitudeUpdate[operator][strategies[i]][operatorSetKey].decrementAtAndFutureCheckpoints({ + key: uint32(block.timestamp), + decrementValue: slashedMagnitude + }); } - - /// TODO: add wrapping library for rounding up for slashing accounting - slashedMagnitude = uint64(uint256(bipsToSlash) * uint256(currentMagnitude) / BIPS_FACTOR); - - _magnitudeUpdate[operator][strategies[i]][operatorSetKey].decrementAtAndFutureCheckpoints({ - key: uint32(block.timestamp), - decrementValue: slashedMagnitude - }); } // 2. if there are any pending deallocations then need to update and decrement if they fall within slashable window @@ -768,9 +763,9 @@ contract AVSDirectory is function _getLatestTotalMagnitude(address operator, IStrategy strategy) internal returns (uint64) { (bool exists,, uint224 totalMagnitude) = _totalMagnitudeUpdate[operator][strategy].latestCheckpoint(); if (!exists) { - totalMagnitude = ShareScalingLib.INITIAL_TOTAL_MAGNITUDE; + totalMagnitude = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; _totalMagnitudeUpdate[operator][strategy].push({key: uint32(block.timestamp), value: totalMagnitude}); - operatorMagnitudeInfo[operator][strategy].freeMagnitude = ShareScalingLib.INITIAL_TOTAL_MAGNITUDE; + operatorMagnitudeInfo[operator][strategy].freeMagnitude = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; } return uint64(totalMagnitude); @@ -1051,19 +1046,24 @@ contract AVSDirectory is OperatorSetRegistrationStatus memory registrationStatus = operatorSetStatus[operatorSet.avs][operator][operatorSet.operatorSetId]; return isMember(operator, operatorSet) - || registrationStatus.lastDeregisteredTimestamp + DEALLOCATION_DELAY >= block.timestamp; + || registrationStatus.lastDeregisteredTimestamp + SlashingConstants.DEALLOCATION_DELAY >= block.timestamp; } - /// @notice Returns the total magnitude of an operator for a given set of strategies + /** + * @notice Returns the current total magnitudes of an operator for a given set of strategies + * @param operator the operator to get the total magnitude for + * @param strategies the strategies to get the total magnitudes for + * @return totalMagnitudes the total magnitudes for each strategy + */ function getTotalMagnitudes( address operator, IStrategy[] calldata strategies ) external view returns (uint64[] memory) { uint64[] memory totalMagnitudes = new uint64[](strategies.length); for (uint256 i = 0; i < strategies.length;) { - (bool exists, uint32 key, uint224 value) = _totalMagnitudeUpdate[operator][strategies[i]].latestCheckpoint(); + (bool exists, , uint224 value) = _totalMagnitudeUpdate[operator][strategies[i]].latestCheckpoint(); if (!exists) { - totalMagnitudes[i] = ShareScalingLib.INITIAL_TOTAL_MAGNITUDE; + totalMagnitudes[i] = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; } else { totalMagnitudes[i] = uint64(value); } @@ -1075,7 +1075,13 @@ contract AVSDirectory is return totalMagnitudes; } - /// @notice Returns the total magnitude of an operator for a given set of strategies at a given timestamp + /** + * @notice Returns the total magnitudes of an operator for a given set of strategies at a given timestamp + * @param operator the operator to get the total magnitude for + * @param strategies the strategies to get the total magnitudes for + * @param timestamp the timestamp to get the total magnitudes at + * @return totalMagnitudes the total magnitudes for each strategy + */ function getTotalMagnitudesAtTimestamp( address operator, IStrategy[] calldata strategies, @@ -1083,12 +1089,12 @@ contract AVSDirectory is ) external view returns (uint64[] memory) { uint64[] memory totalMagnitudes = new uint64[](strategies.length); for (uint256 i = 0; i < strategies.length;) { - (uint224 value, uint256 pos, uint256 length) = + (uint224 value, uint256 pos, ) = _totalMagnitudeUpdate[operator][strategies[i]].upperLookupRecentWithPos(timestamp); // if there is no existing total magnitude checkpoint - if (value != 0 || pos != 0) { - totalMagnitudes[i] = ShareScalingLib.INITIAL_TOTAL_MAGNITUDE; + if (value == 0 && pos == 0) { + totalMagnitudes[i] = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; } else { totalMagnitudes[i] = uint64(value); } @@ -1100,6 +1106,55 @@ contract AVSDirectory is return totalMagnitudes; } + /** + * @notice Returns the current total magnitude of an operator for a given strategy + * @param operator the operator to get the total magnitude for + * @param strategy the strategy to get the total magnitude for + * @return totalMagnitude the total magnitude for the strategy + */ + function getTotalMagnitude( + address operator, + IStrategy strategy + ) external view returns (uint64) { + uint64 totalMagnitude; + (bool exists, , uint224 value) = _totalMagnitudeUpdate[operator][strategy].latestCheckpoint(); + if (!exists) { + totalMagnitude = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; + } else { + totalMagnitude = uint64(value); + } + + return totalMagnitude; + } + + /** + * @notice Returns the total magnitude of an operator for a given strategy at a given timestamp + * @param operator the operator to get the total magnitude for + * @param strategy the strategy to get the total magnitude for + * @param timestamp the timestamp to get the total magnitude at + * @return totalMagnitude the total magnitude for the strategy + */ + function getTotalMagnitudeAtTimestamp( + address operator, + IStrategy strategy, + uint32 timestamp + ) external view returns (uint64) { + uint64 totalMagnitude = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; + ( + uint224 value, + uint256 pos, + ) = _totalMagnitudeUpdate[operator][strategy].upperLookupRecentWithPos(timestamp); + + // if there is no existing total magnitude checkpoint + if (value == 0 && pos == 0) { + totalMagnitude = SlashingConstants.INITIAL_TOTAL_MAGNITUDE; + } else { + totalMagnitude = uint64(value); + } + + return totalMagnitude; + } + // /** // * @notice fetches the minimum slashable shares for a certain operator and operatorSet for a list of strategies // * from the current timestamp until the given timestamp diff --git a/src/contracts/core/DelegationManager.sol b/src/contracts/core/DelegationManager.sol index 83e8173f78..2055fac867 100644 --- a/src/contracts/core/DelegationManager.sol +++ b/src/contracts/core/DelegationManager.sol @@ -6,7 +6,7 @@ import "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol"; import "@openzeppelin-upgrades/contracts/security/ReentrancyGuardUpgradeable.sol"; import "../permissions/Pausable.sol"; import "../libraries/EIP1271SignatureUtils.sol"; -import "../libraries/ShareScalingLib.sol"; +import "../libraries/SlashingConstants.sol"; import "./DelegationManagerStorage.sol"; /** @@ -252,9 +252,9 @@ contract DelegationManager is "DelegationManager.undelegate: caller cannot undelegate staker" ); - // Gather strategies and scaled shares to remove from staker/operator during undelegation + // Gather strategies and shares from the staker. Caluclate scaled shares to remove from operator during undelegation // Undelegation removes ALL currently-active strategies and shares - (IStrategy[] memory strategies, uint256[] memory scaledShares) = getDelegatableScaledShares(staker); + (IStrategy[] memory strategies, uint256[] memory shares) = getDelegatableShares(staker); // emit an event if this action was not initiated by the staker themselves if (msg.sender != staker) { @@ -270,18 +270,23 @@ contract DelegationManager is withdrawalRoots = new bytes32[](0); } else { withdrawalRoots = new bytes32[](strategies.length); + uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); + for (uint256 i = 0; i < strategies.length; i++) { IStrategy[] memory singleStrategy = new IStrategy[](1); - uint256[] memory singleScaledShare = new uint256[](1); + uint256[] memory singleShare = new uint256[](1); + uint64[] memory singleTotalMagnitude = new uint64[](1); singleStrategy[0] = strategies[i]; - singleScaledShare[0] = scaledShares[i]; + singleShare[0] = shares[i]; + singleTotalMagnitude[0] = totalMagnitudes[i]; withdrawalRoots[i] = _removeSharesAndQueueWithdrawal({ staker: staker, operator: operator, withdrawer: staker, strategies: singleStrategy, - scaledShares: singleScaledShare + sharesToWithdraw: singleShare, + totalMagnitudes: singleTotalMagnitude }); } } @@ -314,13 +319,9 @@ contract DelegationManager is "DelegationManager.queueWithdrawal: withdrawer must be staker" ); - // Withdrawer inputs the number of real Strategy shares they expect to withdraw, - // convert accordingly to scaledShares in storage - uint256[] memory scaledShares = ShareScalingLib.scaleShares( - avsDirectory, + uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes( operator, - queuedWithdrawalParams[i].strategies, - queuedWithdrawalParams[i].shares + queuedWithdrawalParams[i].strategies ); // Remove shares from staker's strategies and place strategies/shares in queue. @@ -331,7 +332,8 @@ contract DelegationManager is operator: operator, withdrawer: queuedWithdrawalParams[i].withdrawer, strategies: queuedWithdrawalParams[i].strategies, - scaledShares: scaledShares + sharesToWithdraw: queuedWithdrawalParams[i].shares, + totalMagnitudes: totalMagnitudes }); } return withdrawalRoots; @@ -375,53 +377,82 @@ contract DelegationManager is } /** - * @notice Increases a staker's delegated share balance in a strategy. - * @param staker The address to increase the delegated scaled shares for their operator. - * @param strategy The strategy in which to increase the delegated scaled shares. - * @param scaledShares The number of scaled shares to increase. + * @notice Increases a staker's delegated share balance in a strategy. Note that before adding to operator shares, + * the delegated shares are scaled according to the operator's total magnitude as part of slashing accounting. + * The staker's scaling factor 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 existingShares The number of 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 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 shares in `strategy` by `scaledShares`. Otherwise does nothing. + * @dev *If the staker is actively delegated*, then increases the `staker`'s delegated scaled shares in `strategy` after scaling `shares`. + * Otherwise does nothing. * @dev Callable only by the StrategyManager or EigenPodManager. */ - function increaseDelegatedScaledShares( + function increaseDelegatedShares( address staker, IStrategy strategy, - uint256 scaledShares + uint256 existingShares, + uint256 addedShares ) external onlyStrategyManagerOrEigenPodManager { // if the staker is delegated to an operator if (isDelegated(staker)) { address operator = delegatedTo[staker]; + uint64 totalMagnitude = avsDirectory.getTotalMagnitude(operator, strategy); + + // update stakers scaling deposit scaling factor + uint256 newStakerScalingFactor = _calculateStakerScalingFactor({ + staker: staker, + strategy: strategy, + totalMagnitude: totalMagnitude, + existingShares: existingShares, + addedShares: addedShares + }); + stakerScalingFactors[staker][strategy] = newStakerScalingFactor; // add strategy shares to delegate's shares - _increaseOperatorScaledShares({operator: operator, staker: staker, strategy: strategy, scaledShares: scaledShares}); + _increaseOperatorScaledShares({ + operator: operator, + staker: staker, + strategy: strategy, + shares: addedShares, + totalMagnitude: totalMagnitude + }); } } /** - * @notice Decreases a staker's delegated share balance in a strategy. + * @notice Decreases a staker's delegated share balance in a strategy. Note that before removing from operator shares, + * the delegated shares are scaled according to the operator's total magnitude as part of slashing accounting. Unlike + * `increaseDelegatedShares`, the staker's scaling factor is not updated here. * @param staker The address to increase the delegated scaled shares for their operator. * @param strategy The strategy in which to decrease the delegated scaled shares. - * @param scaledShares The number of scaled shares to decrease. + * @param removedShares The number of shares to decremented for the strategy in the + * StrategyManager/EigenPodManager * * @dev *If the staker is actively delegated*, then decreases the `staker`'s delegated scaled shares in `strategy` by `scaledShares`. Otherwise does nothing. * @dev Callable only by the StrategyManager or EigenPodManager. */ - function decreaseDelegatedScaledShares( + function decreaseDelegatedShares( address staker, IStrategy strategy, - uint256 scaledShares + uint256 removedShares ) external onlyStrategyManagerOrEigenPodManager { // if the staker is delegated to an operator if (isDelegated(staker)) { address operator = delegatedTo[staker]; - // forgefmt: disable-next-item + uint64 totalMagnitude = avsDirectory.getTotalMagnitude(operator, strategy); + // subtract strategy shares from delegated scaled shares _decreaseOperatorScaledShares({ - operator: operator, - staker: staker, - strategy: strategy, - scaledShares: scaledShares + operator: operator, + staker: staker, + strategy: strategy, + shares: removedShares, + totalMagnitude: totalMagnitude }); } } @@ -568,41 +599,31 @@ contract DelegationManager is delegatedTo[staker] = operator; emit StakerDelegated(staker, operator); - (IStrategy[] memory strategies, uint256[] memory shares) = getDelegatableScaledShares(staker); - - // NOTE: technically the function above returns scaled shares, so it needs to be descaled before scaling to the - // the new operator according to their totalMagnitude. However, since the staker is not delegated, - // scaledShares = shares so we can skip descaling and read these as descaled shares. - uint256[] memory scaledShares = ShareScalingLib.scaleShares(avsDirectory, operator, strategies, shares); + // read staker's delegatable shares and strategies to add to operator's scaled shares + // and also update the staker scaling factor for each strategy + (IStrategy[] memory strategies, uint256[] memory shares) = getDelegatableShares(staker); + uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); for (uint256 i = 0; i < strategies.length;) { + // update stakers scaling deposit scaling factor + uint256 newStakerScalingFactor = _calculateStakerScalingFactor({ + staker: staker, + strategy: strategies[i], + totalMagnitude: totalMagnitudes[i], + existingShares: 0, + addedShares: shares[i] + }); + stakerScalingFactors[staker][strategies[i]] = newStakerScalingFactor; + // forgefmt: disable-next-item _increaseOperatorScaledShares({ operator: operator, staker: staker, strategy: strategies[i], - scaledShares: scaledShares[i] + shares: shares[i], + totalMagnitude: totalMagnitudes[i] }); - // stakerStrategyScaledShares(staker, strategies[i]) needs to be updated to be equal to newly scaled shares. - // we take the difference between the shares and the scaledShares to update the staker's scaledShares. The key property - // here is that scaledShares should be >= shares and can never be less than shares due to totalMagnitude being - // a monotonically decreasing value (see ShareScalingLib.scaleShares for more info). - // The exact same property applies to the EigenPodManager and podOwnerScaledShares where real ETH shares are scaled based on - // the delegated operator (if any) prior to being stored as scaledShares in the EigenPodManager. - if (shares[i] < scaledShares[i]) { - if (strategies[i] == beaconChainETHStrategy) { - eigenPodManager.addScaledShares(staker, scaledShares[i] - shares[i]); - } else { - strategyManager.addScaledShares( - staker, - strategies[i].underlyingToken(), //TODO possibly remove this from the interface - strategies[i], - scaledShares[i] - shares[i] - ); - } - } - unchecked { ++i; } @@ -610,10 +631,12 @@ contract DelegationManager is } /** - * @dev This function "descales" the scaledShares according to the totalMagnitude at the time of withdrawal completion. - * This will apply any slashing that has occurred since the scaledShares were initially set in storage. - * If receiveAsTokens is true, then these descaled shares will be withdrawn as tokens. - * If receiveAsTokens is false, then they will be rescaled according to the current operator the staker is delegated to, and added back to the operator's scaledShares. + * @dev This function completes a queued withdrawal for a staker. + * This will apply any slashing that has occurred since the scaledShares were initially set in the Withdrawal struct up + * until the totalMagnitude at completion time. + * If receiveAsTokens is true, then these shares will be withdrawn as tokens. + * If receiveAsTokens is false, then they will be redeposited according to the current operator the staker is delegated to, + * and added back to the operator's scaledShares. */ function _completeQueuedWithdrawal( Withdrawal calldata withdrawal, @@ -638,30 +661,27 @@ contract DelegationManager is "DelegationManager._completeQueuedWithdrawal: input length mismatch" ); - // read delegated operator for scaling and adding shares back if needed - address currentOperator = delegatedTo[msg.sender]; - // descale shares to get the "real" strategy share values of the withdrawal - uint256[] memory descaledShares = ShareScalingLib.descaleSharesAtTimestamp( - avsDirectory, - withdrawal.delegatedTo, - withdrawal.strategies, - withdrawal.scaledShares, - withdrawal.startTimestamp + avsDirectory.DEALLOCATION_DELAY() - ); + // read delegated operator's totalMagnitudes at time of withdrawal to scale shares again if any slashing has occurred + // during withdrawal delay period + uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudesAtTimestamp({ + operator: withdrawal.delegatedTo, + strategies: withdrawal.strategies, + timestamp: withdrawal.startTimestamp + SlashingConstants.DEALLOCATION_DELAY + }); if (receiveAsTokens) { - // complete the withdrawal by converting descaled shares to tokens + // complete the withdrawal by converting shares to tokens _completeReceiveAsTokens( withdrawal, tokens, - descaledShares + totalMagnitudes ); } else { // Award shares back in StrategyManager/EigenPodManager. _completeReceiveAsShares( withdrawal, tokens, - descaledShares + totalMagnitudes ); } @@ -670,11 +690,15 @@ contract DelegationManager is emit WithdrawalCompleted(withdrawalRoot); } - /// TODO: natspec + /** + * @dev This function completes a queued withdrawal for a staker by converting shares to tokens and transferring to the withdrawer. + * This will apply any slashing that has occurred since the scaledShares were initially set in the Withdrawal struct + * by reading the original delegated operator's totalMagnitude at the time of withdrawal completion. + */ function _completeReceiveAsTokens( Withdrawal calldata withdrawal, IERC20[] calldata tokens, - uint256[] memory descaledShares + uint64[] memory totalMagnitudes ) internal { // Finalize action by converting scaled shares to tokens for each strategy, or // by re-awarding shares in each strategy. @@ -684,11 +708,16 @@ contract DelegationManager is "DelegationManager._completeReceiveAsTokens: withdrawalDelay period has not yet passed for this strategy" ); + // Take already scaled staker shares and scale again according to current operator totalMagnitude + // This is because the totalMagnitude may have changed since withdrawal was queued and the staker shares + // are still susceptible to slashing + uint256 sharesToWithdraw = _calculateSharesToCompleteWithdraw(withdrawal.scaledShares[i], totalMagnitudes[i]); + _withdrawSharesAsTokens({ staker: withdrawal.staker, withdrawer: msg.sender, strategy: withdrawal.strategies[i], - shares: descaledShares[i], + shares: sharesToWithdraw, token: tokens[i] }); unchecked { @@ -697,21 +726,20 @@ contract DelegationManager is } } - /// TODO: natspec + /** + * @dev This function completes a queued withdrawal for a staker by receiving them back as shares + * in the StrategyManager/EigenPodManager. If the withdrawer is delegated to an operator, this will also + * increase the operator's scaled shares. + * This will apply any slashing that has occurred since the scaledShares were initially set in the Withdrawal struct + * by reading the original delegated operator's totalMagnitude at the time of withdrawal completion. + */ function _completeReceiveAsShares( Withdrawal calldata withdrawal, IERC20[] calldata tokens, - uint256[] memory descaledShares + uint64[] memory totalMagnitudes ) internal { // read delegated operator for scaling and adding shares back if needed address currentOperator = delegatedTo[msg.sender]; - // We scale shares again to the new totalMagnitude of the currentOperator. - uint256[] memory scaledShares = ShareScalingLib.scaleShares( - avsDirectory, - currentOperator, - withdrawal.strategies, - descaledShares - ); for (uint256 i = 0; i < withdrawal.strategies.length;) { require( @@ -719,6 +747,14 @@ contract DelegationManager is "DelegationManager._completeReceiveAsShares: withdrawalDelay period has not yet passed for this strategy" ); + // Take already scaled staker shares and scale again according to current operator totalMagnitude + // This is because the totalMagnitude may have changed since withdrawal was queued and the staker shares + // are still susceptible to slashing + uint256 shares = _calculateSharesToCompleteWithdraw(withdrawal.scaledShares[i], totalMagnitudes[i]); + + // store existing shares to calculate new staker scaling factor later + uint256 existingShares; + /** * When awarding podOwnerShares in EigenPodManager, we need to be sure to only give them back to the original podOwner. * Other strategy shares can + will be awarded to the withdrawer. @@ -729,8 +765,10 @@ contract DelegationManager is * Update shares amount depending upon the returned value. * The return value will be lower than the input value in the case where the staker has an existing share deficit */ - uint256 increaseInDelegateableScaledShares = - eigenPodManager.addScaledShares({podOwner: staker, scaledShares: scaledShares[i]}); + ( + shares, + existingShares + ) = eigenPodManager.addShares({podOwner: staker, shares: shares}); address podOwnerOperator = delegatedTo[staker]; // Similar to `isDelegated` logic if (podOwnerOperator != address(0)) { @@ -739,11 +777,12 @@ contract DelegationManager is // the 'staker' here is the address receiving new shares staker: staker, strategy: withdrawal.strategies[i], - scaledShares: increaseInDelegateableScaledShares + shares: shares, + totalMagnitude: totalMagnitudes[i] }); } } else { - strategyManager.addScaledShares(msg.sender, tokens[i], withdrawal.strategies[i], scaledShares[i]); + existingShares = strategyManager.addShares(msg.sender, tokens[i], withdrawal.strategies[i], shares); // Similar to `isDelegated` logic if (currentOperator != address(0)) { _increaseOperatorScaledShares({ @@ -751,26 +790,68 @@ contract DelegationManager is // the 'staker' here is the address receiving new shares staker: msg.sender, strategy: withdrawal.strategies[i], - scaledShares: scaledShares[i] + shares: shares, + totalMagnitude: totalMagnitudes[i] }); } } + + // update stakers scaling deposit scaling factor + uint256 newStakerScalingFactor = _calculateStakerScalingFactor({ + staker: withdrawal.staker, + strategy: withdrawal.strategies[i], + totalMagnitude: totalMagnitudes[i], + existingShares: existingShares, + addedShares: shares + }); + stakerScalingFactors[withdrawal.staker][withdrawal.strategies[i]] = newStakerScalingFactor; + unchecked { ++i; } } } - // @notice Increases `operator`s delegated scaled shares in `strategy` by `scaledShares` and emits an `OperatorSharesIncreased` event - function _increaseOperatorScaledShares(address operator, address staker, IStrategy strategy, uint256 scaledShares) internal { + /** + * @notice Increases `operator`s delegated scaled shares in `strategy` based on staker's added shares and operator's totalMagnitude + * @param operator The operator to increase the delegated scaled shares for + * @param staker The staker to increase the delegated scaled shares for + * @param strategy The strategy to increase the delegated scaled shares for + * @param shares The shares added to the staker in the StrategyManager/EigenPodManager + * @param totalMagnitude The current total magnitude of the operator for the strategy + */ + function _increaseOperatorScaledShares( + address operator, + address staker, + IStrategy strategy, + uint256 shares, + uint64 totalMagnitude + ) internal { + // based on total magnitude, update operators scaled shares + uint256 scaledShares = _scaleShares(shares, totalMagnitude); operatorScaledShares[operator][strategy] += scaledShares; + // TODO: What to do about event wrt scaling? emit OperatorSharesIncreased(operator, staker, strategy, scaledShares); } - // @notice Decreases `operator`s delegated scaled shares in `strategy` by `scaledShares` and emits an `OperatorSharesDecreased` event - function _decreaseOperatorScaledShares(address operator, address staker, IStrategy strategy, uint256 scaledShares) internal { - // This will revert on underflow, so no check needed + /** + * @notice Decreases `operator`s delegated scaled shares in `strategy` based on staker's removed shares and operator's totalMagnitude + * @param operator The operator to decrease the delegated scaled shares for + * @param staker The staker to decrease the delegated scaled shares for + * @param strategy The strategy to decrease the delegated scaled shares for + * @param shares The shares removed from the staker in the StrategyManager/EigenPodManager + * @param totalMagnitude The current total magnitude of the operator for the strategy + */ + function _decreaseOperatorScaledShares( + address operator, + address staker, + IStrategy strategy, + uint256 shares, + uint64 totalMagnitude + ) internal { + // based on total magnitude, decrement operator's scaled shares + uint256 scaledShares = _scaleShares(shares, totalMagnitude); operatorScaledShares[operator][strategy] -= scaledShares; // TODO: What to do about event wrt scaling? emit OperatorSharesDecreased(operator, staker, strategy, scaledShares); @@ -786,16 +867,44 @@ contract DelegationManager is address operator, address withdrawer, IStrategy[] memory strategies, - uint256[] memory scaledShares + uint256[] memory sharesToWithdraw, + uint64[] memory totalMagnitudes ) internal returns (bytes32) { require( staker != address(0), "DelegationManager._removeSharesAndQueueWithdrawal: staker cannot be zero address" ); require(strategies.length != 0, "DelegationManager._removeSharesAndQueueWithdrawal: strategies cannot be empty"); + uint256[] memory scaledStakerShares = new uint256[](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;) { + + // check sharesToWithdraw is valid + // TODO maybe have a getter to get totalShares for all strategies, like getDelegatableShares + // but for inputted strategies + uint256 totalShares; + if (strategies[i] == beaconChainETHStrategy) { + int256 podShares = eigenPodManager.podOwnerShares(staker); + totalShares = podShares <= 0 ? 0 : uint256(podShares); + } else { + totalShares = strategyManager.stakerStrategyShares(staker, strategies[i]); + } + require( + sharesToWithdraw[i] <= _getWithdrawableShares(staker, strategies[i], totalShares, totalMagnitudes[i]), + "DelegationManager._removeSharesAndQueueWithdrawal: shares to withdraw exceeds withdrawable shares" + ); + // calculate scaledShares to place into queue withdrawal and shares to decrement from SM/EPM + uint256 stakerScalingFactor = _getStakerScalingFactor(staker, strategies[i]); + ( + // shares to decrement from StrategyManager/EigenPodManager + uint256 sharesToDecrement, + // scaledShares for staker to place into queueWithdrawal + uint256 scaledShares + ) = _calculateSharesToQueueWithdraw(sharesToWithdraw[i], stakerScalingFactor, totalMagnitudes[i]); + scaledStakerShares[i] = scaledShares; + // Similar to `isDelegated` logic if (operator != address(0)) { // forgefmt: disable-next-item @@ -803,7 +912,8 @@ contract DelegationManager is operator: operator, staker: staker, strategy: strategies[i], - scaledShares: scaledShares[i] + shares: sharesToWithdraw[i], + totalMagnitude: totalMagnitudes[i] }); } @@ -815,14 +925,14 @@ contract DelegationManager is * shares from the operator to whom the staker is delegated. * It will also revert if the share amount being withdrawn is not a whole Gwei amount. */ - eigenPodManager.removeScaledShares(staker, scaledShares[i]); + eigenPodManager.removeShares(staker, sharesToDecrement); } else { require( staker == withdrawer || !strategyManager.thirdPartyTransfersForbidden(strategies[i]), "DelegationManager._removeSharesAndQueueWithdrawal: withdrawer must be same address as staker if thirdPartyTransfersForbidden are set" ); // this call will revert if `scaledShares[i]` exceeds the Staker's current shares in `strategies[i]` - strategyManager.removeScaledShares(staker, strategies[i], scaledShares[i]); + strategyManager.removeShares(staker, strategies[i], sharesToDecrement); } unchecked { @@ -841,7 +951,7 @@ contract DelegationManager is nonce: nonce, startTimestamp: uint32(block.timestamp), strategies: strategies, - scaledShares: scaledShares + scaledShares: scaledStakerShares }); bytes32 withdrawalRoot = calculateWithdrawalRoot(withdrawal); @@ -864,11 +974,15 @@ contract DelegationManager is uint256 shares, IERC20 token ) internal { - // TODO: rename params if (strategy == beaconChainETHStrategy) { eigenPodManager.withdrawSharesAsTokens({podOwner: staker, destination: withdrawer, shares: shares}); } else { - strategyManager.withdrawSharesAsTokens(withdrawer, strategy, shares, token); + strategyManager.withdrawSharesAsTokens({ + recipient: withdrawer, + strategy: strategy, + shares: shares, + token: token + }); } } @@ -941,6 +1055,118 @@ contract DelegationManager is } } + /** + * + * SLASHING AND SHARES HELPER FUNCTIONS + * + */ + + /** + * @notice helper pure to calculate the scaledShares to store in queue withdrawal + * and the shares to decrement from StrategyManager/EigenPodManager + */ + function _calculateSharesToQueueWithdraw( + uint256 sharesToWithdraw, + uint256 stakerScalingFactor, + uint64 totalMagnitude + ) internal pure returns (uint256 sharesToDecrement, uint256 scaledStakerShares) { + // TODO: DOUBLE CHECK THIS BEHAVIOR + // NOTE that to prevent numerator overflow, the max sharesToWithdraw is + // x*1e36 <= 2^256-1 + // => x <= 1.1579e41 + sharesToDecrement = (sharesToWithdraw * SlashingConstants.PRECISION_FACTOR_SQUARED) / (stakerScalingFactor * totalMagnitude); + scaledStakerShares = sharesToWithdraw * stakerScalingFactor / SlashingConstants.PRECISION_FACTOR; + return (sharesToDecrement, scaledStakerShares); + } + + /** + * @notice helper pure to calculate the shares to complete a withdrawal after slashing + * We use the totalMagnitude of the delegated operator at the time of withdrawal completion to scale the shares + * back to real StrategyManager/EigenPodManager shares + */ + function _calculateSharesToCompleteWithdraw( + uint256 scaledStakerShares, + uint64 totalMagnitudeAtCompletion + ) internal pure returns (uint256 shares) { + shares = scaledStakerShares * totalMagnitudeAtCompletion / SlashingConstants.PRECISION_FACTOR; + } + + /** + * @notice helper pure to return scaledShares given shares and current totalMagnitude. Used for + * adding/removing staker shares from operatorScaledShares + */ + function _scaleShares(uint256 shares, uint64 totalMagnitude) internal pure returns (uint256) { + return shares * SlashingConstants.PRECISION_FACTOR / totalMagnitude; + } + + /** + * @notice helper pure to return real strategy shares given operator scaledShares and current totalMagnitude. + * Used for returning the total delegated shares for an operator and strategy + */ + function _descaleShares(uint256 scaledShares, uint64 totalMagnitude) internal pure returns (uint256) { + return scaledShares * totalMagnitude / SlashingConstants.PRECISION_FACTOR; + } + + /** + * @notice staker scaling factor should be initialized and lower bounded to 1e18 + */ + function _getStakerScalingFactor(address staker, IStrategy strategy) internal view returns (uint256) { + uint256 currStakerScalingFactor = stakerScalingFactors[staker][strategy]; + if (currStakerScalingFactor == 0) { + currStakerScalingFactor = SlashingConstants.PRECISION_FACTOR; + } + return currStakerScalingFactor; + } + + /** + * @notice helper to calculate the new staker scaling factor after adding shares. This is only used + * when a staker is depositing through the StrategyManager or EigenPodManager. A stakers scaling factor + * is only updated when they have new deposits and their shares are being increased. + */ + function _calculateStakerScalingFactor( + address staker, + IStrategy strategy, + uint64 totalMagnitude, + uint256 existingShares, + uint256 addedShares + ) internal view returns (uint256) { + uint256 newStakerScalingFactor; + + if (existingShares == 0) { + // existing shares are 0, meaning no existing delegated shares. In this case, the new staker scaling factor + // is re-initialized to + newStakerScalingFactor = SlashingConstants.PRECISION_FACTOR / (totalMagnitude); + } else { + uint256 currStakerScalingFactor = _getStakerScalingFactor(staker, strategy); + + // TODO: DOUBLE CHECK THIS BEHAVIOR AND OVERFLOWS + // staker scaling factor is initialized to PRECISION_FACTOR(1e18) and totalMagnitude is initialized to INITIAL_TOTAL_MAGNITUDE(1e18) + // and is monotonically decreasing. You can deduce that the newStakerScalingFactor will never decrease to less than the PRECISION_FACTOR + // so this won't round to 0. + newStakerScalingFactor = + (currStakerScalingFactor * existingShares * totalMagnitude / SlashingConstants.PRECISION_FACTOR + + addedShares * SlashingConstants.PRECISION_FACTOR) + / + ((existingShares + addedShares) * totalMagnitude); + } + + return newStakerScalingFactor; + } + + /** + * @notice helper to calculate the withdrawable shares for a staker given their shares and the + * current totalMagnitude. `shares` should be the staker's shares in storage in the StrategyManager/EigenPodManager. + */ + function _getWithdrawableShares( + address staker, + IStrategy strategy, + uint256 shares, + uint64 currTotalMagnitude + ) internal view returns (uint256) { + uint256 stakerScalingFactor = _getStakerScalingFactor(staker, strategy); + return stakerScalingFactor * shares * currTotalMagnitude / SlashingConstants.PRECISION_FACTOR; + } + /** * * VIEW FUNCTIONS @@ -1005,64 +1231,10 @@ contract DelegationManager is return _operatorDetails[operator].stakerOptOutWindowBlocks; } - /// @notice a legacy function that returns the operatorShares for an operator and strategy + /// @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) { - return ShareScalingLib.descaleShares({ - avsDirectory: avsDirectory, - operator: operator, - strategy: strategy, - scaledShares: operatorScaledShares[operator][strategy] - }); - } - - /// @notice Given array of strategies, returns array of scaled shares for the operator - function getOperatorScaledShares( - address operator, - IStrategy[] memory strategies - ) public view returns (uint256[] memory) { - uint256[] memory scaledShares = new uint256[](strategies.length); - for (uint256 i = 0; i < strategies.length; ++i) { - scaledShares[i] = operatorScaledShares[operator][strategies[i]]; - } - return scaledShares; - } - - /// @notice Given array of strategies, returns array of shares for the operator - function getOperatorShares( - address operator, - IStrategy[] memory strategies - ) public view returns (uint256[] memory) { - return ShareScalingLib.descaleShares({ - avsDirectory: avsDirectory, - operator: operator, - strategies: strategies, - scaledShares: getOperatorScaledShares(operator, strategies) - }); - } - - /** - * @notice Given a staker and shares amounts of deposits, return the scaled shares calculated if - * the staker were to deposit. Depends on which operator the staker is delegated to. - */ - function getStakerScaledShares( - address staker, - IStrategy strategy, - uint256 shares - ) public view returns (uint256 scaledShares) { - address operator = delegatedTo[staker]; - if (operator == address(0)) { - // if the staker is not delegated to an operator, return the shares as is - // as no slashing and scaling applied - scaledShares = shares; - } else { - // if the staker is delegated to an operator, scale the shares accordingly - scaledShares = ShareScalingLib.scaleShares({ - avsDirectory: avsDirectory, - operator: operator, - strategy: strategy, - shares: shares - }); - } + uint64 totalMagnitude = avsDirectory.getTotalMagnitude(operator, strategy); + return _descaleShares(operatorScaledShares[operator][strategy], totalMagnitude); } /** @@ -1071,62 +1243,68 @@ contract DelegationManager is * 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 getStakerShares( + function getWithdrawableStakerShares( address staker, - IStrategy strategy, - uint256 scaledShares - ) public view returns (uint256 shares) { + IStrategy[] calldata strategies + ) external view returns (uint256[] memory shares) { address operator = delegatedTo[staker]; - if (operator == address(0)) { - // if the staker is not delegated to an operator, return the shares as is - // as no slashing and scaling applied - shares = scaledShares; - } else { - // if the staker is delegated to an operator, descale the shares accordingly - shares = ShareScalingLib.descaleShares({ - avsDirectory: avsDirectory, - operator: operator, - strategy: strategy, - scaledShares: scaledShares - }); + for (uint256 i = 0; i < strategies.length; ++i) { + // 1. read strategy shares + if (strategies[i] == beaconChainETHStrategy) { + int256 podShares = eigenPodManager.podOwnerShares(staker); + shares[i] = podShares <= 0 ? 0 : uint256(podShares); + } else { + shares[i] = strategyManager.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 + if (operator != address(0)) { + uint64 totalMagnitude = avsDirectory.getTotalMagnitude(operator, strategies[i]); + shares[i] = _getWithdrawableShares(staker, strategies[i], shares[i], totalMagnitude); + } } + return shares; } /** - * @notice Returns the number of actively-delegatable scaled shares a staker has across all strategies. + * @notice Returns the number of actively-delegatable shares a staker has across all strategies. + * NOTE: If you are delegated to an operator and have been slashed, these values won't be your real actual + * delegatable shares! * @dev Returns two empty arrays in the case that the Staker has no actively-delegateable shares. */ - function getDelegatableScaledShares(address staker) public view returns (IStrategy[] memory, uint256[] memory) { - // Get currently active scaled shares and strategies for `staker` - int256 scaledPodShares = eigenPodManager.podOwnerScaledShares(staker); + function getDelegatableShares(address staker) public view returns (IStrategy[] memory, uint256[] memory) { + // Get current StrategyManager/EigenPodManager shares and strategies for `staker` + // If `staker` is already delegated, these may not be the full withdrawable amounts due to slashing + int256 podShares = eigenPodManager.podOwnerShares(staker); (IStrategy[] memory strategyManagerStrats, uint256[] memory strategyManagerShares) = strategyManager.getDeposits(staker); // Has no shares in EigenPodManager, but potentially some in StrategyManager - if (scaledPodShares <= 0) { + if (podShares <= 0) { return (strategyManagerStrats, strategyManagerShares); } IStrategy[] memory strategies; - uint256[] memory scaledShares; + uint256[] memory shares; if (strategyManagerStrats.length == 0) { // Has shares in EigenPodManager, but not in StrategyManager strategies = new IStrategy[](1); - scaledShares = new uint256[](1); + shares = new uint256[](1); strategies[0] = beaconChainETHStrategy; - scaledShares[0] = uint256(scaledPodShares); + shares[0] = uint256(podShares); } else { // Has shares in both -// TODO: make more efficient by resizing array + // TODO: make more efficient by resizing array // 1. Allocate return arrays strategies = new IStrategy[](strategyManagerStrats.length + 1); - scaledShares = new uint256[](strategies.length); + shares = new uint256[](strategies.length); // 2. Place StrategyManager strats/shares in return arrays for (uint256 i = 0; i < strategyManagerStrats.length;) { strategies[i] = strategyManagerStrats[i]; - scaledShares[i] = strategyManagerShares[i]; + shares[i] = strategyManagerShares[i]; unchecked { ++i; @@ -1135,10 +1313,10 @@ contract DelegationManager is // 3. Place EigenPodManager strat/shares in return arrays strategies[strategies.length - 1] = beaconChainETHStrategy; - scaledShares[strategies.length - 1] = uint256(scaledPodShares); + shares[strategies.length - 1] = uint256(podShares); } - return (strategies, scaledShares); + return (strategies, shares); } /** diff --git a/src/contracts/core/DelegationManagerStorage.sol b/src/contracts/core/DelegationManagerStorage.sol index c1595f1e8f..86e3149709 100644 --- a/src/contracts/core/DelegationManagerStorage.sol +++ b/src/contracts/core/DelegationManagerStorage.sol @@ -34,7 +34,7 @@ abstract contract DelegationManagerStorage is IDelegationManager { */ bytes32 internal _DOMAIN_SEPARATOR; - /// TODO @dev actually make this immutable and add to constructor + /// @notice The AVSDirectory contract for EigenLayer IAVSDirectory public immutable avsDirectory; /// @notice The StrategyManager contract for EigenLayer @@ -121,6 +121,10 @@ abstract contract DelegationManagerStorage is IDelegationManager { */ mapping(IStrategy => uint256) public strategyWithdrawalDelays; + /// @notice Mapping: staker => strategy => scaling factor used to calculate the staker's withdrawable shares in the strategy. + /// This is updated upon each deposit based on the staker's currently delegated operator's totalMagnitude. + mapping(address => mapping(IStrategy => uint256)) public stakerScalingFactors; + /// @notice Mapping: operator => allocation delay (in seconds) for the operator. /// This determines how long it takes for allocations to take effect in the future. Can only be set one time for each operator mapping(address => AllocationDelayDetails) internal _operatorAllocationDelay; @@ -137,5 +141,5 @@ abstract contract DelegationManagerStorage is IDelegationManager { * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[38] private __gap; + uint256[37] private __gap; } diff --git a/src/contracts/core/StrategyManager.sol b/src/contracts/core/StrategyManager.sol index abe7f07d39..8f0fca302c 100644 --- a/src/contracts/core/StrategyManager.sol +++ b/src/contracts/core/StrategyManager.sol @@ -96,7 +96,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 scaledShares The amount of new scaled 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 @@ -106,8 +106,8 @@ contract StrategyManager is IStrategy strategy, IERC20 token, uint256 amount - ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 scaledShares) { - scaledShares = _depositIntoStrategy(msg.sender, strategy, token, amount); + ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 shares) { + shares = _depositIntoStrategy(msg.sender, strategy, token, amount); } /** @@ -122,7 +122,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 scaledShares The amount of new scaled 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. @@ -138,7 +138,7 @@ contract StrategyManager is address staker, uint256 expiry, bytes memory signature - ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 scaledShares) { + ) external onlyWhenNotPaused(PAUSED_DEPOSITS) nonReentrant returns (uint256 shares) { require( !thirdPartyTransfersForbidden[strategy], "StrategyManager.depositIntoStrategyWithSignature: third transfers disabled" @@ -163,34 +163,35 @@ contract StrategyManager is EIP1271SignatureUtils.checkSignature_EIP1271(staker, digestHash, signature); // deposit the tokens (from the `msg.sender`) and credit the new shares to the `staker` - scaledShares = _depositIntoStrategy(staker, strategy, token, amount); + shares = _depositIntoStrategy(staker, strategy, token, amount); } - /// @notice Used by the DelegationManager to remove a Staker's scaled shares from a particular strategy when entering the withdrawal queue - function removeScaledShares(address staker, IStrategy strategy, uint256 scaledShares) external onlyDelegationManager { - _removeScaledShares(staker, strategy, scaledShares); + /// @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, uint256 shares) external onlyDelegationManager { + _removeShares(staker, strategy, shares); } - /// @notice Used by the DelegationManager to award a Staker some scaled shares that have passed through the withdrawal queue - function addScaledShares( + /// @notice Used by the DelegationManager to award a Staker some shares that have passed through the withdrawal queue + function addShares( address staker, IERC20 token, IStrategy strategy, - uint256 scaledShares - ) external onlyDelegationManager { - _addScaledShares(staker, token, strategy, scaledShares); + uint256 shares + ) external onlyDelegationManager returns (uint256 existingShares) { + uint256 existingShares = _addShares(staker, token, strategy, shares); + return existingShares; } /// @notice Used by the DelegationManager to convert withdrawn shares to tokens and send them to a recipient - /// Assumes that shares being passed in have already been descaled accordingly to account for any slashing + /// Assumes that shares being passed in have already been accounted for any slashing /// and are the `real` shares in the strategy to withdraw function withdrawSharesAsTokens( address recipient, IStrategy strategy, - uint256 strategyShares, + uint256 shares, IERC20 token ) external onlyDelegationManager { - strategy.withdraw(recipient, token, strategyShares); + strategy.withdraw(recipient, token, shares); } /** @@ -265,93 +266,102 @@ contract StrategyManager is // INTERNAL FUNCTIONS /** - * @notice This function adds `scaledShares` for a given `strategy` to the `staker` and runs through the necessary update logic. - * @param staker The address to add scaledShares to + * @notice This function adds `shares` for a given `strategy` to the `staker` and runs through the necessary update logic. + * @param staker The address to add shares to * @param token The token that is being deposited (used for indexing) - * @param strategy The Strategy in which the `staker` is receiving scaledShares - * @param scaledShares The amount of scaled shares to grant to the `staker` - * @dev In particular, this function calls `delegation.increaseDelegatedScaledShares(staker, strategy, scaledShares)` to ensure that all - * delegated scaledShares are tracked, increases the stored share amount in `stakerStrategyScaledShares[staker][strategy]`, and adds `strategy` + * @param strategy The Strategy in which the `staker` is receiving shares + * @param shares The amount of shares 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` * to the `staker`'s list of strategies, if it is not in the list already. */ - function _addScaledShares(address staker, IERC20 token, IStrategy strategy, uint256 scaledShares) internal { + function _addShares( + address staker, + IERC20 token, + IStrategy strategy, + uint256 shares + ) internal returns (uint256 existingShares) { // sanity checks on inputs - require(staker != address(0), "StrategyManager._addScaledShares: staker cannot be zero address"); - require(scaledShares != 0, "StrategyManager._addScaledShares: shares should not be zero!"); + require(staker != address(0), "StrategyManager._addShares: staker cannot be zero address"); + require(shares != 0, "StrategyManager._addShares: shares should not be zero!"); - // if they dont have existing scaled shares of this strategy, add it to their strats - if (stakerStrategyScaledShares[staker][strategy] == 0) { + // if they dont have existing shares of this strategy, add it to their strats + if (stakerStrategyShares[staker][strategy] == 0) { require( stakerStrategyList[staker].length < MAX_STAKER_STRATEGY_LIST_LENGTH, - "StrategyManager._addScaledShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH" + "StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH" ); stakerStrategyList[staker].push(strategy); } - // add the returned scaled shares to their existing scaled shares for this strategy - stakerStrategyScaledShares[staker][strategy] += scaledShares; + // add the returned shares to their existing shares for this strategy + uint256 existingShares = stakerStrategyShares[staker][strategy]; + stakerStrategyShares[staker][strategy] += shares; - emit Deposit(staker, token, strategy, scaledShares); + emit Deposit(staker, token, strategy, shares); + return existingShares; } /** * @notice Internal function in which `amount` of ERC20 `token` is transferred from `msg.sender` to the Strategy-type contract - * `strategy`, with the resulting scaledShares credited to `staker`. - * @param staker The address that will be credited with the new scaledShares. + * `strategy`, with the resulting shares credited to `staker`. + * @param staker The address that will be credited with the new shares. * @param strategy The Strategy contract to deposit into. * @param token The ERC20 token to deposit. * @param amount The amount of `token` to deposit. - * @return scaledShares The amount of *new* scaled shares in `strategy` that have been credited to the `staker`. + * @return shares The amount of *new* shares in `strategy` that have been credited to the `staker`. */ function _depositIntoStrategy( address staker, IStrategy strategy, IERC20 token, uint256 amount - ) internal onlyStrategiesWhitelistedForDeposit(strategy) returns (uint256 scaledShares) { + ) internal onlyStrategiesWhitelistedForDeposit(strategy) returns (uint256 shares) { // transfer tokens from the sender to the strategy token.safeTransferFrom(msg.sender, address(strategy), amount); // deposit the assets into the specified strategy and get the equivalent amount of shares in that strategy uint256 shares = strategy.deposit(token, amount); - // scale strategy shares before storing - scaledShares = delegation.getStakerScaledShares(staker, strategy, shares); + // add the returned shares to the staker's existing shares for this strategy + uint256 existingShares = _addShares(staker, token, strategy, shares); - // add the returned scaled shares to the staker's existing scaled shares for this strategy - _addScaledShares(staker, token, strategy, scaledShares); + // Increase shares delegated to operator, if needed + delegation.increaseDelegatedShares({ + staker: staker, + strategy: strategy, + existingShares: existingShares, + addedShares: shares + }); - // Increase scaled shares delegated to operator, if needed - delegation.increaseDelegatedScaledShares(staker, strategy, scaledShares); - - return scaledShares; + return shares; } /** - * @notice Decreases the scaled shares that `staker` holds in `strategy` by `scaledSharesAmount`. - * @param staker The address to decrement scaled shares from - * @param strategy The strategy for which the `staker`'s scaled shares are being decremented - * @param scaledSharesAmount The amount of scaled shares to decrement - * @dev If the amount of scaled shares represents all of the staker`s shares in said strategy, + * @notice Decreases the shares that `staker` holds in `strategy` by `sharesAmount`. + * @param staker The address to decrement shares from + * @param strategy The strategy for which the `staker`'s shares are being decremented + * @param sharesAmount 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 _removeScaledShares(address staker, IStrategy strategy, uint256 scaledSharesAmount) internal returns (bool) { + function _removeShares(address staker, IStrategy strategy, uint256 sharesAmount) internal returns (bool) { // sanity checks on inputs - require(scaledSharesAmount != 0, "StrategyManager._removeScaledShares: scaledSharesAmount should not be zero!"); + require(sharesAmount != 0, "StrategyManager._removeShares: sharesAmount should not be zero!"); - //check that the user has sufficient scaled shares - uint256 userShares = stakerStrategyScaledShares[staker][strategy]; + //check that the user has sufficient shares + uint256 userShares = stakerStrategyShares[staker][strategy]; - require(scaledSharesAmount <= userShares, "StrategyManager._removeScaledShares: scaledSharesAmount too high"); + require(sharesAmount <= userShares, "StrategyManager._removeShares: sharesAmount too high"); //unchecked arithmetic since we just checked this above unchecked { - userShares = userShares - scaledSharesAmount; + userShares = userShares - sharesAmount; } // subtract the shares from the staker's existing shares for this strategy - stakerStrategyScaledShares[staker][strategy] = userShares; + stakerStrategyShares[staker][strategy] = userShares; - // if no existing scaled shares, remove the strategy from the staker's dynamic array of strategies + // if no existing shares, remove the strategy from the staker's dynamic array of strategies if (userShares == 0) { _removeStrategyFromStakerStrategyList(staker, strategy); @@ -408,31 +418,23 @@ contract StrategyManager is } // VIEW FUNCTIONS + /** - * @notice Get all details on the staker's deposits and corresponding scaled shares + * @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, scaled shares in these strategies) + * @return (staker's strategies, shares in these strategies) */ function getDeposits(address staker) external view returns (IStrategy[] memory, uint256[] memory) { uint256 strategiesLength = stakerStrategyList[staker].length; - uint256[] memory scaledShares = new uint256[](strategiesLength); + uint256[] memory shares = new uint256[](strategiesLength); for (uint256 i = 0; i < strategiesLength;) { - scaledShares[i] = stakerStrategyScaledShares[staker][stakerStrategyList[staker][i]]; + shares[i] = stakerStrategyShares[staker][stakerStrategyList[staker][i]]; unchecked { ++i; } } - return (stakerStrategyList[staker], scaledShares); - } - - /// @notice Returns the current shares of `user` in `strategy` - function stakerStrategyShares( - address user, - IStrategy strategy - ) public view returns (uint256 shares) { - uint256 scaledShares = stakerStrategyScaledShares[user][strategy]; - return delegation.getStakerShares(user, strategy, scaledShares); + return (stakerStrategyList[staker], shares); } /// @notice Simple getter function that returns `stakerStrategyList[staker].length`. diff --git a/src/contracts/core/StrategyManagerStorage.sol b/src/contracts/core/StrategyManagerStorage.sol index 15492f9151..4e6c5dde74 100644 --- a/src/contracts/core/StrategyManagerStorage.sol +++ b/src/contracts/core/StrategyManagerStorage.sol @@ -45,9 +45,9 @@ abstract contract StrategyManagerStorage is IStrategyManager { * This variable was migrated to the DelegationManager instead. */ uint256 private __deprecated_withdrawalDelayBlocks; - /// @notice Mapping: staker => Strategy => number of scaled shares which they currently hold - mapping(address => mapping(IStrategy => uint256)) public stakerStrategyScaledShares; - /// @notice Mapping: staker => array of strategies in which they have nonzero scaled shares + /// @notice Mapping: staker => Strategy => number of shares which they currently hold + mapping(address => mapping(IStrategy => uint256)) public stakerStrategyShares; + /// @notice Mapping: staker => array of strategies in which they have nonzero shares mapping(address => IStrategy[]) public stakerStrategyList; /// @notice *Deprecated* mapping: hash of withdrawal inputs, aka 'withdrawalRoot' => whether the withdrawal is pending /// @dev This mapping is preserved to allow the migration of withdrawals to the DelegationManager contract. @@ -68,7 +68,7 @@ abstract contract StrategyManagerStorage is IStrategyManager { mapping(address => uint256) internal beaconChainETHSharesToDecrementOnWithdrawal; /** - * @notice Mapping: strategy => whether or not stakers are allowed to transfer strategy scaled shares to another address + * @notice Mapping: strategy => whether or not stakers are allowed to transfer strategy shares to another address * if true for a strategy, a user cannot depositIntoStrategyWithSignature into that strategy for another staker * and also when performing queueWithdrawals, a staker can only withdraw to themselves */ diff --git a/src/contracts/interfaces/IAVSDirectory.sol b/src/contracts/interfaces/IAVSDirectory.sol index dd19a3e96d..ad2e98567c 100644 --- a/src/contracts/interfaces/IAVSDirectory.sol +++ b/src/contracts/interfaces/IAVSDirectory.sol @@ -311,9 +311,6 @@ interface IAVSDirectory is ISignatureUtils { uint16 numToComplete ) external view returns (uint64); - /// @dev Delay before deallocations take effect and are added back into freeMagnitude - function DEALLOCATION_DELAY() external view returns (uint32); - /** * @notice operator is slashable by operatorSet if currently registered OR last deregistered within 21 days * @param operator the operator to check slashability for @@ -364,18 +361,54 @@ interface IAVSDirectory is ISignatureUtils { bool linear ) external view returns (uint24[] memory); - /// @notice Returns the total magnitude of an operator for a given set of strategies - /// TODO: finish natspec - function getTotalMagnitudes(address operator, IStrategy[] calldata strategies) external view returns (uint64[] memory); + /** + * @notice Returns the current total magnitudes of an operator for a given set of strategies + * @param operator the operator to get the total magnitude for + * @param strategies the strategies to get the total magnitudes for + * @return totalMagnitudes the total magnitudes for each strategy + */ + function getTotalMagnitudes( + address operator, + IStrategy[] calldata strategies + ) external view returns (uint64[] memory); - /// @notice Returns the total magnitude of an operator for a given set of strategies at a given timestamp - /// TODO: finish natspec + /** + * @notice Returns the total magnitudes of an operator for a given set of strategies at a given timestamp + * @param operator the operator to get the total magnitude for + * @param strategies the strategies to get the total magnitudes for + * @param timestamp the timestamp to get the total magnitudes at + * @return totalMagnitudes the total magnitudes for each strategy + */ function getTotalMagnitudesAtTimestamp( address operator, IStrategy[] calldata strategies, uint32 timestamp ) external view returns (uint64[] memory); + /** + * @notice Returns the current total magnitude of an operator for a given strategy + * @param operator the operator to get the total magnitude for + * @param strategy the strategy to get the total magnitude for + * @return totalMagnitude the total magnitude for the strategy + */ + function getTotalMagnitude( + address operator, + IStrategy strategy + ) external view returns (uint64); + + /** + * @notice Returns the total magnitude of an operator for a given strategy at a given timestamp + * @param operator the operator to get the total magnitude for + * @param strategy the strategy to get the total magnitude for + * @param timestamp the timestamp to get the total magnitude at + * @return totalMagnitude the total magnitude for the strategy + */ + function getTotalMagnitudeAtTimestamp( + address operator, + IStrategy strategy, + uint32 timestamp + ) external view returns (uint64); + function operatorSaltIsSpent(address operator, bytes32 salt) external view returns (bool); function isMember(address operator, OperatorSet memory operatorSet) external view returns (bool); diff --git a/src/contracts/interfaces/IDelegationManager.sol b/src/contracts/interfaces/IDelegationManager.sol index 5f3fe8130c..18595a9af5 100644 --- a/src/contracts/interfaces/IDelegationManager.sol +++ b/src/contracts/interfaces/IDelegationManager.sol @@ -99,7 +99,9 @@ interface IDelegationManager is ISignatureUtils { uint32 startTimestamp; // Array of strategies that the Withdrawal contains IStrategy[] strategies; - // Array containing the amount of scaled shares for withdrawal in each Strategy in the `strategies` array + // Array containing the amount of staker scaled shares for withdrawal in each Strategy in the `strategies` array + // Note that these shares need to be rescaled again at completion based on the operator's totalMagnitude in case + // of slashing occurring during the queue withdrawal period uint256[] scaledShares; } @@ -294,30 +296,44 @@ interface IDelegationManager is ISignatureUtils { ) external; /** - * @notice Increases a staker's delegated share balance in a strategy. - * @param staker The address to increase the delegated scaled shares for their operator. - * @param strategy The strategy in which to increase the delegated scaled shares. - * @param scaledShares The number of scaled shares to increase. + * @notice Increases a staker's delegated share balance in a strategy. Note that before adding to operator shares, + * the delegated shares are scaled according to the operator's total magnitude as part of slashing accounting. + * The staker's scaling factor 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 existingShares The number of 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 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 shares in `strategy` by `scaledShares`. Otherwise does nothing. + * @dev *If the staker is actively delegated*, then increases the `staker`'s delegated scaled shares in `strategy` after scaling `shares`. + * Otherwise does nothing. * @dev Callable only by the StrategyManager or EigenPodManager. */ - function increaseDelegatedScaledShares( + function increaseDelegatedShares( address staker, IStrategy strategy, - uint256 scaledShares + uint256 existingShares, + uint256 addedShares ) external; /** - * @notice Decreases a staker's delegated share balance in a strategy. + * @notice Decreases a staker's delegated share balance in a strategy. Note that before removing from operator shares, + * the delegated shares are scaled according to the operator's total magnitude as part of slashing accounting. Unlike + * `increaseDelegatedShares`, the staker's scaling factor is not updated here. * @param staker The address to increase the delegated scaled shares for their operator. * @param strategy The strategy in which to decrease the delegated scaled shares. - * @param scaledShares The number of scaled shares to decrease. + * @param removedShares The number of shares to decremented for the strategy in the + * StrategyManager/EigenPodManager * * @dev *If the staker is actively delegated*, then decreases the `staker`'s delegated scaled shares in `strategy` by `scaledShares`. Otherwise does nothing. * @dev Callable only by the StrategyManager or EigenPodManager. */ - function decreaseDelegatedScaledShares(address staker, IStrategy strategy, uint256 scaledShares) external; + function decreaseDelegatedShares( + address staker, + IStrategy strategy, + uint256 removedShares + ) external; /** * @notice returns the address of the operator that `staker` is delegated to. @@ -347,44 +363,6 @@ interface IDelegationManager is ISignatureUtils { */ function stakerOptOutWindowBlocks(address operator) external view returns (uint256); - /** - * @notice Given array of strategies, returns array of shares for the operator - */ - function getOperatorShares( - address operator, - IStrategy[] memory strategies - ) external view returns (uint256[] memory); - - /** - * @notice Given array of strategies, returns array of scaled shares for the operator - */ - function getOperatorScaledShares( - address operator, - IStrategy[] memory strategies - ) external view returns (uint256[] memory); - - /** - * @notice Given a staker and shares amounts of deposits, return the scaled shares calculated if - * the staker were to deposit. Depends on who the operator the staker is delegated to. - */ - function getStakerScaledShares( - address staker, - IStrategy strategy, - uint256 shares - ) external view returns (uint256 scaledShares); - - /** - * @notice Given a staker and scaled shares amounts of deposits, return the shares calculated if - * the staker were to withdraw. 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 getStakerShares( - address staker, - IStrategy strategy, - uint256 scaledShares - ) external view returns (uint256 shares); - /** * @notice Given a list of strategies, return the minimum number of blocks that must pass to withdraw * from all the inputted strategies. Return value is >= minWithdrawalDelayBlocks as this is the global min withdrawal delay. diff --git a/src/contracts/interfaces/IEigenPodManager.sol b/src/contracts/interfaces/IEigenPodManager.sol index cfed6fe8a2..ba8967feba 100644 --- a/src/contracts/interfaces/IEigenPodManager.sol +++ b/src/contracts/interfaces/IEigenPodManager.sol @@ -97,9 +97,6 @@ interface IEigenPodManager is IPausable { */ function podOwnerShares(address podOwner) external view returns (int256); - /// TODO: natspec - function podOwnerScaledShares(address podOwner) external view returns (int256); - /// @notice returns canonical, virtual beaconChainETH strategy function beaconChainETHStrategy() external view returns (IStrategy); @@ -111,16 +108,20 @@ interface IEigenPodManager is IPausable { * shares from the operator to whom the staker is delegated. * @dev Reverts if `shares` is not a whole Gwei amount */ - function removeScaledShares(address podOwner, uint256 scaledShares) external; + function removeShares(address podOwner, uint256 shares) external; /** * @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 `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) + * 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 addScaledShares(address podOwner, uint256 scaledShares) external returns (uint256); + function addShares( + address podOwner, + uint256 shares + ) external returns (uint256 increaseInDelegateableShares, uint256 existingPodShares); /** * @notice Used by the DelegationManager to complete a withdrawal, sending tokens to some destination address diff --git a/src/contracts/interfaces/IStrategyManager.sol b/src/contracts/interfaces/IStrategyManager.sol index c420dedc60..37c96712c7 100644 --- a/src/contracts/interfaces/IStrategyManager.sol +++ b/src/contracts/interfaces/IStrategyManager.sol @@ -39,14 +39,14 @@ interface IStrategyManager { * @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 scaledShares The amount of new scaled 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 Cannot be called by an address that is 'frozen' (this function will revert if the `msg.sender` is frozen). * * WARNING: Depositing tokens that allow reentrancy (eg. ERC-777) into a strategy is not recommended. This can lead to attack vectors * where the token balance and corresponding strategy shares are not in sync upon reentrancy. */ - function depositIntoStrategy(IStrategy strategy, IERC20 token, uint256 amount) external returns (uint256 scaledShares); + function depositIntoStrategy(IStrategy strategy, IERC20 token, uint256 amount) external returns (uint256 shares); /** * @notice Used for depositing an asset into the specified strategy with the resultant shares credited to `staker`, @@ -60,7 +60,7 @@ interface IStrategyManager { * @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 scaledShares The amount of new scaled 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. @@ -76,23 +76,17 @@ interface IStrategyManager { address staker, uint256 expiry, bytes memory signature - ) external returns (uint256 scaledShares); + ) external returns (uint256 shares); /// @notice Used by the DelegationManager to remove a Staker's shares from a particular strategy when entering the withdrawal queue - function removeScaledShares(address staker, IStrategy strategy, uint256 shares) external; + function removeShares(address staker, IStrategy strategy, uint256 shares) external; - /// @notice Used by the DelegationManager to award a Staker some scaled shares that have passed through the withdrawal queue - function addScaledShares(address staker, IERC20 token, IStrategy strategy, uint256 scaledShares) external; + /// @notice Used by the DelegationManager to award a Staker some shares that have passed through the withdrawal queue + function addShares(address staker, IERC20 token, IStrategy strategy, uint256 shares) external returns (uint256 existingShares); /// @notice Used by the DelegationManager to convert withdrawn descaled shares to tokens and send them to a recipient function withdrawSharesAsTokens(address recipient, IStrategy strategy, uint256 shares, IERC20 token) external; - /// @notice Returns the current scaled shares of `user` in `strategy` - function stakerStrategyScaledShares( - address user, - IStrategy strategy - ) external view returns (uint256 scaledShares); - /// @notice Returns the current shares of `user` in `strategy` function stakerStrategyShares( address user, diff --git a/src/contracts/libraries/ShareScalingLib.sol b/src/contracts/libraries/ShareScalingLib.sol deleted file mode 100644 index e534407219..0000000000 --- a/src/contracts/libraries/ShareScalingLib.sol +++ /dev/null @@ -1,121 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.12; - -import "../interfaces/IAVSDirectory.sol"; - -library ShareScalingLib { - /// @dev The initial total magnitude for an operator - uint64 public constant INITIAL_TOTAL_MAGNITUDE = 1e18; - - /** - * @notice Returns the scaled shares of for an operator for a particular set of strategies and shares delegated to an operator - * @param avsDirectory The AVS directory - * @param operator The operator to scale shares for - * @param strategies The strategies of which shares are - * @param shares The shares to scale - */ - function scaleShares( - IAVSDirectory avsDirectory, - address operator, - IStrategy[] memory strategies, - uint256[] memory shares - ) internal view returns (uint256[] memory) { - uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); - uint256[] memory scaledShares = new uint256[](shares.length); - for (uint256 i = 0; i < shares.length; i++) { - scaledShares[i] = _scaleShares(shares[i], totalMagnitudes[i]); - } - return scaledShares; - } - - /** - * @notice Returns the shares of for an operator for a particular set of strategies and scaled shares delegated to an operator - * @param avsDirectory The AVS directory - * @param operator The operator to scale shares for - * @param strategies The strategies of which scaled shares are - * @param scaledShares The scaled shares to descale - */ - function descaleShares( - IAVSDirectory avsDirectory, - address operator, - IStrategy[] memory strategies, - uint256[] memory scaledShares - ) internal view returns (uint256[] memory) { - uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); - uint256[] memory descaledShares = new uint256[](scaledShares.length); - for (uint256 i = 0; i < scaledShares.length; i++) { - descaledShares[i] = _descaleShares(scaledShares[i], totalMagnitudes[i]); - } - return descaledShares; - } - - /** - * @notice Returns the shares of for an operator for a particular set of strategies and scaled shares delegated to an operator - * @param avsDirectory The AVS directory - * @param operator The operator to scale shares for - * @param strategies The strategies of which scaled shares are - * @param scaledShares The scaled shares to descale - */ - function descaleSharesAtTimestamp( - IAVSDirectory avsDirectory, - address operator, - IStrategy[] memory strategies, - uint256[] memory scaledShares, - uint32 timestamp - ) internal view returns (uint256[] memory) { - uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudesAtTimestamp(operator, strategies, timestamp); - uint256[] memory descaledShares = new uint256[](scaledShares.length); - for (uint256 i = 0; i < scaledShares.length; i++) { - descaledShares[i] = _descaleShares(scaledShares[i], totalMagnitudes[i]); - } - return descaledShares; - } - - /** - * @notice Returns the scaled shares of for an operator for a particular strategy and shares delegated to an operator - * @param avsDirectory The AVS directory - * @param operator The operator to scale shares for - * @param strategy The strategy of which shares are - * @param shares The shares to scale - */ - function scaleShares( - IAVSDirectory avsDirectory, - address operator, - IStrategy strategy, - uint256 shares - ) internal view returns (uint256) { - IStrategy[] memory strategies = new IStrategy[](1); - strategies[0] = strategy; - uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); - return _scaleShares(shares, totalMagnitudes[0]); - } - - /** - * @notice Returns the shares of for an operator for a particular strategy and scaled shares delegated to an operator - * @param avsDirectory The AVS directory - * @param operator The operator to scale shares for - * @param strategy The strategy of which scaled shares are - * @param scaledShares The scaled shares to descale - */ - function descaleShares( - IAVSDirectory avsDirectory, - address operator, - IStrategy strategy, - uint256 scaledShares - ) internal view returns (uint256) { - IStrategy[] memory strategies = new IStrategy[](1); - strategies[0] = strategy; - uint64[] memory totalMagnitudes = avsDirectory.getTotalMagnitudes(operator, strategies); - return _descaleShares(scaledShares, totalMagnitudes[0]); - } - - /// @notice helper pure to return scaledShares given shares and totalMagnitude - function _scaleShares(uint256 shares, uint64 totalMagnitude) internal pure returns (uint256) { - return shares * INITIAL_TOTAL_MAGNITUDE / totalMagnitude; - } - - /// @notice helper pure to return shares(descaledShares) given scaledShares and totalMagnitude - function _descaleShares(uint256 scaledShares, uint64 totalMagnitude) internal pure returns (uint256) { - return scaledShares * totalMagnitude / INITIAL_TOTAL_MAGNITUDE; - } -} diff --git a/src/contracts/libraries/SlashingConstants.sol b/src/contracts/libraries/SlashingConstants.sol new file mode 100644 index 0000000000..3d501d4099 --- /dev/null +++ b/src/contracts/libraries/SlashingConstants.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.12; + +import "../interfaces/IAVSDirectory.sol"; + +library SlashingConstants { + /// @dev The initial total magnitude for an operator + uint64 public constant INITIAL_TOTAL_MAGNITUDE = 1e18; + + /// @notice that stakerScalingFactor and totalMagnitude have initial default values to 1e18 as "1" + /// to preserve precision with uint256 math. We use `PRECISION_FACTOR` where these variables are used + /// and divide to represent as 1 + uint256 public constant PRECISION_FACTOR = 1e18; + uint256 public constant PRECISION_FACTOR_SQUARED = 1e36; + + /// @dev Delay before deallocations are completable and can be added back into freeMagnitude + uint32 public constant DEALLOCATION_DELAY = 17.5 days; +} diff --git a/src/contracts/pods/EigenPodManager.sol b/src/contracts/pods/EigenPodManager.sol index 056690b017..b2685668d0 100644 --- a/src/contracts/pods/EigenPodManager.sol +++ b/src/contracts/pods/EigenPodManager.sol @@ -111,61 +111,57 @@ contract EigenPodManager is sharesDelta % int256(GWEI_TO_WEI) == 0, "EigenPodManager.recordBeaconChainETHBalanceUpdate: sharesDelta must be a whole Gwei amount" ); - int256 currentPodOwnerScaledShares = podOwnerScaledShares[podOwner]; - int256 updatedPodOwnerScaledShares; - // scale the sharesDelta and add to the podOwnerShares - if (sharesDelta < 0) { - updatedPodOwnerScaledShares = currentPodOwnerScaledShares - - int256(delegationManager.getStakerScaledShares(podOwner, beaconChainETHStrategy, uint256(-sharesDelta))); - } else { - updatedPodOwnerScaledShares = currentPodOwnerScaledShares - + int256(delegationManager.getStakerScaledShares(podOwner, beaconChainETHStrategy, uint256(sharesDelta))); - } - podOwnerScaledShares[podOwner] = updatedPodOwnerScaledShares; + int256 currentPodOwnerShares = podOwnerShares[podOwner]; + int256 updatedPodOwnerShares = currentPodOwnerShares + sharesDelta; + podOwnerShares[podOwner] = updatedPodOwnerShares; // inform the DelegationManager of the change in delegateable shares - int256 changeInDelegatableScaledShares = _calculateChangeInDelegatableScaledShares({ - scaledSharesBefore: currentPodOwnerScaledShares, - scaledSharesAfter: updatedPodOwnerScaledShares + int256 changeInDelegatableShares = _calculateChangeInDelegatableShares({ + sharesBefore: currentPodOwnerShares, + sharesAfter: updatedPodOwnerShares }); // skip making a call to the DelegationManager if there is no change in delegateable shares - if (changeInDelegatableScaledShares != 0) { - if (changeInDelegatableScaledShares < 0) { - delegationManager.decreaseDelegatedScaledShares({ + // or if the currentPodShares < 0 and updatedPodShares is still < 0. Means no update required + // in delegated shares + if (changeInDelegatableShares != 0) { + if (changeInDelegatableShares < 0) { + delegationManager.decreaseDelegatedShares({ staker: podOwner, strategy: beaconChainETHStrategy, - scaledShares: uint256(-changeInDelegatableScaledShares) + removedShares: uint256(-changeInDelegatableShares) }); } else { - delegationManager.increaseDelegatedScaledShares({ + delegationManager.increaseDelegatedShares({ staker: podOwner, strategy: beaconChainETHStrategy, - scaledShares: uint256(changeInDelegatableScaledShares) + // existing shares from standpoint of the DelegationManager + existingShares: currentPodOwnerShares < 0 ? 0 : uint256(currentPodOwnerShares), + addedShares: uint256(changeInDelegatableShares) }); } } emit PodSharesUpdated(podOwner, sharesDelta); - emit NewTotalShares(podOwner, updatedPodOwnerScaledShares); + emit NewTotalShares(podOwner, updatedPodOwnerShares); } /** * @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 `podOwnerScaledShares[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 removeScaledShares(address podOwner, uint256 scaledShares) external onlyDelegationManager { - require(int256(scaledShares) >= 0, "EigenPodManager.removeShares: shares cannot be negative"); - require(scaledShares % GWEI_TO_WEI == 0, "EigenPodManager.removeShares: shares must be a whole Gwei amount"); - int256 updatedPodOwnerShares = podOwnerScaledShares[podOwner] - int256(scaledShares); + function removeShares(address podOwner, uint256 shares) external onlyDelegationManager { + require(int256(shares) >= 0, "EigenPodManager.removeShares: shares cannot be negative"); + require(shares % GWEI_TO_WEI == 0, "EigenPodManager.removeShares: shares must be a whole Gwei amount"); + int256 updatedPodOwnerShares = podOwnerShares[podOwner] - int256(shares); require( updatedPodOwnerShares >= 0, "EigenPodManager.removeShares: cannot result in pod owner having negative shares" ); - podOwnerScaledShares[podOwner] = updatedPodOwnerShares; + podOwnerShares[podOwner] = updatedPodOwnerShares; emit NewTotalShares(podOwner, updatedPodOwnerShares); } @@ -173,27 +169,34 @@ 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 `podOwnerScaledShares[podOwner]` above zero, which will be less than the `shares` input - * in the event that the podOwner has an existing shares deficit (i.e. `podOwnerScaledShares[podOwner]` starts below zero) + * @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 addScaledShares(address podOwner, uint256 scaledShares) external onlyDelegationManager returns (uint256) { + function addShares( + address podOwner, + uint256 shares + ) external onlyDelegationManager returns (uint256 increaseInDelegateableShares, uint256 existingPodShares) { require(podOwner != address(0), "EigenPodManager.addShares: podOwner cannot be zero address"); - require(int256(scaledShares) >= 0, "EigenPodManager.addShares: shares cannot be negative"); - require(scaledShares % GWEI_TO_WEI == 0, "EigenPodManager.addShares: shares must be a whole Gwei amount"); - int256 currentPodOwnerScaledShares = podOwnerScaledShares[podOwner]; - int256 updatedPodOwnerScaledShares = currentPodOwnerScaledShares + int256(scaledShares); - podOwnerScaledShares[podOwner] = updatedPodOwnerScaledShares; + require(int256(shares) >= 0, "EigenPodManager.addShares: shares cannot be negative"); + require(shares % GWEI_TO_WEI == 0, "EigenPodManager.addShares: shares must be a whole Gwei amount"); + int256 currentPodOwnerShares = podOwnerShares[podOwner]; + int256 updatedPodOwnerShares = currentPodOwnerShares + int256(shares); + podOwnerShares[podOwner] = updatedPodOwnerShares; - emit PodSharesUpdated(podOwner, int256(scaledShares)); - emit NewTotalShares(podOwner, updatedPodOwnerScaledShares); + emit PodSharesUpdated(podOwner, int256(shares)); + emit NewTotalShares(podOwner, updatedPodOwnerShares); - return uint256( - _calculateChangeInDelegatableScaledShares({ - scaledSharesBefore: currentPodOwnerScaledShares, - scaledSharesAfter: updatedPodOwnerScaledShares + increaseInDelegateableShares = uint256( + _calculateChangeInDelegatableShares({ + sharesBefore: currentPodOwnerShares, + sharesAfter: updatedPodOwnerShares }) ); + existingPodShares = currentPodOwnerShares < 0 ? 0 : uint256(currentPodOwnerShares); + + return (increaseInDelegateableShares, existingPodShares); } /** @@ -212,22 +215,22 @@ contract EigenPodManager is require(destination != address(0), "EigenPodManager.withdrawSharesAsTokens: destination cannot be zero address"); require(int256(shares) >= 0, "EigenPodManager.withdrawSharesAsTokens: shares cannot be negative"); require(shares % GWEI_TO_WEI == 0, "EigenPodManager.withdrawSharesAsTokens: shares must be a whole Gwei amount"); - int256 currentPodOwnerScaledShares = podOwnerScaledShares[podOwner]; + int256 currentPodOwnerShares = podOwnerShares[podOwner]; // if there is an existing shares deficit, prioritize decreasing the deficit first - if (currentPodOwnerScaledShares < 0) { - uint256 currentShareDeficit = uint256(-currentPodOwnerScaledShares); + if (currentPodOwnerShares < 0) { + uint256 currentShareDeficit = uint256(-currentPodOwnerShares); if (shares > currentShareDeficit) { // get rid of the whole deficit if possible, and pass any remaining shares onto destination - podOwnerScaledShares[podOwner] = 0; + podOwnerShares[podOwner] = 0; shares -= currentShareDeficit; emit PodSharesUpdated(podOwner, int256(currentShareDeficit)); emit NewTotalShares(podOwner, 0); } else { // otherwise get rid of as much deficit as possible, and return early, since there is nothing left over to forward on - int256 updatedPodOwnerShares = podOwnerScaledShares[podOwner] + int256(shares); - podOwnerScaledShares[podOwner] = updatedPodOwnerShares; + int256 updatedPodOwnerShares = podOwnerShares[podOwner] + int256(shares); + podOwnerShares[podOwner] = updatedPodOwnerShares; emit PodSharesUpdated(podOwner, int256(shares)); emit NewTotalShares(podOwner, updatedPodOwnerShares); return; @@ -259,53 +262,33 @@ contract EigenPodManager is /** * @notice Calculates the change in a pod owner's delegateable shares as a result of their beacon chain ETH shares changing - * from `scaledSharesBefore` to `scaledSharesAfter`. The key concept here is that negative/"deficit" scaledShares are not delegateable. + * from `sharesBefore` to `sharesAfter`. The key concept here is that negative/"deficit" shares are not delegateable. */ - function _calculateChangeInDelegatableScaledShares( - int256 scaledSharesBefore, - int256 scaledSharesAfter + function _calculateChangeInDelegatableShares( + int256 sharesBefore, + int256 sharesAfter ) internal pure returns (int256) { - if (scaledSharesBefore <= 0) { - if (scaledSharesAfter <= 0) { + if (sharesBefore <= 0) { + if (sharesAfter <= 0) { // if the shares started negative and stayed negative, then there cannot have been an increase in delegateable shares return 0; - // if the shares started negative and became positive, then the increase in delegateable shares is the ending share amount } else { // if the shares started negative and became positive, then the increase in delegateable shares is the ending share amount - return scaledSharesAfter; + return sharesAfter; } } else { - if (scaledSharesAfter <= 0) { + if (sharesAfter <= 0) { // if the shares started positive and became negative, then the decrease in delegateable shares is the starting share amount - return (-scaledSharesBefore); + return (-sharesBefore); } else { // if the shares started positive and stayed positive, then the change in delegateable shares // is the difference between starting and ending amounts - return (scaledSharesAfter - scaledSharesBefore); + return (sharesAfter - sharesBefore); } } } // VIEW FUNCTIONS - /// @notice Return the real number of ETH shares that the `podOwner` has - function podOwnerShares(address podOwner) public view returns (int256) { - int256 podOwnerShares; - if (podOwnerScaledShares[podOwner] < 0) { - podOwnerShares = -int256( - delegationManager.getStakerShares( - podOwner, beaconChainETHStrategy, uint256(-podOwnerScaledShares[podOwner]) - ) - ); - } else { - podOwnerShares = int256( - delegationManager.getStakerShares( - podOwner, beaconChainETHStrategy, uint256(podOwnerScaledShares[podOwner]) - ) - ); - } - return podOwnerShares; - } - /// @notice Returns the address of the `podOwner`'s EigenPod (whether it is deployed yet or not). function getPod(address podOwner) public view returns (IEigenPod) { IEigenPod pod = ownerToPod[podOwner]; diff --git a/src/contracts/pods/EigenPodManagerStorage.sol b/src/contracts/pods/EigenPodManagerStorage.sol index 5ff0e4137c..5b0d1c590b 100644 --- a/src/contracts/pods/EigenPodManagerStorage.sol +++ b/src/contracts/pods/EigenPodManagerStorage.sol @@ -68,17 +68,17 @@ abstract contract EigenPodManagerStorage is IEigenPodManager { // BEGIN STORAGE VARIABLES ADDED AFTER MAINNET DEPLOYMENT -- DO NOT SUGGEST REORDERING TO CONVENTIONAL ORDER /** - * @notice Mapping from Pod owner owner to the number of scaled shares they have in the virtual beacon chain ETH strategy. - * NOTE: This mapping was previously named `podOwnerShares` but will now store scaled shares based on the currently delegated operator + * @notice Mapping from Pod owner owner to the number of shares they have in the virtual beacon chain ETH strategy. + * NOTE: This mapping was previously named `podOwnerShares` but will now store shares based on the currently delegated operator * for the podOwner to account for Slashing in EigenLayer. There is a backwards compatible view function `podOwnerShares` that will convert - * scaled shares to its actual ETH shares. + * shares to its actual ETH shares. * @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 podOwnerScaledShares; + mapping(address => int256) public podOwnerShares; uint64 internal __deprecated_denebForkTimestamp; diff --git a/src/contracts/strategies/StrategyBase.sol b/src/contracts/strategies/StrategyBase.sol index 608adb1a6e..a4fab630a6 100644 --- a/src/contracts/strategies/StrategyBase.sol +++ b/src/contracts/strategies/StrategyBase.sol @@ -291,7 +291,7 @@ contract StrategyBase is Initializable, Pausable, IStrategy { * querying the `strategyManager` contract */ function shares(address user) public view virtual returns (uint256) { - return strategyManager.stakerStrategyScaledShares(user, IStrategy(address(this))); + return strategyManager.stakerStrategyShares(user, IStrategy(address(this))); } /// @notice Internal function used to fetch this contract's current balance of `underlyingToken`. diff --git a/src/test/EigenLayerTestHelper.t.sol b/src/test/EigenLayerTestHelper.t.sol index 2cfe5b8764..dc4632e495 100644 --- a/src/test/EigenLayerTestHelper.t.sol +++ b/src/test/EigenLayerTestHelper.t.sol @@ -299,7 +299,7 @@ pragma solidity ^0.8.12; // uint256 amountDeposited = _testDepositWeth(staker, amountToDeposit); // // We can't withdraw more than we deposit // if (shareAmounts[0] > amountDeposited) { -// cheats.expectRevert("StrategyManager._removeScaledShares: shareAmount too high"); +// cheats.expectRevert("StrategyManager._removeShares: shareAmount too high"); // } // } diff --git a/src/test/harnesses/EigenPodManagerWrapper.sol b/src/test/harnesses/EigenPodManagerWrapper.sol index 4955b144e9..15eec05fdc 100644 --- a/src/test/harnesses/EigenPodManagerWrapper.sol +++ b/src/test/harnesses/EigenPodManagerWrapper.sol @@ -14,8 +14,8 @@ contract EigenPodManagerWrapper is EigenPodManager { IDelegationManager _delegationManager ) EigenPodManager(_ethPOS, _eigenPodBeacon, _strategyManager, _slasher, _delegationManager) {} - function calculateChangeInDelegatableScaledShares(int256 sharesBefore, int256 sharesAfter) external pure returns (int256) { - return _calculateChangeInDelegatableScaledShares(sharesBefore, sharesAfter); + function calculateChangeInDelegatableShares(int256 sharesBefore, int256 sharesAfter) external pure returns (int256) { + return _calculateChangeInDelegatableShares(sharesBefore, sharesAfter); } function setPodAddress(address owner, IEigenPod pod) external { diff --git a/src/test/integration/IntegrationBase.t.sol b/src/test/integration/IntegrationBase.t.sol index 5c8c14dbbf..dc232a98ca 100644 --- a/src/test/integration/IntegrationBase.t.sol +++ b/src/test/integration/IntegrationBase.t.sol @@ -185,7 +185,7 @@ abstract contract IntegrationBase is IntegrationDeployer { function assert_HasNoDelegatableShares(User user, string memory err) internal { (IStrategy[] memory strategies, uint[] memory shares) = - delegationManager.getDelegatableScaledShares(address(user)); + delegationManager.getDelegatableShares(address(user)); assertEq(strategies.length, 0, err); assertEq(strategies.length, shares.length, "assert_HasNoDelegatableShares: return length mismatch"); @@ -238,7 +238,7 @@ abstract contract IntegrationBase is IntegrationDeployer { actualShares = uint(shares); } else { - actualShares = strategyManager.stakerStrategyScaledShares(address(user), strat); + actualShares = strategyManager.stakerStrategyShares(address(user), strat); } assertApproxEqAbs(expectedShares[i], actualShares, 1, err); @@ -1051,7 +1051,7 @@ abstract contract IntegrationBase is IntegrationDeployer { curShares[i] = uint(shares); } else { - curShares[i] = strategyManager.stakerStrategyScaledShares(address(staker), strat); + curShares[i] = strategyManager.stakerStrategyShares(address(staker), strat); } } @@ -1076,7 +1076,7 @@ abstract contract IntegrationBase is IntegrationDeployer { if (strat == BEACONCHAIN_ETH_STRAT) { curShares[i] = eigenPodManager.podOwnerShares(address(staker)); } else { - curShares[i] = int(strategyManager.stakerStrategyScaledShares(address(staker), strat)); + curShares[i] = int(strategyManager.stakerStrategyShares(address(staker), strat)); } } diff --git a/src/test/integration/users/User.t.sol b/src/test/integration/users/User.t.sol index d7f9c04e25..4ed72e7ce2 100644 --- a/src/test/integration/users/User.t.sol +++ b/src/test/integration/users/User.t.sol @@ -468,7 +468,7 @@ contract User is PrintUtils { /// @notice Assumes staker and withdrawer are the same and that all strategies and shares are withdrawn function _getExpectedWithdrawalStructsForStaker(address staker) internal view returns (IDelegationManager.Withdrawal[] memory) { (IStrategy[] memory strategies, uint256[] memory scaledShares) - = delegationManager.getDelegatableScaledShares(staker); + = delegationManager.getDelegatableShares(staker); IDelegationManager.Withdrawal[] memory expectedWithdrawals = new IDelegationManager.Withdrawal[](strategies.length); address delegatedTo = delegationManager.delegatedTo(staker); diff --git a/src/test/mocks/AVSDirectoryMock.sol b/src/test/mocks/AVSDirectoryMock.sol index 51b99367c3..a7219ef466 100644 --- a/src/test/mocks/AVSDirectoryMock.sol +++ b/src/test/mocks/AVSDirectoryMock.sol @@ -98,9 +98,27 @@ contract AVSDirectoryMock is IAVSDirectory, Test { function DEALLOCATION_DELAY() external view returns (uint32) {} - function getTotalMagnitudes(address operator, IStrategy[] calldata strategies) external view returns (uint64[] memory) {} + function getTotalMagnitudes( + address operator, + IStrategy[] calldata strategies + ) external view returns (uint64[] memory) {} - function getTotalMagnitudesAtTimestamp(address operator, IStrategy[] calldata strategies, uint32 timestamp) external view returns (uint64[] memory) {} + function getTotalMagnitudesAtTimestamp( + address operator, + IStrategy[] calldata strategies, + uint32 timestamp + ) external view returns (uint64[] memory) {} + + function getTotalMagnitude( + address operator, + IStrategy strategy + ) external view returns (uint64) {} + + function getTotalMagnitudeAtTimestamp( + address operator, + IStrategy strategy, + uint32 timestamp + ) external view returns (uint64) {} function calculateOperatorAVSRegistrationDigestHash( address operator, diff --git a/src/test/mocks/DelegationManagerMock.sol b/src/test/mocks/DelegationManagerMock.sol index 3485086dde..80b8250a17 100644 --- a/src/test/mocks/DelegationManagerMock.sol +++ b/src/test/mocks/DelegationManagerMock.sol @@ -52,13 +52,14 @@ contract DelegationManagerMock is IDelegationManager, Test { return withdrawalRoot; } - function increaseDelegatedScaledShares( + function increaseDelegatedShares( address staker, IStrategy strategy, - uint256 scaledShares + uint256 existingShares, + uint256 addedShares ) external {} - function decreaseDelegatedScaledShares(address staker, IStrategy strategy, uint256 shares) external {} + function decreaseDelegatedShares(address staker, IStrategy strategy, uint256 shares) external {} function operatorDetails(address operator) external pure returns (OperatorDetails memory) { OperatorDetails memory returnValue = OperatorDetails({ @@ -94,28 +95,6 @@ contract DelegationManagerMock is IDelegationManager, Test { return 0; } - function getOperatorScaledShares( - address operator, - IStrategy[] memory strategies - ) external view returns (uint256[] memory) {} - - function getOperatorShares( - address operator, - IStrategy[] memory strategies - ) external view returns (uint256[] memory) {} - - function getStakerScaledShares( - address staker, - IStrategy strategy, - uint256 shares - ) external view returns (uint256 scaledShares) {} - - function getStakerShares( - address staker, - IStrategy strategy, - uint256 scaledShares - ) public view returns (uint256 shares) {} - function getWithdrawalDelay(IStrategy[] calldata /*strategies*/) public pure returns (uint256) { return 0; } @@ -192,23 +171,23 @@ contract DelegationManagerMock is IDelegationManager, Test { ) external {} // onlyDelegationManager functions in StrategyManager - function addScaledShares( + function addShares( IStrategyManager strategyManager, address staker, IERC20 token, IStrategy strategy, uint256 shares ) external { - strategyManager.addScaledShares(staker, token, strategy, shares); + strategyManager.addShares(staker, token, strategy, shares); } - function removeScaledShares( + function removeShares( IStrategyManager strategyManager, address staker, IStrategy strategy, uint256 shares ) external { - strategyManager.removeScaledShares(staker, strategy, shares); + strategyManager.removeShares(staker, strategy, shares); } function withdrawSharesAsTokens( diff --git a/src/test/mocks/EigenPodManagerMock.sol b/src/test/mocks/EigenPodManagerMock.sol index ad48c0a060..855fc54e6a 100644 --- a/src/test/mocks/EigenPodManagerMock.sol +++ b/src/test/mocks/EigenPodManagerMock.sol @@ -53,14 +53,14 @@ contract EigenPodManagerMock is IEigenPodManager, Test, Pausable { podShares[podOwner] = shares; } - function addScaledShares(address /*podOwner*/, uint256 scaledShares) external pure returns (uint256) { + function addShares(address /*podOwner*/, uint256 shares) external pure returns (uint256, uint256) { // this is the "increase in delegateable tokens" - return (scaledShares); + // return (shares); } function withdrawSharesAsTokens(address podOwner, address destination, uint256 shares) external {} - function removeScaledShares(address podOwner, uint256 scaledShares) external {} + function removeShares(address podOwner, uint256 shares) external {} function numPods() external view returns (uint256) {} diff --git a/src/test/mocks/StrategyManagerMock.sol b/src/test/mocks/StrategyManagerMock.sol index 43cf298f3a..14f2d1a749 100644 --- a/src/test/mocks/StrategyManagerMock.sol +++ b/src/test/mocks/StrategyManagerMock.sol @@ -66,9 +66,6 @@ contract StrategyManagerMock is external returns (uint256 shares) {} - /// @notice Returns the current shares of `user` in `strategy` - function stakerStrategyScaledShares(address user, IStrategy strategy) external view returns (uint256 shares) {} - /** * @notice mocks the return value of getDeposits * @param staker staker whose deposits are being mocked @@ -118,9 +115,9 @@ contract StrategyManagerMock is strategyIsWhitelistedForDeposit[strategy] = value; } - function removeScaledShares(address staker, IStrategy strategy, uint256 shares) external {} + function removeShares(address staker, IStrategy strategy, uint256 shares) external {} - function addScaledShares(address staker, IERC20 token, IStrategy strategy, uint256 shares) external {} + function addShares(address staker, IERC20 token, IStrategy strategy, uint256 shares) external returns (uint256 existingShares) {} function withdrawSharesAsTokens(address recipient, IStrategy strategy, uint256 shares, IERC20 token) external {} diff --git a/src/test/unit/EigenPodManagerUnit.t.sol b/src/test/unit/EigenPodManagerUnit.t.sol index ad11425da5..a0c5d0819c 100644 --- a/src/test/unit/EigenPodManagerUnit.t.sol +++ b/src/test/unit/EigenPodManagerUnit.t.sol @@ -190,35 +190,35 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { Add Shares Tests *******************************************************************************/ - function testFuzz_addScaledShares_revert_notDelegationManager(address notDelegationManager) public filterFuzzedAddressInputs(notDelegationManager){ + function testFuzz_addShares_revert_notDelegationManager(address notDelegationManager) public filterFuzzedAddressInputs(notDelegationManager){ cheats.assume(notDelegationManager != address(delegationManagerMock)); cheats.prank(notDelegationManager); cheats.expectRevert("EigenPodManager.onlyDelegationManager: not the DelegationManager"); - eigenPodManager.addScaledShares(defaultStaker, 0); + eigenPodManager.addShares(defaultStaker, 0); } - function test_addScaledShares_revert_podOwnerZeroAddress() public { + function test_addShares_revert_podOwnerZeroAddress() public { cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.addScaledShares: podOwner cannot be zero address"); - eigenPodManager.addScaledShares(address(0), 0); + cheats.expectRevert("EigenPodManager.addShares: podOwner cannot be zero address"); + eigenPodManager.addShares(address(0), 0); } - function testFuzz_addScaledShares_revert_sharesNegative(int256 shares) public { + function testFuzz_addShares_revert_sharesNegative(int256 shares) public { cheats.assume(shares < 0); cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.addScaledShares: shares cannot be negative"); - eigenPodManager.addScaledShares(defaultStaker, uint256(shares)); + cheats.expectRevert("EigenPodManager.addShares: shares cannot be negative"); + eigenPodManager.addShares(defaultStaker, uint256(shares)); } - function testFuzz_addScaledShares_revert_sharesNotWholeGwei(uint256 shares) public { + function testFuzz_addShares_revert_sharesNotWholeGwei(uint256 shares) public { cheats.assume(int256(shares) >= 0); cheats.assume(shares % GWEI_TO_WEI != 0); cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.addScaledShares: shares must be a whole Gwei amount"); - eigenPodManager.addScaledShares(defaultStaker, shares); + cheats.expectRevert("EigenPodManager.addShares: shares must be a whole Gwei amount"); + eigenPodManager.addShares(defaultStaker, shares); } - function testFuzz_addScaledShares(uint256 shares) public { + function testFuzz_addShares(uint256 shares) public { // Fuzz inputs cheats.assume(defaultStaker != address(0)); shares = shares - (shares % GWEI_TO_WEI); // Round down to nearest Gwei @@ -226,7 +226,7 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { // Add shares cheats.prank(address(delegationManagerMock)); - eigenPodManager.addScaledShares(defaultStaker, shares); + eigenPodManager.addShares(defaultStaker, shares); // Check storage update assertEq(eigenPodManager.podOwnerShares(defaultStaker), int256(shares), "Incorrect number of shares added"); @@ -236,29 +236,29 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { Remove Shares Tests ******************************************************************************/ - function testFuzz_removeScaledShares_revert_notDelegationManager(address notDelegationManager) public filterFuzzedAddressInputs(notDelegationManager) { + function testFuzz_removeShares_revert_notDelegationManager(address notDelegationManager) public filterFuzzedAddressInputs(notDelegationManager) { cheats.assume(notDelegationManager != address(delegationManagerMock)); cheats.prank(notDelegationManager); cheats.expectRevert("EigenPodManager.onlyDelegationManager: not the DelegationManager"); - eigenPodManager.removeScaledShares(defaultStaker, 0); + eigenPodManager.removeShares(defaultStaker, 0); } - function testFuzz_removeScaledShares_revert_sharesNegative(int256 shares) public { + function testFuzz_removeShares_revert_sharesNegative(int256 shares) public { cheats.assume(shares < 0); cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.removeScaledShares: shares cannot be negative"); - eigenPodManager.removeScaledShares(defaultStaker, uint256(shares)); + cheats.expectRevert("EigenPodManager.removeShares: shares cannot be negative"); + eigenPodManager.removeShares(defaultStaker, uint256(shares)); } - function testFuzz_removeScaledShares_revert_sharesNotWholeGwei(uint256 shares) public { + function testFuzz_removeShares_revert_sharesNotWholeGwei(uint256 shares) public { cheats.assume(int256(shares) >= 0); cheats.assume(shares % GWEI_TO_WEI != 0); cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.removeScaledShares: shares must be a whole Gwei amount"); - eigenPodManager.removeScaledShares(defaultStaker, shares); + cheats.expectRevert("EigenPodManager.removeShares: shares must be a whole Gwei amount"); + eigenPodManager.removeShares(defaultStaker, shares); } - function testFuzz_removeScaledShares_revert_tooManySharesRemoved(uint224 sharesToAdd, uint224 sharesToRemove) public { + function testFuzz_removeShares_revert_tooManySharesRemoved(uint224 sharesToAdd, uint224 sharesToRemove) public { // Constrain inputs cheats.assume(sharesToRemove > sharesToAdd); uint256 sharesAdded = sharesToAdd * GWEI_TO_WEI; @@ -269,11 +269,11 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { // Remove shares cheats.prank(address(delegationManagerMock)); - cheats.expectRevert("EigenPodManager.removeScaledShares: cannot result in pod owner having negative shares"); - eigenPodManager.removeScaledShares(defaultStaker, sharesRemoved); + cheats.expectRevert("EigenPodManager.removeShares: cannot result in pod owner having negative shares"); + eigenPodManager.removeShares(defaultStaker, sharesRemoved); } - function testFuzz_removeScaledShares(uint224 sharesToAdd, uint224 sharesToRemove) public { + function testFuzz_removeShares(uint224 sharesToAdd, uint224 sharesToRemove) public { // Constain inputs cheats.assume(sharesToRemove <= sharesToAdd); uint256 sharesAdded = sharesToAdd * GWEI_TO_WEI; @@ -284,13 +284,13 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { // Remove shares cheats.prank(address(delegationManagerMock)); - eigenPodManager.removeScaledShares(defaultStaker, sharesRemoved); + eigenPodManager.removeShares(defaultStaker, sharesRemoved); // Check storage assertEq(eigenPodManager.podOwnerShares(defaultStaker), int256(sharesAdded - sharesRemoved), "Incorrect number of shares removed"); } - function testFuzz_removeScaledShares_zeroShares(address podOwner, uint256 shares) public filterFuzzedAddressInputs(podOwner) { + function testFuzz_removeShares_zeroShares(address podOwner, uint256 shares) public filterFuzzedAddressInputs(podOwner) { // Constrain inputs cheats.assume(podOwner != address(0)); cheats.assume(shares < type(uint256).max / 2); @@ -302,7 +302,7 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { // Remove shares cheats.prank(address(delegationManagerMock)); - eigenPodManager.removeScaledShares(podOwner, shares); + eigenPodManager.removeShares(podOwner, shares); // Check storage update assertEq(eigenPodManager.podOwnerShares(podOwner), 0, "Shares not reset to zero"); @@ -342,7 +342,7 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests { /** * @notice The `withdrawSharesAsTokens` is called in the `completeQueuedWithdrawal` function from the - * delegationManager. When a withdrawal is queued in the delegationManager, `removeScaledShares is called` + * delegationManager. When a withdrawal is queued in the delegationManager, `removeShares is called` */ function test_withdrawSharesAsTokens_reduceEntireDeficit() public { // Shares to initialize & withdraw @@ -439,7 +439,7 @@ contract EigenPodManagerUnitTests_BeaconChainETHBalanceUpdateTests is EigenPodMa } contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodManagerUnitTests { - // Wrapper contract that exposes the internal `_calculateChangeInDelegatableScaledShares` function + // Wrapper contract that exposes the internal `_calculateChangeInDelegatableShares` function EigenPodManagerWrapper public eigenPodManagerWrapper; function setUp() virtual override public { @@ -460,7 +460,7 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan cheats.assume(scaledSharesBefore <= 0); cheats.assume(scaledSharesAfter <= 0); - int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableScaledShares(scaledSharesBefore, scaledSharesAfter); + int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableShares(scaledSharesBefore, scaledSharesAfter); assertEq(sharesDelta, 0, "Shares delta must be 0"); } @@ -468,7 +468,7 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan cheats.assume(scaledSharesBefore <= 0); cheats.assume(scaledSharesAfter > 0); - int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableScaledShares(scaledSharesBefore, scaledSharesAfter); + int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableShares(scaledSharesBefore, scaledSharesAfter); assertEq(sharesDelta, scaledSharesAfter, "Shares delta must be equal to scaledSharesAfter"); } @@ -476,7 +476,7 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan cheats.assume(scaledSharesBefore > 0); cheats.assume(scaledSharesAfter <= 0); - int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableScaledShares(scaledSharesBefore, scaledSharesAfter); + int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableShares(scaledSharesBefore, scaledSharesAfter); assertEq(sharesDelta, -scaledSharesBefore, "Shares delta must be equal to the negative of scaledSharesBefore"); } @@ -484,7 +484,7 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan cheats.assume(scaledSharesBefore > 0); cheats.assume(scaledSharesAfter > 0); - int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableScaledShares(scaledSharesBefore, scaledSharesAfter); + int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableShares(scaledSharesBefore, scaledSharesAfter); assertEq(sharesDelta, scaledSharesAfter - scaledSharesBefore, "Shares delta must be equal to the difference between scaledSharesAfter and scaledSharesBefore"); } } diff --git a/src/test/unit/StrategyManagerUnit.t.sol b/src/test/unit/StrategyManagerUnit.t.sol index 26ab872975..309ad37577 100644 --- a/src/test/unit/StrategyManagerUnit.t.sol +++ b/src/test/unit/StrategyManagerUnit.t.sol @@ -101,14 +101,14 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv ) internal filterFuzzedAddressInputs(staker) { IERC20 token = dummyToken; - // filter out zero case since it will revert with "StrategyManager._addScaledShares: shares should not be zero!" + // filter out zero case since it will revert with "StrategyManager._addShares: shares should not be zero!" cheats.assume(amount != 0); // filter out zero address because the mock ERC20 we are using will revert on using it cheats.assume(staker != address(0)); // sanity check / filter cheats.assume(amount <= token.balanceOf(address(this))); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); // needed for expecting an event with the right parameters @@ -119,7 +119,7 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv emit Deposit(staker, token, strategy, expectedShares); uint256 shares = strategyManager.depositIntoStrategy(strategy, token, amount); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); assertEq(sharesAfter, sharesBefore + shares, "sharesAfter != sharesBefore + shares"); @@ -144,7 +144,7 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv uint256 expiry, string memory expectedRevertMessage ) internal returns (bytes memory) { - // filter out zero case since it will revert with "StrategyManager._addScaledShares: shares should not be zero!" + // filter out zero case since it will revert with "StrategyManager._addShares: shares should not be zero!" cheats.assume(amount != 0); // sanity check / filter cheats.assume(amount <= dummyToken.balanceOf(address(this))); @@ -163,7 +163,7 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv signature = abi.encodePacked(r, s, v); } - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, dummyStrat); bool expectedRevertMessageIsempty; { @@ -190,7 +190,7 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv signature ); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, dummyStrat); uint256 nonceAfter = strategyManager.nonces(staker); if (expiry >= block.timestamp && expectedRevertMessageIsempty) { @@ -271,7 +271,7 @@ contract StrategyManagerUnitTests_depositIntoStrategy is StrategyManagerUnitTest IERC20 token = dummyToken; IStrategy strategy = dummyStrat; - // filter out zero case since it will revert with "StrategyManager._addScaledShares: shares should not be zero!" + // filter out zero case since it will revert with "StrategyManager._addShares: shares should not be zero!" cheats.assume(amount != 0); // filter out zero address because the mock ERC20 we are using will revert on using it cheats.assume(staker != address(0)); @@ -281,7 +281,7 @@ contract StrategyManagerUnitTests_depositIntoStrategy is StrategyManagerUnitTest cheats.assume(amount <= token.balanceOf(address(this))); cheats.assume(amount >= 1); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); // needed for expecting an event with the right parameters @@ -292,7 +292,7 @@ contract StrategyManagerUnitTests_depositIntoStrategy is StrategyManagerUnitTest emit Deposit(staker, token, strategy, expectedShares); uint256 shares = strategyManager.depositIntoStrategy(strategy, token, amount); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); assertEq(sharesAfter, sharesBefore + shares, "sharesAfter != sharesBefore + shares"); @@ -472,7 +472,7 @@ contract StrategyManagerUnitTests_depositIntoStrategy is StrategyManagerUnitTest strategyManager.depositIntoStrategy(strategy, token, amount); } - function test_addScaledShares_Revert_WhenSharesIsZero() external { + function test_addShares_Revert_WhenSharesIsZero() external { // replace dummyStrat with Reenterer contract reenterer = new Reenterer(); dummyStrat = StrategyBase(address(reenterer)); @@ -494,7 +494,7 @@ contract StrategyManagerUnitTests_depositIntoStrategy is StrategyManagerUnitTest reenterer.prepareReturnData(abi.encode(uint256(0))); cheats.prank(staker); - cheats.expectRevert("StrategyManager._addScaledShares: shares should not be zero!"); + cheats.expectRevert("StrategyManager._addShares: shares should not be zero!"); strategyManager.depositIntoStrategy(strategy, token, amount); } } @@ -521,14 +521,14 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa signature = abi.encodePacked(r, s, v); } - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); cheats.expectRevert("EIP1271SignatureUtils.checkSignature_EIP1271: signature not from signer"); // call with `notStaker` as input instead of `staker` address address notStaker = address(3333); strategyManager.depositIntoStrategyWithSignature(strategy, token, amount, notStaker, expiry, signature); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); uint256 nonceAfter = strategyManager.nonces(staker); assertEq(sharesAfter, sharesBefore, "sharesAfter != sharesBefore"); @@ -572,7 +572,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa ERC1271WalletMock wallet = new ERC1271WalletMock(staker); staker = address(wallet); - // filter out zero case since it will revert with "StrategyManager._addScaledShares: shares should not be zero!" + // filter out zero case since it will revert with "StrategyManager._addShares: shares should not be zero!" cheats.assume(amount != 0); // sanity check / filter cheats.assume(amount <= token.balanceOf(address(this))); @@ -617,7 +617,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa ERC1271MaliciousMock wallet = new ERC1271MaliciousMock(); staker = address(wallet); - // filter out zero case since it will revert with "StrategyManager._addScaledShares: shares should not be zero!" + // filter out zero case since it will revert with "StrategyManager._addShares: shares should not be zero!" cheats.assume(amount != 0); // sanity check / filter cheats.assume(amount <= token.balanceOf(address(this))); @@ -763,12 +763,12 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa signature = abi.encodePacked(r, s, v); } - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); cheats.expectRevert("StrategyManager.depositIntoStrategyWithSignature: signature expired"); strategyManager.depositIntoStrategyWithSignature(strategy, token, amount, staker, expiry, signature); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); uint256 nonceAfter = strategyManager.nonces(staker); assertEq(sharesAfter, sharesBefore, "sharesAfter != sharesBefore"); @@ -801,18 +801,18 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa } } -contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests { +contract StrategyManagerUnitTests_removeShares is StrategyManagerUnitTests { /** * @notice Should revert if not called by DelegationManager */ function test_Revert_DelegationManagerModifier() external { DelegationManagerMock invalidDelegationManager = new DelegationManagerMock(); cheats.expectRevert("StrategyManager.onlyDelegationManager: not the DelegationManager"); - invalidDelegationManager.removeScaledShares(strategyManager, address(this), dummyStrat, 1); + invalidDelegationManager.removeShares(strategyManager, address(this), dummyStrat, 1); } /** - * @notice deposits a single strategy and tests removeScaledShares() function reverts when sharesAmount is 0 + * @notice deposits a single strategy and tests removeShares() function reverts when sharesAmount is 0 */ function testFuzz_Revert_ZeroShares( address staker, @@ -822,48 +822,48 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests cheats.assume(depositAmount > 0 && depositAmount < dummyToken.totalSupply()); IStrategy strategy = dummyStrat; _depositIntoStrategySuccessfully(strategy, staker, depositAmount); - cheats.expectRevert("StrategyManager._removeScaledShares: shareAmount should not be zero!"); - delegationManagerMock.removeScaledShares(strategyManager, staker, strategy, 0); + cheats.expectRevert("StrategyManager._removeShares: shareAmount should not be zero!"); + delegationManagerMock.removeShares(strategyManager, staker, strategy, 0); } /** - * @notice deposits a single strategy and tests removeScaledShares() function reverts when sharesAmount is + * @notice deposits a single strategy and tests removeShares() function reverts when sharesAmount is * higher than depositAmount */ function testFuzz_Revert_ShareAmountTooHigh( address staker, uint256 depositAmount, - uint256 removeScaledSharesAmount + uint256 removeSharesAmount ) external filterFuzzedAddressInputs(staker) { cheats.assume(staker != address(0)); cheats.assume(depositAmount > 0 && depositAmount < dummyToken.totalSupply()); - cheats.assume(removeScaledSharesAmount > depositAmount); + cheats.assume(removeSharesAmount > depositAmount); IStrategy strategy = dummyStrat; _depositIntoStrategySuccessfully(strategy, staker, depositAmount); - cheats.expectRevert("StrategyManager._removeScaledShares: shareAmount too high"); - delegationManagerMock.removeScaledShares(strategyManager, staker, strategy, removeScaledSharesAmount); + cheats.expectRevert("StrategyManager._removeShares: shareAmount too high"); + delegationManagerMock.removeShares(strategyManager, staker, strategy, removeSharesAmount); } /** - * @notice deposit single strategy and removeScaledShares() for less than the deposited amount + * @notice deposit single strategy and removeShares() for less than the deposited amount * Shares should be updated correctly with stakerStrategyListLength unchanged */ function testFuzz_RemoveScaledSharesLessThanDeposit( address staker, uint256 depositAmount, - uint256 removeScaledSharesAmount + uint256 removeSharesAmount ) external filterFuzzedAddressInputs(staker) { cheats.assume(staker != address(0)); cheats.assume(depositAmount > 0 && depositAmount < dummyToken.totalSupply()); - cheats.assume(removeScaledSharesAmount > 0 && removeScaledSharesAmount < depositAmount); + cheats.assume(removeSharesAmount > 0 && removeSharesAmount < depositAmount); IStrategy strategy = dummyStrat; _depositIntoStrategySuccessfully(strategy, staker, depositAmount); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); - delegationManagerMock.removeScaledShares(strategyManager, staker, strategy, removeScaledSharesAmount); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); + delegationManagerMock.removeShares(strategyManager, staker, strategy, removeSharesAmount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); - assertEq(sharesBefore, sharesAfter + removeScaledSharesAmount, "Remove incorrect amount of shares"); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); + assertEq(sharesBefore, sharesAfter + removeSharesAmount, "Remove incorrect amount of shares"); assertEq( stakerStrategyListLengthBefore, stakerStrategyListLengthAfter, @@ -872,7 +872,7 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests } /** - * @notice testing removeScaledShares() + * @notice testing removeShares() * deposits 1 strategy and tests it is removed from staker strategy list after removing all shares */ function testFuzz_RemovesStakerStrategyListSingleStrat( @@ -885,12 +885,12 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests _depositIntoStrategySuccessfully(strategy, staker, sharesAmount); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, strategy); assertEq(sharesBefore, sharesAmount, "Staker has not deposited amount into strategy"); - delegationManagerMock.removeScaledShares(strategyManager, staker, strategy, sharesAmount); + delegationManagerMock.removeShares(strategyManager, staker, strategy, sharesAmount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, strategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, strategy); assertEq( stakerStrategyListLengthAfter, stakerStrategyListLengthBefore - 1, @@ -901,7 +901,7 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests } /** - * @notice testing removeScaledShares() function with 3 strategies deposited. + * @notice testing removeShares() function with 3 strategies deposited. * Randomly selects one of the 3 strategies to be fully removed from staker strategy list. * Only callable by DelegationManager */ @@ -925,14 +925,14 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); uint256[] memory sharesBefore = new uint256[](3); for (uint256 i = 0; i < 3; ++i) { - sharesBefore[i] = strategyManager.stakerStrategyScaledShares(staker, strategies[i]); + sharesBefore[i] = strategyManager.stakerStrategyShares(staker, strategies[i]); assertEq(sharesBefore[i], amounts[i], "Staker has not deposited amount into strategy"); assertTrue(_isDepositedStrategy(staker, strategies[i]), "strategy should be deposited"); } - delegationManagerMock.removeScaledShares(strategyManager, staker, removeStrategy, removeAmount); + delegationManagerMock.removeShares(strategyManager, staker, removeStrategy, removeAmount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, removeStrategy); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, removeStrategy); assertEq( stakerStrategyListLengthAfter, stakerStrategyListLengthBefore - 1, @@ -946,7 +946,7 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests } /** - * @notice testing removeScaledShares() function with 3 strategies deposited. + * @notice testing removeShares() function with 3 strategies deposited. * Removing Shares could result in removing from staker strategy list if depositAmounts[i] == sharesAmounts[i]. * Only callable by DelegationManager */ @@ -961,7 +961,7 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests depositAmounts[i] = bound(depositAmounts[i], 1, strategies[i].underlyingToken().totalSupply()); sharesAmounts[i] = bound(sharesAmounts[i], 1, depositAmounts[i]); _depositIntoStrategySuccessfully(strategies[i], staker, depositAmounts[i]); - sharesBefore[i] = strategyManager.stakerStrategyScaledShares(staker, strategies[i]); + sharesBefore[i] = strategyManager.stakerStrategyShares(staker, strategies[i]); assertEq(sharesBefore[i], depositAmounts[i], "Staker has not deposited amount into strategy"); assertTrue(_isDepositedStrategy(staker, strategies[i]), "strategy should be deposited"); } @@ -970,11 +970,11 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests uint256 numPoppedStrategies = 0; uint256[] memory sharesAfter = new uint256[](3); for (uint256 i = 0; i < 3; ++i) { - delegationManagerMock.removeScaledShares(strategyManager, staker, strategies[i], sharesAmounts[i]); + delegationManagerMock.removeShares(strategyManager, staker, strategies[i], sharesAmounts[i]); } for (uint256 i = 0; i < 3; ++i) { - sharesAfter[i] = strategyManager.stakerStrategyScaledShares(staker, strategies[i]); + sharesAfter[i] = strategyManager.stakerStrategyShares(staker, strategies[i]); if (sharesAmounts[i] == depositAmounts[i]) { ++numPoppedStrategies; assertFalse( @@ -1002,22 +1002,22 @@ contract StrategyManagerUnitTests_removeScaledShares is StrategyManagerUnitTests } } -contract StrategyManagerUnitTests_addScaledShares is StrategyManagerUnitTests { +contract StrategyManagerUnitTests_addShares is StrategyManagerUnitTests { function test_Revert_DelegationManagerModifier() external { DelegationManagerMock invalidDelegationManager = new DelegationManagerMock(); cheats.expectRevert("StrategyManager.onlyDelegationManager: not the DelegationManager"); - invalidDelegationManager.addScaledShares(strategyManager, address(this), dummyToken, dummyStrat, 1); + invalidDelegationManager.addShares(strategyManager, address(this), dummyToken, dummyStrat, 1); } function testFuzz_Revert_StakerZeroAddress(uint256 amount) external { - cheats.expectRevert("StrategyManager._addScaledShares: staker cannot be zero address"); - delegationManagerMock.addScaledShares(strategyManager, address(0), dummyToken, dummyStrat, amount); + cheats.expectRevert("StrategyManager._addShares: staker cannot be zero address"); + delegationManagerMock.addShares(strategyManager, address(0), dummyToken, dummyStrat, amount); } function testFuzz_Revert_ZeroShares(address staker) external filterFuzzedAddressInputs(staker) { cheats.assume(staker != address(0)); - cheats.expectRevert("StrategyManager._addScaledShares: shares should not be zero!"); - delegationManagerMock.addScaledShares(strategyManager, staker, dummyToken, dummyStrat, 0); + cheats.expectRevert("StrategyManager._addShares: shares should not be zero!"); + delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, 0); } function testFuzz_AppendsStakerStrategyList( @@ -1026,13 +1026,13 @@ contract StrategyManagerUnitTests_addScaledShares is StrategyManagerUnitTests { ) external filterFuzzedAddressInputs(staker) { cheats.assume(staker != address(0) && amount != 0); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq(sharesBefore, 0, "Staker has already deposited into this strategy"); assertFalse(_isDepositedStrategy(staker, dummyStrat), "strategy should not be deposited"); - delegationManagerMock.addScaledShares(strategyManager, staker, dummyToken, dummyStrat, amount); + delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, amount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq( stakerStrategyListLengthAfter, stakerStrategyListLengthBefore + 1, @@ -1051,13 +1051,13 @@ contract StrategyManagerUnitTests_addScaledShares is StrategyManagerUnitTests { IStrategy strategy = dummyStrat; _depositIntoStrategySuccessfully(strategy, staker, initialAmount); uint256 stakerStrategyListLengthBefore = strategyManager.stakerStrategyListLength(staker); - uint256 sharesBefore = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesBefore = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq(sharesBefore, initialAmount, "Staker has not deposited amount into strategy"); assertTrue(_isDepositedStrategy(staker, strategy), "strategy should be deposited"); - delegationManagerMock.addScaledShares(strategyManager, staker, dummyToken, dummyStrat, sharesAmount); + delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, sharesAmount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); - uint256 sharesAfter = strategyManager.stakerStrategyScaledShares(staker, dummyStrat); + uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq( stakerStrategyListLengthAfter, stakerStrategyListLengthBefore, @@ -1068,7 +1068,7 @@ contract StrategyManagerUnitTests_addScaledShares is StrategyManagerUnitTests { } /** - * @notice When _addScaledShares() called either by depositIntoStrategy or addScaledShares() results in appending to + * @notice When _addShares() called either by depositIntoStrategy or addShares() results in appending to * stakerStrategyListLength when the staker has MAX_STAKER_STRATEGY_LIST_LENGTH strategies, it should revert */ function test_Revert_WhenMaxStrategyListLength() external { @@ -1104,10 +1104,10 @@ contract StrategyManagerUnitTests_addScaledShares is StrategyManagerUnitTests { ); cheats.prank(staker); - cheats.expectRevert("StrategyManager._addScaledShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); - delegationManagerMock.addScaledShares(strategyManager, staker, dummyToken, strategy, amount); + cheats.expectRevert("StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); + delegationManagerMock.addShares(strategyManager, staker, dummyToken, strategy, amount); - cheats.expectRevert("StrategyManager._addScaledShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); + cheats.expectRevert("StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); strategyManager.depositIntoStrategy(strategy, token, amount); } } @@ -1116,7 +1116,7 @@ contract StrategyManagerUnitTests_withdrawSharesAsTokens is StrategyManagerUnitT function test_Revert_DelegationManagerModifier() external { DelegationManagerMock invalidDelegationManager = new DelegationManagerMock(); cheats.expectRevert("StrategyManager.onlyDelegationManager: not the DelegationManager"); - invalidDelegationManager.removeScaledShares(strategyManager, address(this), dummyStrat, 1); + invalidDelegationManager.removeShares(strategyManager, address(this), dummyStrat, 1); } /**