Horizon: bug fixes

packages/horizon/contracts/interfaces/IGraphTallyCollector.sol

@@ -99,6 +99,12 @@ interface IGraphTallyCollector is IPaymentsCollector {
      */
     error GraphTallyCollectorInvalidTokensToCollectAmount(uint256 tokensToCollect, uint256 maxTokensToCollect);
 
+    /**
+     * @notice Thrown when the payment type is invalid
+     * @param paymentType The payment type
+     */
+    error GraphTallyCollectorInvalidPaymentType(IGraphPayments.PaymentTypes paymentType);
+
     /**
      * @notice See {IPaymentsCollector.collect}
      * This variant adds the ability to partially collect a RAV by specifying the amount of tokens to collect.

packages/horizon/contracts/payments/collectors/GraphTallyCollector.sol

@@ -102,6 +102,8 @@ contract GraphTallyCollector is EIP712, GraphDirectory, Authorizable, IGraphTall
         bytes calldata _data,
         uint256 _tokensToCollect
     ) private returns (uint256) {
+        require(_paymentType == IGraphPayments.PaymentTypes.QueryFee, GraphTallyCollectorInvalidPaymentType(_paymentType));
+
         (SignedRAV memory signedRAV, uint256 dataServiceCut, address receiverDestination) = abi.decode(
             _data,
             (SignedRAV, uint256, address)

packages/horizon/contracts/staking/HorizonStakingExtension.sol

@@ -356,7 +356,7 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
         // Process non-zero-allocation rewards tracking
         if (alloc.tokens > 0) {
             // Distribute rewards if proof of indexing was presented by the indexer or operator
-            if (isIndexerOrOperator && _poi != 0) {
+            if (isIndexerOrOperator && _poi != 0 && epochs > 0) {
                 _distributeRewards(_allocationID, alloc.indexer);
             } else {
                 _updateRewards(alloc.subgraphDeploymentID);

packages/horizon/test/unit/payments/graph-tally-collector/collect/collect.t.sol

@@ -483,4 +483,31 @@ contract GraphTallyCollectTest is GraphTallyTest {
         bytes memory allocation0Data = _getQueryFeeEncodedData(signerPrivateKey, collectTestParams[0]);
         _collect(IGraphPayments.PaymentTypes.QueryFee, allocation0Data);
     }
+
+    function testGraphTally_Collect_RevertWhen_IncorrectPaymentType(
+        uint256 tokens
+    ) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
+        tokens = bound(tokens, 1, type(uint128).max);
+
+        _depositTokens(address(graphTallyCollector), users.indexer, tokens);
+
+        CollectTestParams memory params = CollectTestParams({
+            tokens: tokens,
+            allocationId: _allocationId,
+            payer: users.gateway,
+            indexer: users.indexer,
+            collector: users.verifier
+        });
+
+        bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, params);
+
+        resetPrank(users.verifier);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IGraphTallyCollector.GraphTallyCollectorInvalidPaymentType.selector,
+                IGraphPayments.PaymentTypes.IndexingRewards
+            )
+        );
+        graphTallyCollector.collect(IGraphPayments.PaymentTypes.IndexingRewards, data);
+    }
 }

packages/subgraph-service/contracts/DisputeManager.sol

@@ -120,12 +120,16 @@ contract DisputeManager is
     }
 
     /// @inheritdoc IDisputeManager
-    function createIndexingDispute(address allocationId, bytes32 poi) external override returns (bytes32) {
+    function createIndexingDispute(
+        address allocationId,
+        bytes32 poi,
+        uint256 blockNumber
+    ) external override returns (bytes32) {
         // Get funds from fisherman
         _graphToken().pullTokens(msg.sender, disputeDeposit);
 
         // Create a dispute
-        return _createIndexingDisputeWithAllocation(msg.sender, disputeDeposit, allocationId, poi);
+        return _createIndexingDisputeWithAllocation(msg.sender, disputeDeposit, allocationId, poi, blockNumber);
     }
 
     /// @inheritdoc IDisputeManager
@@ -340,11 +344,7 @@ contract DisputeManager is
 
     /// @inheritdoc IDisputeManager
     function getStakeSnapshot(address indexer) external view override returns (uint256) {
-        IHorizonStaking.Provision memory provision = _graphStaking().getProvision(
-            indexer,
-            address(_getSubgraphService())
-        );
-        return _getStakeSnapshot(indexer, provision.tokens);
+        return _getStakeSnapshot(indexer);
     }
 
     /// @inheritdoc IDisputeManager
@@ -394,13 +394,6 @@ contract DisputeManager is
         // Get the indexer that signed the attestation
         address indexer = getAttestationIndexer(_attestation);
 
-        // The indexer is disputable
-        IHorizonStaking.Provision memory provision = _graphStaking().getProvision(
-            indexer,
-            address(_getSubgraphService())
-        );
-        require(provision.tokens != 0, DisputeManagerZeroTokens());
-
         // Create a disputeId
         bytes32 disputeId = keccak256(
             abi.encodePacked(
@@ -415,8 +408,11 @@ contract DisputeManager is
         // Only one dispute at a time
         require(!isDisputeCreated(disputeId), DisputeManagerDisputeAlreadyCreated(disputeId));
 
+        // The indexer is disputable
+        uint256 stakeSnapshot = _getStakeSnapshot(indexer);
+        require(stakeSnapshot != 0, DisputeManagerZeroTokens());
+
         // Store dispute
-        uint256 stakeSnapshot = _getStakeSnapshot(indexer, provision.tokens);
         uint256 cancellableAt = block.timestamp + disputePeriod;
         disputes[disputeId] = Dispute(
             indexer,
@@ -450,16 +446,18 @@ contract DisputeManager is
      * @param _deposit Amount of tokens staked as deposit
      * @param _allocationId Allocation disputed
      * @param _poi The POI being disputed
+     * @param _blockNumber The block number for which the POI was calculated
      * @return The dispute id
      */
     function _createIndexingDisputeWithAllocation(
         address _fisherman,
         uint256 _deposit,
         address _allocationId,
-        bytes32 _poi
+        bytes32 _poi,
+        uint256 _blockNumber
     ) private returns (bytes32) {
         // Create a disputeId
-        bytes32 disputeId = keccak256(abi.encodePacked(_allocationId, _poi));
+        bytes32 disputeId = keccak256(abi.encodePacked(_allocationId, _poi, _blockNumber));
 
         // Only one dispute for an allocationId at a time
         require(!isDisputeCreated(disputeId), DisputeManagerDisputeAlreadyCreated(disputeId));
@@ -471,11 +469,10 @@ contract DisputeManager is
         require(indexer != address(0), DisputeManagerIndexerNotFound(_allocationId));
 
         // The indexer must be disputable
-        IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService_));
-        require(provision.tokens != 0, DisputeManagerZeroTokens());
+        uint256 stakeSnapshot = _getStakeSnapshot(indexer);
+        require(stakeSnapshot != 0, DisputeManagerZeroTokens());
 
         // Store dispute
