Skip to content

Commit 3aab3a7

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 02a6c1c commit 3aab3a7

File tree

6 files changed

+161
-6
lines changed

6 files changed

+161
-6
lines changed

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

+6-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)) {
@@ -417,6 +418,7 @@ contract DisputeManager is
417418

418419
// Store dispute
419420
uint256 stakeSnapshot = _getStakeSnapshot(indexer, provision.tokens);
421+
uint256 cancellableAt = block.timestamp + disputePeriod;
420422
disputes[disputeId] = Dispute(
421423
indexer,
422424
_fisherman,
@@ -425,6 +427,7 @@ contract DisputeManager is
425427
DisputeType.QueryDispute,
426428
IDisputeManager.DisputeStatus.Pending,
427429
block.timestamp,
430+
cancellableAt,
428431
stakeSnapshot
429432
);
430433

@@ -435,6 +438,7 @@ contract DisputeManager is
435438
_deposit,
436439
_attestation.subgraphDeploymentId,
437440
_attestationData,
441+
cancellableAt,
438442
stakeSnapshot
439443
);
440444

@@ -481,6 +485,7 @@ contract DisputeManager is
481485
DisputeType.IndexingDispute,
482486
IDisputeManager.DisputeStatus.Pending,
483487
block.timestamp,
488+
block.timestamp + disputePeriod,
484489
stakeSnapshot
485490
);
486491

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

+5-1
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

@@ -97,6 +99,7 @@ interface IDisputeManager {
9799
* @param tokens The amount of tokens deposited by the fisherman
98100
* @param subgraphDeploymentId The subgraph deployment id
99101
* @param attestation The attestation
102+
* @param cancellableAt The timestamp when the dispute can be cancelled
100103
* @param stakeSnapshot The stake snapshot of the indexer at the time of the dispute
101104
*/
102105
event QueryDisputeCreated(
@@ -106,7 +109,8 @@ interface IDisputeManager {
106109
uint256 tokens,
107110
bytes32 subgraphDeploymentId,
108111
bytes attestation,
109-
uint256 stakeSnapshot
112+
uint256 stakeSnapshot,
113+
uint256 cancellableAt
110114
);
111115

112116
/**

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
145145
uint256 disputeDeposit = disputeManager.disputeDeposit();
146146
uint256 beforeFishermanBalance = token.balanceOf(fisherman);
147147
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(indexer);
148+
uint256 cancellableAt = block.timestamp + disputeManager.disputePeriod();
148149

149150
// Approve the dispute deposit
150151
token.approve(address(disputeManager), disputeDeposit);
@@ -157,6 +158,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
157158
disputeDeposit,
158159
attestation.subgraphDeploymentId,
159160
_attestationData,
161+
cancellableAt,
160162
stakeSnapshot
161163
);
162164

@@ -318,6 +320,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
318320
)
319321
);
320322

323+
uint256 cancellableAt = block.timestamp + disputeManager.disputePeriod();
324+
321325
// createQueryDisputeConflict
322326
vm.expectEmit(address(disputeManager));
323327
emit IDisputeManager.QueryDisputeCreated(
@@ -327,6 +331,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
327331
disputeDeposit / 2,
328332
beforeValues.attestation1.subgraphDeploymentId,
329333
attestationData1,
334+
cancellableAt,
330335
beforeValues.stakeSnapshot1
331336
);
332337
vm.expectEmit(address(disputeManager));
@@ -337,6 +342,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
337342
disputeDeposit / 2,
338343
beforeValues.attestation2.subgraphDeploymentId,
339344
attestationData2,
345+
cancellableAt,
340346
beforeValues.stakeSnapshot2
341347
);
342348

@@ -680,12 +686,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
680686
if (isDisputeInConflict) relatedDispute = _getDispute(dispute.relatedDisputeId);
681687
address fisherman = dispute.fisherman;
682688
uint256 fishermanPreviousBalance = token.balanceOf(fisherman);
683-
uint256 disputePeriod = disputeManager.disputePeriod();
684689
uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService));
685690

686-
// skip to end of dispute period
687-
skip(disputePeriod + 1);
688-
689691
vm.expectEmit(address(disputeManager));
690692
emit IDisputeManager.DisputeCancelled(_disputeId, dispute.indexer, dispute.fisherman, dispute.deposit);
691693

@@ -792,6 +794,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
792794
IDisputeManager.DisputeType disputeType,
793795
IDisputeManager.DisputeStatus status,
794796
uint256 createdAt,
797+
uint256 cancellableAt,
795798
uint256 stakeSnapshot
796799
) = disputeManager.disputes(_disputeId);
797800
return
@@ -803,6 +806,7 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
803806
disputeType: disputeType,
804807
status: status,
805808
createdAt: createdAt,
809+
cancellableAt: cancellableAt,
806810
stakeSnapshot: stakeSnapshot
807811
});
808812
}

‎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)