Merge pull request #977 from graphprotocol/mde/fix-zero-tokens-withdr…

…aw-delegation Horizon fix: withdraw delegation with zero tokens
graphprotocol · May 29, 2024 · 6093c53 · 6093c53
2 parents 75d3cb9 + c212cbe
commit 6093c53
Showing 3 changed files with 82 additions and 5 deletions.
diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol
@@ -795,10 +795,12 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         pool.sharesThawing = sharesThawing;
         pool.tokensThawing = tokensThawing;
 
-        if (_newServiceProvider != address(0)) {
-            _delegate(_newServiceProvider, _verifier, tokensThawed, _minSharesForNewProvider);
-        } else {
-            _graphToken().pushTokens(msg.sender, tokensThawed);
+        if (tokensThawed != 0) {
+            if (_newServiceProvider != address(0)) {
+                _delegate(_newServiceProvider, _verifier, tokensThawed, _minSharesForNewProvider);
+            } else {
+                _graphToken().pushTokens(msg.sender, tokensThawed);
+            }
         }
 
         emit DelegatedTokensWithdrawn(_serviceProvider, _verifier, msg.sender, tokensThawed);

diff --git a/packages/horizon/test/GraphBase.t.sol b/packages/horizon/test/GraphBase.t.sol
@@ -161,7 +161,7 @@ abstract contract GraphBaseTest is Test, Constants {
         controller.setPaused(false);
     }
 
-    function createUser(string memory name) private returns (address) {
+    function createUser(string memory name) internal returns (address) {
         address user = makeAddr(name);
         vm.deal({ account: user, newBalance: 100 ether });
         deal({ token: address(token), to: user, give: type(uint256).max });

diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol
@@ -36,6 +36,18 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
         return (thawRequest.shares * pool.tokensThawing) / pool.sharesThawing;
     }
 
+    function _setupNewIndexer(uint256 tokens) private returns(address) {
+        address msgSender;
+        (, msgSender,) = vm.readCallers();
+        address newIndexer = createUser("newIndexer");
+        vm.startPrank(newIndexer);
+        token.approve(address(staking), tokens);
+        staking.stakeTo(newIndexer, tokens);
+        staking.provision(newIndexer,subgraphDataServiceAddress, tokens, 0, MAX_THAWING_PERIOD);
+        vm.startPrank(msgSender);
+        return newIndexer;
+    }
+
     function testWithdrawDelegation_Tokens(
         uint256 delegationAmount,
         uint256 withdrawShares
@@ -71,4 +83,67 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
         vm.expectRevert(expectedError);
         _withdrawDelegated();
     }
+
+    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);
+
+        uint256 previousBalance = token.balanceOf(users.delegator);
+        staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, newIndexer, 0, 0);
+
+        uint256 newBalance = token.balanceOf(users.delegator);
+        assertEq(newBalance, previousBalance);
+
+        uint256 delegatedTokens = staking.getDelegatedTokensAvailable(newIndexer, subgraphDataServiceAddress);
+        assertEq(delegatedTokens, delegationAmount);
+    }
+
+    function testWithdrawDelegation_ZeroTokens(
+        uint256 delegationAmount
+    )
+        public
+        useIndexer
+        useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD)
+        useDelegation(delegationAmount)
+        useUndelegate(delegationAmount)
+    {
+        uint256 previousBalance = token.balanceOf(users.delegator);
+        _withdrawDelegated();
+
+        // Nothing changed since thawing period haven'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);
+        staking.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);
+    }
 }