From 838e7bf3a0962875df7c20c481380600095dadf9 Mon Sep 17 00:00:00 2001 From: unknownunknown1 Date: Wed, 8 Nov 2023 18:31:15 +1000 Subject: [PATCH 1/2] fix(KlerosCore): dispute kit structure fix --- contracts/src/arbitration/KlerosCore.sol | 142 ++++++----------------- contracts/test/arbitration/index.ts | 1 - 2 files changed, 37 insertions(+), 106 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index cfee3e003..871df03c6 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -43,7 +43,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 feeForJuror; // Arbitration fee paid per juror. uint256 jurorsForCourtJump; // The appeal after the one that reaches this number of jurors will go to the parent court if any. uint256[4] timesPerPeriod; // The time allotted to each dispute period in the form `timesPerPeriod[period]`. - mapping(uint256 => bool) supportedDisputeKits; // True if DK with this ID is supported by the court. + mapping(uint256 => bool) supportedDisputeKits; // True if DK with this ID is supported by the court. Note that each court must support classic dispute kit. bool disabled; // True if the court is disabled. Unused for now, will be implemented later. } @@ -77,14 +77,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { mapping(uint96 => uint256) stakedPnkByCourt; // The amount of PNKs the juror has staked in the court in the form `stakedPnkByCourt[courtID]`. } - struct DisputeKitNode { - uint256 parent; // Index of the parent dispute kit. If it's 0 then this DK is a root. - uint256[] children; // List of child dispute kits. - IDisputeKit disputeKit; // The dispute kit implementation. - uint256 depthLevel; // How far this DK is from the root. 0 for root DK. - bool disabled; // True if the dispute kit is disabled and can't be used. This parameter is added preemptively to avoid storage changes in the future. - } - // Workaround "stack too deep" errors struct ExecuteParams { uint256 disputeID; // The ID of the dispute to execute. @@ -107,14 +99,13 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 private constant ALPHA_DIVISOR = 1e4; // The number to divide `Court.alpha` by. uint256 private constant NON_PAYABLE_AMOUNT = (2 ** 256 - 2) / 2; // An amount higher than the supply of ETH. - uint256 private constant SEARCH_ITERATIONS = 10; // Number of iterations to search for suitable parent court before jumping to the top court. address public governor; // The governor of the contract. IERC20 public pinakion; // The Pinakion token contract. address public jurorProsecutionModule; // The module for juror's prosecution. ISortitionModule public sortitionModule; // Sortition module for drawing. Court[] public courts; // The courts. - DisputeKitNode[] public disputeKitNodes; // The list of DisputeKitNode, indexed by DisputeKitID. + IDisputeKit[] public disputeKits; // Array of dispute kits. Dispute[] public disputes; // The disputes. mapping(address => Juror) internal jurors; // The jurors. mapping(IERC20 => CurrencyRate) public currencyRates; // The price of each token in ETH. @@ -149,11 +140,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 _jurorsForCourtJump, uint256[4] _timesPerPeriod ); - event DisputeKitCreated( - uint256 indexed _disputeKitID, - IDisputeKit indexed _disputeKitAddress, - uint256 indexed _parent - ); + event DisputeKitCreated(uint256 indexed _disputeKitID, IDisputeKit indexed _disputeKitAddress); event DisputeKitEnabled(uint96 indexed _courtID, uint256 indexed _disputeKitID, bool indexed _enable); event CourtJump( uint256 indexed _disputeID, @@ -228,20 +215,13 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { jurorProsecutionModule = _jurorProsecutionModule; sortitionModule = _sortitionModuleAddress; - // NULL_DISPUTE_KIT: an empty element at index 0 to indicate when a node has no parent. - disputeKitNodes.push(); + // NULL_DISPUTE_KIT: an empty element at index 0 to indicate when a dispute kit is not supported. + disputeKits.push(); // DISPUTE_KIT_CLASSIC - disputeKitNodes.push( - DisputeKitNode({ - parent: Constants.NULL_DISPUTE_KIT, - children: new uint256[](0), - disputeKit: _disputeKit, - depthLevel: 0, - disabled: false - }) - ); - emit DisputeKitCreated(Constants.DISPUTE_KIT_CLASSIC, _disputeKit, Constants.NULL_DISPUTE_KIT); + disputeKits.push(_disputeKit); + + emit DisputeKitCreated(Constants.DISPUTE_KIT_CLASSIC, _disputeKit); // FORKING_COURT // TODO: Fill the properties for the Forking court, emit CourtCreated. @@ -326,33 +306,10 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @dev Add a new supported dispute kit module to the court. /// @param _disputeKitAddress The address of the dispute kit contract. - /// @param _parent The ID of the parent dispute kit. It is left empty when root DK is created. - /// Note that the root DK must be supported by the general court. - function addNewDisputeKit(IDisputeKit _disputeKitAddress, uint256 _parent) external onlyByGovernor { - uint256 disputeKitID = disputeKitNodes.length; - if (_parent >= disputeKitID) revert InvalidDisputKitParent(); - uint256 depthLevel; - if (_parent != Constants.NULL_DISPUTE_KIT) { - depthLevel = disputeKitNodes[_parent].depthLevel + 1; - // It should be always possible to reach the root from the leaf with the defined number of search iterations. - if (depthLevel >= SEARCH_ITERATIONS) revert DepthLevelMax(); - } - disputeKitNodes.push( - DisputeKitNode({ - parent: _parent, - children: new uint256[](0), - disputeKit: _disputeKitAddress, - depthLevel: depthLevel, - disabled: false - }) - ); - - disputeKitNodes[_parent].children.push(disputeKitID); - emit DisputeKitCreated(disputeKitID, _disputeKitAddress, _parent); - if (_parent == Constants.NULL_DISPUTE_KIT) { - // A new dispute kit tree root should always be supported by the General court. - _enableDisputeKit(Constants.GENERAL_COURT, disputeKitID, true); - } + function addNewDisputeKit(IDisputeKit _disputeKitAddress) external onlyByGovernor { + uint256 disputeKitID = disputeKits.length; + disputeKits.push(_disputeKitAddress); + emit DisputeKitCreated(disputeKitID, _disputeKitAddress); } /// @dev Creates a court under a specified parent court. @@ -384,11 +341,13 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { Court storage court = courts.push(); for (uint256 i = 0; i < _supportedDisputeKits.length; i++) { - if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKitNodes.length) { + if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKits.length) { revert WrongDisputeKitIndex(); } court.supportedDisputeKits[_supportedDisputeKits[i]] = true; } + // Check that Classic DK support was added. + if (!court.supportedDisputeKits[Constants.DISPUTE_KIT_CLASSIC]) revert MustSupportDisputeKitClassic(); court.parent = _parent; court.children = new uint256[](0); @@ -458,16 +417,14 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { function enableDisputeKits(uint96 _courtID, uint256[] memory _disputeKitIDs, bool _enable) external onlyByGovernor { for (uint256 i = 0; i < _disputeKitIDs.length; i++) { if (_enable) { - if (_disputeKitIDs[i] == 0 || _disputeKitIDs[i] >= disputeKitNodes.length) { + if (_disputeKitIDs[i] == 0 || _disputeKitIDs[i] >= disputeKits.length) { revert WrongDisputeKitIndex(); } _enableDisputeKit(_courtID, _disputeKitIDs[i], true); } else { - if ( - _courtID == Constants.GENERAL_COURT && - disputeKitNodes[_disputeKitIDs[i]].parent == Constants.NULL_DISPUTE_KIT - ) { - revert CannotDisableRootDKInGeneral(); + // Classic dispute kit must be supported by all courts. + if (_disputeKitIDs[i] == Constants.DISPUTE_KIT_CLASSIC) { + revert CannotDisableClassicDK(); } _enableDisputeKit(_courtID, _disputeKitIDs[i], false); } @@ -547,7 +504,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { dispute.arbitrated = IArbitrableV2(msg.sender); dispute.lastPeriodChange = block.timestamp; - IDisputeKit disputeKit = disputeKitNodes[disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[disputeKitID]; Court storage court = courts[dispute.courtID]; Round storage round = dispute.rounds.push(); @@ -587,7 +544,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { } else if (dispute.period == Period.commit) { if ( block.timestamp - dispute.lastPeriodChange < court.timesPerPeriod[uint256(dispute.period)] && - !disputeKitNodes[round.disputeKitID].disputeKit.areCommitsAllCast(_disputeID) + !disputeKits[round.disputeKitID].areCommitsAllCast(_disputeID) ) { revert CommitPeriodNotPassed(); } @@ -595,7 +552,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { } else if (dispute.period == Period.vote) { if ( block.timestamp - dispute.lastPeriodChange < court.timesPerPeriod[uint256(dispute.period)] && - !disputeKitNodes[round.disputeKitID].disputeKit.areVotesAllCast(_disputeID) + !disputeKits[round.disputeKitID].areVotesAllCast(_disputeID) ) { revert VotePeriodNotPassed(); } @@ -623,7 +580,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { Round storage round = dispute.rounds[currentRound]; if (dispute.period != Period.evidence) revert NotEvidencePeriod(); - IDisputeKit disputeKit = disputeKitNodes[round.disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[round.disputeKitID]; uint256 startIndex = round.drawIterations; // for gas: less storage reads uint256 i; @@ -654,7 +611,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { if (dispute.period != Period.appeal) revert DisputeNotAppealable(); Round storage round = dispute.rounds[dispute.rounds.length - 1]; - if (msg.sender != address(disputeKitNodes[round.disputeKitID].disputeKit)) revert DisputeKitOnly(); + if (msg.sender != address(disputeKits[round.disputeKitID])) revert DisputeKitOnly(); uint96 newCourtID = dispute.courtID; uint256 newDisputeKitID = round.disputeKitID; @@ -666,22 +623,9 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // Jump to parent court. newCourtID = courts[newCourtID].parent; - for (uint256 i = 0; i < SEARCH_ITERATIONS; i++) { - if (courts[newCourtID].supportedDisputeKits[newDisputeKitID]) { - break; - } else if (disputeKitNodes[newDisputeKitID].parent != Constants.NULL_DISPUTE_KIT) { - newDisputeKitID = disputeKitNodes[newDisputeKitID].parent; - } else { - // DK's parent has 0 index, that means we reached the root DK (0 depth level). - // Jump to the next parent court if the current court doesn't support any DK from this tree. - // Note that we don't reset newDisputeKitID in this case as, a precaution. - newCourtID = courts[newCourtID].parent; - } - } - // We didn't find a court that is compatible with DK from this tree, so we jump directly to the top court. - // Note that this can only happen when disputeKitID is at its root, and each root DK is supported by the top court by default. if (!courts[newCourtID].supportedDisputeKits[newDisputeKitID]) { - newCourtID = Constants.GENERAL_COURT; + // Switch to classic dispute kit if parent court doesn't support the current one. + newDisputeKitID = Constants.DISPUTE_KIT_CLASSIC; } if (newCourtID != dispute.courtID) { @@ -704,7 +648,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // Dispute kit was changed, so create a dispute in the new DK contract. if (extraRound.disputeKitID != round.disputeKitID) { emit DisputeKitJump(_disputeID, dispute.rounds.length - 1, round.disputeKitID, extraRound.disputeKitID); - disputeKitNodes[extraRound.disputeKitID].disputeKit.createDispute( + disputeKits[extraRound.disputeKitID].createDispute( _disputeID, _numberOfChoices, _extraData, @@ -725,7 +669,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { if (dispute.period != Period.execution) revert NotExecutionPeriod(); Round storage round = dispute.rounds[_round]; - IDisputeKit disputeKit = disputeKitNodes[round.disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[round.disputeKitID]; uint256 start = round.repartitions; uint256 end = round.repartitions + _iterations; @@ -765,7 +709,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { function _executePenalties(ExecuteParams memory _params) internal returns (uint256) { Dispute storage dispute = disputes[_params.disputeID]; Round storage round = dispute.rounds[_params.round]; - IDisputeKit disputeKit = disputeKitNodes[round.disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[round.disputeKitID]; // [0, 1] value that determines how coherent the juror was in this round, in basis points. uint256 degreeOfCoherence = disputeKit.getDegreeOfCoherence( @@ -833,7 +777,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { function _executeRewards(ExecuteParams memory _params) internal { Dispute storage dispute = disputes[_params.disputeID]; Round storage round = dispute.rounds[_params.round]; - IDisputeKit disputeKit = disputeKitNodes[round.disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[round.disputeKitID]; // [0, 1] value that determines how coherent the juror was in this round, in basis points. uint256 degreeOfCoherence = disputeKit.getDegreeOfCoherence( @@ -988,7 +932,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { function currentRuling(uint256 _disputeID) public view returns (uint256 ruling, bool tied, bool overridden) { Dispute storage dispute = disputes[_disputeID]; Round storage round = dispute.rounds[dispute.rounds.length - 1]; - IDisputeKit disputeKit = disputeKitNodes[round.disputeKitID].disputeKit; + IDisputeKit disputeKit = disputeKits[round.disputeKitID]; (ruling, tied, overridden) = disputeKit.currentRuling(_disputeID); } @@ -1015,13 +959,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { return courts[_courtID].supportedDisputeKits[_disputeKitID]; } - /// @dev Gets non-primitive properties of a specified dispute kit node. - /// @param _disputeKitID The ID of the dispute kit. - /// @return children Indexes of children of this DK. - function getDisputeKitChildren(uint256 _disputeKitID) external view returns (uint256[] memory) { - return disputeKitNodes[_disputeKitID].children; - } - /// @dev Gets the timesPerPeriod array for a given court. /// @param _courtID The ID of the court to get the times from. /// @return timesPerPeriod The timesPerPeriod array for the given court. @@ -1056,14 +993,8 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { return !courts[court.parent].supportedDisputeKits[round.disputeKitID]; } - function getDisputeKitNodesLength() external view returns (uint256) { - return disputeKitNodes.length; - } - - /// @dev Gets the dispute kit for a specific `_disputeKitID`. - /// @param _disputeKitID The ID of the dispute kit. - function getDisputeKit(uint256 _disputeKitID) external view returns (IDisputeKit) { - return disputeKitNodes[_disputeKitID].disputeKit; + function getDisputeKitsLength() external view returns (uint256) { + return disputeKits.length; } /// @dev Gets the court identifiers where a specific `_juror` has staked. @@ -1083,7 +1014,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @dev Toggles the dispute kit support for a given court. /// @param _courtID The ID of the court to toggle the support for. /// @param _disputeKitID The ID of the dispute kit to toggle the support for. - /// @param _enable Whether to enable or disable the support. + /// @param _enable Whether to enable or disable the support. Note that classic dispute kit should always be enabled. function _enableDisputeKit(uint96 _courtID, uint256 _disputeKitID, bool _enable) internal { courts[_courtID].supportedDisputeKits[_disputeKitID] = _enable; emit DisputeKitEnabled(_courtID, _disputeKitID, _enable); @@ -1197,7 +1128,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { if (minJurors == 0) { minJurors = Constants.DEFAULT_NB_OF_JURORS; } - if (disputeKitID == Constants.NULL_DISPUTE_KIT || disputeKitID >= disputeKitNodes.length) { + if (disputeKitID == Constants.NULL_DISPUTE_KIT || disputeKitID >= disputeKits.length) { disputeKitID = Constants.DISPUTE_KIT_CLASSIC; // 0 index is not used. } } else { @@ -1219,12 +1150,13 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { error UnsupportedDisputeKit(); error InvalidForkingCourtAsParent(); error WrongDisputeKitIndex(); - error CannotDisableRootDKInGeneral(); + error CannotDisableClassicDK(); error ArraysLengthMismatch(); error StakingFailed(); error WrongCaller(); error ArbitrationFeesNotEnough(); error DisputeKitNotSupportedByCourt(); + error MustSupportDisputeKitClassic(); error TokenNotAccepted(); error EvidenceNotPassedAndNotAppeal(); error DisputeStillDrawing(); diff --git a/contracts/test/arbitration/index.ts b/contracts/test/arbitration/index.ts index f3530437b..bf8b4bb02 100644 --- a/contracts/test/arbitration/index.ts +++ b/contracts/test/arbitration/index.ts @@ -18,7 +18,6 @@ describe("DisputeKitClassic", async () => { expect(events.length).to.equal(1); expect(events[0].args._disputeKitID).to.equal(1); expect(events[0].args._disputeKitAddress).to.equal(disputeKit.address); - expect(events[0].args._parent).to.equal(0); // Reminder: the Forking court will be added which will break these expectations. events = await core.queryFilter(core.filters.CourtCreated()); From 4a007e39e8be5e078f361f15950ebc42b12b9a70 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Thu, 7 Dec 2023 18:03:18 +0000 Subject: [PATCH 2/2] feat: supporting the simplified DK structure --- subgraph/schema.graphql | 3 --- subgraph/src/entities/DisputeKit.ts | 11 ++--------- subgraph/subgraph.yaml | 2 +- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/subgraph/schema.graphql b/subgraph/schema.graphql index e0826be6e..e8715ab2d 100644 --- a/subgraph/schema.graphql +++ b/subgraph/schema.graphql @@ -198,10 +198,7 @@ type Draw @entity(immutable: true) { type DisputeKit @entity { id: ID! address: Bytes - parent: DisputeKit - children: [DisputeKit!]! @derivedFrom(field: "parent") needsFreezing: Boolean! - depthLevel: BigInt! rounds: [Round!]! @derivedFrom(field: "disputeKit") courts: [Court!]! @derivedFrom(field: "supportedDisputeKits") } diff --git a/subgraph/src/entities/DisputeKit.ts b/subgraph/src/entities/DisputeKit.ts index f10a975b1..150aaed0b 100644 --- a/subgraph/src/entities/DisputeKit.ts +++ b/subgraph/src/entities/DisputeKit.ts @@ -4,21 +4,14 @@ import { ZERO, ONE } from "../utils"; export function createDisputeKitFromEvent(event: DisputeKitCreated): void { const disputeKit = new DisputeKit(event.params._disputeKitID.toString()); - disputeKit.parent = event.params._parent.toString(); disputeKit.address = event.params._disputeKitAddress; disputeKit.needsFreezing = false; - const parent = DisputeKit.load(event.params._parent.toString()); - disputeKit.depthLevel = parent ? parent.depthLevel.plus(ONE) : ZERO; disputeKit.save(); } -export function filterSupportedDisputeKits( - supportedDisputeKits: string[], - disputeKitID: string -): string[] { +export function filterSupportedDisputeKits(supportedDisputeKits: string[], disputeKitID: string): string[] { let result: string[] = []; for (let i = 0; i < supportedDisputeKits.length; i++) - if (supportedDisputeKits[i] !== disputeKitID) - result = result.concat([supportedDisputeKits[i]]); + if (supportedDisputeKits[i] !== disputeKitID) result = result.concat([supportedDisputeKits[i]]); return result; } diff --git a/subgraph/subgraph.yaml b/subgraph/subgraph.yaml index 801616467..0fd7441f9 100644 --- a/subgraph/subgraph.yaml +++ b/subgraph/subgraph.yaml @@ -42,7 +42,7 @@ dataSources: handler: handleCourtCreated - event: CourtModified(indexed uint96,bool,uint256,uint256,uint256,uint256,uint256[4]) handler: handleCourtModified - - event: DisputeKitCreated(indexed uint256,indexed address,indexed uint256) + - event: DisputeKitCreated(indexed uint256,indexed address) handler: handleDisputeKitCreated - event: DisputeKitEnabled(indexed uint96,indexed uint256,indexed bool) handler: handleDisputeKitEnabled