diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 210146bf4..b2786af68 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -413,6 +413,16 @@ interface IHorizonStakingMain { */ error HorizonStakingInvalidBeneficiaryZeroAddress(); + /** + * @notice Thrown when attempting to redelegate with a serivce provider that is the zero address. + */ + error HorizonStakingInvalidServiceProviderZeroAddress(); + + /** + * @notice Thrown when attempting to redelegate with a verifier that is the zero address. + */ + error HorizonStakingInvalidVerifierZeroAddress(); + // -- Errors: thaw requests -- error HorizonStakingNothingThawing(); @@ -739,26 +749,44 @@ interface IHorizonStakingMain { /** * @notice Withdraw undelegated tokens from a provision after thawing. - * Tokens can be automatically re-delegated to another provision by setting `newServiceProvider`. * @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. * * Requirements: * - Must have previously initiated a thaw request using {undelegate}. - * - `newServiceProvider` must either be zero address or have previously provisioned stake to `verifier`. * * Emits {ThawRequestFulfilled}, {ThawRequestsFulfilled} and {DelegatedTokensWithdrawn} events. * * @param serviceProvider The service provider address * @param verifier The verifier address - * @param newServiceProvider The address of a new service provider, if the delegator wants to re-delegate + * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. + */ + function withdrawDelegated(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 + * requests in the event that fulfilling all of them results in a gas limit error. + * + * Requirements: + * - Must have previously initiated a thaw request using {undelegate}. + * - `newServiceProvider` and `newVerifier` must not be the zero address. + * - `newServiceProvider` must have previously provisioned stake to `newVerifier`. + * + * Emits {ThawRequestFulfilled}, {ThawRequestsFulfilled} and {DelegatedTokensWithdrawn} events. + * + * @param oldServiceProvider The old service provider address + * @param oldVerifier The old verifier address + * @param newServiceProvider The address of a new service provider + * @param newVerifier The address of a new verifier * @param minSharesForNewProvider The minimum amount of shares to accept for the new service provider * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. */ - function withdrawDelegated( - address serviceProvider, - address verifier, + function redelegate( + address oldServiceProvider, + address oldVerifier, address newServiceProvider, + address newVerifier, uint256 minSharesForNewProvider, uint256 nThawRequests ) external; diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index a00cda8af..36429126a 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -319,11 +319,32 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { function withdrawDelegated( address serviceProvider, address verifier, + uint256 nThawRequests + ) external override notPaused { + _withdrawDelegated(serviceProvider, verifier, address(0), address(0), 0, nThawRequests); + } + + /** + * @notice See {IHorizonStakingMain-redelegate}. + */ + function redelegate( + address oldServiceProvider, + address oldVerifier, address newServiceProvider, + address newVerifier, uint256 minSharesForNewProvider, uint256 nThawRequests ) external override notPaused { - _withdrawDelegated(serviceProvider, verifier, newServiceProvider, minSharesForNewProvider, nThawRequests); + require(newServiceProvider != address(0), HorizonStakingInvalidServiceProviderZeroAddress()); + require(newVerifier != address(0), HorizonStakingInvalidVerifierZeroAddress()); + _withdrawDelegated( + oldServiceProvider, + oldVerifier, + newServiceProvider, + newVerifier, + minSharesForNewProvider, + nThawRequests + ); } /** @@ -360,7 +381,14 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @notice See {IHorizonStakingMain-withdrawDelegated}. */ function withdrawDelegated(address serviceProvider, address newServiceProvider) external override notPaused { - _withdrawDelegated(serviceProvider, SUBGRAPH_DATA_SERVICE_ADDRESS, newServiceProvider, 0, 0); + _withdrawDelegated( + serviceProvider, + SUBGRAPH_DATA_SERVICE_ADDRESS, + newServiceProvider, + SUBGRAPH_DATA_SERVICE_ADDRESS, + 0, + 0 + ); } /* @@ -805,6 +833,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address _serviceProvider, address _verifier, address _newServiceProvider, + address _newVerifier, uint256 _minSharesForNewProvider, uint256 _nThawRequests ) private { @@ -828,8 +857,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { pool.tokensThawing = tokensThawing; if (tokensThawed != 0) { - if (_newServiceProvider != address(0)) { - _delegate(_newServiceProvider, _verifier, tokensThawed, _minSharesForNewProvider); + if (_newServiceProvider != address(0) && _newVerifier != address(0)) { + _delegate(_newServiceProvider, _newVerifier, tokensThawed, _minSharesForNewProvider); } else { _graphToken().pushTokens(msg.sender, tokensThawed); } diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index f9796cf94..6cd42adf3 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -951,9 +951,26 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } function _withdrawDelegated( + address serviceProvider, + address verifier, + uint256 nThawRequests + ) internal { + __withdrawDelegated( + serviceProvider, + verifier, + address(0), + address(0), + 0, + nThawRequests, + false + ); + } + + function _redelegate( address serviceProvider, address verifier, address newServiceProvider, + address newVerifier, uint256 minSharesForNewProvider, uint256 nThawRequests ) internal { @@ -961,6 +978,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { serviceProvider, verifier, newServiceProvider, + newVerifier, minSharesForNewProvider, nThawRequests, false @@ -968,7 +986,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } function _withdrawDelegated(address serviceProvider, address newServiceProvider) internal { - __withdrawDelegated(serviceProvider, subgraphDataServiceLegacyAddress, newServiceProvider, 0, 0, true); + __withdrawDelegated(serviceProvider, subgraphDataServiceLegacyAddress, newServiceProvider, subgraphDataServiceLegacyAddress, 0, 0, true); } struct BeforeValues_WithdrawDelegated { @@ -991,19 +1009,20 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { address _serviceProvider, address _verifier, address _newServiceProvider, + address _newVerifier, uint256 _minSharesForNewProvider, uint256 _nThawRequests, bool legacy ) private { (, address msgSender, ) = vm.readCallers(); - bool reDelegate = _newServiceProvider != address(0); + bool reDelegate = _newServiceProvider != address(0) && _newVerifier != address(0); // before BeforeValues_WithdrawDelegated memory beforeValues; beforeValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - beforeValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _verifier, legacy); - beforeValues.newDelegation = _getStorage_Delegation(_serviceProvider, _verifier, msgSender, legacy); + beforeValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); + beforeValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); beforeValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); beforeValues.senderBalance = token.balanceOf(msgSender); beforeValues.stakingBalance = token.balanceOf(address(staking)); @@ -1038,26 +1057,33 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { if (calcValues.tokensThawed != 0) { vm.expectEmit(); if (reDelegate) { - emit IHorizonStakingMain.TokensDelegated(_newServiceProvider, _verifier, msgSender, calcValues.tokensThawed); + emit IHorizonStakingMain.TokensDelegated(_newServiceProvider, _newVerifier, msgSender, calcValues.tokensThawed); } else { emit Transfer(address(staking), msgSender, calcValues.tokensThawed); } } vm.expectEmit(); emit IHorizonStakingMain.DelegatedTokensWithdrawn(_serviceProvider, _verifier, msgSender, calcValues.tokensThawed); - staking.withdrawDelegated( - _serviceProvider, - _verifier, - _newServiceProvider, - _minSharesForNewProvider, - _nThawRequests - ); + if (legacy) { + staking.withdrawDelegated(_serviceProvider, _newServiceProvider); + } else if (reDelegate) { + staking.redelegate( + _serviceProvider, + _verifier, + _newServiceProvider, + _newVerifier, + _minSharesForNewProvider, + _nThawRequests + ); + } else { + staking.withdrawDelegated(_serviceProvider, _verifier, _nThawRequests); + } // after AfterValues_WithdrawDelegated memory afterValues; afterValues.pool = _getStorage_DelegationPoolInternal(_serviceProvider, _verifier, legacy); - afterValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _verifier, legacy); - afterValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _verifier, msgSender, legacy); + afterValues.newPool = _getStorage_DelegationPoolInternal(_newServiceProvider, _newVerifier, legacy); + afterValues.newDelegation = _getStorage_Delegation(_newServiceProvider, _newVerifier, msgSender, legacy); afterValues.thawRequestList = staking.getThawRequestList(_serviceProvider, _verifier, msgSender); afterValues.senderBalance = token.balanceOf(msgSender); afterValues.stakingBalance = token.balanceOf(address(staking)); diff --git a/packages/horizon/test/staking/delegation/redelegate.t.sol b/packages/horizon/test/staking/delegation/redelegate.t.sol new file mode 100644 index 000000000..9aa2cdcd6 --- /dev/null +++ b/packages/horizon/test/staking/delegation/redelegate.t.sol @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import "forge-std/Test.sol"; + +import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.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); + _; + } + + /* + * HELPERS + */ + + function _setupNewIndexer(uint256 tokens) private returns(address) { + (, address msgSender,) = vm.readCallers(); + + address newIndexer = createUser("newIndexer"); + vm.startPrank(newIndexer); + _createProvision(newIndexer, subgraphDataServiceAddress, tokens, 0, MAX_THAWING_PERIOD); + + vm.startPrank(msgSender); + return newIndexer; + } + + function _setupNewIndexerAndVerifier(uint256 tokens) private returns(address, address) { + (, address msgSender,) = vm.readCallers(); + + address newIndexer = createUser("newIndexer"); + address newVerifier = makeAddr("newVerifier"); + vm.startPrank(newIndexer); + _createProvision(newIndexer, newVerifier, tokens, 0, MAX_THAWING_PERIOD); + + vm.startPrank(msgSender); + return (newIndexer, newVerifier); + } + + /* + * TESTS + */ + + function testRedelegate_MoveToNewServiceProvider( + uint256 delegationAmount, + uint256 withdrawShares + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(withdrawShares) + { + skip(MAX_THAWING_PERIOD + 1); + + // Setup new service provider + address newIndexer = _setupNewIndexer(10_000_000 ether); + _redelegate(users.indexer, subgraphDataServiceAddress, newIndexer, subgraphDataServiceAddress, 0, 0); + } + + function testRedelegate_MoveToNewServiceProviderAndNewVerifier( + uint256 delegationAmount, + uint256 withdrawShares + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(withdrawShares) + { + skip(MAX_THAWING_PERIOD + 1); + + // Setup new service provider + (address newIndexer, address newVerifier) = _setupNewIndexerAndVerifier(10_000_000 ether); + _redelegate(users.indexer, subgraphDataServiceAddress, newIndexer, newVerifier, 0, 0); + } + + function testRedelegate_RevertWhen_VerifierZeroAddress( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(delegationAmount) + { + skip(MAX_THAWING_PERIOD + 1); + + // Setup new service provider + address newIndexer = _setupNewIndexer(10_000_000 ether); + vm.expectRevert(abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingInvalidVerifierZeroAddress.selector)); + staking.redelegate(users.indexer, subgraphDataServiceAddress, newIndexer, address(0), 0, 0); + } + + function testRedelegate_RevertWhen_ServiceProviderZeroAddress( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(delegationAmount) + { + skip(MAX_THAWING_PERIOD + 1); + + // Setup new verifier + address newVerifier = makeAddr("newVerifier"); + vm.expectRevert(abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingInvalidServiceProviderZeroAddress.selector)); + staking.redelegate(users.indexer, subgraphDataServiceAddress, address(0), newVerifier, 0, 0); + } + + function testRedelegate_MoveZeroTokensToNewServiceProviderAndVerifier( + uint256 delegationAmount, + uint256 withdrawShares + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(withdrawShares) + { + // Setup new service provider + (address newIndexer, address newVerifier) = _setupNewIndexerAndVerifier(10_000_000 ether); + + uint256 previousBalance = token.balanceOf(users.delegator); + _redelegate(users.indexer, subgraphDataServiceAddress, newIndexer, newVerifier, 0, 0); + + uint256 newBalance = token.balanceOf(users.delegator); + assertEq(newBalance, previousBalance); + + uint256 delegatedTokens = staking.getDelegatedTokensAvailable(newIndexer, newVerifier); + assertEq(delegatedTokens, 0); + } +} \ No newline at end of file diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index 866f49942..59559d0a4 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -15,8 +15,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { */ modifier useUndelegate(uint256 shares) { - vm.stopPrank(); - vm.startPrank(users.delegator); + resetPrank(users.delegator); DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); shares = bound(shares, 1, delegation.shares); @@ -24,20 +23,6 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { _; } - /* - * HELPERS - */ - function _setupNewIndexer(uint256 tokens) private returns(address) { - (, address msgSender,) = vm.readCallers(); - - address newIndexer = createUser("newIndexer"); - vm.startPrank(newIndexer); - _createProvision(newIndexer, subgraphDataServiceAddress, tokens, 0, MAX_THAWING_PERIOD); - - vm.startPrank(msgSender); - return newIndexer; - } - /* * TESTS */ @@ -57,7 +42,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { skip(thawRequest.thawingUntil + 1); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 0); + _withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); } function testWithdrawDelegation_RevertWhen_NotThawing( @@ -70,23 +55,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { { bytes memory expectedError = abi.encodeWithSignature("HorizonStakingNothingThawing()"); vm.expectRevert(expectedError); - staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 0); - } - - function testWithdrawDelegation_MoveToNewServiceProvider( - uint256 delegationAmount - ) - public - useIndexer - useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) - useDelegation(delegationAmount) - useUndelegate(delegationAmount) - { - skip(MAX_THAWING_PERIOD + 1); - - // Setup new service provider - address newIndexer = _setupNewIndexer(10_000_000 ether); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, newIndexer, 0, 0); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); } function testWithdrawDelegation_ZeroTokens( @@ -99,35 +68,13 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { useUndelegate(delegationAmount) { uint256 previousBalance = token.balanceOf(users.delegator); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 0); + _withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); - // Nothing changed since thawing period haven't finished + // Nothing changed since thawing period hasn't finished uint256 newBalance = token.balanceOf(users.delegator); assertEq(newBalance, previousBalance); } - function testWithdrawDelegation_MoveZeroTokensToNewServiceProvider( - uint256 delegationAmount - ) - public - useIndexer - useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) - useDelegation(delegationAmount) - useUndelegate(delegationAmount) - { - // Setup new service provider - address newIndexer = _setupNewIndexer(10_000_000 ether); - - uint256 previousBalance = token.balanceOf(users.delegator); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, newIndexer, 0, 0); - - uint256 newBalance = token.balanceOf(users.delegator); - assertEq(newBalance, previousBalance); - - uint256 delegatedTokens = staking.getDelegatedTokensAvailable(newIndexer, subgraphDataServiceAddress); - assertEq(delegatedTokens, 0); - } - function testWithdrawDelegation_LegacySubgraphService(uint256 delegationAmount) public useIndexer { delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, 10_000_000 ether, 0, MAX_THAWING_PERIOD); @@ -166,7 +113,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { users.indexer, subgraphDataServiceAddress )); - staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 0); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); } function testWithdrawDelegation_WithBeneficiary( @@ -192,7 +139,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { // Beneficiary withdraws delegated tokens resetPrank(beneficiary); - _withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 1); + _withdrawDelegated(users.indexer, subgraphDataServiceAddress, 1); } function testWithdrawDelegation_RevertWhen_PreviousOwnerAttemptsToWithdraw( @@ -219,6 +166,6 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { // Delegator attempts to withdraw delegated tokens, should revert since beneficiary is the thaw request owner bytes memory expectedError = abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingNothingThawing.selector); vm.expectRevert(expectedError); - staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, address(0), 0, 1); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 1); } } \ No newline at end of file