fix: ensure dispute cancellation is not affected by dispute period change (OZ L-19)

packages/subgraph-service/contracts/DisputeManager.sol

@@ -219,6 +219,7 @@ contract DisputeManager is
             DisputeType.LegacyDispute,
             IDisputeManager.DisputeStatus.Accepted,
             block.timestamp,
+            block.timestamp + disputePeriod,
             0
         );
 
@@ -285,7 +286,7 @@ contract DisputeManager is
         Dispute storage dispute = disputes[disputeId];
 
         // Check if dispute period has finished
-        require(dispute.createdAt + disputePeriod < block.timestamp, DisputeManagerDisputePeriodNotFinished());
+        require(dispute.cancellableAt <= block.timestamp, DisputeManagerDisputePeriodNotFinished());
         _cancelDispute(disputeId, dispute);
 
         if (_isDisputeInConflict(dispute)) {
@@ -417,6 +418,7 @@ contract DisputeManager is
 
         // Store dispute
         uint256 stakeSnapshot = _getStakeSnapshot(indexer, provision.tokens);
+        uint256 cancellableAt = block.timestamp + disputePeriod;
         disputes[disputeId] = Dispute(
             indexer,
             _fisherman,
@@ -425,6 +427,7 @@ contract DisputeManager is
             DisputeType.QueryDispute,
             IDisputeManager.DisputeStatus.Pending,
             block.timestamp,
+            cancellableAt,
             stakeSnapshot
         );
 
@@ -435,6 +438,7 @@ contract DisputeManager is
             _deposit,
             _attestation.subgraphDeploymentId,
             _attestationData,
+            cancellableAt,
             stakeSnapshot
         );
 
@@ -473,6 +477,7 @@ contract DisputeManager is
 
         // Store dispute
         uint256 stakeSnapshot = _getStakeSnapshot(indexer, provision.tokens);
+        uint256 cancellableAt = block.timestamp + disputePeriod;
         disputes[disputeId] = Dispute(
             alloc.indexer,
             _fisherman,
@@ -481,10 +486,20 @@ contract DisputeManager is
             DisputeType.IndexingDispute,
             IDisputeManager.DisputeStatus.Pending,
             block.timestamp,
+            cancellableAt,
             stakeSnapshot
         );
 
-        emit IndexingDisputeCreated(disputeId, alloc.indexer, _fisherman, _deposit, _allocationId, _poi, stakeSnapshot);
+        emit IndexingDisputeCreated(
+            disputeId,
+            alloc.indexer,
+            _fisherman,
+            _deposit,
+            _allocationId,
+            _poi,
+            stakeSnapshot,
+            cancellableAt
+        );
 
         return disputeId;
     }

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

