From ce749be7f8d08c3ba9908a3a8f7675f5184dae0b Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 28 Nov 2024 14:47:11 -0300 Subject: [PATCH 01/23] fix: separate delegation and provision thaw request lists (TRST-H02) --- .../internal/IHorizonStakingBase.sol | 16 +- .../internal/IHorizonStakingTypes.sol | 32 ++++ .../contracts/staking/HorizonStaking.sol | 174 +++++++++++------- .../contracts/staking/HorizonStakingBase.sol | 96 +++++++++- .../staking/HorizonStakingStorage.sol | 13 +- .../HorizonStakingShared.t.sol | 114 +++++++++--- .../test/staking/delegation/withdraw.t.sol | 16 +- 7 files changed, 353 insertions(+), 108 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol index e221dc2cf..32e476298 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol @@ -134,21 +134,27 @@ interface IHorizonStakingBase { /** * @notice Gets a thaw request. + * @param thawRequestType The type of thaw request. * @param thawRequestId The id of the thaw request. * @return The thaw request details. */ - function getThawRequest(bytes32 thawRequestId) external view returns (IHorizonStakingTypes.ThawRequest memory); + function getThawRequest( + IHorizonStakingTypes.ThawRequestType thawRequestType, + bytes32 thawRequestId + ) external view returns (IHorizonStakingTypes.ThawRequest memory); /** * @notice Gets the metadata of a thaw request list. * Service provider and delegators each have their own thaw request list per provision. * Metadata includes the head and tail of the list, plus the total number of thaw requests. + * @param thawRequestType The type of thaw request. * @param serviceProvider The address of the service provider. * @param verifier The address of the verifier. * @param owner The owner of the thaw requests. Use either the service provider or delegator address. * @return The thaw requests list metadata. */ function getThawRequestList( + IHorizonStakingTypes.ThawRequestType thawRequestType, address serviceProvider, address verifier, address owner @@ -156,12 +162,18 @@ interface IHorizonStakingBase { /** * @notice Gets the amount of thawed tokens for a given provision. + * @param thawRequestType The type of thaw request. * @param serviceProvider The address of the service provider. * @param verifier The address of the verifier. * @param owner The owner of the thaw requests. Use either the service provider or delegator address. * @return The amount of thawed tokens. */ - function getThawedTokens(address serviceProvider, address verifier, address owner) external view returns (uint256); + function getThawedTokens( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + address owner + ) external view returns (uint256); /** * @notice Gets the maximum allowed thawing period for a provision. diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol index 0dfc6c774..8aff1a6e0 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol @@ -131,6 +131,16 @@ interface IHorizonStakingTypes { uint256 __DEPRECATED_tokensLockedUntil; } + /** + * @dev Enum to specify the type of thaw request. + * @param Provision Represents a thaw request for a provision. + * @param Delegation Represents a thaw request for a delegation. + */ + enum ThawRequestType { + Provision, + Delegation + } + /** * @notice Details of a stake thawing operation. * @dev ThawRequests are stored in linked lists by service provider/delegator, @@ -146,4 +156,26 @@ interface IHorizonStakingTypes { // Used to invalidate unfulfilled thaw requests uint256 thawingNonce; } + + /** + * @notice Parameters to fulfill thaw requests. + * @param requestType The type of thaw request (Provision or Delegation) + * @param serviceProvider The address of the service provider + * @param verifier The address of the verifier + * @param owner The address of the owner of the thaw request + * @param tokensThawing The current amount of tokens already thawing + * @param sharesThawing The current amount of shares already thawing + * @param nThawRequests The number of thaw requests to fulfill. If set to 0, all thaw requests are fulfilled. + * @param thawingNonce The current valid thawing nonce. Any thaw request with a different nonce is invalid and should be ignored. + */ + struct FulfillThawRequestsParams { + ThawRequestType requestType; + address serviceProvider; + address verifier; + address owner; + uint256 tokensThawing; + uint256 sharesThawing; + uint256 nThawRequests; + uint256 thawingNonce; + } } diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 8df8f20f6..4ad8136c4 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -741,6 +741,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { prov.tokensThawing = prov.tokensThawing + _tokens; bytes32 thawRequestId = _createThawRequest( + ThawRequestType.Provision, _serviceProvider, _verifier, _serviceProvider, @@ -765,15 +766,18 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensThawed_ = 0; uint256 sharesThawing = prov.sharesThawing; uint256 tokensThawing = prov.tokensThawing; - (tokensThawed_, tokensThawing, sharesThawing) = _fulfillThawRequests( - _serviceProvider, - _verifier, - _serviceProvider, - tokensThawing, - sharesThawing, - _nThawRequests, - prov.thawingNonce - ); + + FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ + requestType: ThawRequestType.Provision, + serviceProvider: _serviceProvider, + verifier: _verifier, + owner: _serviceProvider, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing, + nThawRequests: _nThawRequests, + thawingNonce: prov.thawingNonce + }); + (tokensThawed_, tokensThawing, sharesThawing) = _fulfillThawRequests(params); prov.tokens = prov.tokens - tokensThawed_; prov.sharesThawing = sharesThawing; @@ -860,6 +864,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { delegation.shares = delegation.shares - _shares; bytes32 thawRequestId = _createThawRequest( + ThawRequestType.Delegation, _serviceProvider, _verifier, _beneficiary, @@ -894,15 +899,18 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensThawed = 0; uint256 sharesThawing = pool.sharesThawing; uint256 tokensThawing = pool.tokensThawing; - (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests( - _serviceProvider, - _verifier, - msg.sender, - tokensThawing, - sharesThawing, - _nThawRequests, - pool.thawingNonce - ); + + FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ + requestType: ThawRequestType.Delegation, + serviceProvider: _serviceProvider, + verifier: _verifier, + owner: msg.sender, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing, + nThawRequests: _nThawRequests, + thawingNonce: pool.thawingNonce + }); + (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests(params); // The next subtraction should never revert becase: pool.tokens >= pool.tokensThawing and pool.tokensThawing >= tokensThawed // In the event the pool gets completely slashed tokensThawed will fulfil to 0. @@ -936,6 +944,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @return The ID of the thaw request */ function _createThawRequest( + ThawRequestType _requestType, address _serviceProvider, address _verifier, address _owner, @@ -943,18 +952,22 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint64 _thawingUntil, uint256 _thawingNonce ) private returns (bytes32) { - LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner]; + LinkedList.List storage thawRequestList = _getThawRequestList( + _requestType, + _serviceProvider, + _verifier, + _owner + ); require(thawRequestList.count < MAX_THAW_REQUESTS, HorizonStakingTooManyThawRequests()); bytes32 thawRequestId = keccak256(abi.encodePacked(_serviceProvider, _verifier, _owner, thawRequestList.nonce)); - _thawRequests[thawRequestId] = ThawRequest({ - shares: _shares, - thawingUntil: _thawingUntil, - next: bytes32(0), - thawingNonce: _thawingNonce - }); + ThawRequest storage thawRequest = _getThawRequest(_requestType, thawRequestId); + thawRequest.shares = _shares; + thawRequest.thawingUntil = _thawingUntil; + thawRequest.next = bytes32(0); + thawRequest.thawingNonce = _thawingNonce; - if (thawRequestList.count != 0) _thawRequests[thawRequestList.tail].next = thawRequestId; + if (thawRequestList.count != 0) _getThawRequest(_requestType, thawRequestList.tail).next = thawRequestId; thawRequestList.addTail(thawRequestId); emit ThawRequestCreated(_serviceProvider, _verifier, _owner, _shares, _thawingUntil, thawRequestId); @@ -964,41 +977,49 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /** * @notice Traverses a thaw request list and fulfills expired thaw requests. * @dev Emits a {ThawRequestsFulfilled} event and a {ThawRequestFulfilled} event for each thaw request fulfilled. - * @param _serviceProvider The address of the service provider - * @param _verifier The address of the verifier - * @param _owner The address of the owner of the thaw request - * @param _tokensThawing The current amount of tokens already thawing - * @param _sharesThawing The current amount of shares already thawing - * @param _nThawRequests The number of thaw requests to fulfill. If set to 0, all thaw requests are fulfilled. - * @param _thawingNonce The current valid thawing nonce. Any thaw request with a different nonce is invalid and should be ignored. + * @param params The parameters for fulfilling thaw requests * @return The amount of thawed tokens * @return The amount of tokens still thawing * @return The amount of shares still thawing */ - function _fulfillThawRequests( - address _serviceProvider, - address _verifier, - address _owner, - uint256 _tokensThawing, - uint256 _sharesThawing, - uint256 _nThawRequests, - uint256 _thawingNonce - ) private returns (uint256, uint256, uint256) { - LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner]; + function _fulfillThawRequests(FulfillThawRequestsParams memory params) private returns (uint256, uint256, uint256) { + LinkedList.List storage thawRequestList = _getThawRequestList( + params.requestType, + params.serviceProvider, + params.verifier, + params.owner + ); require(thawRequestList.count > 0, HorizonStakingNothingThawing()); - uint256 tokensThawed = 0; + function(bytes32) view returns (bytes32) getNextItem = _getNextThawRequest(params.requestType); + function(bytes32) deleteItem = _getDeleteThawRequest(params.requestType); + bytes memory acc = abi.encode( + params.requestType, + uint256(0), + params.tokensThawing, + params.sharesThawing, + params.thawingNonce + ); (uint256 thawRequestsFulfilled, bytes memory data) = thawRequestList.traverse( - _getNextThawRequest, + getNextItem, _fulfillThawRequest, - _deleteThawRequest, - abi.encode(tokensThawed, _tokensThawing, _sharesThawing, _thawingNonce), - _nThawRequests + deleteItem, + acc, + params.nThawRequests ); - (tokensThawed, _tokensThawing, _sharesThawing) = abi.decode(data, (uint256, uint256, uint256)); - emit ThawRequestsFulfilled(_serviceProvider, _verifier, _owner, thawRequestsFulfilled, tokensThawed); - return (tokensThawed, _tokensThawing, _sharesThawing); + (, uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing) = abi.decode( + data, + (ThawRequestType, uint256, uint256, uint256) + ); + emit ThawRequestsFulfilled( + params.serviceProvider, + params.verifier, + params.owner, + thawRequestsFulfilled, + tokensThawed + ); + return (tokensThawed, tokensThawing, sharesThawing); } /** @@ -1013,19 +1034,22 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @return The updated accumulator data */ function _fulfillThawRequest(bytes32 _thawRequestId, bytes memory _acc) private returns (bool, bytes memory) { - ThawRequest storage thawRequest = _thawRequests[_thawRequestId]; + // decode + ( + ThawRequestType requestType, + uint256 tokensThawed, + uint256 tokensThawing, + uint256 sharesThawing, + uint256 thawingNonce + ) = abi.decode(_acc, (ThawRequestType, uint256, uint256, uint256, uint256)); + + ThawRequest storage thawRequest = _getThawRequest(requestType, _thawRequestId); // early exit if (thawRequest.thawingUntil > block.timestamp) { return (true, LinkedList.NULL_BYTES); } - // decode - (uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing, uint256 thawingNonce) = abi.decode( - _acc, - (uint256, uint256, uint256, uint256) - ); - // process - only fulfill thaw requests for the current valid nonce uint256 tokens = 0; bool validThawRequest = thawRequest.thawingNonce == thawingNonce; @@ -1044,17 +1068,39 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { ); // encode - _acc = abi.encode(tokensThawed, tokensThawing, sharesThawing, thawingNonce); + _acc = abi.encode(requestType, tokensThawed, tokensThawing, sharesThawing, thawingNonce); return (false, _acc); } /** - * @notice Deletes a ThawRequest. - * @dev This function is used as a callback in the thaw requests linked list traversal. - * @param _thawRequestId The ID of the thaw request to delete + * @notice Determines the correct callback function for `deleteItem` based on the request type. + * @param _requestType The type of thaw request (Provision or Delegation). + * @return A function pointer to the appropriate `deleteItem` callback. + */ + function _getDeleteThawRequest(ThawRequestType _requestType) private pure returns (function(bytes32)) { + if (_requestType == ThawRequestType.Provision) { + return _deleteProvisionThawRequest; + } else if (_requestType == ThawRequestType.Delegation) { + return _deleteDelegationThawRequest; + } else { + revert("Unknown ThawRequestType"); + } + } + + /** + * @notice Deletes a thaw request for a provision. + * @param _thawRequestId The ID of the thaw request to delete. + */ + function _deleteProvisionThawRequest(bytes32 _thawRequestId) private { + delete _provisionThawRequests[_thawRequestId]; + } + + /** + * @notice Deletes a thaw request for a delegation. + * @param _thawRequestId The ID of the thaw request to delete. */ - function _deleteThawRequest(bytes32 _thawRequestId) private { - delete _thawRequests[_thawRequestId]; + function _deleteDelegationThawRequest(bytes32 _thawRequestId) private { + delete _delegationThawRequests[_thawRequestId]; } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index d29d3edec..cdc45bb20 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -173,30 +173,35 @@ abstract contract HorizonStakingBase is /** * @notice See {IHorizonStakingBase-getThawRequest}. */ - function getThawRequest(bytes32 thawRequestId) external view override returns (ThawRequest memory) { - return _thawRequests[thawRequestId]; + function getThawRequest( + ThawRequestType requestType, + bytes32 thawRequestId + ) external view override returns (ThawRequest memory) { + return _getThawRequest(requestType, thawRequestId); } /** * @notice See {IHorizonStakingBase-getThawRequestList}. */ function getThawRequestList( + ThawRequestType requestType, address serviceProvider, address verifier, address owner ) external view override returns (LinkedList.List memory) { - return _thawRequestLists[serviceProvider][verifier][owner]; + return _getThawRequestList(requestType, serviceProvider, verifier, owner); } /** * @notice See {IHorizonStakingBase-getThawedTokens}. */ function getThawedTokens( + ThawRequestType requestType, address serviceProvider, address verifier, address owner ) external view override returns (uint256) { - LinkedList.List storage thawRequestList = _thawRequestLists[serviceProvider][verifier][owner]; + LinkedList.List storage thawRequestList = _getThawRequestList(requestType, serviceProvider, verifier, owner); if (thawRequestList.count == 0) { return 0; } @@ -206,7 +211,7 @@ abstract contract HorizonStakingBase is bytes32 thawRequestId = thawRequestList.head; while (thawRequestId != bytes32(0)) { - ThawRequest storage thawRequest = _thawRequests[thawRequestId]; + ThawRequest storage thawRequest = _getThawRequest(requestType, thawRequestId); if (thawRequest.thawingUntil <= block.timestamp) { tokens += (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; } else { @@ -289,11 +294,84 @@ abstract contract HorizonStakingBase is } /** - * @notice Gets the next thaw request after `_thawRequestId`. - * @dev This function is used as a callback in the thaw requests linked list traversal. + * @notice Determines the correct callback function for `getNextItem` based on the request type. + * @param _requestType The type of thaw request (Provision or Delegation). + * @return A function pointer to the appropriate `getNextItem` callback. */ - function _getNextThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { - return _thawRequests[_thawRequestId].next; + function _getNextThawRequest( + ThawRequestType _requestType + ) internal pure returns (function(bytes32) view returns (bytes32)) { + if (_requestType == ThawRequestType.Provision) { + return _getNextProvisionThawRequest; + } else if (_requestType == ThawRequestType.Delegation) { + return _getNextDelegationThawRequest; + } else { + revert("Unknown ThawRequestType"); + } + } + + /** + * @notice Retrieves the next thaw request for a provision. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextProvisionThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _provisionThawRequests[_thawRequestId].next; + } + + /** + * @notice Retrieves the next thaw request for a delegation. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextDelegationThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _delegationThawRequests[_thawRequestId].next; + } + + /** + * @notice Retrieves the thaw request list for the given request type. + * @dev Uses the `ThawRequestType` to determine which mapping to access. + * Reverts if the request type is unknown. + * @param _requestType The type of thaw request (Provision or Delegation). + * @param _serviceProvider The address of the service provider. + * @param _verifier The address of the verifier. + * @param _owner The address of the owner of the thaw request. + * @return The linked list of thaw requests for the specified request type. + */ + function _getThawRequestList( + ThawRequestType _requestType, + address _serviceProvider, + address _verifier, + address _owner + ) internal view returns (LinkedList.List storage) { + if (_requestType == ThawRequestType.Provision) { + return _provisionThawRequestLists[_serviceProvider][_verifier][_owner]; + } else if (_requestType == ThawRequestType.Delegation) { + return _delegationThawRequestLists[_serviceProvider][_verifier][_owner]; + } else { + revert("Unknown ThawRequestType"); + } + } + + /** + * @notice Retrieves a specific thaw request for the given request type. + * @dev Uses the `ThawRequestType` to determine which mapping to access. + * Reverts if the request type is unknown. + * @param _requestType The type of thaw request (Provision or Delegation). + * @param _thawRequestId The unique ID of the thaw request. + * @return The thaw request data for the specified request type and ID. + */ + function _getThawRequest( + ThawRequestType _requestType, + bytes32 _thawRequestId + ) internal view returns (IHorizonStakingTypes.ThawRequest storage) { + if (_requestType == ThawRequestType.Provision) { + return _provisionThawRequests[_thawRequestId]; + } else if (_requestType == ThawRequestType.Delegation) { + return _delegationThawRequests[_thawRequestId]; + } else { + revert("Unknown ThawRequestType"); + } } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingStorage.sol b/packages/horizon/contracts/staking/HorizonStakingStorage.sol index ce2755468..9ed0cef26 100644 --- a/packages/horizon/contracts/staking/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/staking/HorizonStakingStorage.sol @@ -149,12 +149,21 @@ abstract contract HorizonStakingV1Storage { /// @dev Thaw requests /// Details for each thawing operation in the staking contract (for both service providers and delegators). - mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _thawRequests; + mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _provisionThawRequests; /// @dev Thaw request lists /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) - internal _thawRequestLists; + internal _provisionThawRequestLists; + + /// @dev Thaw requests + /// Details for each thawing operation in the staking contract (for both service providers and delegators). + mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _delegationThawRequests; + + /// @dev Thaw request lists + /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) + mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) + internal _delegationThawRequestLists; /// @dev Operator allow list /// Used for all verifiers except the subgraph data service. diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 1fe4ceba3..39e2c5c34 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -8,6 +8,7 @@ import { IGraphPayments } from "../../../contracts/interfaces/IGraphPayments.sol import { IHorizonStakingBase } from "../../../contracts/interfaces/internal/IHorizonStakingBase.sol"; import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { MathUtils } from "../../../contracts/libraries/MathUtils.sol"; @@ -400,6 +401,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // before Provision memory beforeProvision = staking.getProvision(serviceProvider, verifier); LinkedList.List memory beforeThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -428,13 +430,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // after Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); - ThawRequest memory afterThawRequest = staking.getThawRequest(thawRequestId); - LinkedList.List memory afterThawRequestList = staking.getThawRequestList( - serviceProvider, - verifier, - serviceProvider - ); - ThawRequest memory afterPreviousTailThawRequest = staking.getThawRequest(beforeThawRequestList.tail); + ThawRequest memory afterThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, thawRequestId); + LinkedList.List memory afterThawRequestList = _getProvisionThawRequestList(serviceProvider, verifier, serviceProvider); + ThawRequest memory afterPreviousTailThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, beforeThawRequestList.tail); // assert assertEq(afterProvision.tokens, beforeProvision.tokens); @@ -472,6 +470,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory beforeProvision = staking.getProvision(serviceProvider, verifier); ServiceProviderInternal memory beforeServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory beforeThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -513,6 +512,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); ServiceProviderInternal memory afterServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory afterThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -540,7 +540,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beforeServiceProvider.__DEPRECATED_tokensLockedUntil ); for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -583,7 +583,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { provision: staking.getProvision(serviceProvider, verifier), provisionNewVerifier: staking.getProvision(serviceProvider, newVerifier), serviceProvider: _getStorage_ServiceProviderInternal(serviceProvider), - thawRequestList: staking.getThawRequestList(serviceProvider, verifier, serviceProvider) + thawRequestList: staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider) }); // calc @@ -626,6 +626,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { Provision memory afterProvisionNewVerifier = staking.getProvision(serviceProvider, newVerifier); ServiceProviderInternal memory afterServiceProvider = _getStorage_ServiceProviderInternal(serviceProvider); LinkedList.List memory afterThawRequestList = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider @@ -680,7 +681,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // assert: thaw request list old verifier for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -882,8 +883,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 deltaShares = afterDelegation.shares - beforeDelegation.shares; // assertions - assertEq(beforePool.tokens + tokens, afterPool.tokens); - assertEq(beforePool.shares + calcShares, afterPool.shares); + assertEq(beforePool.tokens + tokens, afterPool.tokens, "afterPool.tokens FAIL"); + assertEq(beforePool.shares + calcShares, afterPool.shares, "afterPool.shares FAIL"); assertEq(beforePool.tokensThawing, afterPool.tokensThawing); assertEq(beforePool.sharesThawing, afterPool.sharesThawing); assertEq(beforePool.thawingNonce, afterPool.thawingNonce); @@ -930,7 +931,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { BeforeValues_Undelegate memory beforeValues; beforeValues.pool = _getStorage_DelegationPoolInternal(serviceProvider, verifier, legacy); beforeValues.delegation = _getStorage_Delegation(serviceProvider, verifier, delegator, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(serviceProvider, verifier, delegator); + beforeValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, delegator); beforeValues.delegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // calc @@ -978,8 +979,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beneficiary, legacy ); - LinkedList.List memory afterThawRequestList = staking.getThawRequestList(serviceProvider, verifier, beneficiary); - ThawRequest memory afterThawRequest = staking.getThawRequest(calcValues.thawRequestId); + LinkedList.List memory afterThawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, beneficiary); + ThawRequest memory afterThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, calcValues.thawRequestId); uint256 afterDelegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // assertions @@ -1077,7 +1078,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beforeValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); beforeValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); beforeValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); + beforeValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, _serviceProvider, _verifier, msgSender); beforeValues.senderBalance = token.balanceOf(msgSender); beforeValues.stakingBalance = token.balanceOf(address(staking)); @@ -1150,7 +1151,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { afterValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); afterValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); afterValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - afterValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); + afterValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, _serviceProvider, _verifier, msgSender); afterValues.senderBalance = token.balanceOf(msgSender); afterValues.stakingBalance = token.balanceOf(address(staking)); @@ -1162,7 +1163,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { assertEq(afterValues.pool.thawingNonce, beforeValues.pool.thawingNonce); for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -1797,7 +1798,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address operator, bool legacy ) internal view returns (bool) { - uint256 slotNumber = legacy ? 21 : 31; + uint256 slotNumber = legacy ? 21 : 33; uint256 slot; if (legacy) { @@ -1879,7 +1880,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address verifier, bool legacy ) internal view returns (DelegationPoolInternalTest memory) { - uint256 slotNumber = legacy ? 20 : 33; + uint256 slotNumber = legacy ? 20 : 35; uint256 baseSlot; if (legacy) { baseSlot = uint256(keccak256(abi.encode(serviceProvider, slotNumber))); @@ -1911,7 +1912,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address delegator, bool legacy ) internal view returns (DelegationInternal memory) { - uint256 slotNumber = legacy ? 20 : 33; + uint256 slotNumber = legacy ? 20 : 35; uint256 baseSlot; // DelegationPool @@ -2207,6 +2208,13 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256[] thawRequestsFulfilledListTokens; } + struct ThawingData { + uint256 tokensThawed; + uint256 tokensThawing; + uint256 sharesThawing; + uint256 thawRequestsFulfilled; + } + function calcThawRequestData( address serviceProvider, address verifier, @@ -2214,7 +2222,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 iterations, bool delegation ) private view returns (CalcValues_ThawRequestData memory) { - LinkedList.List memory thawRequestList = staking.getThawRequestList(serviceProvider, verifier, owner); + LinkedList.List memory thawRequestList = delegation + ? _getDelegationThawRequestList(serviceProvider, verifier, owner) + : _getProvisionThawRequestList(serviceProvider, verifier, owner); if (thawRequestList.count == 0) { return CalcValues_ThawRequestData(0, 0, 0, new ThawRequest[](0), new bytes32[](0), new uint256[](0)); } @@ -2229,7 +2239,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { bytes32 thawRequestId = thawRequestList.head; while (thawRequestId != bytes32(0) && (iterations == 0 || thawRequestsFulfilled < iterations)) { - ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId); + ThawRequest memory thawRequest = delegation + ? _getDelegationThawRequest(thawRequestId) + : _getProvisionThawRequest(thawRequestId); bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { thawRequestsFulfilled++; @@ -2254,7 +2266,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 i = 0; thawRequestId = thawRequestList.head; while (thawRequestId != bytes32(0) && (iterations == 0 || i < iterations)) { - ThawRequest memory thawRequest = staking.getThawRequest(thawRequestId); + ThawRequest memory thawRequest = delegation + ? _getDelegationThawRequest(thawRequestId) + : _getProvisionThawRequest(thawRequestId); bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { @@ -2264,7 +2278,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { : (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; } thawRequestsFulfilledListIds[i] = thawRequestId; - thawRequestsFulfilledList[i] = staking.getThawRequest(thawRequestId); + thawRequestsFulfilledList[i] = delegation + ? _getDelegationThawRequest(thawRequestId) + : _getProvisionThawRequest(thawRequestId); thawRequestId = thawRequestsFulfilledList[i].next; i++; } else { @@ -2287,4 +2303,52 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { thawRequestsFulfilledListTokens ); } + + function _getProvisionThawRequestList(address serviceProvider, address verifier, address owner) + private + view + returns (LinkedList.List memory) + { + return staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Provision, + serviceProvider, + verifier, + owner + ); + } + + function _getDelegationThawRequestList(address serviceProvider, address verifier, address owner) + private + view + returns (LinkedList.List memory) + { + return staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider, + verifier, + owner + ); + } + + function _getProvisionThawRequest(bytes32 thawRequestId) + private + view + returns (ThawRequest memory) + { + return staking.getThawRequest( + IHorizonStakingTypes.ThawRequestType.Provision, + thawRequestId + ); + } + + function _getDelegationThawRequest(bytes32 thawRequestId) + private + view + returns (ThawRequest memory) + { + return staking.getThawRequest( + IHorizonStakingTypes.ThawRequestType.Delegation, + thawRequestId + ); + } } diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index c9aa04dbc..45bdbb1a8 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.27; import "forge-std/Test.sol"; import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { HorizonStakingTest } from "../HorizonStaking.t.sol"; @@ -42,11 +43,12 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { useUndelegate(withdrawShares) { LinkedList.List memory thawingRequests = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, users.delegator ); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); @@ -93,11 +95,12 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { _undelegate(users.indexer, delegation.shares); LinkedList.List memory thawingRequests = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceLegacyAddress, users.delegator ); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); @@ -180,6 +183,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { useDelegation(delegationAmount) { vm.assume(beneficiary != address(0)); + vm.assume(beneficiary != address(staking)); // Skip beneficiary if balance will overflow vm.assume(token.balanceOf(beneficiary) < type(uint256).max - delegationAmount); @@ -189,8 +193,8 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends - LinkedList.List memory thawingRequests = staking.getThawRequestList(users.indexer, subgraphDataServiceAddress, beneficiary); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, beneficiary); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); // Beneficiary withdraws delegated tokens @@ -216,8 +220,8 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends - LinkedList.List memory thawingRequests = staking.getThawRequestList(users.indexer, subgraphDataServiceAddress, users.delegator); - ThawRequest memory thawRequest = staking.getThawRequest(thawingRequests.tail); + LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, users.delegator); + ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, thawingRequests.tail); skip(thawRequest.thawingUntil + 1); // Delegator attempts to withdraw delegated tokens, should revert since beneficiary is the thaw request owner From d02f410eb0c3967e9b6f80e36a1a5bde43492941 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 29 Nov 2024 18:17:15 -0300 Subject: [PATCH 02/23] fix: separate delegation and delegationWithBeneficiary thaw requests (TRST-H03) --- .../internal/IHorizonStakingBase.sol | 5 + .../internal/IHorizonStakingMain.sol | 24 +- .../internal/IHorizonStakingTypes.sol | 3 +- .../contracts/staking/HorizonStaking.sol | 57 ++- .../contracts/staking/HorizonStakingBase.sol | 19 +- .../staking/HorizonStakingStorage.sol | 18 +- .../HorizonStakingShared.t.sol | 382 ++++++++++-------- .../test/staking/delegation/undelegate.t.sol | 4 +- .../test/staking/delegation/withdraw.t.sol | 6 +- 9 files changed, 327 insertions(+), 191 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol index 32e476298..0145dac16 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingBase.sol @@ -24,6 +24,11 @@ interface IHorizonStakingBase { */ event StakeDeposited(address indexed serviceProvider, uint256 tokens); + /** + * @notice Thrown when using an invalid thaw request type. + */ + error HorizonStakingInvalidThawRequestType(); + /** * @notice Gets the details of a service provider. * @param serviceProvider The address of the service provider. diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index b144b0ce6..9122153c5 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -747,7 +747,7 @@ interface IHorizonStakingMain { * @param beneficiary The address where the tokens will be withdrawn after thawing * @return The ID of the thaw request */ - function undelegate( + function undelegateWithBeneficiary( address serviceProvider, address verifier, uint256 shares, @@ -772,6 +772,28 @@ interface IHorizonStakingMain { */ function withdrawDelegated(address serviceProvider, address verifier, uint256 nThawRequests) external; + /** + * @notice Withdraw undelegated with beneficiary tokens from a provision after thawing. + * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw + * requests in the event that fulfilling all of them results in a gas limit error. + * @dev If the delegation pool was completely slashed before withdrawing, calling this function will fulfill + * the thaw requests with an amount equal to zero. + * + * Requirements: + * - Must have previously initiated a thaw request using {undelegateWithBeneficiary}. + * + * Emits {ThawRequestFulfilled}, {ThawRequestsFulfilled} and {DelegatedTokensWithdrawn} events. + * + * @param serviceProvider The service provider address + * @param verifier The verifier address + * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. + */ + function withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) external; + /** * @notice Re-delegate undelegated tokens from a provision after thawing to a `newServiceProvider` and `newVerifier`. * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol index 8aff1a6e0..9da150d0f 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol @@ -138,7 +138,8 @@ interface IHorizonStakingTypes { */ enum ThawRequestType { Provision, - Delegation + Delegation, + DelegationWithBeneficiary } /** diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 4ad8136c4..0e9e4bba2 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -297,20 +297,20 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address verifier, uint256 shares ) external override notPaused returns (bytes32) { - return _undelegate(serviceProvider, verifier, shares, msg.sender); + return _undelegate(ThawRequestType.Delegation, serviceProvider, verifier, shares, msg.sender); } /** * @notice See {IHorizonStakingMain-undelegate}. */ - function undelegate( + function undelegateWithBeneficiary( address serviceProvider, address verifier, uint256 shares, address beneficiary ) external override notPaused returns (bytes32) { require(beneficiary != address(0), HorizonStakingInvalidBeneficiaryZeroAddress()); - return _undelegate(serviceProvider, verifier, shares, beneficiary); + return _undelegate(ThawRequestType.DelegationWithBeneficiary, serviceProvider, verifier, shares, beneficiary); } /** @@ -321,7 +321,34 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address verifier, uint256 nThawRequests ) external override notPaused { - _withdrawDelegated(serviceProvider, verifier, address(0), address(0), 0, nThawRequests); + _withdrawDelegated( + ThawRequestType.Delegation, + serviceProvider, + verifier, + address(0), + address(0), + 0, + nThawRequests + ); + } + + /** + * @notice See {IHorizonStakingMain-withdrawDelegatedWithBeneficiary}. + */ + function withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) external override notPaused { + _withdrawDelegated( + ThawRequestType.DelegationWithBeneficiary, + serviceProvider, + verifier, + address(0), + address(0), + 0, + nThawRequests + ); } /** @@ -338,6 +365,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { require(newServiceProvider != address(0), HorizonStakingInvalidServiceProviderZeroAddress()); require(newVerifier != address(0), HorizonStakingInvalidVerifierZeroAddress()); _withdrawDelegated( + ThawRequestType.Delegation, oldServiceProvider, oldVerifier, newServiceProvider, @@ -374,7 +402,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @notice See {IHorizonStakingMain-undelegate}. */ function undelegate(address serviceProvider, uint256 shares) external override notPaused { - _undelegate(serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, shares, msg.sender); + _undelegate(ThawRequestType.Delegation, serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, shares, msg.sender); } /** @@ -382,6 +410,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { */ function withdrawDelegated(address serviceProvider, address newServiceProvider) external override notPaused { _withdrawDelegated( + ThawRequestType.Delegation, serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, newServiceProvider, @@ -837,6 +866,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * that were not thawing will be preserved. */ function _undelegate( + ThawRequestType _requestType, address _serviceProvider, address _verifier, uint256 _shares, @@ -864,7 +894,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { delegation.shares = delegation.shares - _shares; bytes32 thawRequestId = _createThawRequest( - ThawRequestType.Delegation, + _requestType, _serviceProvider, _verifier, _beneficiary, @@ -881,6 +911,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @notice See {IHorizonStakingMain-withdrawDelegated}. */ function _withdrawDelegated( + ThawRequestType _requestType, address _serviceProvider, address _verifier, address _newServiceProvider, @@ -901,7 +932,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensThawing = pool.tokensThawing; FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ - requestType: ThawRequestType.Delegation, + requestType: _requestType, serviceProvider: _serviceProvider, verifier: _verifier, owner: msg.sender, @@ -1082,8 +1113,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { return _deleteProvisionThawRequest; } else if (_requestType == ThawRequestType.Delegation) { return _deleteDelegationThawRequest; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _deleteDelegationWithBeneficiaryThawRequest; } else { - revert("Unknown ThawRequestType"); + revert HorizonStakingInvalidThawRequestType(); } } @@ -1103,6 +1136,14 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { delete _delegationThawRequests[_thawRequestId]; } + /** + * @notice Deletes a thaw request for a delegation with a beneficiary. + * @param _thawRequestId The ID of the thaw request to delete. + */ + function _deleteDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) private { + delete _delegationWithBeneficiaryThawRequests[_thawRequestId]; + } + /** * @notice See {IHorizonStakingMain-setOperator}. * @dev Note that this function handles the special case where the verifier is the subgraph data service, diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index cdc45bb20..01210d157 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -305,8 +305,10 @@ abstract contract HorizonStakingBase is return _getNextProvisionThawRequest; } else if (_requestType == ThawRequestType.Delegation) { return _getNextDelegationThawRequest; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _getNextDelegationWithBeneficiaryThawRequest; } else { - revert("Unknown ThawRequestType"); + revert HorizonStakingInvalidThawRequestType(); } } @@ -328,6 +330,15 @@ abstract contract HorizonStakingBase is return _delegationThawRequests[_thawRequestId].next; } + /** + * @notice Retrieves the next thaw request for a delegation with a beneficiary. + * @param _thawRequestId The ID of the current thaw request. + * @return The ID of the next thaw request in the list. + */ + function _getNextDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { + return _delegationWithBeneficiaryThawRequests[_thawRequestId].next; + } + /** * @notice Retrieves the thaw request list for the given request type. * @dev Uses the `ThawRequestType` to determine which mapping to access. @@ -348,8 +359,10 @@ abstract contract HorizonStakingBase is return _provisionThawRequestLists[_serviceProvider][_verifier][_owner]; } else if (_requestType == ThawRequestType.Delegation) { return _delegationThawRequestLists[_serviceProvider][_verifier][_owner]; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _delegationWithBeneficiaryThawRequestLists[_serviceProvider][_verifier][_owner]; } else { - revert("Unknown ThawRequestType"); + revert HorizonStakingInvalidThawRequestType(); } } @@ -369,6 +382,8 @@ abstract contract HorizonStakingBase is return _provisionThawRequests[_thawRequestId]; } else if (_requestType == ThawRequestType.Delegation) { return _delegationThawRequests[_thawRequestId]; + } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + return _delegationWithBeneficiaryThawRequests[_thawRequestId]; } else { revert("Unknown ThawRequestType"); } diff --git a/packages/horizon/contracts/staking/HorizonStakingStorage.sol b/packages/horizon/contracts/staking/HorizonStakingStorage.sol index 9ed0cef26..ca6a1450f 100644 --- a/packages/horizon/contracts/staking/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/staking/HorizonStakingStorage.sol @@ -148,23 +148,33 @@ abstract contract HorizonStakingV1Storage { internal _delegationFeeCut; /// @dev Thaw requests - /// Details for each thawing operation in the staking contract (for both service providers and delegators). + /// Details for each thawing operation in the staking contract for both service providers. mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _provisionThawRequests; /// @dev Thaw request lists - /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) + /// Metadata defining linked lists of thaw requests for each service provider (owner). mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) internal _provisionThawRequestLists; /// @dev Thaw requests - /// Details for each thawing operation in the staking contract (for both service providers and delegators). + /// Details for each thawing operation in the staking contract for delegators. mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _delegationThawRequests; /// @dev Thaw request lists - /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) + /// Metadata defining linked lists of thaw requests for each delegator (owner). mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) internal _delegationThawRequestLists; + /// @dev Thaw requests + /// Details for each thawing operation in the staking contract for both delegators undelegating to a beneficiary. + mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) + internal _delegationWithBeneficiaryThawRequests; + + /// @dev Thaw request lists + /// Metadata defining linked lists of thaw requests for each delegator (owner) undelegating to a beneficiary. + mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) + internal _delegationWithBeneficiaryThawRequestLists; + /// @dev Operator allow list /// Used for all verifiers except the subgraph data service. mapping(address serviceProvider => mapping(address verifier => mapping(address operator => bool authorized))) diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 39e2c5c34..ac86c83f5 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -431,7 +431,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // after Provision memory afterProvision = staking.getProvision(serviceProvider, verifier); ThawRequest memory afterThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, thawRequestId); - LinkedList.List memory afterThawRequestList = _getProvisionThawRequestList(serviceProvider, verifier, serviceProvider); + LinkedList.List memory afterThawRequestList = _getThawRequestList(IHorizonStakingTypes.ThawRequestType.Provision, serviceProvider, verifier, serviceProvider); ThawRequest memory afterPreviousTailThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Provision, beforeThawRequestList.tail); // assert @@ -476,13 +476,15 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { serviceProvider ); - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - serviceProvider, - verifier, - serviceProvider, - nThawRequests, - false - ); + Params_CalcThawRequestData memory params = Params_CalcThawRequestData({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Provision, + serviceProvider: serviceProvider, + verifier: verifier, + owner: serviceProvider, + iterations: nThawRequests, + delegation: false + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(params); // deprovision for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -587,13 +589,15 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { }); // calc - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - serviceProvider, - verifier, - serviceProvider, - nThawRequests, - false - ); + Params_CalcThawRequestData memory params = Params_CalcThawRequestData({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Provision, + serviceProvider: serviceProvider, + verifier: verifier, + owner: serviceProvider, + iterations: nThawRequests, + delegation: false + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(params); // reprovision for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -899,16 +903,30 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { function _undelegate(address serviceProvider, address verifier, uint256 shares) internal { (, address caller, ) = vm.readCallers(); - __undelegate(serviceProvider, verifier, shares, false, caller); + __undelegate(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, shares, false, caller); } - function _undelegate(address serviceProvider, address verifier, uint256 shares, address beneficiary) internal { - __undelegate(serviceProvider, verifier, shares, false, beneficiary); + function _undelegateWithBeneficiary(address serviceProvider, address verifier, uint256 shares, address beneficiary) internal { + __undelegate( + IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary, + serviceProvider, + verifier, + shares, + false, + beneficiary + ); } function _undelegate(address serviceProvider, uint256 shares) internal { (, address caller, ) = vm.readCallers(); - __undelegate(serviceProvider, subgraphDataServiceLegacyAddress, shares, true, caller); + __undelegate( + IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider, + subgraphDataServiceLegacyAddress, + shares, + true, + caller + ); } struct BeforeValues_Undelegate { @@ -924,14 +942,21 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { bytes32 thawRequestId; } - function __undelegate(address serviceProvider, address verifier, uint256 shares, bool legacy, address beneficiary) private { + function __undelegate( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + uint256 shares, + bool legacy, + address beneficiary + ) private { (, address delegator, ) = vm.readCallers(); // before BeforeValues_Undelegate memory beforeValues; beforeValues.pool = _getStorage_DelegationPoolInternal(serviceProvider, verifier, legacy); beforeValues.delegation = _getStorage_Delegation(serviceProvider, verifier, delegator, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, delegator); + beforeValues.thawRequestList = staking.getThawRequestList(thawRequestType, serviceProvider, verifier, delegator); beforeValues.delegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // calc @@ -963,8 +988,12 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { emit IHorizonStakingMain.TokensUndelegated(serviceProvider, verifier, delegator, calcValues.tokens); if (legacy) { staking.undelegate(serviceProvider, shares); + } else if (thawRequestType == IHorizonStakingTypes.ThawRequestType.Delegation) { + staking.undelegate(serviceProvider, verifier, shares); + } else if (thawRequestType == IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary) { + staking.undelegateWithBeneficiary(serviceProvider, verifier, shares, beneficiary); } else { - staking.undelegate(serviceProvider, verifier, shares, beneficiary); + revert("Invalid thaw request type"); } // after @@ -979,8 +1008,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beneficiary, legacy ); - LinkedList.List memory afterThawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, serviceProvider, verifier, beneficiary); - ThawRequest memory afterThawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, calcValues.thawRequestId); + LinkedList.List memory afterThawRequestList = staking.getThawRequestList(thawRequestType, serviceProvider, verifier, beneficiary); + ThawRequest memory afterThawRequest = staking.getThawRequest(thawRequestType, calcValues.thawRequestId); uint256 afterDelegatedTokens = staking.getDelegatedTokensAvailable(serviceProvider, verifier); // assertions @@ -1009,15 +1038,35 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address verifier, uint256 nThawRequests ) internal { - __withdrawDelegated( - serviceProvider, - verifier, - address(0), - address(0), - 0, - nThawRequests, - false - ); + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: address(0), + newVerifier: address(0), + minSharesForNewProvider: 0, + nThawRequests: nThawRequests, + legacy: false + }); + __withdrawDelegated(params); + } + + function _withdrawDelegatedWithBeneficiary( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) internal { + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: address(0), + newVerifier: address(0), + minSharesForNewProvider: 0, + nThawRequests: nThawRequests, + legacy: false + }); + __withdrawDelegated(params); } function _redelegate( @@ -1028,19 +1077,31 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 minSharesForNewProvider, uint256 nThawRequests ) internal { - __withdrawDelegated( - serviceProvider, - verifier, - newServiceProvider, - newVerifier, - minSharesForNewProvider, - nThawRequests, - false - ); + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider: serviceProvider, + verifier: verifier, + newServiceProvider: newServiceProvider, + newVerifier: newVerifier, + minSharesForNewProvider: minSharesForNewProvider, + nThawRequests: nThawRequests, + legacy: false + }); + __withdrawDelegated(params); } function _withdrawDelegated(address serviceProvider, address newServiceProvider) internal { - __withdrawDelegated(serviceProvider, subgraphDataServiceLegacyAddress, newServiceProvider, subgraphDataServiceLegacyAddress, 0, 0, true); + Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ + thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, + serviceProvider: serviceProvider, + verifier: subgraphDataServiceLegacyAddress, + newServiceProvider: newServiceProvider, + newVerifier: subgraphDataServiceLegacyAddress, + minSharesForNewProvider: 0, + nThawRequests: 0, + legacy: true + }); + __withdrawDelegated(params); } struct BeforeValues_WithdrawDelegated { @@ -1060,35 +1121,40 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 stakingBalance; } - function __withdrawDelegated( - address _serviceProvider, - address _verifier, - address _newServiceProvider, - address _newVerifier, - uint256 _minSharesForNewProvider, - uint256 _nThawRequests, - bool legacy - ) private { + struct Params_WithdrawDelegated { + IHorizonStakingTypes.ThawRequestType thawRequestType; + address serviceProvider; + address verifier; + address newServiceProvider; + address newVerifier; + uint256 minSharesForNewProvider; + uint256 nThawRequests; + bool legacy; + } + + function __withdrawDelegated(Params_WithdrawDelegated memory params) private { (, address msgSender, ) = vm.readCallers(); - bool reDelegate = _newServiceProvider != address(0) && _newVerifier != address(0); + bool reDelegate = params.newServiceProvider != address(0) && params.newVerifier != address(0); // before BeforeValues_WithdrawDelegated memory beforeValues; - beforeValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - beforeValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); - beforeValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - beforeValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, _serviceProvider, _verifier, msgSender); + beforeValues.pool = _getStorage_DelegationPoolInternal(params.serviceProvider, params.verifier, params.legacy); + beforeValues.newPool = _getStorage_DelegationPoolInternal(params.newServiceProvider, params.newVerifier, params.legacy); + beforeValues.newDelegation = _getStorage_Delegation(params.newServiceProvider, params.newVerifier, msgSender, params.legacy); + beforeValues.thawRequestList = staking.getThawRequestList(params.thawRequestType, params.serviceProvider, params.verifier, msgSender); beforeValues.senderBalance = token.balanceOf(msgSender); beforeValues.stakingBalance = token.balanceOf(address(staking)); - CalcValues_ThawRequestData memory calcValues = calcThawRequestData( - _serviceProvider, - _verifier, - msgSender, - _nThawRequests, - true - ); + Params_CalcThawRequestData memory paramsCalc = Params_CalcThawRequestData({ + thawRequestType: params.thawRequestType, + serviceProvider: params.serviceProvider, + verifier: params.verifier, + owner: msgSender, + iterations: params.nThawRequests, + delegation: true + }); + CalcValues_ThawRequestData memory calcValues = calcThawRequestData(paramsCalc); // withdrawDelegated for (uint i = 0; i < calcValues.thawRequestsFulfilledList.length; i++) { @@ -1104,8 +1170,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } vm.expectEmit(address(staking)); emit IHorizonStakingMain.ThawRequestsFulfilled( - _serviceProvider, - _verifier, + params.serviceProvider, + params.verifier, msgSender, calcValues.thawRequestsFulfilledList.length, calcValues.tokensThawed @@ -1114,8 +1180,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { vm.expectEmit(); if (reDelegate) { emit IHorizonStakingMain.TokensDelegated( - _newServiceProvider, - _newVerifier, + params.newServiceProvider, + params.newVerifier, msgSender, calcValues.tokensThawed ); @@ -1126,32 +1192,36 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { vm.expectEmit(); emit IHorizonStakingMain.DelegatedTokensWithdrawn( - _serviceProvider, - _verifier, + params.serviceProvider, + params.verifier, msgSender, calcValues.tokensThawed ); - if (legacy) { - staking.withdrawDelegated(_serviceProvider, _newServiceProvider); + if (params.legacy) { + staking.withdrawDelegated(params.serviceProvider, params.newServiceProvider); } else if (reDelegate) { staking.redelegate( - _serviceProvider, - _verifier, - _newServiceProvider, - _newVerifier, - _minSharesForNewProvider, - _nThawRequests + params.serviceProvider, + params.verifier, + params.newServiceProvider, + params.newVerifier, + params.minSharesForNewProvider, + params.nThawRequests ); + } else if (params.thawRequestType == IHorizonStakingTypes.ThawRequestType.Delegation) { + staking.withdrawDelegated(params.serviceProvider, params.verifier, params.nThawRequests); + } else if (params.thawRequestType == IHorizonStakingTypes.ThawRequestType.DelegationWithBeneficiary) { + staking.withdrawDelegatedWithBeneficiary(params.serviceProvider, params.verifier, params.nThawRequests); } else { - staking.withdrawDelegated(_serviceProvider, _verifier, _nThawRequests); + revert("Invalid thaw request type"); } // after AfterValues_WithdrawDelegated memory afterValues; - afterValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - afterValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); - afterValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); - afterValues.thawRequestList = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, _serviceProvider, _verifier, msgSender); + afterValues.pool = _getStorage_DelegationPoolInternal(params.serviceProvider, params.verifier, params.legacy); + afterValues.newPool = _getStorage_DelegationPoolInternal(params.newServiceProvider, params.newVerifier, params.legacy); + afterValues.newDelegation = _getStorage_Delegation(params.newServiceProvider, params.newVerifier, msgSender, params.legacy); + afterValues.thawRequestList = staking.getThawRequestList(params.thawRequestType, params.serviceProvider, params.verifier, msgSender); afterValues.senderBalance = token.balanceOf(msgSender); afterValues.stakingBalance = token.balanceOf(address(staking)); @@ -1163,7 +1233,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { assertEq(afterValues.pool.thawingNonce, beforeValues.pool.thawingNonce); for (uint i = 0; i < calcValues.thawRequestsFulfilledListIds.length; i++) { - ThawRequest memory thawRequest = staking.getThawRequest(IHorizonStakingTypes.ThawRequestType.Delegation, calcValues.thawRequestsFulfilledListIds[i]); + ThawRequest memory thawRequest = staking.getThawRequest(params.thawRequestType, calcValues.thawRequestsFulfilledListIds[i]); assertEq(thawRequest.shares, 0); assertEq(thawRequest.thawingUntil, 0); assertEq(thawRequest.next, bytes32(0)); @@ -1211,7 +1281,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { afterValues.newDelegation.__DEPRECATED_tokensLockedUntil, beforeValues.newDelegation.__DEPRECATED_tokensLockedUntil ); - assertGe(deltaShares, _minSharesForNewProvider); + assertGe(deltaShares, params.minSharesForNewProvider); assertEq(calcShares, deltaShares); assertEq(afterValues.senderBalance - beforeValues.senderBalance, 0); assertEq(beforeValues.stakingBalance - afterValues.stakingBalance, 0); @@ -1798,7 +1868,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address operator, bool legacy ) internal view returns (bool) { - uint256 slotNumber = legacy ? 21 : 33; + uint256 slotNumber = legacy ? 21 : 35; uint256 slot; if (legacy) { @@ -1880,7 +1950,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address verifier, bool legacy ) internal view returns (DelegationPoolInternalTest memory) { - uint256 slotNumber = legacy ? 20 : 35; + uint256 slotNumber = legacy ? 20 : 37; uint256 baseSlot; if (legacy) { baseSlot = uint256(keccak256(abi.encode(serviceProvider, slotNumber))); @@ -1912,7 +1982,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address delegator, bool legacy ) internal view returns (DelegationInternal memory) { - uint256 slotNumber = legacy ? 20 : 35; + uint256 slotNumber = legacy ? 20 : 37; uint256 baseSlot; // DelegationPool @@ -2215,38 +2285,42 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 thawRequestsFulfilled; } - function calcThawRequestData( - address serviceProvider, - address verifier, - address owner, - uint256 iterations, - bool delegation - ) private view returns (CalcValues_ThawRequestData memory) { - LinkedList.List memory thawRequestList = delegation - ? _getDelegationThawRequestList(serviceProvider, verifier, owner) - : _getProvisionThawRequestList(serviceProvider, verifier, owner); + struct Params_CalcThawRequestData { + IHorizonStakingTypes.ThawRequestType thawRequestType; + address serviceProvider; + address verifier; + address owner; + uint256 iterations; + bool delegation; + } + + function calcThawRequestData(Params_CalcThawRequestData memory params) private view returns (CalcValues_ThawRequestData memory) { + LinkedList.List memory thawRequestList = _getThawRequestList( + params.thawRequestType, + params.serviceProvider, + params.verifier, + params.owner + ); if (thawRequestList.count == 0) { return CalcValues_ThawRequestData(0, 0, 0, new ThawRequest[](0), new bytes32[](0), new uint256[](0)); } - Provision memory prov = staking.getProvision(serviceProvider, verifier); - DelegationPool memory pool = staking.getDelegationPool(serviceProvider, verifier); + Provision memory prov = staking.getProvision(params.serviceProvider, params.verifier); + DelegationPool memory pool = staking.getDelegationPool(params.serviceProvider, params.verifier); uint256 tokensThawed = 0; - uint256 tokensThawing = delegation ? pool.tokensThawing : prov.tokensThawing; - uint256 sharesThawing = delegation ? pool.sharesThawing : prov.sharesThawing; + uint256 tokensThawing = params.delegation ? pool.tokensThawing : prov.tokensThawing; + uint256 sharesThawing = params.delegation ? pool.sharesThawing : prov.sharesThawing; uint256 thawRequestsFulfilled = 0; bytes32 thawRequestId = thawRequestList.head; - while (thawRequestId != bytes32(0) && (iterations == 0 || thawRequestsFulfilled < iterations)) { - ThawRequest memory thawRequest = delegation - ? _getDelegationThawRequest(thawRequestId) - : _getProvisionThawRequest(thawRequestId); - bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); + while (thawRequestId != bytes32(0) && (params.iterations == 0 || thawRequestsFulfilled < params.iterations)) { + ThawRequest memory thawRequest = _getThawRequest(params.thawRequestType, thawRequestId); + bool isThawRequestValid = thawRequest.thawingNonce == (params.delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { thawRequestsFulfilled++; if (isThawRequestValid) { - uint256 tokens = delegation + uint256 tokens = params.delegation ? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing : (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; tokensThawed += tokens; @@ -2260,28 +2334,28 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } // we need to do a second pass because solidity doesnt allow dynamic arrays on memory - ThawRequest[] memory thawRequestsFulfilledList = new ThawRequest[](thawRequestsFulfilled); - bytes32[] memory thawRequestsFulfilledListIds = new bytes32[](thawRequestsFulfilled); - uint256[] memory thawRequestsFulfilledListTokens = new uint256[](thawRequestsFulfilled); + CalcValues_ThawRequestData memory thawRequestData; + thawRequestData.tokensThawed = tokensThawed; + thawRequestData.tokensThawing = tokensThawing; + thawRequestData.sharesThawing = sharesThawing; + thawRequestData.thawRequestsFulfilledList = new ThawRequest[](thawRequestsFulfilled); + thawRequestData.thawRequestsFulfilledListIds = new bytes32[](thawRequestsFulfilled); + thawRequestData.thawRequestsFulfilledListTokens = new uint256[](thawRequestsFulfilled); uint256 i = 0; thawRequestId = thawRequestList.head; - while (thawRequestId != bytes32(0) && (iterations == 0 || i < iterations)) { - ThawRequest memory thawRequest = delegation - ? _getDelegationThawRequest(thawRequestId) - : _getProvisionThawRequest(thawRequestId); - bool isThawRequestValid = thawRequest.thawingNonce == (delegation ? pool.thawingNonce : prov.thawingNonce); + while (thawRequestId != bytes32(0) && (params.iterations == 0 || i < params.iterations)) { + ThawRequest memory thawRequest = _getThawRequest(params.thawRequestType, thawRequestId); + bool isThawRequestValid = thawRequest.thawingNonce == (params.delegation ? pool.thawingNonce : prov.thawingNonce); if (thawRequest.thawingUntil <= block.timestamp) { if (isThawRequestValid) { - thawRequestsFulfilledListTokens[i] = delegation + thawRequestData.thawRequestsFulfilledListTokens[i] = params.delegation ? (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing : (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; } - thawRequestsFulfilledListIds[i] = thawRequestId; - thawRequestsFulfilledList[i] = delegation - ? _getDelegationThawRequest(thawRequestId) - : _getProvisionThawRequest(thawRequestId); - thawRequestId = thawRequestsFulfilledList[i].next; + thawRequestData.thawRequestsFulfilledListIds[i] = thawRequestId; + thawRequestData.thawRequestsFulfilledList[i] = _getThawRequest(params.thawRequestType, thawRequestId); + thawRequestId = thawRequestData.thawRequestsFulfilledList[i].next; i++; } else { break; @@ -2289,65 +2363,33 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { thawRequestId = thawRequest.next; } - assertEq(thawRequestsFulfilled, thawRequestsFulfilledList.length); - assertEq(thawRequestsFulfilled, thawRequestsFulfilledListIds.length); - assertEq(thawRequestsFulfilled, thawRequestsFulfilledListTokens.length); - - return - CalcValues_ThawRequestData( - tokensThawed, - tokensThawing, - sharesThawing, - thawRequestsFulfilledList, - thawRequestsFulfilledListIds, - thawRequestsFulfilledListTokens - ); - } + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledList.length); + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledListIds.length); + assertEq(thawRequestsFulfilled, thawRequestData.thawRequestsFulfilledListTokens.length); - function _getProvisionThawRequestList(address serviceProvider, address verifier, address owner) - private - view - returns (LinkedList.List memory) - { - return staking.getThawRequestList( - IHorizonStakingTypes.ThawRequestType.Provision, - serviceProvider, - verifier, - owner - ); + return thawRequestData; } - function _getDelegationThawRequestList(address serviceProvider, address verifier, address owner) - private - view - returns (LinkedList.List memory) - { + function _getThawRequestList( + IHorizonStakingTypes.ThawRequestType thawRequestType, + address serviceProvider, + address verifier, + address owner + ) private view returns (LinkedList.List memory) { return staking.getThawRequestList( - IHorizonStakingTypes.ThawRequestType.Delegation, + thawRequestType, serviceProvider, verifier, owner ); } - function _getProvisionThawRequest(bytes32 thawRequestId) - private - view - returns (ThawRequest memory) - { - return staking.getThawRequest( - IHorizonStakingTypes.ThawRequestType.Provision, - thawRequestId - ); - } - - function _getDelegationThawRequest(bytes32 thawRequestId) - private - view - returns (ThawRequest memory) - { + function _getThawRequest( + IHorizonStakingTypes.ThawRequestType thawRequestType, + bytes32 thawRequestId + ) private view returns (ThawRequest memory) { return staking.getThawRequest( - IHorizonStakingTypes.ThawRequestType.Delegation, + thawRequestType, thawRequestId ); } diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index 4cad2e0c3..c824832c4 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/packages/horizon/test/staking/delegation/undelegate.t.sol @@ -57,7 +57,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { vm.assume(beneficiary != address(0)); resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); } function testUndelegate_RevertWhen_TooManyUndelegations() @@ -232,6 +232,6 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); bytes memory expectedError = abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingInvalidBeneficiaryZeroAddress.selector); vm.expectRevert(expectedError); - staking.undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, address(0)); + staking.undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, address(0)); } } diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index 45bdbb1a8..ff3fb3db0 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -190,7 +190,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { // Delegator undelegates to beneficiary resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, beneficiary); @@ -199,7 +199,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { // Beneficiary withdraws delegated tokens resetPrank(beneficiary); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, 1); + _withdrawDelegatedWithBeneficiary(users.indexer, subgraphDataServiceAddress, 1); } function testWithdrawDelegation_RevertWhen_PreviousOwnerAttemptsToWithdraw( @@ -217,7 +217,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { // Delegator undelegates to beneficiary resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); + _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); // Thawing period ends LinkedList.List memory thawingRequests = staking.getThawRequestList(IHorizonStakingTypes.ThawRequestType.Delegation, users.indexer, subgraphDataServiceAddress, users.delegator); From 167055ac64d5aedbac264da23189bd7dbede8fd0 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Mon, 2 Dec 2024 15:45:25 -0300 Subject: [PATCH 03/23] fix: round down tokens thawing when slashing (TRST-H04) --- .../contracts/staking/HorizonStaking.sol | 7 ++-- .../HorizonStakingShared.t.sol | 4 +- .../horizon/test/staking/slash/slash.t.sol | 42 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 0e9e4bba2..151bcaff4 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -461,8 +461,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _graphToken().burnTokens(providerTokensSlashed - tokensVerifier); // Provision accounting - // TODO check for rounding issues - uint256 provisionFractionSlashed = (providerTokensSlashed * FIXED_POINT_PRECISION) / prov.tokens; + uint256 provisionFractionSlashed = (providerTokensSlashed * FIXED_POINT_PRECISION + prov.tokens - 1) / + prov.tokens; prov.tokensThawing = (prov.tokensThawing * (FIXED_POINT_PRECISION - provisionFractionSlashed)) / (FIXED_POINT_PRECISION); @@ -497,7 +497,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { _graphToken().burnTokens(tokensToSlash); // Delegation pool accounting - uint256 delegationFractionSlashed = (tokensToSlash * FIXED_POINT_PRECISION) / pool.tokens; + uint256 delegationFractionSlashed = (tokensToSlash * FIXED_POINT_PRECISION + pool.tokens - 1) / + pool.tokens; pool.tokens = pool.tokens - tokensToSlash; pool.tokensThawing = (pool.tokensThawing * (FIXED_POINT_PRECISION - delegationFractionSlashed)) / diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index ac86c83f5..c960b19bd 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -1488,7 +1488,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 tokensSlashed = calcValues.providerTokensSlashed + (isDelegationSlashingEnabled ? calcValues.delegationTokensSlashed : 0); uint256 provisionThawingTokens = (before.provision.tokensThawing * - (1e18 - ((calcValues.providerTokensSlashed * 1e18) / before.provision.tokens))) / (1e18); + (1e18 - ((calcValues.providerTokensSlashed * 1e18 + before.provision.tokens - 1) / before.provision.tokens))) / (1e18); // assert assertEq(afterProvision.tokens + calcValues.providerTokensSlashed, before.provision.tokens); @@ -1506,7 +1506,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { (before.provision.sharesThawing != 0 && afterProvision.sharesThawing == 0) ? before.provision.thawingNonce + 1 : before.provision.thawingNonce); if (isDelegationSlashingEnabled) { uint256 poolThawingTokens = (before.pool.tokensThawing * - (1e18 - ((calcValues.delegationTokensSlashed * 1e18) / before.pool.tokens))) / (1e18); + (1e18 - ((calcValues.delegationTokensSlashed * 1e18 + before.pool.tokens - 1) / before.pool.tokens))) / (1e18); assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens); assertEq(afterPool.shares, before.pool.shares); assertEq(afterPool.tokensThawing, poolThawingTokens); diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index 7c7933419..a74403ed7 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/packages/horizon/test/staking/slash/slash.t.sol @@ -140,4 +140,46 @@ contract HorizonStakingSlashTest is HorizonStakingTest { vm.startPrank(subgraphDataServiceAddress); _slash(users.indexer, subgraphDataServiceAddress, tokens + delegationTokens, 0); } + + function testSlash_RoundDown_TokensThawing_Provision() public useIndexer { + uint256 tokens = 1 ether + 1; + _useProvision(subgraphDataServiceAddress, tokens, MAX_PPM, MAX_THAWING_PERIOD); + + _thaw(users.indexer, subgraphDataServiceAddress, tokens); + + resetPrank(subgraphDataServiceAddress); + _slash(users.indexer, subgraphDataServiceAddress, 1, 0); + + resetPrank(users.indexer); + Provision memory provision = staking.getProvision(users.indexer, subgraphDataServiceAddress); + assertEq(provision.tokens, tokens - 1); + // Tokens thawing should be rounded down + assertEq(provision.tokensThawing, tokens - 2); + } + + function testSlash_RoundDown_TokensThawing_Delegation( + uint256 tokens + ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing { + resetPrank(users.delegator); + uint256 delegationTokens = 1 ether + 1; + _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); + + resetPrank(subgraphDataServiceAddress); + // Slash 1 token from delegation + _slash(users.indexer, subgraphDataServiceAddress, tokens + 1, 0); + + resetPrank(users.delegator); + DelegationPool memory pool = staking.getDelegationPool(users.indexer, subgraphDataServiceAddress); + assertEq(pool.tokens, delegationTokens - 1); + // Tokens thawing should be rounded down + assertEq(pool.tokensThawing, delegationTokens - 2); + } } From abe3321a049ff524625178bf69b7f445183e7d0a Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Tue, 3 Dec 2024 15:03:33 -0300 Subject: [PATCH 04/23] fix: add legacy slasher for transition period (TRST-H06) --- .../internal/IHorizonStakingExtension.sol | 16 ++ .../contracts/staking/HorizonStaking.sol | 18 ++ .../staking/HorizonStakingExtension.sol | 57 ++++++ packages/horizon/test/GraphBase.t.sol | 3 +- .../test/staking/slash/legacySlash.t.sol | 183 ++++++++++++++++++ packages/horizon/test/utils/Users.sol | 1 + 6 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 packages/horizon/test/staking/slash/legacySlash.t.sol diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol index a0b2dc1af..84318f536 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol @@ -77,6 +77,12 @@ interface IHorizonStakingExtension is IRewardsIssuer { uint256 delegationRewards ); + /** + * @dev Emitted when `indexer` was slashed for a total of `tokens` amount. + * Tracks `reward` amount of tokens given to `beneficiary`. + */ + event StakeSlashed(address indexed indexer, uint256 tokens, uint256 reward, address beneficiary); + /** * @notice Close an allocation and free the staked tokens. * To be eligible for rewards a proof of indexing must be presented. @@ -148,4 +154,14 @@ interface IHorizonStakingExtension is IRewardsIssuer { */ // solhint-disable-next-line func-name-mixedcase function __DEPRECATED_getThawingPeriod() external view returns (uint64); + + /** + * @notice Slash the indexer stake. Delegated tokens are not subject to slashing. + * @dev Can only be called by the slasher role. + * @param indexer Address of indexer to slash + * @param tokens Amount of tokens to slash from the indexer stake + * @param reward Amount of reward tokens to send to a beneficiary + * @param beneficiary Address of a beneficiary to receive a reward for the slashing + */ + function legacySlash(address indexer, uint256 tokens, uint256 reward, address beneficiary) external; } diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 151bcaff4..659d0f089 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.27; import { IGraphToken } from "@graphprotocol/contracts/contracts/token/IGraphToken.sol"; import { IHorizonStakingMain } from "../interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingExtension } from "../interfaces/internal/IHorizonStakingExtension.sol"; import { IGraphPayments } from "../interfaces/IGraphPayments.sol"; import { TokenUtils } from "@graphprotocol/contracts/contracts/utils/TokenUtils.sol"; @@ -433,6 +434,23 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 tokensVerifier, address verifierDestination ) external override notPaused { + // TODO remove after the transition period + // Check if sender is authorized to slash on the deprecated list + if (__DEPRECATED_slashers[msg.sender]) { + // Forward call to staking extension + (bool success, ) = STAKING_EXTENSION_ADDRESS.delegatecall( + abi.encodeWithSelector( + IHorizonStakingExtension.legacySlash.selector, + serviceProvider, + tokens, + tokensVerifier, + verifierDestination + ) + ); + require(success, "Delegatecall to legacySlash failed"); + return; + } + address verifier = msg.sender; Provision storage prov = _provisions[serviceProvider][verifier]; DelegationPoolInternal storage pool = _getDelegationPool(serviceProvider, verifier); diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index bc878a4f5..bb523d564 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -36,6 +36,14 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension _; } + /** + * @dev Check if the caller is the slasher. + */ + modifier onlySlasher() { + require(__DEPRECATED_slashers[msg.sender] == true, "!slasher"); + _; + } + /** * @dev The staking contract is upgradeable however we still use the constructor to set * a few immutable variables. @@ -255,6 +263,55 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension return __DEPRECATED_thawingPeriod; } + function legacySlash( + address indexer, + uint256 tokens, + uint256 reward, + address beneficiary + ) external override onlySlasher notPaused { + ServiceProviderInternal storage indexerStake = _serviceProviders[indexer]; + + // Only able to slash a non-zero number of tokens + require(tokens > 0, "!tokens"); + + // Rewards comes from tokens slashed balance + require(tokens >= reward, "rewards>slash"); + + // Cannot slash stake of an indexer without any or enough stake + require(indexerStake.tokensStaked > 0, "!stake"); + require(tokens <= indexerStake.tokensStaked, "slash>stake"); + + // Validate beneficiary of slashed tokens + require(beneficiary != address(0), "!beneficiary"); + + // Slashing more tokens than freely available (over allocation condition) + // Unlock locked tokens to avoid the indexer to withdraw them + uint256 tokensAvailable = indexerStake.tokensStaked - + indexerStake.__DEPRECATED_tokensAllocated - + indexerStake.__DEPRECATED_tokensLocked; + if (tokens > tokensAvailable && indexerStake.__DEPRECATED_tokensLocked > 0) { + uint256 tokensOverAllocated = tokens - tokensAvailable; + uint256 tokensToUnlock = MathUtils.min(tokensOverAllocated, indexerStake.__DEPRECATED_tokensLocked); + indexerStake.__DEPRECATED_tokensLocked = indexerStake.__DEPRECATED_tokensLocked - tokensToUnlock; + if (indexerStake.__DEPRECATED_tokensLocked == 0) { + indexerStake.__DEPRECATED_tokensLockedUntil = 0; + } + } + + // Remove tokens to slash from the stake + indexerStake.tokensStaked = indexerStake.tokensStaked - tokens; + + // -- Interactions -- + + // Set apart the reward for the beneficiary and burn remaining slashed stake + _graphToken().burnTokens(tokens - reward); + + // Give the beneficiary a reward for slashing + _graphToken().pushTokens(beneficiary, reward); + + emit StakeSlashed(indexer, tokens, reward, beneficiary); + } + /** * @notice (Legacy) Return true if operator is allowed for the service provider on the subgraph data service. * @dev TODO: Delete after the transition period diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol index 7aa44d6f5..b58479fcf 100644 --- a/packages/horizon/test/GraphBase.t.sol +++ b/packages/horizon/test/GraphBase.t.sol @@ -71,7 +71,8 @@ abstract contract GraphBaseTest is IHorizonStakingTypes, Utils, Constants { operator: createUser("operator"), gateway: createUser("gateway"), verifier: createUser("verifier"), - delegator: createUser("delegator") + delegator: createUser("delegator"), + legacySlasher: createUser("legacySlasher") }); // Deploy protocol contracts diff --git a/packages/horizon/test/staking/slash/legacySlash.t.sol b/packages/horizon/test/staking/slash/legacySlash.t.sol new file mode 100644 index 000000000..760978aae --- /dev/null +++ b/packages/horizon/test/staking/slash/legacySlash.t.sol @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; + +import { HorizonStakingTest } from "../HorizonStaking.t.sol"; + +contract HorizonStakingLegacySlashTest is HorizonStakingTest { + + /* + * MODIFIERS + */ + + modifier useLegacySlasher(address slasher) { + bytes32 storageKey = keccak256(abi.encode(slasher, 18)); + vm.store(address(staking), storageKey, bytes32(uint256(1))); + _; + } + + /* + * HELPERS + */ + + function _setIndexer( + address _indexer, + uint256 _tokensStaked, + uint256 _tokensAllocated, + uint256 _tokensLocked, + uint256 _tokensLockedUntil + ) public { + bytes32 baseSlot = keccak256(abi.encode(_indexer, 14)); + + vm.store(address(staking), bytes32(uint256(baseSlot)), bytes32(_tokensStaked)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 1), bytes32(_tokensAllocated)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 2), bytes32(_tokensLocked)); + vm.store(address(staking), bytes32(uint256(baseSlot) + 3), bytes32(_tokensLockedUntil)); + } + + /* + * ACTIONS + */ + + function _legacySlash(address _indexer, uint256 _tokens, uint256 _rewards, address _beneficiary) internal { + // before + uint256 beforeStakingBalance = token.balanceOf(address(staking)); + uint256 beforeRewardsDestinationBalance = token.balanceOf(_beneficiary); + + // slash + vm.expectEmit(address(staking)); + emit IHorizonStakingExtension.StakeSlashed(_indexer, _tokens, _rewards, _beneficiary); + staking.slash(_indexer, _tokens, _rewards, _beneficiary); + + // after + uint256 afterStakingBalance = token.balanceOf(address(staking)); + uint256 afterRewardsDestinationBalance = token.balanceOf(_beneficiary); + + assertEq(beforeStakingBalance - _tokens, afterStakingBalance); + assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - _rewards); + } + + /* + * TESTS + */ + + function testSlash_Legacy( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_UsingLockedTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _setIndexer(users.indexer, tokens, 0, tokens, block.timestamp + 1); + // Send tokens manually to staking + token.transfer(address(staking), tokens); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_UsingAllocatedTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 1); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _setIndexer(users.indexer, tokens, 0, tokens, 0); + // Send tokens manually to staking + token.transfer(address(staking), tokens); + + resetPrank(users.legacySlasher); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_CallerNotSlasher( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer { + vm.assume(tokens > 0); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + vm.expectRevert("!slasher"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_RewardsOverSlashTokens( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + vm.assume(slashTokens > 0); + vm.assume(reward > slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("rewards>slash"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_NoStake( + uint256 slashTokens, + uint256 reward + ) public useLegacySlasher(users.legacySlasher) { + vm.assume(slashTokens > 0); + reward = bound(reward, 0, slashTokens); + + resetPrank(users.legacySlasher); + vm.expectRevert("!stake"); + staking.legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_ZeroTokens( + uint256 tokens + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("!tokens"); + staking.legacySlash(users.indexer, 0, 0, makeAddr("fisherman")); + } + + function testSlash_Legacy_RevertWhen_NoBeneficiary( + uint256 tokens, + uint256 slashTokens, + uint256 reward + ) public useIndexer useLegacySlasher(users.legacySlasher) { + vm.assume(tokens > 0); + slashTokens = bound(slashTokens, 1, tokens); + reward = bound(reward, 0, slashTokens); + + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + resetPrank(users.legacySlasher); + vm.expectRevert("!beneficiary"); + staking.legacySlash(users.indexer, slashTokens, reward, address(0)); + } +} diff --git a/packages/horizon/test/utils/Users.sol b/packages/horizon/test/utils/Users.sol index b26329f91..ecd3927ab 100644 --- a/packages/horizon/test/utils/Users.sol +++ b/packages/horizon/test/utils/Users.sol @@ -9,4 +9,5 @@ struct Users { address gateway; address verifier; address delegator; + address legacySlasher; } \ No newline at end of file From f254897802b6cbfbc8e9e972798fa89a0bbf6398 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 10:56:36 -0300 Subject: [PATCH 05/23] fix: add missing legacy withdraw delegated (TRST-H07) --- .../internal/IHorizonStakingExtension.sol | 14 +++ .../staking/HorizonStakingExtension.sol | 37 +++++++ .../staking/delegation/legacyWithdraw.t.sol | 98 +++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 packages/horizon/test/staking/delegation/legacyWithdraw.t.sol diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol index 84318f536..4c6f83075 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol @@ -83,6 +83,11 @@ interface IHorizonStakingExtension is IRewardsIssuer { */ event StakeSlashed(address indexed indexer, uint256 tokens, uint256 reward, address beneficiary); + /** + * @dev Emitted when `delegator` withdrew delegated `tokens` from `indexer` using `legacyWithdrawDelegated`. + */ + event StakeDelegatedWithdrawn(address indexed indexer, address indexed delegator, uint256 tokens); + /** * @notice Close an allocation and free the staked tokens. * To be eligible for rewards a proof of indexing must be presented. @@ -164,4 +169,13 @@ interface IHorizonStakingExtension is IRewardsIssuer { * @param beneficiary Address of a beneficiary to receive a reward for the slashing */ function legacySlash(address indexer, uint256 tokens, uint256 reward, address beneficiary) external; + + /** + * @notice Withdraw undelegated tokens once the unbonding period has passed. + * @param _indexer Withdraw available tokens delegated to indexer + */ + function legacyWithdrawDelegated( + address _indexer, + address /* _newIndexer, deprecated */ + ) external returns (uint256); } diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index bb523d564..4eaeb9a12 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -312,6 +312,43 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension emit StakeSlashed(indexer, tokens, reward, beneficiary); } + /** + * @notice Withdraw undelegated tokens once the unbonding period has passed. + * @param indexer Withdraw available tokens delegated to indexer + */ + function legacyWithdrawDelegated( + address indexer, + address // newIndexer, deprecated + ) external override notPaused returns (uint256) { + // Get the delegation pool of the indexer + address delegator = msg.sender; + DelegationPoolInternal storage pool = _legacyDelegationPools[indexer]; + DelegationInternal storage delegation = pool.delegators[delegator]; + + // Validation + uint256 tokensToWithdraw = 0; + uint256 currentEpoch = _graphEpochManager().currentEpoch(); + if ( + delegation.__DEPRECATED_tokensLockedUntil > 0 && currentEpoch >= delegation.__DEPRECATED_tokensLockedUntil + ) { + tokensToWithdraw = delegation.__DEPRECATED_tokensLocked; + } + require(tokensToWithdraw > 0, "!tokens"); + + // Reset lock + delegation.__DEPRECATED_tokensLocked = 0; + delegation.__DEPRECATED_tokensLockedUntil = 0; + + emit StakeDelegatedWithdrawn(indexer, delegator, tokensToWithdraw); + + // -- Interactions -- + + // Return tokens to the delegator + _graphToken().pushTokens(delegator, tokensToWithdraw); + + return tokensToWithdraw; + } + /** * @notice (Legacy) Return true if operator is allowed for the service provider on the subgraph data service. * @dev TODO: Delete after the transition period diff --git a/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol new file mode 100644 index 000000000..c70fb73a3 --- /dev/null +++ b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "../../../contracts/interfaces/internal/IHorizonStakingTypes.sol"; +import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol"; +import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; + +import { HorizonStakingTest } from "../HorizonStaking.t.sol"; + +contract HorizonStakingLegacyWithdrawDelegationTest is HorizonStakingTest { + /* + * MODIFIERS + */ + + modifier useDelegator() { + resetPrank(users.delegator); + _; + } + + /* + * HELPERS + */ + + function _setLegacyDelegation( + address _indexer, + address _delegator, + uint256 _shares, + uint256 __DEPRECATED_tokensLocked, + uint256 __DEPRECATED_tokensLockedUntil + ) public { + // Calculate the base storage slot for the serviceProvider in the mapping + bytes32 baseSlot = keccak256(abi.encode(_indexer, uint256(20))); + + // Calculate the slot for the delegator's DelegationInternal struct + bytes32 delegatorSlot = keccak256(abi.encode(_delegator, bytes32(uint256(baseSlot) + 4))); + + // Use vm.store to set each field of the struct + vm.store(address(staking), bytes32(uint256(delegatorSlot)), bytes32(_shares)); + vm.store(address(staking), bytes32(uint256(delegatorSlot) + 1), bytes32(__DEPRECATED_tokensLocked)); + vm.store(address(staking), bytes32(uint256(delegatorSlot) + 2), bytes32(__DEPRECATED_tokensLockedUntil)); + } + + /* + * ACTIONS + */ + + function _legacyWithdrawDelegated(address _indexer) internal { + (, address delegator, ) = vm.readCallers(); + IHorizonStakingTypes.DelegationPool memory pool = staking.getDelegationPool(_indexer, subgraphDataServiceLegacyAddress); + uint256 beforeStakingBalance = token.balanceOf(address(staking)); + uint256 beforeDelegatorBalance = token.balanceOf(users.delegator); + + vm.expectEmit(address(staking)); + emit IHorizonStakingExtension.StakeDelegatedWithdrawn(_indexer, delegator, pool.tokens); + staking.legacyWithdrawDelegated(users.indexer, address(0)); + + uint256 afterStakingBalance = token.balanceOf(address(staking)); + uint256 afterDelegatorBalance = token.balanceOf(users.delegator); + + assertEq(afterStakingBalance, beforeStakingBalance - pool.tokens); + assertEq(afterDelegatorBalance - pool.tokens, beforeDelegatorBalance); + + DelegationInternal memory delegation = _getStorage_Delegation( + _indexer, + subgraphDataServiceLegacyAddress, + delegator, + true + ); + assertEq(delegation.shares, 0); + assertEq(delegation.__DEPRECATED_tokensLocked, 0); + assertEq(delegation.__DEPRECATED_tokensLockedUntil, 0); + } + + /* + * TESTS + */ + + function testWithdraw_Legacy(uint256 tokensLocked) public useDelegator { + vm.assume(tokensLocked > 0); + + _setStorage_DelegationPool(users.indexer, tokensLocked, 0, 0); + _setLegacyDelegation(users.indexer, users.delegator, 0, tokensLocked, 1); + token.transfer(address(staking), tokensLocked); + + _legacyWithdrawDelegated(users.indexer); + } + + function testWithdraw_Legacy_RevertWhen_NoTokens() public useDelegator { + _setStorage_DelegationPool(users.indexer, 0, 0, 0); + _setLegacyDelegation(users.indexer, users.delegator, 0, 0, 0); + + vm.expectRevert("!tokens"); + staking.legacyWithdrawDelegated(users.indexer, address(0)); + } +} From 91cda5619ee96954cfaa7526c24499521e66ee3f Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 15:15:40 -0300 Subject: [PATCH 06/23] fix: added minimum delegation for provisions (TRST-M03) --- .../contracts/staking/HorizonStaking.sol | 11 +++++ packages/horizon/test/escrow/collect.t.sol | 2 +- .../horizon/test/payments/GraphPayments.t.sol | 2 +- .../horizon/test/staking/HorizonStaking.t.sol | 28 ++++++++++++- .../test/staking/delegation/addToPool.t.sol | 5 ++- .../test/staking/delegation/delegate.t.sol | 24 +++++++++-- .../test/staking/delegation/redelegate.t.sol | 13 ------ .../test/staking/delegation/undelegate.t.sol | 41 ++++++++++++++++--- .../test/staking/delegation/withdraw.t.sol | 21 +--------- .../horizon/test/staking/slash/slash.t.sol | 6 +-- packages/horizon/test/utils/Constants.sol | 1 + 11 files changed, 105 insertions(+), 49 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 659d0f089..5d7961574 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -41,6 +41,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /// @dev Address of the staking extension contract address private immutable STAKING_EXTENSION_ADDRESS; + /// @dev Minimum amount of delegation. + uint256 private constant MIN_DELEGATION = 1e18; + /** * @notice Checks that the caller is authorized to operate over a provision. * @param serviceProvider The address of the service provider. @@ -842,6 +845,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * have been done before calling this function. */ function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private { + require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientTokens(_tokens, MIN_DELEGATION)); require( _provisions[_serviceProvider][_verifier].createdAt != 0, HorizonStakingInvalidProvision(_serviceProvider, _verifier) @@ -911,6 +915,13 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { pool.shares = pool.shares - _shares; delegation.shares = delegation.shares - _shares; + if (delegation.shares != 0) { + uint256 remainingTokens = (delegation.shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + require( + remainingTokens >= MIN_DELEGATION, + HorizonStakingInsufficientTokens(remainingTokens, MIN_DELEGATION) + ); + } bytes32 thawRequestId = _createThawRequest( _requestType, diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 72b795ee9..e47c04ec5 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -101,7 +101,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { uint256 tokensDelegatoion = tokens * delegationFeeCut / MAX_PPM; vm.assume(tokensDataService < tokens - tokensProtocol - tokensDelegatoion); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 19028bde1..47740b4cd 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -119,7 +119,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { address escrowAddress = address(escrow); // Delegate tokens - tokensDelegate = bound(tokensDelegate, 1, MAX_STAKING_TOKENS); + tokensDelegate = bound(tokensDelegate, MIN_DELEGATION, MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, tokensDelegate, 0); diff --git a/packages/horizon/test/staking/HorizonStaking.t.sol b/packages/horizon/test/staking/HorizonStaking.t.sol index b1b45d118..d57b1c1b8 100644 --- a/packages/horizon/test/staking/HorizonStaking.t.sol +++ b/packages/horizon/test/staking/HorizonStaking.t.sol @@ -31,7 +31,7 @@ contract HorizonStakingTest is HorizonStakingSharedTest { modifier useDelegation(uint256 delegationAmount) { address msgSender; (, msgSender, ) = vm.readCallers(); - vm.assume(delegationAmount > 1); + vm.assume(delegationAmount >= MIN_DELEGATION); vm.assume(delegationAmount <= MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -56,4 +56,30 @@ contract HorizonStakingTest is HorizonStakingSharedTest { resetPrank(msgSender); _; } + + modifier useUndelegate(uint256 shares) { + resetPrank(users.delegator); + + DelegationPoolInternalTest memory pool = _getStorage_DelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + + shares = bound(shares, 1, delegation.shares); + uint256 tokens = (shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + if (shares < delegation.shares) { + uint256 remainingTokens = (shares * (pool.tokens - pool.tokensThawing - tokens)) / pool.shares; + vm.assume(remainingTokens >= MIN_DELEGATION); + } + + _undelegate(users.indexer, subgraphDataServiceAddress, shares); + _; + } } diff --git a/packages/horizon/test/staking/delegation/addToPool.t.sol b/packages/horizon/test/staking/delegation/addToPool.t.sol index ff5b957ca..2bd73f28f 100644 --- a/packages/horizon/test/staking/delegation/addToPool.t.sol +++ b/packages/horizon/test/staking/delegation/addToPool.t.sol @@ -10,6 +10,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { modifier useValidDelegationAmount(uint256 tokens) { vm.assume(tokens <= MAX_STAKING_TOKENS); + vm.assume(tokens >= MIN_DELEGATION); _; } @@ -93,7 +94,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); @@ -116,7 +117,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/delegate.t.sol b/packages/horizon/test/staking/delegation/delegate.t.sol index ab58e4bde..5fc6681df 100644 --- a/packages/horizon/test/staking/delegation/delegate.t.sol +++ b/packages/horizon/test/staking/delegation/delegate.t.sol @@ -59,9 +59,25 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { staking.delegate(users.indexer, subgraphDataServiceAddress, 0, 0); } + function testDelegate_RevertWhen_UnderMinDelegation( + uint256 amount, + uint256 delegationAmount + ) public useIndexer useProvision(amount, 0, 0) { + delegationAmount = bound(delegationAmount, 1, MIN_DELEGATION - 1); + vm.startPrank(users.delegator); + token.approve(address(staking), delegationAmount); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + delegationAmount, + MIN_DELEGATION + ); + vm.expectRevert(expectedError); + staking.delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); + } + function testDelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer { amount = bound(amount, 1 ether, MAX_STAKING_TOKENS); - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -72,7 +88,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -96,7 +112,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -126,7 +142,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/redelegate.t.sol b/packages/horizon/test/staking/delegation/redelegate.t.sol index 605e6601f..6e30348c4 100644 --- a/packages/horizon/test/staking/delegation/redelegate.t.sol +++ b/packages/horizon/test/staking/delegation/redelegate.t.sol @@ -9,19 +9,6 @@ import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } - /* * HELPERS */ diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index c824832c4..a2d41c061 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/packages/horizon/test/staking/delegation/undelegate.t.sol @@ -32,7 +32,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 undelegateSteps ) public useIndexer useProvision(amount, 0, 0) { undelegateSteps = bound(undelegateSteps, 1, 10); - delegationAmount = bound(delegationAmount, 10 wei, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION * undelegateSteps, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -44,9 +44,17 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { ); uint256 undelegateAmount = delegation.shares / undelegateSteps; - for (uint i = 0; i < undelegateSteps; i++) { + for (uint i = 0; i < undelegateSteps - 1; i++) { _undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); } + + delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); } function testUndelegate_WithBeneficiary( @@ -60,6 +68,29 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); } + function testUndelegate_RevertWhen_InsuficientTokens( + uint256 amount, + uint256 delegationAmount, + uint256 undelegateAmount + ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) { + undelegateAmount = bound(undelegateAmount, 1, delegationAmount); + resetPrank(users.delegator); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + undelegateAmount = bound(undelegateAmount, delegation.shares - MIN_DELEGATION + 1, delegation.shares - 1); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + delegation.shares - undelegateAmount, + MIN_DELEGATION + ); + vm.expectRevert(expectedError); + staking.undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); + } + function testUndelegate_RevertWhen_TooManyUndelegations() public useIndexer @@ -112,7 +143,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { function testUndelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer { amount = bound(amount, 1, MAX_STAKING_TOKENS); - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -131,7 +162,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -162,7 +193,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // delegate resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index ff3fb3db0..c8ab59122 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -10,23 +10,6 @@ import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation( - users.indexer, - subgraphDataServiceAddress, - users.delegator, - false - ); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } /* * TESTS @@ -81,7 +64,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { } function testWithdrawDelegation_LegacySubgraphService(uint256 delegationAmount) public useIndexer { - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, 10_000_000 ether, 0, MAX_THAWING_PERIOD); resetPrank(users.delegator); @@ -111,7 +94,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index a74403ed7..fc5676b0f 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/packages/horizon/test/staking/slash/slash.t.sol @@ -54,7 +54,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) { vm.assume(slashTokens > tokens); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); verifierCutAmount = bound(verifierCutAmount, 0, MAX_PPM); vm.assume(verifierCutAmount <= tokens); @@ -71,7 +71,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 verifierCutAmount, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); slashTokens = bound(slashTokens, tokens + 1, tokens + 1 + delegationTokens); verifierCutAmount = bound(verifierCutAmount, 0, tokens); @@ -110,7 +110,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index cd5cc2bfb..9af54087d 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -14,6 +14,7 @@ abstract contract Constants { uint256 internal constant MAX_THAW_REQUESTS = 100; uint64 internal constant MAX_THAWING_PERIOD = 28 days; uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300; + uint256 internal constant MIN_DELEGATION = 1e18; // Epoch manager uint256 internal constant EPOCH_LENGTH = 1; // Rewards manager From 07ef418ecbd1641de3bdad7e5efa9a96cddf9219 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 16:11:15 -0300 Subject: [PATCH 07/23] fix: operator check in closeAllocation (TRST-M12) --- .../staking/HorizonStakingExtension.sol | 3 +-- .../test/staking/allocation/close.t.sol | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index 4eaeb9a12..33d59b61b 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -443,8 +443,7 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension // Anyone is allowed to close ONLY under two concurrent conditions // - After maxAllocationEpochs passed // - When the allocation is for non-zero amount of tokens - bool isIndexerOrOperator = msg.sender == alloc.indexer || - isOperator(alloc.indexer, SUBGRAPH_DATA_SERVICE_ADDRESS); + bool isIndexerOrOperator = msg.sender == alloc.indexer || isOperator(msg.sender, alloc.indexer); if (epochs <= __DEPRECATED_maxAllocationEpochs || alloc.tokens == 0) { require(isIndexerOrOperator, "!auth"); } diff --git a/packages/horizon/test/staking/allocation/close.t.sol b/packages/horizon/test/staking/allocation/close.t.sol index ce3cab273..6257e30ec 100644 --- a/packages/horizon/test/staking/allocation/close.t.sol +++ b/packages/horizon/test/staking/allocation/close.t.sol @@ -12,6 +12,18 @@ contract HorizonStakingCloseAllocationTest is HorizonStakingTest { bytes32 internal constant _poi = keccak256("poi"); + /* + * MODIFIERS + */ + + modifier useLegacyOperator() { + resetPrank(users.indexer); + _setOperator(subgraphDataServiceLegacyAddress, users.operator, true); + vm.startPrank(users.operator); + _; + vm.stopPrank(); + } + /* * TESTS */ @@ -26,6 +38,16 @@ contract HorizonStakingCloseAllocationTest is HorizonStakingTest { _closeAllocation(_allocationId, _poi); } + function testCloseAllocation_Operator(uint256 tokens) public useLegacyOperator() useAllocation(1 ether) { + tokens = bound(tokens, 1, MAX_STAKING_TOKENS); + _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); + + // Skip 15 epochs + vm.roll(15); + + _closeAllocation(_allocationId, _poi); + } + function testCloseAllocation_WithBeneficiaryAddress(uint256 tokens) public useIndexer useAllocation(1 ether) { tokens = bound(tokens, 1, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0); From 26e4dc75a43645a3bfc9936106490ed7f4e5597b Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 18:02:13 -0300 Subject: [PATCH 08/23] fix: getThawedTokens calculation (TRST-L03) --- .../contracts/staking/HorizonStakingBase.sol | 11 ++++++--- .../horizon/test/staking/provision/thaw.t.sol | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index 01210d157..3563dba2e 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -206,20 +206,25 @@ abstract contract HorizonStakingBase is return 0; } - uint256 tokens = 0; + uint256 thawedTokens = 0; Provision storage prov = _provisions[serviceProvider][verifier]; + uint256 tokensThawing = prov.tokensThawing; + uint256 sharesThawing = prov.sharesThawing; bytes32 thawRequestId = thawRequestList.head; while (thawRequestId != bytes32(0)) { ThawRequest storage thawRequest = _getThawRequest(requestType, thawRequestId); if (thawRequest.thawingUntil <= block.timestamp) { - tokens += (thawRequest.shares * prov.tokensThawing) / prov.sharesThawing; + uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing; + tokensThawing = tokensThawing - tokens; + sharesThawing = sharesThawing - thawRequest.shares; + thawedTokens = thawedTokens + tokens; } else { break; } thawRequestId = thawRequest.next; } - return tokens; + return thawedTokens; } /** diff --git a/packages/horizon/test/staking/provision/thaw.t.sol b/packages/horizon/test/staking/provision/thaw.t.sol index c3b4d6903..c69792f50 100644 --- a/packages/horizon/test/staking/provision/thaw.t.sol +++ b/packages/horizon/test/staking/provision/thaw.t.sol @@ -139,4 +139,28 @@ contract HorizonStakingThawTest is HorizonStakingTest { resetPrank(users.indexer); _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); } + + function testThaw_GetThawedTokens( + uint256 amount, + uint64 thawingPeriod, + uint256 thawSteps + ) public useIndexer useProvision(amount, 0, thawingPeriod) { + thawSteps = bound(thawSteps, 1, 10); + + uint256 thawAmount = amount / thawSteps; + vm.assume(thawAmount > 0); + for (uint256 i = 0; i < thawSteps; i++) { + _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); + } + + skip(thawingPeriod + 1); + + uint256 thawedTokens = staking.getThawedTokens( + ThawRequestType.Provision, + users.indexer, + subgraphDataServiceAddress, + users.indexer + ); + vm.assertEq(thawedTokens, thawAmount * thawSteps); + } } From c9f8a2f348da657d046c91ce0aa3df9e1fbd01f2 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 18:09:17 -0300 Subject: [PATCH 09/23] fix: documentation on unstake (TRST-M11) --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 9122153c5..c9d57492a 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -515,6 +515,8 @@ interface IHorizonStakingMain { * - During the transition period it's locked for a period of time before it can be withdrawn * by calling {withdraw}. * - After the transition period it's immediately withdrawn. + * Note that after the transition period if there are tokens still locked they will have to be + * withdrawn by calling {withdraw}. * @dev Requirements: * - `_tokens` cannot be zero. * - `_serviceProvider` must have enough idle stake to cover the staking amount and any From 9271b99aa5c3e34f9f709378f9dfec52f77b1a3f Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 18:47:31 -0300 Subject: [PATCH 10/23] fix: remove unused value from event (TRST-R01) --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 3 +-- packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- .../test/shared/horizon-staking/HorizonStakingShared.t.sol | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index c9d57492a..728d36e7c 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -303,9 +303,8 @@ interface IHorizonStakingMain { /** * @notice Emitted when the delegation slashing global flag is set. - * @param enabled Whether delegation slashing is enabled or disabled. */ - event DelegationSlashingEnabled(bool enabled); + event DelegationSlashingEnabled(); // -- Errors: tokens diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 5d7961574..18eb64ee6 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -585,7 +585,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { */ function setDelegationSlashingEnabled() external override onlyGovernor { _delegationSlashingEnabled = true; - emit DelegationSlashingEnabled(_delegationSlashingEnabled); + emit DelegationSlashingEnabled(); } /** diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index c960b19bd..ecaa37492 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -1367,7 +1367,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { function _setDelegationSlashingEnabled() internal { // setDelegationSlashingEnabled vm.expectEmit(); - emit IHorizonStakingMain.DelegationSlashingEnabled(true); + emit IHorizonStakingMain.DelegationSlashingEnabled(); staking.setDelegationSlashingEnabled(); // after From 151e63a54b824d63cf53f5b39f23478d6b56047a Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 5 Dec 2024 13:31:40 -0300 Subject: [PATCH 11/23] fix: round thawing shares up (TRST-R07) --- packages/horizon/contracts/staking/HorizonStaking.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 18eb64ee6..4f7246859 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -783,9 +783,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // Calculate shares to issue // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0 + // Round thawing shares up to ensure fairness and avoid undervaluing the shares due to rounding down. uint256 thawingShares = prov.tokensThawing == 0 ? _tokens - : ((prov.sharesThawing * _tokens) / prov.tokensThawing); + : ((prov.sharesThawing * _tokens + prov.tokensThawing - 1) / prov.tokensThawing); uint64 thawingUntil = uint64(block.timestamp + uint256(prov.thawingPeriod)); prov.sharesThawing = prov.sharesThawing + thawingShares; @@ -907,6 +908,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // delegation pool shares -> delegation pool tokens -> thawing pool shares // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0 uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + // Thawing shares are rounded down to protect the pool and avoid taking extra tokens from other participants. uint256 thawingShares = pool.tokensThawing == 0 ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing); uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod)); From 6e5a295f9b86ba71ccf66729e88fa8a626c22a1b Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 5 Dec 2024 16:39:15 -0300 Subject: [PATCH 12/23] fix: check shares are not zero when creating a thaw request (TRST-R14) --- packages/horizon/contracts/staking/HorizonStaking.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 4f7246859..f9ad7fb7b 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -1015,6 +1015,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint64 _thawingUntil, uint256 _thawingNonce ) private returns (bytes32) { + require(_shares != 0, HorizonStakingInvalidZeroShares()); LinkedList.List storage thawRequestList = _getThawRequestList( _requestType, _serviceProvider, From d9c61907cb74aa10d11599e0ced278fc46227a82 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 6 Dec 2024 11:41:16 -0300 Subject: [PATCH 13/23] fix: move legacyWithdrawDelegated to withdrawDelegated (TRST-H07) --- .../internal/IHorizonStakingExtension.sol | 14 ------- .../internal/IHorizonStakingMain.sol | 18 ++++++-- .../contracts/staking/HorizonStaking.sol | 41 ++++++++++++++----- .../staking/HorizonStakingExtension.sol | 37 ----------------- .../HorizonStakingShared.t.sol | 20 +-------- .../staking/delegation/legacyWithdraw.t.sol | 6 +-- .../test/staking/delegation/withdraw.t.sol | 2 +- 7 files changed, 51 insertions(+), 87 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol index 4c6f83075..84318f536 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingExtension.sol @@ -83,11 +83,6 @@ interface IHorizonStakingExtension is IRewardsIssuer { */ event StakeSlashed(address indexed indexer, uint256 tokens, uint256 reward, address beneficiary); - /** - * @dev Emitted when `delegator` withdrew delegated `tokens` from `indexer` using `legacyWithdrawDelegated`. - */ - event StakeDelegatedWithdrawn(address indexed indexer, address indexed delegator, uint256 tokens); - /** * @notice Close an allocation and free the staked tokens. * To be eligible for rewards a proof of indexing must be presented. @@ -169,13 +164,4 @@ interface IHorizonStakingExtension is IRewardsIssuer { * @param beneficiary Address of a beneficiary to receive a reward for the slashing */ function legacySlash(address indexer, uint256 tokens, uint256 reward, address beneficiary) external; - - /** - * @notice Withdraw undelegated tokens once the unbonding period has passed. - * @param _indexer Withdraw available tokens delegated to indexer - */ - function legacyWithdrawDelegated( - address _indexer, - address /* _newIndexer, deprecated */ - ) external returns (uint256); } diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 728d36e7c..489c54554 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -205,6 +205,15 @@ interface IHorizonStakingMain { uint256 tokens ); + /** + * @notice Emitted when `delegator` withdrew delegated `tokens` from `indexer` using `withdrawDelegated`. + * @dev This event is for the legacy `withdrawDelegated` function. + * @param indexer The address of the indexer + * @param delegator The address of the delegator + * @param tokens The amount of tokens withdrawn + */ + event StakeDelegatedWithdrawn(address indexed indexer, address indexed delegator, uint256 tokens); + /** * @notice Emitted when tokens are added to a delegation pool's reserve. * @param serviceProvider The address of the service provider @@ -861,13 +870,14 @@ interface IHorizonStakingMain { /** * @notice Withdraw undelegated tokens from the subgraph data service provision after thawing. * This function is for backwards compatibility with the legacy staking contract. - * It only allows withdrawing from the subgraph data service and DOES NOT have slippage protection in - * case the caller opts for re-delegating. + * It only allows withdrawing tokens undelegated before horizon upgrade. * @dev See {delegate}. * @param serviceProvider The service provider address - * @param newServiceProvider The address of a new service provider, if the delegator wants to re-delegate */ - function withdrawDelegated(address serviceProvider, address newServiceProvider) external; + function withdrawDelegated( + address serviceProvider, + address // newServiceProvider, deprecated + ) external returns (uint256); /** * @notice Slash a service provider. This can only be called by a verifier to which diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index f9ad7fb7b..5cd605acf 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -412,16 +412,37 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /** * @notice See {IHorizonStakingMain-withdrawDelegated}. */ - function withdrawDelegated(address serviceProvider, address newServiceProvider) external override notPaused { - _withdrawDelegated( - ThawRequestType.Delegation, - serviceProvider, - SUBGRAPH_DATA_SERVICE_ADDRESS, - newServiceProvider, - SUBGRAPH_DATA_SERVICE_ADDRESS, - 0, - 0 - ); + function withdrawDelegated( + address serviceProvider, + address // newServiceProvider, deprecated + ) external override notPaused returns (uint256) { + // Get the delegation pool of the indexer + address delegator = msg.sender; + DelegationPoolInternal storage pool = _legacyDelegationPools[serviceProvider]; + DelegationInternal storage delegation = pool.delegators[delegator]; + + // Validation + uint256 tokensToWithdraw = 0; + uint256 currentEpoch = _graphEpochManager().currentEpoch(); + if ( + delegation.__DEPRECATED_tokensLockedUntil > 0 && currentEpoch >= delegation.__DEPRECATED_tokensLockedUntil + ) { + tokensToWithdraw = delegation.__DEPRECATED_tokensLocked; + } + require(tokensToWithdraw > 0, "!tokens"); + + // Reset lock + delegation.__DEPRECATED_tokensLocked = 0; + delegation.__DEPRECATED_tokensLockedUntil = 0; + + emit StakeDelegatedWithdrawn(serviceProvider, delegator, tokensToWithdraw); + + // -- Interactions -- + + // Return tokens to the delegator + _graphToken().pushTokens(delegator, tokensToWithdraw); + + return tokensToWithdraw; } /* diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index 33d59b61b..ee8a29a22 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -312,43 +312,6 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension emit StakeSlashed(indexer, tokens, reward, beneficiary); } - /** - * @notice Withdraw undelegated tokens once the unbonding period has passed. - * @param indexer Withdraw available tokens delegated to indexer - */ - function legacyWithdrawDelegated( - address indexer, - address // newIndexer, deprecated - ) external override notPaused returns (uint256) { - // Get the delegation pool of the indexer - address delegator = msg.sender; - DelegationPoolInternal storage pool = _legacyDelegationPools[indexer]; - DelegationInternal storage delegation = pool.delegators[delegator]; - - // Validation - uint256 tokensToWithdraw = 0; - uint256 currentEpoch = _graphEpochManager().currentEpoch(); - if ( - delegation.__DEPRECATED_tokensLockedUntil > 0 && currentEpoch >= delegation.__DEPRECATED_tokensLockedUntil - ) { - tokensToWithdraw = delegation.__DEPRECATED_tokensLocked; - } - require(tokensToWithdraw > 0, "!tokens"); - - // Reset lock - delegation.__DEPRECATED_tokensLocked = 0; - delegation.__DEPRECATED_tokensLockedUntil = 0; - - emit StakeDelegatedWithdrawn(indexer, delegator, tokensToWithdraw); - - // -- Interactions -- - - // Return tokens to the delegator - _graphToken().pushTokens(delegator, tokensToWithdraw); - - return tokensToWithdraw; - } - /** * @notice (Legacy) Return true if operator is allowed for the service provider on the subgraph data service. * @dev TODO: Delete after the transition period diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index ecaa37492..4e024f899 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -1046,7 +1046,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { newVerifier: address(0), minSharesForNewProvider: 0, nThawRequests: nThawRequests, - legacy: false + legacy: verifier == subgraphDataServiceLegacyAddress }); __withdrawDelegated(params); } @@ -1090,20 +1090,6 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { __withdrawDelegated(params); } - function _withdrawDelegated(address serviceProvider, address newServiceProvider) internal { - Params_WithdrawDelegated memory params = Params_WithdrawDelegated({ - thawRequestType: IHorizonStakingTypes.ThawRequestType.Delegation, - serviceProvider: serviceProvider, - verifier: subgraphDataServiceLegacyAddress, - newServiceProvider: newServiceProvider, - newVerifier: subgraphDataServiceLegacyAddress, - minSharesForNewProvider: 0, - nThawRequests: 0, - legacy: true - }); - __withdrawDelegated(params); - } - struct BeforeValues_WithdrawDelegated { DelegationPoolInternalTest pool; DelegationPoolInternalTest newPool; @@ -1197,9 +1183,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { msgSender, calcValues.tokensThawed ); - if (params.legacy) { - staking.withdrawDelegated(params.serviceProvider, params.newServiceProvider); - } else if (reDelegate) { + if (reDelegate) { staking.redelegate( params.serviceProvider, params.verifier, diff --git a/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol index c70fb73a3..c90fa1200 100644 --- a/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol +++ b/packages/horizon/test/staking/delegation/legacyWithdraw.t.sol @@ -54,8 +54,8 @@ contract HorizonStakingLegacyWithdrawDelegationTest is HorizonStakingTest { uint256 beforeDelegatorBalance = token.balanceOf(users.delegator); vm.expectEmit(address(staking)); - emit IHorizonStakingExtension.StakeDelegatedWithdrawn(_indexer, delegator, pool.tokens); - staking.legacyWithdrawDelegated(users.indexer, address(0)); + emit IHorizonStakingMain.StakeDelegatedWithdrawn(_indexer, delegator, pool.tokens); + staking.withdrawDelegated(users.indexer, address(0)); uint256 afterStakingBalance = token.balanceOf(address(staking)); uint256 afterDelegatorBalance = token.balanceOf(users.delegator); @@ -93,6 +93,6 @@ contract HorizonStakingLegacyWithdrawDelegationTest is HorizonStakingTest { _setLegacyDelegation(users.indexer, users.delegator, 0, 0, 0); vm.expectRevert("!tokens"); - staking.legacyWithdrawDelegated(users.indexer, address(0)); + staking.withdrawDelegated(users.indexer, address(0)); } } diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index c8ab59122..0d791ff4c 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -87,7 +87,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { skip(thawRequest.thawingUntil + 1); - _withdrawDelegated(users.indexer, address(0)); + _withdrawDelegated(users.indexer, subgraphDataServiceLegacyAddress, 0); } function testWithdrawDelegation_RevertWhen_InvalidPool( From 161f8a2382c0add3b28994f620e536f6451b82e1 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 6 Dec 2024 15:19:04 -0300 Subject: [PATCH 14/23] fix: added comment for stack too deep solution --- .../contracts/interfaces/internal/IHorizonStakingTypes.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol index 9da150d0f..0d615a7b6 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol @@ -160,6 +160,7 @@ interface IHorizonStakingTypes { /** * @notice Parameters to fulfill thaw requests. + * @dev This struct is used to avoid stack too deep error in the `fulfillThawRequests` function. * @param requestType The type of thaw request (Provision or Delegation) * @param serviceProvider The address of the service provider * @param verifier The address of the verifier From 43bc72be75980baefd63dafe3c415b9a02aa730a Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 6 Dec 2024 15:24:02 -0300 Subject: [PATCH 15/23] fix: added comment to explain minimum delegation (TRST-M03) --- packages/horizon/contracts/staking/HorizonStaking.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 5cd605acf..ac2af54c2 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -867,6 +867,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * have been done before calling this function. */ function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private { + // Enforces a minimum delegation amount to prevent share manipulation attacks. + // This stops attackers from inflating share value and blocking other delegators. require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientTokens(_tokens, MIN_DELEGATION)); require( _provisions[_serviceProvider][_verifier].createdAt != 0, From c59c1866299c88c1fbe7836508712a2f79ba3890 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 6 Dec 2024 15:30:35 -0300 Subject: [PATCH 16/23] fix: new event for minimum delegation not met (TRST-M03) --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 7 +++++++ packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- packages/horizon/test/staking/delegation/delegate.t.sol | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 489c54554..dcbe3fce5 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -423,6 +423,13 @@ interface IHorizonStakingMain { */ error HorizonStakingInvalidDelegationPool(address serviceProvider, address verifier); + /** + * @notice Thrown when the minimum token amount required for delegation is not met. + * @param tokens The actual token amount + * @param minTokens The minimum required token amount + */ + error HorizonStakingInsufficientDelegationTokens(uint256 tokens, uint256 minTokens); + /** * @notice Thrown when attempting to undelegate with a beneficiary that is the zero address. */ diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index ac2af54c2..db29cf12a 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -869,7 +869,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private { // Enforces a minimum delegation amount to prevent share manipulation attacks. // This stops attackers from inflating share value and blocking other delegators. - require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientTokens(_tokens, MIN_DELEGATION)); + require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientDelegationTokens(_tokens, MIN_DELEGATION)); require( _provisions[_serviceProvider][_verifier].createdAt != 0, HorizonStakingInvalidProvision(_serviceProvider, _verifier) diff --git a/packages/horizon/test/staking/delegation/delegate.t.sol b/packages/horizon/test/staking/delegation/delegate.t.sol index 5fc6681df..043453e10 100644 --- a/packages/horizon/test/staking/delegation/delegate.t.sol +++ b/packages/horizon/test/staking/delegation/delegate.t.sol @@ -67,7 +67,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { vm.startPrank(users.delegator); token.approve(address(staking), delegationAmount); bytes memory expectedError = abi.encodeWithSelector( - IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + IHorizonStakingMain.HorizonStakingInsufficientDelegationTokens.selector, delegationAmount, MIN_DELEGATION ); From d1c5cc7556641eb83976f35b863eefe42bae2830 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 6 Dec 2024 16:44:11 -0300 Subject: [PATCH 17/23] fix: add a new mapping instead of splitting thaw requests (TRST-H02) --- .../contracts/staking/HorizonStaking.sol | 6 ++-- .../contracts/staking/HorizonStakingBase.sol | 26 ++++------------ .../staking/HorizonStakingStorage.sol | 30 ++++--------------- .../HorizonStakingShared.t.sol | 6 ++-- 4 files changed, 17 insertions(+), 51 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index db29cf12a..257202d3e 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -1181,7 +1181,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @param _thawRequestId The ID of the thaw request to delete. */ function _deleteProvisionThawRequest(bytes32 _thawRequestId) private { - delete _provisionThawRequests[_thawRequestId]; + delete _thawRequests[ThawRequestType.Provision][_thawRequestId]; } /** @@ -1189,7 +1189,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @param _thawRequestId The ID of the thaw request to delete. */ function _deleteDelegationThawRequest(bytes32 _thawRequestId) private { - delete _delegationThawRequests[_thawRequestId]; + delete _thawRequests[ThawRequestType.Delegation][_thawRequestId]; } /** @@ -1197,7 +1197,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @param _thawRequestId The ID of the thaw request to delete. */ function _deleteDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) private { - delete _delegationWithBeneficiaryThawRequests[_thawRequestId]; + delete _thawRequests[ThawRequestType.DelegationWithBeneficiary][_thawRequestId]; } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index 3563dba2e..6745fb2e1 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -323,7 +323,7 @@ abstract contract HorizonStakingBase is * @return The ID of the next thaw request in the list. */ function _getNextProvisionThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { - return _provisionThawRequests[_thawRequestId].next; + return _thawRequests[ThawRequestType.Provision][_thawRequestId].next; } /** @@ -332,7 +332,7 @@ abstract contract HorizonStakingBase is * @return The ID of the next thaw request in the list. */ function _getNextDelegationThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { - return _delegationThawRequests[_thawRequestId].next; + return _thawRequests[ThawRequestType.Delegation][_thawRequestId].next; } /** @@ -341,7 +341,7 @@ abstract contract HorizonStakingBase is * @return The ID of the next thaw request in the list. */ function _getNextDelegationWithBeneficiaryThawRequest(bytes32 _thawRequestId) internal view returns (bytes32) { - return _delegationWithBeneficiaryThawRequests[_thawRequestId].next; + return _thawRequests[ThawRequestType.DelegationWithBeneficiary][_thawRequestId].next; } /** @@ -360,15 +360,7 @@ abstract contract HorizonStakingBase is address _verifier, address _owner ) internal view returns (LinkedList.List storage) { - if (_requestType == ThawRequestType.Provision) { - return _provisionThawRequestLists[_serviceProvider][_verifier][_owner]; - } else if (_requestType == ThawRequestType.Delegation) { - return _delegationThawRequestLists[_serviceProvider][_verifier][_owner]; - } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { - return _delegationWithBeneficiaryThawRequestLists[_serviceProvider][_verifier][_owner]; - } else { - revert HorizonStakingInvalidThawRequestType(); - } + return _thawRequestLists[_requestType][_serviceProvider][_verifier][_owner]; } /** @@ -383,15 +375,7 @@ abstract contract HorizonStakingBase is ThawRequestType _requestType, bytes32 _thawRequestId ) internal view returns (IHorizonStakingTypes.ThawRequest storage) { - if (_requestType == ThawRequestType.Provision) { - return _provisionThawRequests[_thawRequestId]; - } else if (_requestType == ThawRequestType.Delegation) { - return _delegationThawRequests[_thawRequestId]; - } else if (_requestType == ThawRequestType.DelegationWithBeneficiary) { - return _delegationWithBeneficiaryThawRequests[_thawRequestId]; - } else { - revert("Unknown ThawRequestType"); - } + return _thawRequests[_requestType][_thawRequestId]; } /** diff --git a/packages/horizon/contracts/staking/HorizonStakingStorage.sol b/packages/horizon/contracts/staking/HorizonStakingStorage.sol index ca6a1450f..a470ac363 100644 --- a/packages/horizon/contracts/staking/HorizonStakingStorage.sol +++ b/packages/horizon/contracts/staking/HorizonStakingStorage.sol @@ -148,32 +148,14 @@ abstract contract HorizonStakingV1Storage { internal _delegationFeeCut; /// @dev Thaw requests - /// Details for each thawing operation in the staking contract for both service providers. - mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _provisionThawRequests; + /// Details for each thawing operation in the staking contract (for both service providers and delegators). + mapping(IHorizonStakingTypes.ThawRequestType thawRequestType => mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest)) + internal _thawRequests; /// @dev Thaw request lists - /// Metadata defining linked lists of thaw requests for each service provider (owner). - mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) - internal _provisionThawRequestLists; - - /// @dev Thaw requests - /// Details for each thawing operation in the staking contract for delegators. - mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) internal _delegationThawRequests; - - /// @dev Thaw request lists - /// Metadata defining linked lists of thaw requests for each delegator (owner). - mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) - internal _delegationThawRequestLists; - - /// @dev Thaw requests - /// Details for each thawing operation in the staking contract for both delegators undelegating to a beneficiary. - mapping(bytes32 thawRequestId => IHorizonStakingTypes.ThawRequest thawRequest) - internal _delegationWithBeneficiaryThawRequests; - - /// @dev Thaw request lists - /// Metadata defining linked lists of thaw requests for each delegator (owner) undelegating to a beneficiary. - mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list))) - internal _delegationWithBeneficiaryThawRequestLists; + /// Metadata defining linked lists of thaw requests for each service provider or delegator (owner) + mapping(IHorizonStakingTypes.ThawRequestType thawRequestType => mapping(address serviceProvider => mapping(address verifier => mapping(address owner => LinkedList.List list)))) + internal _thawRequestLists; /// @dev Operator allow list /// Used for all verifiers except the subgraph data service. diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 4e024f899..b47ac7d7e 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -1852,7 +1852,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address operator, bool legacy ) internal view returns (bool) { - uint256 slotNumber = legacy ? 21 : 35; + uint256 slotNumber = legacy ? 21 : 31; uint256 slot; if (legacy) { @@ -1934,7 +1934,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address verifier, bool legacy ) internal view returns (DelegationPoolInternalTest memory) { - uint256 slotNumber = legacy ? 20 : 37; + uint256 slotNumber = legacy ? 20 : 33; uint256 baseSlot; if (legacy) { baseSlot = uint256(keccak256(abi.encode(serviceProvider, slotNumber))); @@ -1966,7 +1966,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address delegator, bool legacy ) internal view returns (DelegationInternal memory) { - uint256 slotNumber = legacy ? 20 : 37; + uint256 slotNumber = legacy ? 20 : 33; uint256 baseSlot; // DelegationPool From 6e00d1764ded92317f9830d5ec7cc0403ba035d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 9 Dec 2024 15:35:25 -0300 Subject: [PATCH 18/23] docs: fix documentation errors (TRST-R09) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- .../horizon/contracts/staking/HorizonStakingExtension.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 257202d3e..e42930546 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -735,7 +735,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { } /** - * @notice See {IHorizonStakingMain-createProvision}. + * @notice See {IHorizonStakingMain-provision}. */ function _createProvision( address _serviceProvider, diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index ee8a29a22..ac9dc6bc8 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -19,8 +19,8 @@ import { HorizonStakingBase } from "./HorizonStakingBase.sol"; * to the Horizon Staking contract. It allows indexers to close allocations and collect pending query fees, but it * does not allow for the creation of new allocations. This should allow indexers to migrate to a subgraph data service * without losing rewards or having service interruptions. - * @dev TODO: Once the transition period passes this contract can be removed. It's expected the transition period to - * last for a full allocation cycle (28 epochs). + * @dev TODO: Once the transition period passes this contract can be removed (note that an upgrade to the RewardsManager + * will also be required). It's expected the transition period to last for a full allocation cycle (28 epochs). * @custom:security-contact Please email security+contracts@thegraph.com if you find any * bugs. We may have an active bug bounty program. */ From d6d376c27330c022b18d68bb2445ddff7800a5de Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 12 Dec 2024 15:39:28 -0300 Subject: [PATCH 19/23] fix: legacy slashing underflow (TRST-H08) --- .../staking/HorizonStakingExtension.sol | 5 ++-- .../test/staking/slash/legacySlash.t.sol | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index ac9dc6bc8..3b42ebf1a 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/packages/horizon/contracts/staking/HorizonStakingExtension.sol @@ -286,9 +286,8 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension // Slashing more tokens than freely available (over allocation condition) // Unlock locked tokens to avoid the indexer to withdraw them - uint256 tokensAvailable = indexerStake.tokensStaked - - indexerStake.__DEPRECATED_tokensAllocated - - indexerStake.__DEPRECATED_tokensLocked; + uint256 tokensUsed = indexerStake.__DEPRECATED_tokensAllocated + indexerStake.__DEPRECATED_tokensLocked; + uint256 tokensAvailable = tokensUsed > indexerStake.tokensStaked ? 0 : indexerStake.tokensStaked - tokensUsed; if (tokens > tokensAvailable && indexerStake.__DEPRECATED_tokensLocked > 0) { uint256 tokensOverAllocated = tokens - tokensAvailable; uint256 tokensToUnlock = MathUtils.min(tokensOverAllocated, indexerStake.__DEPRECATED_tokensLocked); diff --git a/packages/horizon/test/staking/slash/legacySlash.t.sol b/packages/horizon/test/staking/slash/legacySlash.t.sol index 760978aae..f5595fdac 100644 --- a/packages/horizon/test/staking/slash/legacySlash.t.sol +++ b/packages/horizon/test/staking/slash/legacySlash.t.sol @@ -180,4 +180,29 @@ contract HorizonStakingLegacySlashTest is HorizonStakingTest { vm.expectRevert("!beneficiary"); staking.legacySlash(users.indexer, slashTokens, reward, address(0)); } + + function test_LegacySlash_WhenTokensAllocatedGreaterThanStake() + public + useIndexer + useLegacySlasher(users.legacySlasher) + { + // Setup indexer with: + // - tokensStaked = 1000 GRT + // - tokensAllocated = 800 GRT + // - tokensLocked = 300 GRT + // This means tokensUsed (1100 GRT) > tokensStaked (1000 GRT) + _setIndexer( + users.indexer, + 1000 ether, // tokensStaked + 800 ether, // tokensAllocated + 300 ether, // tokensLocked + 0 // tokensLockedUntil + ); + + // Send tokens manually to staking + token.transfer(address(staking), 1100 ether); + + resetPrank(users.legacySlasher); + _legacySlash(users.indexer, 1000 ether, 500 ether, makeAddr("fisherman")); + } } From 7d90ad28f7302e1eacf9c6850c4d649d2a6436b6 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 12 Dec 2024 17:14:58 -0300 Subject: [PATCH 20/23] fix: underflow in getIdleStake (TRST-L14) --- packages/horizon/contracts/staking/HorizonStakingBase.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index 6745fb2e1..5f2808255 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -268,11 +268,11 @@ abstract contract HorizonStakingBase is * TODO: update the calculation after the transition period. */ function _getIdleStake(address _serviceProvider) internal view returns (uint256) { - return - _serviceProviders[_serviceProvider].tokensStaked - - _serviceProviders[_serviceProvider].tokensProvisioned - - _serviceProviders[_serviceProvider].__DEPRECATED_tokensAllocated - + uint256 tokensUsed = _serviceProviders[_serviceProvider].tokensProvisioned + + _serviceProviders[_serviceProvider].__DEPRECATED_tokensAllocated + _serviceProviders[_serviceProvider].__DEPRECATED_tokensLocked; + uint256 tokensStaked = _serviceProviders[_serviceProvider].tokensStaked; + return tokensStaked > tokensUsed ? tokensStaked - tokensUsed : 0; } /** From 57aea4439978faa2e896e41c799a8e280cba3199 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Thu, 12 Dec 2024 17:39:14 -0300 Subject: [PATCH 21/23] fix: added thaw request type to thaw request fulfilled event (TRST-R15) --- .../internal/IHorizonStakingMain.sol | 5 ++- .../internal/IHorizonStakingTypes.sol | 15 +++++++ .../contracts/staking/HorizonStaking.sol | 41 +++++++++++++++---- .../HorizonStakingShared.t.sol | 9 ++-- 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index dcbe3fce5..6df04cf54 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.27; import { IGraphPayments } from "../../interfaces/IGraphPayments.sol"; +import { IHorizonStakingTypes } from "./IHorizonStakingTypes.sol"; /** * @title Inferface for the {HorizonStaking} contract. @@ -280,13 +281,15 @@ interface IHorizonStakingMain { * @param owner The address of the owner of the thaw requests * @param thawRequestsFulfilled The number of thaw requests fulfilled * @param tokens The total amount of tokens being released + * @param requestType The type of thaw request */ event ThawRequestsFulfilled( address indexed serviceProvider, address indexed verifier, address indexed owner, uint256 thawRequestsFulfilled, - uint256 tokens + uint256 tokens, + IHorizonStakingTypes.ThawRequestType requestType ); // -- Events: governance -- diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol index 0d615a7b6..e2376bf18 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol @@ -180,4 +180,19 @@ interface IHorizonStakingTypes { uint256 nThawRequests; uint256 thawingNonce; } + + /** + * @notice Results of the traversal of thaw requests. + * @dev This struct is used to avoid stack too deep error in the `fulfillThawRequests` function. + * @param requestsFulfilled The number of thaw requests fulfilled + * @param tokensThawed The total amount of tokens thawed + * @param tokensThawing The total amount of tokens thawing + * @param sharesThawing The total amount of shares thawing + */ + struct TraverseThawRequestsResults { + uint256 requestsFulfilled; + uint256 tokensThawed; + uint256 tokensThawing; + uint256 sharesThawing; + } } diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index e42930546..c035125e8 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -1078,8 +1078,33 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { ); require(thawRequestList.count > 0, HorizonStakingNothingThawing()); + TraverseThawRequestsResults memory results = _traverseThawRequests(params, thawRequestList); + + emit ThawRequestsFulfilled( + params.serviceProvider, + params.verifier, + params.owner, + results.requestsFulfilled, + results.tokensThawed, + params.requestType + ); + + return (results.tokensThawed, results.tokensThawing, results.sharesThawing); + } + + /** + * @notice Traverses a thaw request list and fulfills expired thaw requests. + * @param params The parameters for fulfilling thaw requests + * @param thawRequestList The list of thaw requests to traverse + * @return The results of the traversal + */ + function _traverseThawRequests( + FulfillThawRequestsParams memory params, + LinkedList.List storage thawRequestList + ) private returns (TraverseThawRequestsResults memory) { function(bytes32) view returns (bytes32) getNextItem = _getNextThawRequest(params.requestType); function(bytes32) deleteItem = _getDeleteThawRequest(params.requestType); + bytes memory acc = abi.encode( params.requestType, uint256(0), @@ -1099,14 +1124,14 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { data, (ThawRequestType, uint256, uint256, uint256) ); - emit ThawRequestsFulfilled( - params.serviceProvider, - params.verifier, - params.owner, - thawRequestsFulfilled, - tokensThawed - ); - return (tokensThawed, tokensThawing, sharesThawing); + + return + TraverseThawRequestsResults({ + requestsFulfilled: thawRequestsFulfilled, + tokensThawed: tokensThawed, + tokensThawing: tokensThawing, + sharesThawing: sharesThawing + }); } /** diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index b47ac7d7e..3be6633fb 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -504,7 +504,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { verifier, serviceProvider, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + IHorizonStakingTypes.ThawRequestType.Provision ); vm.expectEmit(address(staking)); emit IHorizonStakingMain.TokensDeprovisioned(serviceProvider, verifier, calcValues.tokensThawed); @@ -617,7 +618,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { verifier, serviceProvider, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + IHorizonStakingTypes.ThawRequestType.Provision ); vm.expectEmit(address(staking)); emit IHorizonStakingMain.TokensDeprovisioned(serviceProvider, verifier, calcValues.tokensThawed); @@ -1160,7 +1162,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { params.verifier, msgSender, calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed + calcValues.tokensThawed, + params.thawRequestType ); if (calcValues.tokensThawed != 0) { vm.expectEmit(); From bbd23f5e2ab000816375b31f0e250b886eb74346 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 13 Dec 2024 11:27:37 -0300 Subject: [PATCH 22/23] fix: add minimum tokens amount for undelegate with beneficiary (TRST-H03) --- .../interfaces/internal/IHorizonStakingMain.sol | 7 +++++++ .../horizon/contracts/staking/HorizonStaking.sol | 16 +++++++++++++++- .../test/staking/delegation/undelegate.t.sol | 3 ++- .../test/staking/delegation/withdraw.t.sol | 2 ++ .../test/staking/provision/deprovision.t.sol | 4 ++-- .../horizon/test/staking/provision/thaw.t.sol | 11 +++-------- packages/horizon/test/utils/Constants.sol | 3 ++- 7 files changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 6df04cf54..f9e3969b6 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -433,6 +433,13 @@ interface IHorizonStakingMain { */ error HorizonStakingInsufficientDelegationTokens(uint256 tokens, uint256 minTokens); + /** + * @notice Thrown when the minimum token amount required for delegation with beneficiary is not met. + * @param tokens The actual token amount + * @param minTokens The minimum required token amount + */ + error HorizonStakingInsufficientUndelegationTokens(uint256 tokens, uint256 minTokens); + /** * @notice Thrown when attempting to undelegate with a beneficiary that is the zero address. */ diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index c035125e8..3414fe555 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -36,7 +36,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 private constant FIXED_POINT_PRECISION = 1e18; /// @dev Maximum number of simultaneous stake thaw requests (per provision) or undelegations (per delegation) - uint256 private constant MAX_THAW_REQUESTS = 100; + uint256 private constant MAX_THAW_REQUESTS = 1_000; /// @dev Address of the staking extension contract address private immutable STAKING_EXTENSION_ADDRESS; @@ -44,6 +44,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /// @dev Minimum amount of delegation. uint256 private constant MIN_DELEGATION = 1e18; + /// @dev Minimum amount of undelegation with beneficiary. + uint256 private constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18; + /** * @notice Checks that the caller is authorized to operate over a provision. * @param serviceProvider The address of the service provider. @@ -931,6 +934,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // delegation pool shares -> delegation pool tokens -> thawing pool shares // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0 uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + + // Since anyone can undelegate for any beneficiary, we require a minimum amount to prevent + // malicious actors from flooding the thaw request list with tiny amounts and causing a + // denial of service attack by hitting the MAX_THAW_REQUESTS limit + if (_requestType == ThawRequestType.DelegationWithBeneficiary) { + require( + tokens >= MIN_UNDELEGATION_WITH_BENEFICIARY, + HorizonStakingInsufficientUndelegationTokens(tokens, MIN_UNDELEGATION_WITH_BENEFICIARY) + ); + } + // Thawing shares are rounded down to protect the pool and avoid taking extra tokens from other participants. uint256 thawingShares = pool.tokensThawing == 0 ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing); uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod)); diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index a2d41c061..e23fdadb8 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/packages/horizon/test/staking/delegation/undelegate.t.sol @@ -63,6 +63,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { address beneficiary ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) { vm.assume(beneficiary != address(0)); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); @@ -95,7 +96,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { public useIndexer useProvision(1000 ether, 0, 0) - useDelegation(1000 ether) + useDelegation(10000 ether) { resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index 0d791ff4c..ab286c279 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -167,6 +167,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { { vm.assume(beneficiary != address(0)); vm.assume(beneficiary != address(staking)); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); // Skip beneficiary if balance will overflow vm.assume(token.balanceOf(beneficiary) < type(uint256).max - delegationAmount); @@ -196,6 +197,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { { vm.assume(beneficiary != address(0)); vm.assume(beneficiary != users.delegator); + vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY); // Delegator undelegates to beneficiary resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/provision/deprovision.t.sol b/packages/horizon/test/staking/provision/deprovision.t.sol index ff022e1aa..4fa97da6c 100644 --- a/packages/horizon/test/staking/provision/deprovision.t.sol +++ b/packages/horizon/test/staking/provision/deprovision.t.sol @@ -17,7 +17,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest { uint256 thawCount, uint256 deprovisionCount ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { - thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 1, 100); deprovisionCount = bound(deprovisionCount, 0, thawCount); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; @@ -37,7 +37,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest { uint64 thawingPeriod, uint256 thawCount ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) { - thawCount = bound(thawCount, 2, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 2, 100); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; diff --git a/packages/horizon/test/staking/provision/thaw.t.sol b/packages/horizon/test/staking/provision/thaw.t.sol index c69792f50..eb58e8e86 100644 --- a/packages/horizon/test/staking/provision/thaw.t.sol +++ b/packages/horizon/test/staking/provision/thaw.t.sol @@ -26,7 +26,7 @@ contract HorizonStakingThawTest is HorizonStakingTest { uint64 thawingPeriod, uint256 thawCount ) public useIndexer useProvision(amount, 0, thawingPeriod) { - thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS); + thawCount = bound(thawCount, 1, 100); vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step uint256 individualThawAmount = amount / thawCount; @@ -72,13 +72,8 @@ contract HorizonStakingThawTest is HorizonStakingTest { staking.thaw(users.indexer, subgraphDataServiceAddress, thawAmount); } - function testThaw_RevertWhen_OverMaxThawRequests( - uint256 amount, - uint64 thawingPeriod, - uint256 thawAmount - ) public useIndexer useProvision(amount, 0, thawingPeriod) { - vm.assume(amount >= MAX_THAW_REQUESTS + 1); - thawAmount = bound(thawAmount, 1, amount / (MAX_THAW_REQUESTS + 1)); + function testThaw_RevertWhen_OverMaxThawRequests() public useIndexer useProvision(10000 ether, 0, 0) { + uint256 thawAmount = 1 ether; for (uint256 i = 0; i < MAX_THAW_REQUESTS; i++) { _thaw(users.indexer, subgraphDataServiceAddress, thawAmount); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index 9af54087d..d96c39202 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -11,10 +11,11 @@ abstract contract Constants { // GraphPayments parameters uint256 internal constant protocolPaymentCut = 10000; // Staking constants - uint256 internal constant MAX_THAW_REQUESTS = 100; + uint256 internal constant MAX_THAW_REQUESTS = 1_000; uint64 internal constant MAX_THAWING_PERIOD = 28 days; uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300; uint256 internal constant MIN_DELEGATION = 1e18; + uint256 internal constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18; // Epoch manager uint256 internal constant EPOCH_LENGTH = 1; // Rewards manager From 399b7a91f89248d95a224dc5369286b3f39ec590 Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 13 Dec 2024 15:46:25 -0300 Subject: [PATCH 23/23] fix: natspec for new undelegate error --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index f9e3969b6..50ca90fc3 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -434,7 +434,7 @@ interface IHorizonStakingMain { error HorizonStakingInsufficientDelegationTokens(uint256 tokens, uint256 minTokens); /** - * @notice Thrown when the minimum token amount required for delegation with beneficiary is not met. + * @notice Thrown when the minimum token amount required for undelegation with beneficiary is not met. * @param tokens The actual token amount * @param minTokens The minimum required token amount */