diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index de998adbb..8122b86b7 100644 --- a/contracts/disputes/DisputeManager.sol +++ b/contracts/disputes/DisputeManager.sol @@ -154,6 +154,15 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa _; } + modifier onlyPendingDispute(bytes32 _disputeID) { + require(isDisputeCreated(_disputeID), "Dispute does not exist"); + require( + disputes[_disputeID].status == IDisputeManager.DisputeStatus.Pending, + "Dispute must be pending" + ); + _; + } + // -- Functions -- /** @@ -290,7 +299,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _disputeID True if dispute already exists */ function isDisputeCreated(bytes32 _disputeID) public view override returns (bool) { - return disputes[_disputeID].fisherman != address(0); + return disputes[_disputeID].status != DisputeStatus.Null; } /** @@ -479,7 +488,8 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa _fisherman, _deposit, 0, // no related dispute, - DisputeType.QueryDispute + DisputeType.QueryDispute, + IDisputeManager.DisputeStatus.Pending ); emit QueryDisputeCreated( @@ -546,7 +556,8 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa _fisherman, _deposit, 0, - DisputeType.IndexingDispute + DisputeType.IndexingDispute, + IDisputeManager.DisputeStatus.Pending ); emit IndexingDisputeCreated(disputeID, alloc.indexer, _fisherman, _deposit, _allocationID); @@ -562,8 +573,16 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @notice Accept a dispute with ID `_disputeID` * @param _disputeID ID of the dispute to be accepted */ - function acceptDispute(bytes32 _disputeID) external override onlyArbitrator { - Dispute memory dispute = _resolveDispute(_disputeID); + function acceptDispute(bytes32 _disputeID) + external + override + onlyArbitrator + onlyPendingDispute(_disputeID) + { + Dispute storage dispute = disputes[_disputeID]; + + // store the dispute status + dispute.status = IDisputeManager.DisputeStatus.Accepted; // Slash (, uint256 tokensToReward) = _slashIndexer( @@ -575,8 +594,9 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa // Give the fisherman their deposit back TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); - // Resolve the conflicting dispute if any - _resolveDisputeInConflict(dispute); + if (_isDisputeInConflict(dispute)) { + rejectDispute(dispute.relatedDisputeID); + } emit DisputeAccepted( _disputeID, @@ -591,8 +611,16 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @notice Reject a dispute with ID `_disputeID` * @param _disputeID ID of the dispute to be rejected */ - function rejectDispute(bytes32 _disputeID) external override onlyArbitrator { - Dispute memory dispute = _resolveDispute(_disputeID); + function rejectDispute(bytes32 _disputeID) + public + override + onlyArbitrator + onlyPendingDispute(_disputeID) + { + Dispute storage dispute = disputes[_disputeID]; + + // store dispute status + dispute.status = IDisputeManager.DisputeStatus.Rejected; // Handle conflicting dispute if any require( @@ -611,32 +639,24 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @notice Ignore a dispute with ID `_disputeID` * @param _disputeID ID of the dispute to be disregarded */ - function drawDispute(bytes32 _disputeID) external override onlyArbitrator { - Dispute memory dispute = _resolveDispute(_disputeID); + function drawDispute(bytes32 _disputeID) + public + override + onlyArbitrator + onlyPendingDispute(_disputeID) + { + Dispute storage dispute = disputes[_disputeID]; // Return deposit to the fisherman TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); - // Resolve the conflicting dispute if any - _resolveDisputeInConflict(dispute); - - emit DisputeDrawn(_disputeID, dispute.indexer, dispute.fisherman, dispute.deposit); - } - - /** - * @dev Resolve a dispute by removing it from storage and returning a memory copy. - * @param _disputeID ID of the dispute to resolve - * @return Dispute - */ - function _resolveDispute(bytes32 _disputeID) private returns (Dispute memory) { - require(isDisputeCreated(_disputeID), "Dispute does not exist"); + // resolve related dispute if any + _drawDisputeInConflict(dispute); - Dispute memory dispute = disputes[_disputeID]; + // store dispute status + dispute.status = IDisputeManager.DisputeStatus.Drawn; - // Resolve dispute - delete disputes[_disputeID]; // Re-entrancy - - return dispute; + emit DisputeDrawn(_disputeID, dispute.indexer, dispute.fisherman, dispute.deposit); } /** @@ -644,8 +664,11 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _dispute Dispute * @return True conflicting attestation dispute */ - function _isDisputeInConflict(Dispute memory _dispute) private pure returns (bool) { - return _dispute.relatedDisputeID != 0; + function _isDisputeInConflict(Dispute memory _dispute) private view returns (bool) { + bytes32 relatedID = _dispute.relatedDisputeID; + // this is so the check returns false when rejecting the related dispute. + return + relatedID != 0 && disputes[relatedID].status == IDisputeManager.DisputeStatus.Pending; } /** @@ -653,10 +676,11 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _dispute Dispute * @return True if resolved */ - function _resolveDisputeInConflict(Dispute memory _dispute) private returns (bool) { + function _drawDisputeInConflict(Dispute memory _dispute) private returns (bool) { if (_isDisputeInConflict(_dispute)) { bytes32 relatedDisputeID = _dispute.relatedDisputeID; - delete disputes[relatedDisputeID]; + Dispute storage relatedDispute = disputes[relatedDisputeID]; + relatedDispute.status = IDisputeManager.DisputeStatus.Drawn; return true; } return false; diff --git a/contracts/disputes/IDisputeManager.sol b/contracts/disputes/IDisputeManager.sol index 962e2f2d9..9c872f890 100644 --- a/contracts/disputes/IDisputeManager.sol +++ b/contracts/disputes/IDisputeManager.sol @@ -12,6 +12,14 @@ interface IDisputeManager { QueryDispute } + enum DisputeStatus { + Null, + Accepted, + Rejected, + Drawn, + Pending + } + // Disputes contain info necessary for the Arbitrator to verify and resolve struct Dispute { address indexer; @@ -19,6 +27,7 @@ interface IDisputeManager { uint256 deposit; bytes32 relatedDisputeID; DisputeType disputeType; + DisputeStatus status; } // -- Attestation -- diff --git a/test/disputes/query.test.ts b/test/disputes/query.test.ts index b4ef9bcdf..e8eb8a2a1 100644 --- a/test/disputes/query.test.ts +++ b/test/disputes/query.test.ts @@ -21,6 +21,7 @@ import { } from '../lib/testHelpers' import { Dispute, createQueryDisputeID, encodeAttestation, MAX_PPM } from './common' +import { isExportDeclaration } from 'typescript' const { AddressZero, HashZero } = constants @@ -528,8 +529,10 @@ describe('DisputeManager:Query', async () => { // Do await disputeManager.connect(arbitrator.signer).acceptDispute(dID1) // Check + const mainDispute = await disputeManager.disputes(dID1) + expect(mainDispute.status).to.eq(1) // 1 = DisputeStatus.Accepted const relatedDispute = await disputeManager.disputes(dID2) - expect(relatedDispute.indexer).eq(AddressZero) + expect(relatedDispute.status).to.eq(2) // 2 = DisputeStatus.Rejected }) it('should not allow to reject, user need to accept the related dispute ID to reject it', async function () { @@ -549,7 +552,7 @@ describe('DisputeManager:Query', async () => { await disputeManager.connect(arbitrator.signer).drawDispute(dID1) // Check const relatedDispute = await disputeManager.disputes(dID2) - expect(relatedDispute.indexer).eq(AddressZero) + expect(relatedDispute.status).not.eq(4) // 4 = DisputeStatus.Pending }) }) })