@@ -38,6 +38,7 @@ interface IDisputeManager {
      * @param disputeType The type of dispute
      * @param status The status of the dispute
      * @param createdAt The timestamp when the dispute was created
+     * @param cancellableAt The timestamp when the dispute can be cancelled
      * @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute (includes delegation up to the delegation ratio)
      */
     struct Dispute {
@@ -48,6 +49,7 @@ interface IDisputeManager {
         DisputeType disputeType;
         DisputeStatus status;
         uint256 createdAt;
+        uint256 cancellableAt;
         uint256 stakeSnapshot;
     }
 
@@ -97,6 +99,7 @@ interface IDisputeManager {
      * @param tokens The amount of tokens deposited by the fisherman
      * @param subgraphDeploymentId The subgraph deployment id
      * @param attestation The attestation
+     * @param cancellableAt The timestamp when the dispute can be cancelled
      * @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute
      */
     event QueryDisputeCreated(
@@ -106,7 +109,8 @@ interface IDisputeManager {
         uint256 tokens,
         bytes32 subgraphDeploymentId,
         bytes attestation,
-        uint256 stakeSnapshot
+        uint256 stakeSnapshot,
+        uint256 cancellableAt
     );
 
     /**
@@ -120,6 +124,7 @@ interface IDisputeManager {
      * @param allocationId The allocation id
      * @param poi The POI
      * @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute
+     * @param cancellableAt The timestamp when the dispute can be cancelled
      */
     event IndexingDisputeCreated(
         bytes32 indexed disputeId,
@@ -128,7 +133,8 @@ interface IDisputeManager {
         uint256 tokens,
         address allocationId,
         bytes32 poi,
-        uint256 stakeSnapshot
+        uint256 stakeSnapshot,
+        uint256 cancellableAt
     );
 
     /**

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

@@ -76,6 +76,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
         uint256 beforeFishermanBalance = token.balanceOf(fisherman);
         Allocation.State memory alloc = subgraphService.getAllocation(_allocationId);
         uint256 stakeSnapshot = disputeManager.getStakeSnapshot(alloc.indexer);
+        uint256 cancellableAt = block.timestamp + disputeManager.disputePeriod();
 
         // Approve the dispute deposit
         token.approve(address(disputeManager), disputeDeposit);
@@ -88,7 +89,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             disputeDeposit,
             _allocationId,
             _poi,
-            stakeSnapshot
+            stakeSnapshot,
+            cancellableAt
         );
 
         // Create the indexing dispute
@@ -144,6 +146,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
         uint256 disputeDeposit = disputeManager.disputeDeposit();
         uint256 beforeFishermanBalance = token.balanceOf(fisherman);
         uint256 stakeSnapshot = disputeManager.getStakeSnapshot(indexer);
+        uint256 cancellableAt = block.timestamp + disputeManager.disputePeriod();
 
         // Approve the dispute deposit
         token.approve(address(disputeManager), disputeDeposit);
@@ -156,6 +159,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             disputeDeposit,
             attestation.subgraphDeploymentId,
             _attestationData,
+            cancellableAt,
             stakeSnapshot
         );
 
@@ -317,6 +321,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             )
         );
 
+        uint256 cancellableAt = block.timestamp + disputeManager.disputePeriod();
+
         // createQueryDisputeConflict
         vm.expectEmit(address(disputeManager));
         emit IDisputeManager.QueryDisputeCreated(
@@ -326,6 +332,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             disputeDeposit / 2,
             beforeValues.attestation1.subgraphDeploymentId,
             attestationData1,
+            cancellableAt,
             beforeValues.stakeSnapshot1
         );
         vm.expectEmit(address(disputeManager));
@@ -336,6 +343,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             disputeDeposit / 2,
             beforeValues.attestation2.subgraphDeploymentId,
             attestationData2,
+            cancellableAt,
             beforeValues.stakeSnapshot2
         );
 
@@ -678,12 +686,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
         if (isDisputeInConflict) relatedDispute = _getDispute(dispute.relatedDisputeId);
         address fisherman = dispute.fisherman;
         uint256 fishermanPreviousBalance = token.balanceOf(fisherman);
-        uint256 disputePeriod = disputeManager.disputePeriod();
         uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService));
 
-        // skip to end of dispute period
-        skip(disputePeriod + 1);
-
         vm.expectEmit(address(disputeManager));
         emit IDisputeManager.DisputeCancelled(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit);
 
@@ -790,6 +794,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
             IDisputeManager.DisputeType disputeType,
             IDisputeManager.DisputeStatus status,
             uint256 createdAt,
+            uint256 cancellableAt,
             uint256 stakeSnapshot
         ) = disputeManager.disputes(_disputeId);
         return
@@ -801,6 +806,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
                 disputeType: disputeType,
                 status: status,
                 createdAt: createdAt,
+                cancellableAt: cancellableAt,
                 stakeSnapshot: stakeSnapshot
             });
     }

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

@@ -15,6 +15,10 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
         resetPrank(users.fisherman);
         bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
 
+        // skip to end of dispute period
+        uint256 disputePeriod = disputeManager.disputePeriod();
+        skip(disputePeriod + 1);
+
         _cancelDispute(disputeID);
     }
 
@@ -38,4 +42,39 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
         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"));
+
+        // change the dispute period to a higher value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
+
+        // skip to end of old dispute period
+        skip(oldDisputePeriod + 1);
+
+        // should be able to cancel
+        resetPrank(users.fisherman);
+        _cancelDispute(disputeID);
+    }
+
+    function test_Indexing_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
+        resetPrank(users.fisherman);
+        bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
+
+        // change the dispute period to a lower value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
+
+        // skip to end of new dispute period
+        skip(oldDisputePeriod / 2 + 1);
+
+        // should not be able to cancel
+        resetPrank(users.fisherman);
+        vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
+        disputeManager.cancelDispute(disputeID);
+    }
 }

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

@@ -22,6 +22,10 @@ contract DisputeManagerQueryCancelDisputeTest is DisputeManagerTest {
         bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
         bytes32 disputeID = _createQueryDispute(attestationData);
 
+        // skip to end of dispute period
+        uint256 disputePeriod = disputeManager.disputePeriod();
+        skip(disputePeriod + 1);
+
         _cancelDispute(disputeID);
     }
 
@@ -45,4 +49,43 @@ contract DisputeManagerQueryCancelDisputeTest is DisputeManagerTest {
         vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
         disputeManager.cancelDispute(disputeID);
     }
+
+    function test_Query_Cancel_After_DisputePeriodIncreased(uint256 tokens) public useIndexer useAllocation(tokens) {
+        resetPrank(users.fisherman);
+        Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
+        bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
+        bytes32 disputeID = _createQueryDispute(attestationData);
+
+        // change the dispute period to a higher value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
+
+        // skip to end of old dispute period
+        skip(oldDisputePeriod + 1);
+
+        // should be able to cancel
+        resetPrank(users.fisherman);
+        _cancelDispute(disputeID);
+    }
+
+    function test_Query_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
+        resetPrank(users.fisherman);
+        Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
+        bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
+        bytes32 disputeID = _createQueryDispute(attestationData);
+
+        // change the dispute period to a lower value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
+
+        // skip to end of new dispute period
+        skip(oldDisputePeriod / 2 + 1);
+
+        // should not be able to cancel
+        resetPrank(users.fisherman);
+        vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
+        disputeManager.cancelDispute(disputeID);
+    }
 }

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

@@ -28,6 +28,10 @@ contract DisputeManagerQueryConflictCancelDisputeTest is DisputeManagerTest {
         resetPrank(users.fisherman);
         (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
 
+        // skip to end of dispute period
+        uint256 disputePeriod = disputeManager.disputePeriod();
+        skip(disputePeriod + 1);
+
         _cancelDispute(disputeID1);
     }
 
@@ -70,4 +74,60 @@ contract DisputeManagerQueryConflictCancelDisputeTest is DisputeManagerTest {
         vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
         disputeManager.cancelDispute(disputeID1);
     }
+
+    function test_Query_Conflict_Cancel_After_DisputePeriodIncreased(
+        uint256 tokens
+    ) public useIndexer useAllocation(tokens) {
+        resetPrank(users.fisherman);
+        (bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations(
+            requestCID,
+            subgraphDeployment,
+            responseCID1,
+            responseCID2,
+            allocationIDPrivateKey,
+            allocationIDPrivateKey
+        );
+
+        resetPrank(users.fisherman);
+        (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
+
+        // change the dispute period to a higher value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
+
+        // skip to end of old dispute period
+        skip(oldDisputePeriod + 1);
+
+        // should be able to cancel
+        resetPrank(users.fisherman);
+        _cancelDispute(disputeID1);
+    }
+
+    function test_Query_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
+        (bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations(
+            requestCID,
+            subgraphDeployment,
+            responseCID1,
+            responseCID2,
+            allocationIDPrivateKey,
+            allocationIDPrivateKey
+        );
+
+        resetPrank(users.fisherman);
+        (bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
+
+        // change the dispute period to a lower value
+        uint256 oldDisputePeriod = disputeManager.disputePeriod();
+        resetPrank(users.governor);
+        disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
+
+        // skip to end of new dispute period
+        skip(oldDisputePeriod / 2 + 1);
+
+        // should not be able to cancel
+        resetPrank(users.fisherman);
+        vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
+        disputeManager.cancelDispute(disputeID1);
+    }
 }