fix(KlerosCore): dispute kit structure fix

unknownunknown1 · jaybuidl · commit 838e7bf3a096 · 2023-12-07T17:38:14.000Z
diff --git 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,15 +544,15 @@ 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();
             }
             dispute.period = Period.vote;
         } 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
@@ -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());
