From 7ab82b93fce7565dcd806ea5ffa3bcf30839be3a Mon Sep 17 00:00:00 2001 From: mercuricchloride Date: Fri, 16 Dec 2022 09:57:13 -0700 Subject: [PATCH 1/5] feat: changed dispute manager to not delete bad actors disputes on resolution --- contracts/disputes/DisputeManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index de998adbb..ae132fb9b 100644 --- a/contracts/disputes/DisputeManager.sol +++ b/contracts/disputes/DisputeManager.sol @@ -563,7 +563,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _disputeID ID of the dispute to be accepted */ function acceptDispute(bytes32 _disputeID) external override onlyArbitrator { - Dispute memory dispute = _resolveDispute(_disputeID); + Dispute memory dispute = disputes[_disputeID]; // Slash (, uint256 tokensToReward) = _slashIndexer( From e12d98c596ad250eb266c488a3ca9cf1d128ea08 Mon Sep 17 00:00:00 2001 From: mercuricchloride Date: Sat, 17 Dec 2022 13:37:06 -0700 Subject: [PATCH 2/5] feat: added disputeStatus mapping and enum --- contracts/disputes/DisputeManager.sol | 17 ++++++++++++++++- contracts/disputes/DisputeManagerStorage.sol | 4 ++++ contracts/disputes/IDisputeManager.sol | 7 +++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index ae132fb9b..811b31083 100644 --- a/contracts/disputes/DisputeManager.sol +++ b/contracts/disputes/DisputeManager.sol @@ -482,6 +482,9 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa DisputeType.QueryDispute ); + // store the dispute status + disputeStatus[disputeID] = IDisputeManager.DisputeStatus.Pending; + emit QueryDisputeCreated( disputeID, indexer, @@ -549,6 +552,9 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa DisputeType.IndexingDispute ); + // store dispute status + disputeStatus[disputeID] = IDisputeManager.DisputeStatus.Pending; + emit IndexingDisputeCreated(disputeID, alloc.indexer, _fisherman, _deposit, _allocationID); return disputeID; @@ -563,7 +569,10 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _disputeID ID of the dispute to be accepted */ function acceptDispute(bytes32 _disputeID) external override onlyArbitrator { - Dispute memory dispute = disputes[_disputeID]; + Dispute memory dispute = _resolveDispute(_disputeID); + + // store dispute status + disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Accepted; // Slash (, uint256 tokensToReward) = _slashIndexer( @@ -594,6 +603,9 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa function rejectDispute(bytes32 _disputeID) external override onlyArbitrator { Dispute memory dispute = _resolveDispute(_disputeID); + // store dispute status + disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Rejected; + // Handle conflicting dispute if any require( !_isDisputeInConflict(dispute), @@ -614,6 +626,9 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa function drawDispute(bytes32 _disputeID) external override onlyArbitrator { Dispute memory dispute = _resolveDispute(_disputeID); + // store dispute status + disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Drawn; + // Return deposit to the fisherman TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); diff --git a/contracts/disputes/DisputeManagerStorage.sol b/contracts/disputes/DisputeManagerStorage.sol index 4df6e0ae6..32fec2966 100644 --- a/contracts/disputes/DisputeManagerStorage.sol +++ b/contracts/disputes/DisputeManagerStorage.sol @@ -31,4 +31,8 @@ contract DisputeManagerV1Storage is Managed { // Disputes created : disputeID => Dispute // disputeID - check creation functions to see how disputeID is built mapping(bytes32 => IDisputeManager.Dispute) public disputes; + + // Dispute status : disputeID => DisputeStatus + // disputeID - check creation functions to see how disputeID is built + mapping(bytes32 => IDisputeManager.DisputeStatus) public disputeStatus; } diff --git a/contracts/disputes/IDisputeManager.sol b/contracts/disputes/IDisputeManager.sol index 962e2f2d9..e3e7aef36 100644 --- a/contracts/disputes/IDisputeManager.sol +++ b/contracts/disputes/IDisputeManager.sol @@ -12,6 +12,13 @@ interface IDisputeManager { QueryDispute } + enum DisputeStatus { + Accepted, + Rejected, + Drawn, + Pending + } + // Disputes contain info necessary for the Arbitrator to verify and resolve struct Dispute { address indexer; From 29ea956f6dba0666654283da6e16775760b04848 Mon Sep 17 00:00:00 2001 From: mercuricchloride Date: Wed, 21 Dec 2022 17:24:56 -0500 Subject: [PATCH 3/5] feat: added new formatting --- contracts/disputes/DisputeManager.sol | 102 ++++++++++++------- contracts/disputes/DisputeManagerStorage.sol | 4 - contracts/disputes/IDisputeManager.sol | 1 + test/disputes/query.test.ts | 5 +- 4 files changed, 68 insertions(+), 44 deletions(-) diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index 811b31083..cfb3cbe7d 100644 --- a/contracts/disputes/DisputeManager.sol +++ b/contracts/disputes/DisputeManager.sol @@ -479,12 +479,10 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa _fisherman, _deposit, 0, // no related dispute, - DisputeType.QueryDispute + DisputeType.QueryDispute, + IDisputeManager.DisputeStatus.Pending ); - // store the dispute status - disputeStatus[disputeID] = IDisputeManager.DisputeStatus.Pending; - emit QueryDisputeCreated( disputeID, indexer, @@ -549,17 +547,24 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa _fisherman, _deposit, 0, - DisputeType.IndexingDispute + DisputeType.IndexingDispute, + IDisputeManager.DisputeStatus.Pending ); - // store dispute status - disputeStatus[disputeID] = IDisputeManager.DisputeStatus.Pending; - emit IndexingDisputeCreated(disputeID, alloc.indexer, _fisherman, _deposit, _allocationID); return disputeID; } + modifier onlyPendingDispute(bytes32 _disputeID) { + require(isDisputeCreated(_disputeID), "Dispute does not exist"); + require( + disputes[_disputeID].status == IDisputeManager.DisputeStatus.Pending, + "Dispute must be pending" + ); + _; + } + /** * @dev The arbitrator accepts a dispute as being valid. * This function will revert if the indexer is not slashable, whether because it does not have @@ -568,11 +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 dispute status - disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Accepted; + // store the dispute status + dispute.status = IDisputeManager.DisputeStatus.Accepted; // Slash (, uint256 tokensToReward) = _slashIndexer( @@ -584,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, @@ -600,11 +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 - disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Rejected; + dispute.status = IDisputeManager.DisputeStatus.Rejected; // Handle conflicting dispute if any require( @@ -623,17 +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]; // store dispute status - disputeStatus[_disputeID] = IDisputeManager.DisputeStatus.Drawn; + dispute.status = IDisputeManager.DisputeStatus.Drawn; // Return deposit to the fisherman TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); // Resolve the conflicting dispute if any - _resolveDisputeInConflict(dispute); + if (_isDisputeInConflict(dispute)) { + drawDispute(dispute.relatedDisputeID); + } emit DisputeDrawn(_disputeID, dispute.indexer, dispute.fisherman, dispute.deposit); } @@ -643,24 +666,26 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @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"); + // function _resolveDispute(bytes32 _disputeID) private returns (Dispute memory) { + // require(isDisputeCreated(_disputeID), "Dispute does not exist"); - Dispute memory dispute = disputes[_disputeID]; + // Dispute memory dispute = disputes[_disputeID]; - // Resolve dispute - delete disputes[_disputeID]; // Re-entrancy + // // Resolve dispute + // delete disputes[_disputeID]; // Re-entrancy - return dispute; - } + // return dispute; + // } /** * @dev Returns whether the dispute is for a conflicting attestation or not. * @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; + return + relatedID != 0 && disputes[relatedID].status == IDisputeManager.DisputeStatus.Pending; } /** @@ -668,14 +693,15 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _dispute Dispute * @return True if resolved */ - function _resolveDisputeInConflict(Dispute memory _dispute) private returns (bool) { - if (_isDisputeInConflict(_dispute)) { - bytes32 relatedDisputeID = _dispute.relatedDisputeID; - delete disputes[relatedDisputeID]; - return true; - } - return false; - } + // function _resolveDisputeInConflict(Dispute memory _dispute) private returns (bool) { + // if (_isDisputeInConflict(_dispute)) { + // bytes32 relatedDisputeID = _dispute.relatedDisputeID; + // Dispute storage relatedDispute = disputes[relatedDisputeID]; + // delete disputes[relatedDisputeID]; + // return true; + // } + // return false; + // } /** * @dev Pull deposit from submitter account. diff --git a/contracts/disputes/DisputeManagerStorage.sol b/contracts/disputes/DisputeManagerStorage.sol index 32fec2966..4df6e0ae6 100644 --- a/contracts/disputes/DisputeManagerStorage.sol +++ b/contracts/disputes/DisputeManagerStorage.sol @@ -31,8 +31,4 @@ contract DisputeManagerV1Storage is Managed { // Disputes created : disputeID => Dispute // disputeID - check creation functions to see how disputeID is built mapping(bytes32 => IDisputeManager.Dispute) public disputes; - - // Dispute status : disputeID => DisputeStatus - // disputeID - check creation functions to see how disputeID is built - mapping(bytes32 => IDisputeManager.DisputeStatus) public disputeStatus; } diff --git a/contracts/disputes/IDisputeManager.sol b/contracts/disputes/IDisputeManager.sol index e3e7aef36..5c78e723c 100644 --- a/contracts/disputes/IDisputeManager.sol +++ b/contracts/disputes/IDisputeManager.sol @@ -26,6 +26,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 b7548e842..7365dd199 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 @@ -516,7 +517,7 @@ describe('DisputeManager:Query', async () => { await disputeManager.connect(arbitrator.signer).acceptDispute(dID1) // Check const relatedDispute = await disputeManager.disputes(dID2) - expect(relatedDispute.indexer).eq(AddressZero) + expect(relatedDispute.status).not.eq(3) // 3 = DisputeStatus.Pending }) it('should not allow to reject, user need to accept the related dispute ID to reject it', async function () { @@ -536,7 +537,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(3) // 3 = DisputeStatus.Pending }) }) }) From 0ddea5453c29099862f94fd005d3a3627920275f Mon Sep 17 00:00:00 2001 From: mercuricchloride Date: Wed, 12 Apr 2023 06:45:40 -0700 Subject: [PATCH 4/5] fix: applied requested changes --- contracts/disputes/DisputeManager.sol | 50 +++++++++++++------------- contracts/disputes/IDisputeManager.sol | 1 + test/disputes/query.test.ts | 2 +- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index cfb3cbe7d..1b8d73578 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; } /** @@ -556,15 +565,6 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa return disputeID; } - modifier onlyPendingDispute(bytes32 _disputeID) { - require(isDisputeCreated(_disputeID), "Dispute does not exist"); - require( - disputes[_disputeID].status == IDisputeManager.DisputeStatus.Pending, - "Dispute must be pending" - ); - _; - } - /** * @dev The arbitrator accepts a dispute as being valid. * This function will revert if the indexer is not slashable, whether because it does not have @@ -647,16 +647,14 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa { Dispute storage dispute = disputes[_disputeID]; - // store dispute status - dispute.status = IDisputeManager.DisputeStatus.Drawn; - // Return deposit to the fisherman TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); - // Resolve the conflicting dispute if any - if (_isDisputeInConflict(dispute)) { - drawDispute(dispute.relatedDisputeID); - } + // resolve related dispute if any + _resolveDisputeInConflict(dispute); + + // store dispute status + dispute.status = IDisputeManager.DisputeStatus.Drawn; emit DisputeDrawn(_disputeID, dispute.indexer, dispute.fisherman, dispute.deposit); } @@ -693,15 +691,15 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa * @param _dispute Dispute * @return True if resolved */ - // function _resolveDisputeInConflict(Dispute memory _dispute) private returns (bool) { - // if (_isDisputeInConflict(_dispute)) { - // bytes32 relatedDisputeID = _dispute.relatedDisputeID; - // Dispute storage relatedDispute = disputes[relatedDisputeID]; - // delete disputes[relatedDisputeID]; - // return true; - // } - // return false; - // } + function _resolveDisputeInConflict(Dispute memory _dispute) private returns (bool) { + if (_isDisputeInConflict(_dispute)) { + bytes32 relatedDisputeID = _dispute.relatedDisputeID; + Dispute storage relatedDispute = disputes[relatedDisputeID]; + relatedDispute.status = IDisputeManager.DisputeStatus.Drawn; + return true; + } + return false; + } /** * @dev Pull deposit from submitter account. diff --git a/contracts/disputes/IDisputeManager.sol b/contracts/disputes/IDisputeManager.sol index 5c78e723c..9c872f890 100644 --- a/contracts/disputes/IDisputeManager.sol +++ b/contracts/disputes/IDisputeManager.sol @@ -13,6 +13,7 @@ interface IDisputeManager { } enum DisputeStatus { + Null, Accepted, Rejected, Drawn, diff --git a/test/disputes/query.test.ts b/test/disputes/query.test.ts index 7365dd199..ab3dc1302 100644 --- a/test/disputes/query.test.ts +++ b/test/disputes/query.test.ts @@ -537,7 +537,7 @@ describe('DisputeManager:Query', async () => { await disputeManager.connect(arbitrator.signer).drawDispute(dID1) // Check const relatedDispute = await disputeManager.disputes(dID2) - expect(relatedDispute.status).not.eq(3) // 3 = DisputeStatus.Pending + expect(relatedDispute.status).not.eq(4) // 4 = DisputeStatus.Pending }) }) }) From a928e0b780d0dd4083f084f80b6c1deb4c77a2a1 Mon Sep 17 00:00:00 2001 From: mercuricchloride Date: Wed, 19 Apr 2023 11:14:19 -0700 Subject: [PATCH 5/5] fix: applied the requested changes --- contracts/disputes/DisputeManager.sol | 21 +++------------------ test/disputes/query.test.ts | 4 +++- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/contracts/disputes/DisputeManager.sol b/contracts/disputes/DisputeManager.sol index 1b8d73578..8122b86b7 100644 --- a/contracts/disputes/DisputeManager.sol +++ b/contracts/disputes/DisputeManager.sol @@ -651,7 +651,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit); // resolve related dispute if any - _resolveDisputeInConflict(dispute); + _drawDisputeInConflict(dispute); // store dispute status dispute.status = IDisputeManager.DisputeStatus.Drawn; @@ -659,22 +659,6 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa 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"); - - // Dispute memory dispute = disputes[_disputeID]; - - // // Resolve dispute - // delete disputes[_disputeID]; // Re-entrancy - - // return dispute; - // } - /** * @dev Returns whether the dispute is for a conflicting attestation or not. * @param _dispute Dispute @@ -682,6 +666,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa */ 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; } @@ -691,7 +676,7 @@ 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; Dispute storage relatedDispute = disputes[relatedDisputeID]; diff --git a/test/disputes/query.test.ts b/test/disputes/query.test.ts index ab3dc1302..0a350f775 100644 --- a/test/disputes/query.test.ts +++ b/test/disputes/query.test.ts @@ -516,8 +516,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.status).not.eq(3) // 3 = DisputeStatus.Pending + 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 () {