Skip to content

Commit 65ec5e7

Browse files
tmigonetmigone
committed
fix: ensure dispute cancellation is not affected by dispute period change (OZ L-19)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 0d21723 commit 65ec5e7

File tree

6 files changed

+150
-5
lines changed

6 files changed

+150
-5
lines changed

‎packages/subgraph-service/contracts/DisputeManager.sol

+4-1
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ contract DisputeManager is
219219
DisputeType.LegacyDispute,
220220
IDisputeManager.DisputeStatus.Accepted,
221221
block.timestamp,
222+
block.timestamp + disputePeriod,
222223
0
223224
);
224225

@@ -285,7 +286,7 @@ contract DisputeManager is
285286
Dispute storage dispute = disputes[disputeId];
286287

287288
// Check if dispute period has finished
288-
require(dispute.createdAt + disputePeriod < block.timestamp, DisputeManagerDisputePeriodNotFinished());
289+
require(dispute.cancellableAt <= block.timestamp, DisputeManagerDisputePeriodNotFinished());
289290
_cancelDispute(disputeId, dispute);
290291

291292
if (_isDisputeInConflict(dispute)) {
@@ -425,6 +426,7 @@ contract DisputeManager is
425426
DisputeType.QueryDispute,
426427
IDisputeManager.DisputeStatus.Pending,
427428
block.timestamp,
429+
block.timestamp + disputePeriod,
428430
stakeSnapshot
429431
);
430432

@@ -481,6 +483,7 @@ contract DisputeManager is
481483
DisputeType.IndexingDispute,
482484
IDisputeManager.DisputeStatus.Pending,
483485
block.timestamp,
486+
block.timestamp + disputePeriod,
484487
stakeSnapshot
485488
);
486489

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

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ interface IDisputeManager {
3838
* @param disputeType The type of dispute
3939
* @param status The status of the dispute
4040
* @param createdAt The timestamp when the dispute was created
41+
* @param cancellableAt The timestamp when the dispute can be cancelled
4142
* @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute (includes delegation up to the delegation ratio)
4243
*/
4344
struct Dispute {
@@ -48,6 +49,7 @@ interface IDisputeManager {
4849
DisputeType disputeType;
4950
DisputeStatus status;
5051
uint256 createdAt;
52+
uint256 cancellableAt;
5153
uint256 stakeSnapshot;
5254
}
5355

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -678,12 +678,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
678678
if (isDisputeInConflict) relatedDispute = _getDispute(dispute.relatedDisputeId);
679679
address fisherman = dispute.fisherman;
680680
uint256 fishermanPreviousBalance = token.balanceOf(fisherman);
681-
uint256 disputePeriod = disputeManager.disputePeriod();
682681
uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService));
683682

684-
// skip to end of dispute period
685-
skip(disputePeriod + 1);
686-
687683
vm.expectEmit(address(disputeManager));
688684
emit IDisputeManager.DisputeCancelled(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit);
689685

@@ -790,6 +786,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
790786
IDisputeManager.DisputeType disputeType,
791787
IDisputeManager.DisputeStatus status,
792788
uint256 createdAt,
789+
uint256 cancellableAt,
793790
uint256 stakeSnapshot
794791
) = disputeManager.disputes(_disputeId);
795792
return
@@ -801,6 +798,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
801798
disputeType: disputeType,
802799
status: status,
803800
createdAt: createdAt,
801+
cancellableAt: cancellableAt,
804802
stakeSnapshot: stakeSnapshot
805803
});
806804
}

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

+39
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
1515
resetPrank(users.fisherman);
1616
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
1717

18+
// skip to end of dispute period
19+
uint256 disputePeriod = disputeManager.disputePeriod();
20+
skip(disputePeriod + 1);
21+
1822
_cancelDispute(disputeID);
1923
}
2024

@@ -38,4 +42,39 @@ contract DisputeManagerIndexingCancelDisputeTest is DisputeManagerTest {
3842
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
3943
disputeManager.cancelDispute(disputeID);
4044
}
45+
46+
function test_Indexing_Cancel_After_DisputePeriodIncreased(uint256 tokens) public useIndexer useAllocation(tokens) {
47+
resetPrank(users.fisherman);
48+
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
49+
50+
// change the dispute period to a higher value
51+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
52+
resetPrank(users.governor);
53+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
54+
55+
// skip to end of old dispute period
56+
skip(oldDisputePeriod + 1);
57+
58+
// should be able to cancel
59+
resetPrank(users.fisherman);
60+
_cancelDispute(disputeID);
61+
}
62+
63+
function test_Indexing_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
64+
resetPrank(users.fisherman);
65+
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
66+
67+
// change the dispute period to a lower value
68+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
69+
resetPrank(users.governor);
70+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
71+
72+
// skip to end of new dispute period
73+
skip(oldDisputePeriod / 2 + 1);
74+
75+
// should not be able to cancel
76+
resetPrank(users.fisherman);
77+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
78+
disputeManager.cancelDispute(disputeID);
79+
}
4180
}

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

