feat: dont issue rewards to allos less than one epoch old

tmigone · tmigone · commit 01db6e5b8ee8 · 2025-02-10T16:25:31.000-03:00
Signed-off-by: Tomás Migone &lt;tomas@edgeandnode.com&gt;
diff --git a/packages/subgraph-service/contracts/libraries/Allocation.sol b/packages/subgraph-service/contracts/libraries/Allocation.sol
@@ -30,6 +30,8 @@ library Allocation {
         uint256 accRewardsPerAllocatedToken;
         // Accumulated rewards that are pending to be claimed due allocation resize
         uint256 accRewardsPending;
+        // Epoch when the allocation was created
+        uint256 createdAtEpoch;
     }
 
     /**
@@ -68,7 +70,8 @@ library Allocation {
         address allocationId,
         bytes32 subgraphDeploymentId,
         uint256 tokens,
-        uint256 accRewardsPerAllocatedToken
+        uint256 accRewardsPerAllocatedToken,
+        uint256 createdAtEpoch
     ) internal returns (State memory) {
         require(!self[allocationId].exists(), AllocationAlreadyExists(allocationId));
 
@@ -80,7 +83,8 @@ library Allocation {
             closedAt: 0,
             lastPOIPresentedAt: 0,
             accRewardsPerAllocatedToken: accRewardsPerAllocatedToken,
-            accRewardsPending: 0
+            accRewardsPending: 0,
+            createdAtEpoch: createdAtEpoch
         });
 
         self[allocationId] = allocation;
diff --git a/packages/subgraph-service/contracts/utilities/AllocationManager.sol b/packages/subgraph-service/contracts/utilities/AllocationManager.sol
@@ -40,12 +40,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
      * @param allocationId The id of the allocation
      * @param subgraphDeploymentId The id of the subgraph deployment
      * @param tokens The amount of tokens allocated
+     * @param currentEpoch The current epoch
      */
     event AllocationCreated(
         address indexed indexer,
         address indexed allocationId,
         bytes32 indexed subgraphDeploymentId,
-        uint256 tokens
+        uint256 tokens,
+        uint256 currentEpoch
     );
 
     /**
@@ -157,17 +159,14 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
     // solhint-disable-next-line func-name-mixedcase
     function __AllocationManager_init(string memory _name, string memory _version) internal onlyInitializing {
         __EIP712_init(_name, _version);
-        __AllocationManager_init_unchained(_name, _version);
+        __AllocationManager_init_unchained();
     }
 
     /**
      * @notice Initializes the contract
      */
     // solhint-disable-next-line func-name-mixedcase
