From a1792de35e4a205d45b1442770ce6d7b608f3edd Mon Sep 17 00:00:00 2001 From: "clandestine.eth" <96172957+0xClandestine@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:47:11 -0400 Subject: [PATCH] refactor: natspec/interface improvements --- src/contracts/core/DelegationManager.sol | 25 ++++++++++++++++--- src/contracts/interfaces/IAVSDirectory.sol | 19 ++++++++++++-- .../interfaces/IDelegationManager.sol | 25 ++++++++++++++++++- .../interfaces/IRewardsCoordinator.sol | 17 ++++++++++--- src/contracts/interfaces/IStrategyManager.sol | 13 ++++++++++ src/test/mocks/DelegationManagerMock.sol | 10 ++++++-- src/test/mocks/StrategyManagerMock.sol | 4 +++ 7 files changed, 102 insertions(+), 11 deletions(-) diff --git a/src/contracts/core/DelegationManager.sol b/src/contracts/core/DelegationManager.sol index 7fb0f4a16..cdf71f070 100644 --- a/src/contracts/core/DelegationManager.sol +++ b/src/contracts/core/DelegationManager.sol @@ -330,7 +330,11 @@ contract DelegationManager is * */ - /// @inheritdoc IDelegationManager + /** + * @notice Sets operator parameters in the `_operatorDetails` mapping. + * @param operator The account registered as an operator updating their operatorDetails + * @param newOperatorDetails The new parameters for the operator + */ function _setOperatorDetails(address operator, OperatorDetails calldata newOperatorDetails) internal { require( newOperatorDetails.stakerOptOutWindowBlocks <= MAX_STAKER_OPT_OUT_WINDOW_BLOCKS, @@ -344,7 +348,19 @@ contract DelegationManager is emit OperatorDetailsModified(msg.sender, newOperatorDetails); } - /// @inheritdoc IDelegationManager + /** + * @notice Delegates *from* a `staker` *to* an `operator`. + * @param staker The address to delegate *from* -- this address is delegating control of its own assets. + * @param operator The address to delegate *to* -- this address is being given power to place the `staker`'s assets at risk on services + * @param approverSignatureAndExpiry Verifies the operator approves of this delegation + * @param approverSalt Is a salt used to help guarantee signature uniqueness. Each salt can only be used once by a given approver. + * @dev Assumes the following is checked before calling this function: + * 1) the `staker` is not already delegated to an operator + * 2) the `operator` has indeed registered as an operator in EigenLayer + * Ensures that: + * 1) if applicable, that the approver signature is valid and non-expired + * 2) new delegations are not paused (PAUSED_NEW_DELEGATION) + */ function _delegate( address staker, address operator, @@ -411,7 +427,10 @@ contract DelegationManager is } } - /// @inheritdoc IDelegationManager + /** + * @dev commented-out param (middlewareTimesIndex) is the index in the operator that the staker who triggered the withdrawal was delegated to's middleware times array + * This param is intended to be passed on to the Slasher contract, but is unused in the M2 release of these contracts, and is thus commented-out. + */ function _completeQueuedWithdrawal( Withdrawal calldata withdrawal, IERC20[] calldata tokens, diff --git a/src/contracts/interfaces/IAVSDirectory.sol b/src/contracts/interfaces/IAVSDirectory.sol index b35cabc99..2c3f767d8 100644 --- a/src/contracts/interfaces/IAVSDirectory.sol +++ b/src/contracts/interfaces/IAVSDirectory.sol @@ -23,7 +23,7 @@ interface IAVSDirectory is ISignatureUtils { ); /** - * @notice Called by an avs to register an operator with the avs. + * @notice Called by the AVS's service manager contract to register an operator with the avs. * @param operator The address of the operator to register. * @param operatorSignature The signature, salt, and expiry of the operator's signature. */ @@ -54,7 +54,7 @@ interface IAVSDirectory is ISignatureUtils { /** * @notice Calculates the digest hash to be signed by an operator to register with an AVS * @param operator The account registering as an operator - * @param avs The AVS the operator is registering to + * @param avs The address of the service manager contract for the AVS that the operator is registering to * @param salt A unique and single use value associated with the approver signature. * @param expiry Time after which the approver's signature becomes invalid */ @@ -67,4 +67,19 @@ interface IAVSDirectory is ISignatureUtils { /// @notice The EIP-712 typehash for the Registration struct used by the contract function OPERATOR_AVS_REGISTRATION_TYPEHASH() external view returns (bytes32); + + /** + * @notice Called by an operator to cancel a salt that has been used to register with an AVS. + * @param salt A unique and single use value associated with the approver signature. + */ + function cancelSalt(bytes32 salt) external; + + /** + * @notice Getter function for the current EIP-712 domain separator for this contract. + * + * @dev The domain separator will change in the event of a fork that changes the ChainID. + * @dev By introducing a domain separator the DApp developers are guaranteed that there can be no signature collision. + * for more detailed information please read EIP-712. + */ + function domainSeparator() external view returns (bytes32); } diff --git a/src/contracts/interfaces/IDelegationManager.sol b/src/contracts/interfaces/IDelegationManager.sol index dc42416b6..2fcb8d03d 100644 --- a/src/contracts/interfaces/IDelegationManager.sol +++ b/src/contracts/interfaces/IDelegationManager.sol @@ -148,6 +148,7 @@ interface IDelegationManager is ISignatureUtils { * @param metadataURI is a URI for the operator's metadata, i.e. a link providing more details on the operator. * * @dev Once an operator is registered, they cannot 'deregister' as an operator, and they will forever be considered "delegated to themself". + * @dev This function will revert if the caller is already delegated to an operator. * @dev Note that the `metadataURI` is *never stored * and is only emitted in the `OperatorMetadataURIUpdated` event */ function registerAsOperator( @@ -246,7 +247,7 @@ interface IDelegationManager is ISignatureUtils { * @param receiveAsTokens If true, the shares specified in the withdrawal will be withdrawn from the specified strategies themselves * and sent to the caller, through calls to `withdrawal.strategies[i].withdraw`. If false, then the shares in the specified strategies * will simply be transferred to the caller directly. - * @dev middlewareTimesIndex should be calculated off chain before calling this function by finding the first index that satisfies `slasher.canWithdraw` + * @dev middlewareTimesIndex is unused, but will be used in the Slasher eventually * @dev beaconChainETHStrategy shares are non-transferrable, so if `receiveAsTokens = false` and `withdrawal.withdrawer != withdrawal.staker`, note that * any beaconChainETHStrategy shares in the `withdrawal` will be _returned to the staker_, rather than transferred to the withdrawer, unlike shares in * any other strategies, which will be transferred to the withdrawer. @@ -296,6 +297,21 @@ interface IDelegationManager is ISignatureUtils { */ function decreaseDelegatedShares(address staker, IStrategy strategy, uint256 shares) external; + /** + * @notice Owner-only function for modifying the value of the `minWithdrawalDelayBlocks` variable. + * @param newMinWithdrawalDelayBlocks new value of `minWithdrawalDelayBlocks`. + */ + function setMinWithdrawalDelayBlocks(uint256 newMinWithdrawalDelayBlocks) external; + + /** + * @notice Called by owner to set the minimum withdrawal delay blocks for each passed in strategy + * Note that the min number of blocks to complete a withdrawal of a strategy is + * MAX(minWithdrawalDelayBlocks, strategyWithdrawalDelayBlocks[strategy]) + * @param strategies The strategies to set the minimum withdrawal delay blocks for + * @param withdrawalDelayBlocks The minimum withdrawal delay blocks to set for each strategy + */ + function setStrategyWithdrawalDelayBlocks(IStrategy[] calldata strategies, uint256[] calldata withdrawalDelayBlocks) external; + /** * @notice returns the address of the operator that `staker` is delegated to. * @notice Mapping: staker => operator whom the staker is currently delegated to. @@ -342,6 +358,13 @@ interface IDelegationManager is ISignatureUtils { */ function operatorShares(address operator, IStrategy strategy) external view returns (uint256); + + /** + * @notice Returns the number of actively-delegatable shares a staker has across all strategies. + * @dev Returns two empty arrays in the case that the Staker has no actively-delegateable shares. + */ + function getDelegatableShares(address staker) external view returns (IStrategy[] memory, uint256[] memory); + /** * @notice Returns 'true' if `staker` *is* actively delegated, and 'false' otherwise. */ diff --git a/src/contracts/interfaces/IRewardsCoordinator.sol b/src/contracts/interfaces/IRewardsCoordinator.sol index d3f5beaf2..933758618 100644 --- a/src/contracts/interfaces/IRewardsCoordinator.sol +++ b/src/contracts/interfaces/IRewardsCoordinator.sol @@ -271,6 +271,7 @@ interface IRewardsCoordinator { * @notice similar to `createAVSRewardsSubmission` except the rewards are split amongst *all* stakers * rather than just those delegated to operators who are registered to a single avs and is * a permissioned call based on isRewardsForAllSubmitter mapping. + * @param rewardsSubmission The rewards submission being created */ function createRewardsForAllSubmission(RewardsSubmission[] calldata rewardsSubmission) external; @@ -300,7 +301,7 @@ interface IRewardsCoordinator { /** * @notice Creates a new distribution root. activatedAt is set to block.timestamp + activationDelay * @param root The merkle root of the distribution - * @param rewardsCalculationEndTimestamp The timestamp (seconds) until which rewards have been calculated + * @param rewardsCalculationEndTimestamp The timestamp until which rewards have been calculated * @dev Only callable by the rewardsUpdater */ function submitRoot(bytes32 root, uint32 rewardsCalculationEndTimestamp) external; @@ -313,15 +314,15 @@ interface IRewardsCoordinator { /** * @notice Sets the address of the entity that can call `processClaim` on behalf of the earner (msg.sender) - * @param claimer The address of the entity that can claim rewards on behalf of the earner + * @param claimer The address of the entity that can call `processClaim` on behalf of the earner * @dev Only callable by the `earner` */ function setClaimerFor(address claimer) external; /** * @notice Sets the delay in timestamp before a posted root can be claimed against - * @param _activationDelay Delay in timestamp (seconds) before a posted root can be claimed against * @dev Only callable by the contract owner + * @param _activationDelay The new value for activationDelay */ function setActivationDelay(uint32 _activationDelay) external; @@ -335,6 +336,7 @@ interface IRewardsCoordinator { /** * @notice Sets the permissioned `rewardsUpdater` address which can post new roots * @dev Only callable by the contract owner + * @param _rewardsUpdater The address of the new rewardsUpdater */ function setRewardsUpdater(address _rewardsUpdater) external; @@ -345,4 +347,13 @@ interface IRewardsCoordinator { * @param _newValue The new value for isRewardsForAllSubmitter */ function setRewardsForAllSubmitter(address _submitter, bool _newValue) external; + + /** + * @notice Getter function for the current EIP-712 domain separator for this contract. + * + * @dev The domain separator will change in the event of a fork that changes the ChainID. + * @dev By introducing a domain separator the DApp developers are guaranteed that there can be no signature collision. + * for more detailed information please read EIP-712. + */ + function domainSeparator() external view returns (bytes32); } diff --git a/src/contracts/interfaces/IStrategyManager.sol b/src/contracts/interfaces/IStrategyManager.sol index 565b301cd..784bb1c46 100644 --- a/src/contracts/interfaces/IStrategyManager.sol +++ b/src/contracts/interfaces/IStrategyManager.sol @@ -92,6 +92,7 @@ interface IStrategyManager { /** * @notice Get all details on the staker's deposits and corresponding shares + * @param staker The staker of interest, whose deposits this function will fetch * @return (staker's strategies, shares in these strategies) */ function getDeposits(address staker) external view returns (IStrategy[] memory, uint256[] memory); @@ -139,9 +140,21 @@ interface IStrategyManager { /// @notice Returns bool for whether or not `strategy` is whitelisted for deposit function strategyIsWhitelistedForDeposit(IStrategy strategy) external view returns (bool); + /** + * @notice Owner-only function to change the `strategyWhitelister` address. + * @param newStrategyWhitelister new address for the `strategyWhitelister`. + */ + function setStrategyWhitelister(address newStrategyWhitelister) external; + /** * @notice Returns bool for whether or not `strategy` enables credit transfers. i.e enabling * depositIntoStrategyWithSignature calls or queueing withdrawals to a different address than the staker. */ function thirdPartyTransfersForbidden(IStrategy strategy) external view returns (bool); + + /** + * @notice Getter function for the current EIP-712 domain separator for this contract. + * @dev The domain separator will change in the event of a fork that changes the ChainID. + */ + function domainSeparator() external view returns (bytes32); } diff --git a/src/test/mocks/DelegationManagerMock.sol b/src/test/mocks/DelegationManagerMock.sol index 4fa56d087..7facde3a0 100644 --- a/src/test/mocks/DelegationManagerMock.sol +++ b/src/test/mocks/DelegationManagerMock.sol @@ -10,6 +10,12 @@ contract DelegationManagerMock is IDelegationManager, Test { mapping(address => bool) public isOperator; mapping(address => mapping(IStrategy => uint256)) public operatorShares; + function getDelegatableShares(address staker) external view returns (IStrategy[] memory, uint256[] memory) {} + + function setMinWithdrawalDelayBlocks(uint256 newMinWithdrawalDelayBlocks) external {} + + function setStrategyWithdrawalDelayBlocks(IStrategy[] calldata strategies, uint256[] calldata withdrawalDelayBlocks) external {} + function setIsOperator(address operator, bool _isOperatorReturnValue) external { isOperator[operator] = _isOperatorReturnValue; } @@ -136,8 +142,6 @@ contract DelegationManagerMock is IDelegationManager, Test { function OPERATOR_AVS_REGISTRATION_TYPEHASH() external view returns (bytes32) {} - function domainSeparator() external view returns (bytes32) {} - function cumulativeWithdrawalsQueued(address staker) external view returns (uint256) {} function calculateWithdrawalRoot(Withdrawal memory withdrawal) external pure returns (bytes32) {} @@ -148,6 +152,8 @@ contract DelegationManagerMock is IDelegationManager, Test { function operatorSaltIsSpent(address avs, bytes32 salt) external view returns (bool) {} + function domainSeparator() external view returns (bytes32) {} + function queueWithdrawals( QueuedWithdrawalParams[] calldata queuedWithdrawalParams ) external returns (bytes32[] memory) {} diff --git a/src/test/mocks/StrategyManagerMock.sol b/src/test/mocks/StrategyManagerMock.sol index cefd175fa..4a5f39cb2 100644 --- a/src/test/mocks/StrategyManagerMock.sol +++ b/src/test/mocks/StrategyManagerMock.sol @@ -55,6 +55,8 @@ contract StrategyManagerMock is function recordBeaconChainETHBalanceUpdate(address overcommittedPodOwner, uint256 beaconChainETHStrategyIndex, int256 sharesDelta) external{} + function setStrategyWhitelister(address newStrategyWhitelister) external {} + function depositIntoStrategyWithSignature( IStrategy strategy, IERC20 token, @@ -122,6 +124,8 @@ contract StrategyManagerMock is // function withdrawalDelayBlocks() external view returns (uint256) {} + function domainSeparator() external view returns (bytes32) {} + function addStrategiesToDepositWhitelist( IStrategy[] calldata strategiesToWhitelist, bool[] calldata thirdPartyTransfersForbiddenValues