-        uint256 stakeSnapshot = _getStakeSnapshot(indexer, provision.tokens);
         uint256 cancellableAt = block.timestamp + disputePeriod;
         disputes[disputeId] = Dispute(
             alloc.indexer,
@@ -496,6 +493,7 @@ contract DisputeManager is
             _deposit,
             _allocationId,
             _poi,
+            _blockNumber,
             stakeSnapshot,
             cancellableAt
         );
@@ -684,18 +682,19 @@ contract DisputeManager is
      * @dev A few considerations:
      * - We include both indexer and delegators stake.
      * - Thawing stake is not excluded from the snapshot.
-     * - Delegators stake is capped at the delegation ratio to prevent delegators from inflating the snapshot
-     *   to increase the indexer slash amount.
      *
      * Note that the snapshot can be inflated by delegators front-running the dispute creation with a delegation
      * to the indexer. Given the snapshot is a cap, the dispute outcome is uncertain and considering the cost of capital
      * and slashing risk, this is not a concern.
      * @param _indexer Indexer address
-     * @param _indexerStake Indexer's stake
      * @return Total stake snapshot
      */
-    function _getStakeSnapshot(address _indexer, uint256 _indexerStake) private view returns (uint256) {
-        uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, address(_getSubgraphService())).tokens;
-        return _indexerStake + delegatorsStake;
+    function _getStakeSnapshot(address _indexer) private view returns (uint256) {
+        address subgraphService = address(_getSubgraphService());
+
+        IHorizonStaking.Provision memory provision = _graphStaking().getProvision(_indexer, subgraphService);
+        uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, subgraphService).tokens;
+
+        return provision.tokens + delegatorsStake;
     }
 }

packages/subgraph-service/contracts/interfaces/IDisputeManager.sol

