Merge pull request #1103 from graphprotocol/tmigone/allo-min-duration

tmigone · web-flow · commit d0060b17c040 · 2025-02-11T10:40:27.000-03:00
diff --git a/packages/contracts/contracts/rewards/IRewardsIssuer.sol b/packages/contracts/contracts/rewards/IRewardsIssuer.sol
@@ -31,11 +31,4 @@ interface IRewardsIssuer {
      * @return Total tokens allocated to subgraph
      */
     function getSubgraphAllocatedTokens(bytes32 _subgraphDeploymentId) external view returns (uint256);
-
-    /**
-     * @notice Whether or not an allocation is active (i.e open)
-     * @param _allocationId Allocation Id
-     * @return Whether or not the allocation is active
-     */
-    function isActiveAllocation(address _allocationId) external view returns (bool);
 }
diff --git a/packages/contracts/contracts/rewards/IRewardsManager.sol b/packages/contracts/contracts/rewards/IRewardsManager.sol
@@ -39,7 +39,7 @@ interface IRewardsManager {
 
     function getAccRewardsPerAllocatedToken(bytes32 _subgraphDeploymentID) external view returns (uint256, uint256);
 
-    function getRewards(address _allocationID) external view returns (uint256);
+    function getRewards(address _rewardsIssuer, address _allocationID) external view returns (uint256);
 
     function calcRewards(uint256 _tokens, uint256 _accRewardsPerAllocatedToken) external pure returns (uint256);
 
diff --git a/packages/contracts/contracts/rewards/RewardsManager.sol b/packages/contracts/contracts/rewards/RewardsManager.sol
@@ -322,32 +322,19 @@ contract RewardsManager is RewardsManagerV5Storage, GraphUpgradeable, IRewardsMa
      * @param _allocationID Allocation
      * @return Rewards amount for an allocation
      */
-    function getRewards(address _allocationID) external view override returns (uint256) {
-        address rewardsIssuer = address(0);
-
-        // Check both the legacy and new allocations
-        address[2] memory rewardsIssuers = [address(staking()), address(subgraphService)];
-        for (uint256 i = 0; i < rewardsIssuers.length; i++) {
-            if (rewardsIssuers[i] != address(0)) {
-                if (IRewardsIssuer(rewardsIssuers[i]).isActiveAllocation(_allocationID)) {
-                    rewardsIssuer = address(rewardsIssuers[i]);
-                    break;
-                }
-            }
-        }
-
-        // Bail if allo does not exist
-        if (rewardsIssuer == address(0)) {
-            return 0;
-        }
+    function getRewards(address _rewardsIssuer, address _allocationID) external view override returns (uint256) {
+        require(
+            _rewardsIssuer == address(staking()) || _rewardsIssuer == address(subgraphService),
+            "Not a rewards issuer"
+        );
 
         (
             ,
             bytes32 subgraphDeploymentId,
             uint256 tokens,
             uint256 alloAccRewardsPerAllocatedToken,
             uint256 accRewardsPending
-        ) = IRewardsIssuer(rewardsIssuer).getAllocationData(_allocationID);
+        ) = IRewardsIssuer(_rewardsIssuer).getAllocationData(_allocationID);
 
         (uint256 accRewardsPerAllocatedToken, ) = getAccRewardsPerAllocatedToken(subgraphDeploymentId);
         return
diff --git a/packages/contracts/test/unit/rewards/rewards.test.ts b/packages/contracts/test/unit/rewards/rewards.test.ts
@@ -466,7 +466,7 @@ describe('Rewards', () => {
         await helpers.mine(ISSUANCE_RATE_PERIODS)
 
         // Rewards
-        const contractRewards = await rewardsManager.getRewards(allocationID1)
+        const contractRewards = await rewardsManager.getRewards(staking.address, allocationID1)
 
         // We trust using this function in the test because we tested it
         // standalone in a previous test
@@ -504,12 +504,12 @@ describe('Rewards', () => {
         await staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
 
         // Rewards
-        const contractRewards = await rewardsManager.getRewards(allocationID1)
+        const contractRewards = await rewardsManager.getRewards(staking.address, allocationID1)
         expect(contractRewards).eq(BigNumber.from(0))
       })
       it('rewards should be zero if the allocation does not exist', async function () {
         // Rewards
-        const contractRewards = await rewardsManager.getRewards(allocationIDNull)
+        const contractRewards = await rewardsManager.getRewards(staking.address, allocationIDNull)
         expect(contractRewards).eq(BigNumber.from(0))
       })
     })
@@ -1026,7 +1026,7 @@ describe('Rewards', () => {
       await staking.connect(assetHolder).collect(tokensToCollect, allocationID1)
 
       // check rewards diff
-      await rewardsManager.getRewards(allocationID1).then(formatGRT)
+      await rewardsManager.getRewards(staking.address, allocationID1).then(formatGRT)
 
       await helpers.mine()
       const accrual = await getRewardsAccrual(subgraphs)
diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol
@@ -274,15 +274,6 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
         return __DEPRECATED_subgraphAllocations[subgraphDeploymentID];
     }
 
-    /**
-     * @notice Return true if allocation is active.
-     * @param allocationID Allocation identifier
-     * @return True if allocation is active
-     */
-    function isActiveAllocation(address allocationID) external view override returns (bool) {
-        return _getAllocationState(allocationID) == AllocationState.Active;
-    }
-
     /**
      * @notice Get the total amount of tokens staked by the indexer.
      * @param indexer Address of the indexer
diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol
@@ -449,18 +449,6 @@ contract SubgraphService is
         return _subgraphAllocatedTokens[subgraphDeploymentId];
     }
 
-    /**
-     * @notice Check if an allocation is open
-     * @dev Implements {IRewardsIssuer.isAllocationActive}
-     * @dev To be used by the {RewardsManager}.
-     *
-     * @param allocationId The allocation Id
-     * @return Wether or not the allocation is active
-     */
-    function isActiveAllocation(address allocationId) external view override returns (bool) {
-        return _allocations[allocationId].isOpen();
-    }
-
     /**
      * @notice See {ISubgraphService.getLegacyAllocation}
      */
@@ -475,13 +463,6 @@ contract SubgraphService is
         return _encodeAllocationProof(indexer, allocationId);
     }
 
-    /**
-     * @notice See {ISubgraphService.isStaleAllocation}
-     */
-    function isStaleAllocation(address allocationId) external view override returns (bool) {
-        return _allocations.get(allocationId).isStale(maxPOIStaleness);
-    }
-
     /**
      * @notice See {ISubgraphService.isOverAllocated}
      */
diff --git a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol
@@ -241,13 +241,6 @@ interface ISubgraphService is IDataServiceFees {
      */
     function encodeAllocationProof(address indexer, address allocationId) external view returns (bytes32);
 
-    /**
-     * @notice Checks if an allocation is stale
-     * @param allocationId The id of the allocation
-     * @return True if the allocation is stale, false otherwise
-     */
-    function isStaleAllocation(address allocationId) external view returns (bool);
-
     /**
      * @notice Checks if an indexer is over-allocated
      * @param allocationId The id of the allocation
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/hardhat.config.ts b/packages/subgraph-service/hardhat.config.ts
@@ -17,6 +17,15 @@ if (process.env.BUILD_RUN !== 'true') {
 
 const config: HardhatUserConfig = {
   ...hardhatBaseConfig,
+  solidity: {
+    version: '0.8.27',
+    settings: {
+      optimizer: {
+        enabled: true,
+        runs: 50,
+      },
+    },
+  },
   graph: {
     deployments: {
       ...hardhatBaseConfig.graph?.deployments,
diff --git a/packages/subgraph-service/test/mocks/MockRewardsManager.sol b/packages/subgraph-service/test/mocks/MockRewardsManager.sol
@@ -61,7 +61,7 @@ contract MockRewardsManager is IRewardsManager {
 
     function getAccRewardsPerAllocatedToken(bytes32) external view returns (uint256, uint256) {}
 
-    function getRewards(address) external view returns (uint256) {}
+    function getRewards(address,address) external view returns (uint256) {}
 
     function calcRewards(uint256, uint256) external pure returns (uint256) {}
 
diff --git a/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol b/packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol
diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol
diff --git a/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol b/packages/subgraph-service/test/subgraphService/allocation/resize.t.sol
diff --git a/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol b/packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol
