fix: move legacyWithdrawDelegated to withdrawDelegated (TRST-H07)

Maikol · Maikol · commit d9c61907cb74 · 2024-12-06T11:41:16.000-03:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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(

Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {`
`87`	`87`
`88`	`88`	`skip(thawRequest.thawingUntil + 1);`
`89`	`89`
`90`		`- _withdrawDelegated(users.indexer, address(0));`
	`90`	`+ _withdrawDelegated(users.indexer, subgraphDataServiceLegacyAddress, 0);`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`function testWithdrawDelegation_RevertWhen_InvalidPool(`