@@ -123,6 +123,7 @@ interface IDisputeManager {
      * @param tokens The amount of tokens deposited by the fisherman
      * @param allocationId The allocation id
      * @param poi The POI
+     * @param blockNumber The block number for which the POI was calculated
      * @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute
      * @param cancellableAt The timestamp when the dispute can be cancelled
      */
@@ -133,6 +134,7 @@ interface IDisputeManager {
         uint256 tokens,
         address allocationId,
         bytes32 poi,
+        uint256 blockNumber,
         uint256 stakeSnapshot,
         uint256 cancellableAt
     );
@@ -458,9 +460,10 @@ interface IDisputeManager {
      *
      * @param allocationId The allocation to dispute
      * @param poi The Proof of Indexing (POI) being disputed
+     * @param blockNumber The block number for which the POI was calculated
      * @return The dispute id
      */
-    function createIndexingDispute(address allocationId, bytes32 poi) external returns (bytes32);
+    function createIndexingDispute(address allocationId, bytes32 poi, uint256 blockNumber) external returns (bytes32);
 
     /**
      * @notice Creates and auto-accepts a legacy dispute.

packages/subgraph-service/test/unit/disputeManager/DisputeManager.t.sol

@@ -69,9 +69,9 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
         assertEq(address(disputeManager.subgraphService()), _subgraphService, "Subgraph service should be set.");
     }
 
-    function _createIndexingDispute(address _allocationId, bytes32 _poi) internal returns (bytes32) {
+    function _createIndexingDispute(address _allocationId, bytes32 _poi, uint256 _blockNumber) internal returns (bytes32) {
         (, address fisherman, ) = vm.readCallers();
-        bytes32 expectedDisputeId = keccak256(abi.encodePacked(_allocationId, _poi));
+        bytes32 expectedDisputeId = keccak256(abi.encodePacked(_allocationId, _poi, _blockNumber));
         uint256 disputeDeposit = disputeManager.disputeDeposit();
         uint256 beforeFishermanBalance = token.balanceOf(fisherman);
         Allocation.State memory alloc = subgraphService.getAllocation(_allocationId);
@@ -88,13 +88,14 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             fisherman,
             disputeDeposit,
             _allocationId,
-            _poi,
+            _poi,   
+            _blockNumber,
             stakeSnapshot,
             cancellableAt
         );
 
         // Create the indexing dispute
-        bytes32 _disputeId = disputeManager.createIndexingDispute(_allocationId, _poi);
+        bytes32 _disputeId = disputeManager.createIndexingDispute(_allocationId, _poi, _blockNumber);
 
         // Check that the dispute was created and that it has the correct ID
         assertTrue(disputeManager.isDisputeCreated(_disputeId), "Dispute should be created.");

packages/subgraph-service/test/unit/disputeManager/disputes/disputes.t.sol

@@ -29,7 +29,7 @@ contract DisputeManagerDisputeTest is DisputeManagerTest {
 
     function test_Dispute_Accept_RevertIf_SlashZeroTokens(uint256 tokens) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI101"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI101"), block.number);
 
         // attempt to accept dispute with 0 tokens slashed
         resetPrank(users.arbitrator);

packages/subgraph-service/test/unit/disputeManager/disputes/indexing/accept.t.sol

@@ -18,7 +18,7 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
         tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
 
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         resetPrank(users.arbitrator);
         _acceptDispute(disputeID, tokensSlash);
@@ -31,7 +31,7 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
         tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
 
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         resetPrank(users.arbitrator);
         // clear subgraph service address from storage
@@ -48,7 +48,7 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
         tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
 
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         resetPrank(users.arbitrator);
         _acceptDispute(disputeID, tokensSlash);
@@ -61,7 +61,7 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
         tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
 
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         // attempt to accept dispute as fisherman
         resetPrank(users.fisherman);
@@ -75,7 +75,7 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
     ) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
         tokensSlash = bound(tokensSlash, uint256(maxSlashingPercentage).mulPPM(tokens) + 1, type(uint256).max);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI101"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI101"), block.number);
 
         // max slashing percentage is 50%
         resetPrank(users.arbitrator);

packages/subgraph-service/test/unit/disputeManager/disputes/indexing/cancel.t.sol

@@ -13,7 +13,7 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
 
     function test_Indexing_Cancel_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         // skip to end of dispute period
         uint256 disputePeriod = disputeManager.disputePeriod();
@@ -26,7 +26,7 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
         uint256 tokens
     ) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         resetPrank(users.arbitrator);
         vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerNotFisherman.selector));
@@ -37,15 +37,15 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
         uint256 tokens
     ) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
         disputeManager.cancelDispute(disputeID);
     }
 
     function test_Indexing_Cancel_After_DisputePeriodIncreased(uint256 tokens) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         // change the dispute period to a higher value
         uint256 oldDisputePeriod = disputeManager.disputePeriod();
@@ -62,7 +62,7 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
 
     function test_Indexing_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
         resetPrank(users.fisherman);
-        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number);
 
         // change the dispute period to a lower value
         uint256 oldDisputePeriod = disputeManager.disputePeriod();