-    function __AllocationManager_init_unchained(
-        string memory _name,
-        string memory _version
-    ) internal onlyInitializing {}
+    function __AllocationManager_init_unchained() internal onlyInitializing {}
 
     /**
      * @notice Imports a legacy allocation id into the subgraph service
@@ -213,12 +212,15 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
         // Ensure allocation id is not reused
         // need to check both subgraph service (on allocations.create()) and legacy allocations
         _legacyAllocations.revertIfExists(_graphStaking(), _allocationId);
+
+        uint256 currentEpoch = _graphEpochManager().currentEpoch();
         Allocation.State memory allocation = _allocations.create(
             _indexer,
             _allocationId,
             _subgraphDeploymentId,
             _tokens,
-            _graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId)
+            _graphRewardsManager().onSubgraphAllocationUpdate(_subgraphDeploymentId),
+            currentEpoch
         );
 
         // Check that the indexer has enough tokens available
@@ -230,23 +232,28 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
             _subgraphAllocatedTokens[allocation.subgraphDeploymentId] +
             allocation.tokens;
 
-        emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens);
+        emit AllocationCreated(_indexer, _allocationId, _subgraphDeploymentId, allocation.tokens, currentEpoch);
         return allocation;
     }
 
     /**
      * @notice Present a POI to collect indexing rewards for an allocation
      * This function will mint indexing rewards using the {RewardsManager} and distribute them to the indexer and delegators.
      *
-     * To qualify for indexing rewards:
+     * Conditions to qualify for indexing rewards:
      * - POI must be non-zero
      * - POI must not be stale, i.e: older than `maxPOIStaleness`
      * - allocation must not be altruistic (allocated tokens = 0)
+     * - allocation must be open for at least one epoch
      *
      * Note that indexers are required to periodically (at most every `maxPOIStaleness`) present POIs to collect rewards.
      * Rewards will not be issued to stale POIs, which means that indexers are advised to present a zero POI if they are
      * unable to present a valid one to prevent being locked out of future rewards.
      *
+     * Note on allocation duration restriction: this is required to ensure that non protocol chains have a valid block number for
+     * which to calculate POIs. EBO posts once per epoch typically at each epoch change, so we restrict rewards to allocations
+     * that have gone through at least one epoch change.
+     *
      * Emits a {IndexingRewardsCollected} event.
      *
      * @param _allocationId The id of the allocation to collect rewards for
@@ -260,10 +267,12 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
         Allocation.State memory allocation = _allocations.get(_allocationId);
         require(allocation.isOpen(), AllocationManagerAllocationClosed(_allocationId));
 
+        uint256 currentEpoch = _graphEpochManager().currentEpoch();
+
         // Mint indexing rewards if all conditions are met
         uint256 tokensRewards = (!allocation.isStale(maxPOIStaleness) &&
             !allocation.isAltruistic() &&
-            _poi != bytes32(0))
+            _poi != bytes32(0)) && currentEpoch > allocation.createdAtEpoch
             ? _graphRewardsManager().takeRewards(_allocationId)
             : 0;
 
@@ -318,7 +327,7 @@ abstract contract AllocationManager is EIP712Upgradeable, GraphDirectory, Alloca
             tokensIndexerRewards,
             tokensDelegationRewards,
             _poi,
-            _graphEpochManager().currentEpoch()
+            currentEpoch
         );
 
         // Check if the indexer is over-allocated and close the allocation if necessary
diff --git a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol
@@ -11,7 +11,6 @@ import { ISubgraphService } from "../../contracts/interfaces/ISubgraphService.so
 import { HorizonStakingSharedTest } from "./HorizonStakingShared.t.sol";
 
 abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
-
     /*
      * VARIABLES
      */
@@ -24,7 +23,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
      * MODIFIERS
      */
 
