From fa045965afcc7e66b403d2342766896278c31415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 5 Jun 2025 12:19:20 -0300 Subject: [PATCH 1/4] fix: prevent GraphTallyCollector from accepting non QueryFee payments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../interfaces/IGraphTallyCollector.sol | 6 +++ .../collectors/GraphTallyCollector.sol | 2 + .../collect/collect.t.sol | 27 +++++++++++ .../collect/indexing/indexing.t.sol | 10 ++++ .../subgraphService/collect/query/query.t.sol | 47 +++++++++++++++++++ 5 files changed, 92 insertions(+) diff --git a/packages/horizon/contracts/interfaces/IGraphTallyCollector.sol b/packages/horizon/contracts/interfaces/IGraphTallyCollector.sol index cb665bda5..0fb336ed3 100644 --- a/packages/horizon/contracts/interfaces/IGraphTallyCollector.sol +++ b/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. diff --git a/packages/horizon/contracts/payments/collectors/GraphTallyCollector.sol b/packages/horizon/contracts/payments/collectors/GraphTallyCollector.sol index 9a417fd9b..bab1be09e 100644 --- a/packages/horizon/contracts/payments/collectors/GraphTallyCollector.sol +++ b/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) diff --git a/packages/horizon/test/unit/payments/graph-tally-collector/collect/collect.t.sol b/packages/horizon/test/unit/payments/graph-tally-collector/collect/collect.t.sol index c0c30fb78..ed65dd64c 100644 --- a/packages/horizon/test/unit/payments/graph-tally-collector/collect/collect.t.sol +++ b/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); + } } diff --git a/packages/subgraph-service/test/unit/subgraphService/collect/indexing/indexing.t.sol b/packages/subgraph-service/test/unit/subgraphService/collect/indexing/indexing.t.sol index 6d2d18571..c97416157 100644 --- a/packages/subgraph-service/test/unit/subgraphService/collect/indexing/indexing.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/collect/indexing/indexing.t.sol @@ -171,4 +171,14 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest { ); subgraphService.collect(newIndexer, paymentType, data); } + + function test_SubgraphService_Collect_Indexing_RevertWhen_IncorrectPaymentType(uint256 tokens) public useIndexer useAllocation(tokens) { + bytes memory data = abi.encode(allocationID, bytes32("POI"), _getHardcodedPOIMetadata()); + + // skip time to ensure allocation gets rewards + vm.roll(block.number + EPOCH_LENGTH); + + vm.expectRevert(); + subgraphService.collect(users.indexer, IGraphPayments.PaymentTypes.QueryFee, data); + } } diff --git a/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol b/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol index 433e32375..72da1392b 100644 --- a/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/collect/query/query.t.sol @@ -11,6 +11,7 @@ import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/Mes import { ISubgraphService } from "../../../../../contracts/interfaces/ISubgraphService.sol"; import { SubgraphServiceTest } from "../../SubgraphService.t.sol"; +import { Allocation } from "../../../../../contracts/libraries/Allocation.sol"; contract SubgraphServiceRegisterTest is SubgraphServiceTest { using PPMMath for uint128; @@ -279,4 +280,50 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { ); assertEq(afterTokensCollected, intermediateTokensCollected + tokensToCollect + (oddTokensPayment ? 1 : 0)); } + + function testCollect_QueryFees_ClosedAllocation( + uint256 tokensAllocated, + uint256 tokensPayment + ) public useIndexer useAllocation(tokensAllocated) { + vm.assume(tokensAllocated > minimumProvisionTokens * stakeToFeesRatio); + uint256 maxTokensPayment = tokensAllocated / stakeToFeesRatio > type(uint128).max + ? type(uint128).max + : tokensAllocated / stakeToFeesRatio; + tokensPayment = bound(tokensPayment, minimumProvisionTokens, maxTokensPayment); + + resetPrank(users.gateway); + _deposit(tokensPayment); + _authorizeSigner(); + + // Close the allocation + resetPrank(users.indexer); + bytes memory closeAlloData = abi.encode(allocationID); + _stopService(users.indexer, closeAlloData); + + // Collect the RAV + resetPrank(users.indexer); + bytes memory data = _getQueryFeeEncodedData(users.indexer, uint128(tokensPayment), 0); + _collect(users.indexer, IGraphPayments.PaymentTypes.QueryFee, data); + } + + function testCollect_QueryFees_RevertWhen_IncorrectPaymentType( + uint256 tokensAllocated, + uint256 tokensPayment + ) public useIndexer useAllocation(tokensAllocated) { + vm.assume(tokensAllocated > minimumProvisionTokens * stakeToFeesRatio); + uint256 maxTokensPayment = tokensAllocated / stakeToFeesRatio > type(uint128).max + ? type(uint128).max + : tokensAllocated / stakeToFeesRatio; + tokensPayment = bound(tokensPayment, minimumProvisionTokens, maxTokensPayment); + + resetPrank(users.gateway); + _deposit(tokensPayment); + _authorizeSigner(); + + resetPrank(users.indexer); + bytes memory data = _getQueryFeeEncodedData(users.indexer, uint128(tokensPayment), 0); + + vm.expectRevert(); + subgraphService.collect(users.indexer, IGraphPayments.PaymentTypes.IndexingRewards, data); + } } From 97dceb13bff269ecc80bbe69a7bcb9785a3c363f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 5 Jun 2025 12:58:32 -0300 Subject: [PATCH 2/4] fix: allow creating multiple disputes for same allo/poi tuple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/DisputeManager.sol | 15 +++++++--- .../contracts/interfaces/IDisputeManager.sol | 5 +++- .../unit/disputeManager/DisputeManager.t.sol | 9 +++--- .../disputeManager/disputes/disputes.t.sol | 2 +- .../disputes/indexing/accept.t.sol | 10 +++---- .../disputes/indexing/cancel.t.sol | 10 +++---- .../disputes/indexing/create.t.sol | 30 +++++++++++++------ .../disputes/indexing/draw.t.sol | 4 +-- .../disputes/indexing/reject.t.sol | 4 +-- 9 files changed, 56 insertions(+), 33 deletions(-) diff --git a/packages/subgraph-service/contracts/DisputeManager.sol b/packages/subgraph-service/contracts/DisputeManager.sol index ec6634034..2a4dc6b6b 100644 --- a/packages/subgraph-service/contracts/DisputeManager.sol +++ b/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 @@ -450,16 +454,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)); @@ -496,6 +502,7 @@ contract DisputeManager is _deposit, _allocationId, _poi, + _blockNumber, stakeSnapshot, cancellableAt ); diff --git a/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol b/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol index 611009bef..217b1c154 100644 --- a/packages/subgraph-service/contracts/interfaces/IDisputeManager.sol +++ b/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. diff --git a/packages/subgraph-service/test/unit/disputeManager/DisputeManager.t.sol b/packages/subgraph-service/test/unit/disputeManager/DisputeManager.t.sol index ebe35e4fa..720460bc4 100644 --- a/packages/subgraph-service/test/unit/disputeManager/DisputeManager.t.sol +++ b/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."); diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/disputes.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/disputes.t.sol index 71d40e055..11a49d8e7 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/disputes.t.sol +++ b/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); diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/accept.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/accept.t.sol index e06f105b8..cb7e48fc9 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/accept.t.sol +++ b/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); diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/cancel.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/cancel.t.sol index 8b8e0f587..ac0541f02 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/cancel.t.sol +++ b/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,7 +37,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); vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector)); disputeManager.cancelDispute(disputeID); @@ -45,7 +45,7 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest { 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(); diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol index fdf803d5d..595902557 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol +++ b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol @@ -13,7 +13,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { function test_Indexing_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); - _createIndexingDispute(allocationID, bytes32("POI1")); + _createIndexingDispute(allocationID, bytes32("POI1"), block.number); } function test_Indexing_Create_Dispute_WithDelegation(uint256 tokens, uint256 delegationTokens) public useIndexer { @@ -37,7 +37,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { staking.delegate(users.indexer, address(subgraphService), delegationTokens, 0); resetPrank(users.fisherman); - _createIndexingDispute(allocationID, bytes32("POI1")); + _createIndexingDispute(allocationID, bytes32("POI1"), block.number); } function test_Indexing_Create_Dispute_RevertWhen_SubgraphServiceNotSet( @@ -52,7 +52,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { token.approve(address(disputeManager), disputeDeposit); vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector)); - disputeManager.createIndexingDispute(allocationID, bytes32("POI2")); + disputeManager.createIndexingDispute(allocationID, bytes32("POI2"), block.number); } function test_Indexing_Create_MultipleDisputes() public { @@ -81,7 +81,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { resetPrank(users.fisherman); for (uint i = 0; i < allocationIDPrivateKeys.length; i++) { - _createIndexingDispute(vm.addr(allocationIDPrivateKeys[i]), bytes32("POI1")); + _createIndexingDispute(vm.addr(allocationIDPrivateKeys[i]), bytes32("POI1"), block.number); } } @@ -89,7 +89,7 @@ contract DisputeManagerIndexingCreateDisputeTest 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); // Create another dispute with different fisherman address otherFisherman = makeAddr("otherFisherman"); @@ -101,10 +101,22 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { disputeID ); vm.expectRevert(expectedError); - disputeManager.createIndexingDispute(allocationID, bytes32("POI1")); + disputeManager.createIndexingDispute(allocationID, bytes32("POI1"), block.number); vm.stopPrank(); } + function test_Indexing_Create_DisputesSamePOIAndAllo( + uint256 tokens + ) public useIndexer useAllocation(tokens) { + resetPrank(users.fisherman); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number); + + resetPrank(users.arbitrator); + disputeManager.acceptDispute(disputeID, 100); + + _createIndexingDispute(allocationID, bytes32("POI1"), block.number + 1); + } + function test_Indexing_Create_RevertIf_DepositUnderMinimum(uint256 tokensDeposit) public useFisherman { tokensDeposit = bound(tokensDeposit, 0, disputeDeposit - 1); token.approve(address(disputeManager), tokensDeposit); @@ -115,7 +127,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { disputeDeposit ); vm.expectRevert(expectedError); - disputeManager.createIndexingDispute(allocationID, bytes32("POI3")); + disputeManager.createIndexingDispute(allocationID, bytes32("POI3"), block.number); vm.stopPrank(); } @@ -127,7 +139,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { allocationID ); vm.expectRevert(expectedError); - disputeManager.createIndexingDispute(allocationID, bytes32("POI4")); + disputeManager.createIndexingDispute(allocationID, bytes32("POI4"), block.number); vm.stopPrank(); } @@ -143,6 +155,6 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { resetPrank(users.fisherman); token.approve(address(disputeManager), tokens); vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerZeroTokens.selector)); - disputeManager.createIndexingDispute(allocationID, bytes32("POI1")); + disputeManager.createIndexingDispute(allocationID, bytes32("POI1"), block.number); } } diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/draw.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/draw.t.sol index 62a384b52..53806a258 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/draw.t.sol +++ b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/draw.t.sol @@ -14,7 +14,7 @@ contract DisputeManagerIndexingDrawDisputeTest is DisputeManagerTest { function test_Indexing_Draw_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); - bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI32")); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI32"), block.number); resetPrank(users.arbitrator); _drawDispute(disputeID); @@ -22,7 +22,7 @@ contract DisputeManagerIndexingDrawDisputeTest is DisputeManagerTest { function test_Indexing_Draw_RevertIf_CallerIsNotArbitrator(uint256 tokens) public useIndexer useAllocation(tokens) { resetPrank(users.fisherman); - bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1")); + bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), block.number); // attempt to draw dispute as fisherman vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerNotArbitrator.selector)); diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/reject.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/reject.t.sol index a084bee14..6f8f5bf21 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/reject.t.sol +++ b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/reject.t.sol @@ -13,7 +13,7 @@ contract DisputeManagerIndexingRejectDisputeTest is DisputeManagerTest { function test_Indexing_Reject_Dispute(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); _rejectDispute(disputeID); @@ -23,7 +23,7 @@ contract DisputeManagerIndexingRejectDisputeTest 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); // attempt to accept dispute as fisherman resetPrank(users.fisherman); From d89440a95ffaf9a9abfc1a6836399e3189e66e50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 5 Jun 2025 14:03:12 -0300 Subject: [PATCH 3/4] fix: consider delegation when deciding if an indexer is disputable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- .../contracts/DisputeManager.sol | 36 ++++++++----------- .../disputes/indexing/create.t.sol | 20 +++++++++++ .../disputes/query/create.t.sol | 23 ++++++++++++ 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/packages/subgraph-service/contracts/DisputeManager.sol b/packages/subgraph-service/contracts/DisputeManager.sol index 2a4dc6b6b..573e8f67e 100644 --- a/packages/subgraph-service/contracts/DisputeManager.sol +++ b/packages/subgraph-service/contracts/DisputeManager.sol @@ -344,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 @@ -398,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( @@ -419,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, @@ -477,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, @@ -691,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; } } diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol index 595902557..62b368835 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol +++ b/packages/subgraph-service/test/unit/disputeManager/disputes/indexing/create.t.sol @@ -157,4 +157,24 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest { vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerZeroTokens.selector)); disputeManager.createIndexingDispute(allocationID, bytes32("POI1"), block.number); } + + function test_Indexing_Create_DontRevertIf_IndexerIsBelowStake_WithDelegation(uint256 tokens, uint256 delegationTokens) public useIndexer useAllocation(tokens) { + // Close allocation + bytes memory data = abi.encode(allocationID); + _stopService(users.indexer, data); + // Thaw, deprovision and unstake + address subgraphDataServiceAddress = address(subgraphService); + _thawDeprovisionAndUnstake(users.indexer, subgraphDataServiceAddress, tokens); + + delegationTokens = bound(delegationTokens, 1 ether, 100_000_000 ether); + + resetPrank(users.delegator); + token.approve(address(staking), delegationTokens); + staking.delegate(users.indexer, address(subgraphService), delegationTokens, 0); + + // create dispute + resetPrank(users.fisherman); + token.approve(address(disputeManager), tokens); + _createIndexingDispute(allocationID, bytes32("POI1"), block.number); + } } diff --git a/packages/subgraph-service/test/unit/disputeManager/disputes/query/create.t.sol b/packages/subgraph-service/test/unit/disputeManager/disputes/query/create.t.sol index 6abd890a2..94f2fe615 100644 --- a/packages/subgraph-service/test/unit/disputeManager/disputes/query/create.t.sol +++ b/packages/subgraph-service/test/unit/disputeManager/disputes/query/create.t.sol @@ -155,4 +155,27 @@ contract DisputeManagerQueryCreateDisputeTest is DisputeManagerTest { vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerZeroTokens.selector)); disputeManager.createQueryDispute(attestationData); } + + function test_Query_Create_DontRevertIf_IndexerIsBelowStake_WithDelegation(uint256 tokens, uint256 delegationTokens) public useIndexer useAllocation(tokens) { + // Close allocation + bytes memory data = abi.encode(allocationID); + _stopService(users.indexer, data); + // Thaw, deprovision and unstake + address subgraphDataServiceAddress = address(subgraphService); + _thawDeprovisionAndUnstake(users.indexer, subgraphDataServiceAddress, tokens); + + delegationTokens = bound(delegationTokens, 1 ether, 100_000_000 ether); + + resetPrank(users.delegator); + token.approve(address(staking), delegationTokens); + staking.delegate(users.indexer, address(subgraphService), delegationTokens, 0); + + // create dispute + resetPrank(users.fisherman); + token.approve(address(disputeManager), tokens); + + Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId); + bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey); + _createQueryDispute(attestationData); + } } From 1c7e4cfb280e3e5f7396910e75df263480869897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Tue, 17 Jun 2025 14:35:46 -0300 Subject: [PATCH 4/4] fix: ensure close allo behavior consistent with legacy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- packages/horizon/contracts/staking/HorizonStakingExtension.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/staking/HorizonStakingExtension.sol b/packages/horizon/contracts/staking/HorizonStakingExtension.sol index 17787c4d3..be585fdb9 100644 --- a/packages/horizon/contracts/staking/HorizonStakingExtension.sol +++ b/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);