+43
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ contract DisputeManagerQueryCancelDisputeTest is DisputeManagerTest {
2222
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
2323
bytes32 disputeID = _createQueryDispute(attestationData);
2424

25+
// skip to end of dispute period
26+
uint256 disputePeriod = disputeManager.disputePeriod();
27+
skip(disputePeriod + 1);
28+
2529
_cancelDispute(disputeID);
2630
}
2731

@@ -45,4 +49,43 @@ contract DisputeManagerQueryCancelDisputeTest is DisputeManagerTest {
4549
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
4650
disputeManager.cancelDispute(disputeID);
4751
}
52+
53+
function test_Query_Cancel_After_DisputePeriodIncreased(uint256 tokens) public useIndexer useAllocation(tokens) {
54+
resetPrank(users.fisherman);
55+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
56+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
57+
bytes32 disputeID = _createQueryDispute(attestationData);
58+
59+
// change the dispute period to a higher value
60+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
61+
resetPrank(users.governor);
62+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
63+
64+
// skip to end of old dispute period
65+
skip(oldDisputePeriod + 1);
66+
67+
// should be able to cancel
68+
resetPrank(users.fisherman);
69+
_cancelDispute(disputeID);
70+
}
71+
72+
function test_Query_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
73+
resetPrank(users.fisherman);
74+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
75+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
76+
bytes32 disputeID = _createQueryDispute(attestationData);
77+
78+
// change the dispute period to a lower value
79+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
80+
resetPrank(users.governor);
81+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
82+
83+
// skip to end of new dispute period
84+
skip(oldDisputePeriod / 2 + 1);
85+
86+
// should not be able to cancel
87+
resetPrank(users.fisherman);
88+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
89+
disputeManager.cancelDispute(disputeID);
90+
}
4891
}

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

+60
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ contract DisputeManagerQueryConflictCancelDisputeTest is DisputeManagerTest {
2828
resetPrank(users.fisherman);
2929
(bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
3030

31+
// skip to end of dispute period
32+
uint256 disputePeriod = disputeManager.disputePeriod();
33+
skip(disputePeriod + 1);
34+
3135
_cancelDispute(disputeID1);
3236
}
3337

@@ -70,4 +74,60 @@ contract DisputeManagerQueryConflictCancelDisputeTest is DisputeManagerTest {
7074
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
7175
disputeManager.cancelDispute(disputeID1);
7276
}
77+
78+
function test_Query_Conflict_Cancel_After_DisputePeriodIncreased(
79+
uint256 tokens
80+
) public useIndexer useAllocation(tokens) {
81+
resetPrank(users.fisherman);
82+
(bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations(
83+
requestCID,
84+
subgraphDeployment,
85+
responseCID1,
86+
responseCID2,
87+
allocationIDPrivateKey,
88+
allocationIDPrivateKey
89+
);
90+
91+
resetPrank(users.fisherman);
92+
(bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
93+
94+
// change the dispute period to a higher value
95+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
96+
resetPrank(users.governor);
97+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod * 2));
98+
99+
// skip to end of old dispute period
100+
skip(oldDisputePeriod + 1);
101+
102+
// should be able to cancel
103+
resetPrank(users.fisherman);
104+
_cancelDispute(disputeID1);
105+
}
106+
107+
function test_Query_Cancel_After_DisputePeriodDecreased(uint256 tokens) public useIndexer useAllocation(tokens) {
108+
(bytes memory attestationData1, bytes memory attestationData2) = _createConflictingAttestations(
109+
requestCID,
110+
subgraphDeployment,
111+
responseCID1,
112+
responseCID2,
113+
allocationIDPrivateKey,
114+
allocationIDPrivateKey
115+
);
116+
117+
resetPrank(users.fisherman);
118+
(bytes32 disputeID1, ) = _createQueryDisputeConflict(attestationData1, attestationData2);
119+
120+
// change the dispute period to a lower value
121+
uint256 oldDisputePeriod = disputeManager.disputePeriod();
122+
resetPrank(users.governor);
123+
disputeManager.setDisputePeriod(uint64(oldDisputePeriod / 2));
124+
125+
// skip to end of new dispute period
126+
skip(oldDisputePeriod / 2 + 1);
127+
128+
// should not be able to cancel
129+
resetPrank(users.fisherman);
130+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputePeriodNotFinished.selector));
131+
disputeManager.cancelDispute(disputeID1);
132+
}
73133
}

0 commit comments

Comments
 (0)