-    modifier useIndexer {
+    modifier useIndexer() {
         vm.startPrank(users.indexer);
         _;
         vm.stopPrank();
@@ -35,15 +34,20 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
         vm.assume(tokens < 10_000_000_000 ether);
         _createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
         _register(users.indexer, abi.encode("url", "geoHash", address(0)));
-        bytes memory data = _createSubgraphAllocationData(users.indexer, subgraphDeployment, allocationIDPrivateKey, tokens);
+        bytes memory data = _createSubgraphAllocationData(
+            users.indexer,
+            subgraphDeployment,
+            allocationIDPrivateKey,
+            tokens
+        );
         _startService(users.indexer, data);
         _;
     }
 
     modifier useDelegation(uint256 tokens) {
         vm.assume(tokens > MIN_DELEGATION);
         vm.assume(tokens < 10_000_000_000 ether);
-        (, address msgSender,) = vm.readCallers();
+        (, address msgSender, ) = vm.readCallers();
         resetPrank(users.delegator);
         token.approve(address(staking), tokens);
         _delegate(users.indexer, address(subgraphService), tokens, 0);
@@ -72,10 +76,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
         );
 
         vm.expectEmit(address(subgraphService));
-        emit IDataService.ServiceProviderRegistered(
-            _indexer,
-            _data
-        );
+        emit IDataService.ServiceProviderRegistered(_indexer, _data);
 
         // Register indexer
         subgraphService.register(_indexer, _data);
@@ -91,14 +92,22 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
     }
 
     function _startService(address _indexer, bytes memory _data) internal {
-        (bytes32 subgraphDeploymentId, uint256 tokens, address allocationId,) = abi.decode(
+        (bytes32 subgraphDeploymentId, uint256 tokens, address allocationId, ) = abi.decode(
             _data,
             (bytes32, uint256, address, bytes)
         );
         uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(subgraphDeploymentId);
+        uint256 currentEpoch = epochManager.currentEpoch();
 
         vm.expectEmit(address(subgraphService));
         emit IDataService.ServiceStarted(_indexer, _data);
+        emit AllocationManager.AllocationCreated(
+            _indexer,
+            allocationId,
+            subgraphDeploymentId,
+            tokens,
+            currentEpoch
+        );
 
         // TODO: improve this
         uint256 accRewardsPerAllocatedToken = 0;
@@ -116,9 +125,10 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
         assertEq(allocation.subgraphDeploymentId, subgraphDeploymentId);
         assertEq(allocation.createdAt, block.timestamp);
         assertEq(allocation.closedAt, 0);
-        assertEq(allocation.lastPOIPresentedAt, 0);        
+        assertEq(allocation.lastPOIPresentedAt, 0);
         assertEq(allocation.accRewardsPerAllocatedToken, accRewardsPerAllocatedToken);
         assertEq(allocation.accRewardsPending, 0);
+        assertEq(allocation.createdAtEpoch, currentEpoch);
 
         // Check subgraph deployment allocated tokens
         uint256 subgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(subgraphDeploymentId);
@@ -130,10 +140,17 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
         assertTrue(subgraphService.isActiveAllocation(allocationId));
 
         Allocation.State memory allocation = subgraphService.getAllocation(allocationId);
-        uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(allocation.subgraphDeploymentId);
-        
+        uint256 previousSubgraphAllocatedTokens = subgraphService.getSubgraphAllocatedTokens(
+            allocation.subgraphDeploymentId
+        );
+
         vm.expectEmit(address(subgraphService));
-        emit AllocationManager.AllocationClosed(_indexer, allocationId, allocation.subgraphDeploymentId, allocation.tokens);
+        emit AllocationManager.AllocationClosed(
+            _indexer,
+            allocationId,
+            allocation.subgraphDeploymentId,
+            allocation.tokens
+        );
         emit IDataService.ServiceStopped(_indexer, _data);
 
         // stop allocation
@@ -178,10 +195,6 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
 
     function _getIndexer(address _indexer) private view returns (ISubgraphService.Indexer memory) {
         (uint256 registeredAt, string memory url, string memory geoHash) = subgraphService.indexers(_indexer);
-        return ISubgraphService.Indexer({
-            registeredAt: registeredAt,
-            url: url,
-            geoHash: geoHash
-        });
+        return ISubgraphService.Indexer({ registeredAt: registeredAt, url: url, geoHash: geoHash });
     }
 }
diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol
@@ -302,6 +302,9 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
         // Calculate the payment collected by the indexer for this transaction
         paymentCollected = accRewardsPerTokens - allocation.accRewardsPerAllocatedToken;
 
+        uint256 currentEpoch = epochManager.currentEpoch();
+        paymentCollected = currentEpoch > allocation.createdAtEpoch ? paymentCollected : 0;
+
         uint256 delegatorCut = staking.getDelegationFeeCut(
             allocation.indexer,
             address(subgraphService),
diff --git a/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol b/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol
@@ -29,6 +29,10 @@ contract SubgraphServiceAllocationResizeTest is SubgraphServiceTest {
         vm.assume(resizeTokens != tokens);
 
         mint(users.indexer, resizeTokens);
+
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI1"));
         _collect(users.indexer, paymentType, data);
diff --git a/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol b/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol
@@ -19,6 +19,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
     ) public useIndexer useAllocation(tokens) {
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
+
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         _collect(users.indexer, paymentType, data);
     }
 
@@ -34,6 +38,11 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
             IGraphPayments.PaymentTypes.IndexingRewards,
             delegationFeeCut
         );
+
+
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
         _collect(users.indexer, paymentType, data);
@@ -54,6 +63,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
         // Undelegate
         resetPrank(users.delegator);
         staking.undelegate(users.indexer, address(subgraphService), delegationTokens);
+
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         resetPrank(users.indexer);
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
@@ -63,6 +76,9 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
     function test_SubgraphService_Collect_Indexing_RewardsDestination(
         uint256 tokens
     ) public useIndexer useAllocation(tokens) useRewardsDestination {
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
         _collect(users.indexer, paymentType, data);
@@ -121,6 +137,9 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
         // thaw some tokens to become over allocated
         staking.thaw(users.indexer, address(subgraphService), tokens / 2);
 
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         // this collection should close the allocation
         IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes.IndexingRewards;
         bytes memory collectData = abi.encode(allocationID, bytes32("POI"));
@@ -135,6 +154,10 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
         address newIndexer = makeAddr("newIndexer");
         _createAndStartAllocation(newIndexer, tokens);
         bytes memory data = abi.encode(allocationID, bytes32("POI"));
+
+        // skip time to ensure allocation gets rewards
+        vm.roll(block.number + EPOCH_LENGTH);
+
         // Attempt to collect from other indexer's allocation
         vm.expectRevert(
             abi.encodeWithSelector(ISubgraphService.SubgraphServiceAllocationNotAuthorized.selector, newIndexer, allocationID)
