fix: only add to delegation pool if shares is greater than 0 (#1031)

graphprotocol · Sep 19, 2024 · 4354737 · 4354737
1 parent 613d7f3
commit 4354737
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 6 deletions.
diff --git a/packages/subgraph-service/contracts/utilities/AllocationManager.sol b/packages/subgraph-service/contracts/utilities/AllocationManager.sol
@@ -3,6 +3,7 @@ pragma solidity 0.8.27;
 
 import { IGraphPayments } from "@graphprotocol/horizon/contracts/interfaces/IGraphPayments.sol";
 import { IGraphToken } from "@graphprotocol/contracts/contracts/token/IGraphToken.sol";
+import { IHorizonStakingTypes } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol";
 
 import { GraphDirectory } from "@graphprotocol/horizon/contracts/utilities/GraphDirectory.sol";
 import { AllocationManagerV1Storage } from "./AllocationManagerStorage.sol";
@@ -289,7 +290,12 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
                 address(this),
                 IGraphPayments.PaymentTypes.IndexingFee
             );
-            tokensDelegationRewards = tokensRewards.mulPPM(delegatorCut);
+            IHorizonStakingTypes.DelegationPool memory delegationPool = _graphStaking().getDelegationPool(
+                allocation.indexer,
+                address(this)
+            );
+            // If delegation pool has no shares then we don't need to distribute rewards to delegators
+            tokensDelegationRewards = delegationPool.shares > 0 ? tokensRewards.mulPPM(delegatorCut) : 0;
             if (tokensDelegationRewards > 0) {
                 _graphToken().approve(address(_graphStaking()), tokensDelegationRewards);
                 _graphStaking().addToDelegationPool(allocation.indexer, address(this), tokensDelegationRewards);

diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol
@@ -11,6 +11,7 @@ import { ITAPCollector } from "@graphprotocol/horizon/contracts/interfaces/ITAPC
 import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import { LinkedList } from "@graphprotocol/horizon/contracts/libraries/LinkedList.sol";
 import { IDataServiceFees } from "@graphprotocol/horizon/contracts/data-service/interfaces/IDataServiceFees.sol";
+import { IHorizonStakingTypes } from "@graphprotocol/horizon/contracts/interfaces/internal/IHorizonStakingTypes.sol";
 
 import { Allocation } from "../../contracts/libraries/Allocation.sol";
 import { AllocationManager } from "../../contracts/utilities/AllocationManager.sol";
@@ -255,7 +256,8 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
                 // TODO: this should be fixed in AllocationManager, it should be IndexingRewards instead
                 IGraphPayments.PaymentTypes.IndexingFee
             );
-            indexingRewardsData.tokensDelegationRewards = paymentCollected.mulPPM(delegatorCut);
+            IHorizonStakingTypes.DelegationPool memory delegationPool = staking.getDelegationPool(allocation.indexer, address(subgraphService));
+            indexingRewardsData.tokensDelegationRewards = delegationPool.shares > 0 ? paymentCollected.mulPPM(delegatorCut) : 0;
             indexingRewardsData.tokensIndexerRewards = paymentCollected - indexingRewardsData.tokensDelegationRewards;
 
             vm.expectEmit(address(subgraphService));
@@ -322,7 +324,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
             assertEq(stakeClaim.createdAt, block.timestamp);
             assertEq(stakeClaim.releaseAt, block.timestamp + disputePeriod);
             assertEq(stakeClaim.nextClaim, bytes32(0));
-        } else {
+        } else if (_paymentType == IGraphPayments.PaymentTypes.IndexingRewards) {
             // Update allocation after collecting rewards
             allocation = subgraphService.getAllocation(allocationId);
 

diff --git a/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol b/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol
@@ -27,14 +27,38 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
         uint256 delegationTokens,
         uint256 delegationFeeCut
     ) public useIndexer useAllocation(tokens) useDelegation(delegationTokens) {
+        // Collect reverts if delegationFeeCut is 100%
+        delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM - 1);
+        _setDelegationFeeCut(
+            users.indexer,
+            address(subgraphService),
+            // TODO: this should be fixed in AllocationManager, it should be IndexingRewards instead
+            IGraphPayments.PaymentTypes.IndexingFee,
+            delegationFeeCut
+        );
+        IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
+        bytes memory data = abi.encode(allocationID, bytes32("POI"));
+        _collect(users.indexer, paymentType, data);
+    }
+
+    function test_SubgraphService_Collect_Indexing_AfterUndelegate(
+        uint256 tokens,
+        uint256 delegationTokens,
+        uint256 delegationFeeCut
+    ) public useIndexer useAllocation(tokens) useDelegation(delegationTokens) {
+        // Collect reverts if delegationFeeCut is 100%
         delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM);
         _setDelegationFeeCut(
             users.indexer,
             address(subgraphService),
             // TODO: this should be fixed in AllocationManager, it should be IndexingRewards instead
             IGraphPayments.PaymentTypes.IndexingFee,
-            100_000
+            delegationFeeCut
         );
+        // Undelegate
+        resetPrank(users.delegator);
+        staking.undelegate(users.indexer, address(subgraphService), delegationTokens);
+        resetPrank(users.indexer);
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
         _collect(users.indexer, paymentType, data);
@@ -68,13 +92,14 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
         uint256 delegationTokens,
         uint256 delegationFeeCut
     ) public useIndexer useAllocation(tokens) useDelegation(delegationTokens) {
-        delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM);
+        // Collect reverts if delegationFeeCut is 100%
+        delegationFeeCut = bound(delegationFeeCut, 0, MAX_PPM - 1);
         _setDelegationFeeCut(
             users.indexer,
             address(subgraphService),
             // TODO: this should be fixed in AllocationManager, it should be IndexingRewards instead
             IGraphPayments.PaymentTypes.IndexingFee,
-            100_000
+            delegationFeeCut
         );
 
         uint8 numberOfPOIs = 20;