fix: feedback from reviews

sanity check in Core._getCompatibleNextRoundSettings(),
bug fix in appealCost(),
naming of the NextRoundSettingsChanged parameters,
better comments describing the NextRoundSettings parameters.

Author: jaybuidl

contracts/src/arbitration/KlerosCore.sol

@@ -1080,31 +1080,26 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable {
 
     /// @notice Gets the cost of appealing a specified dispute.
     /// @param _disputeID The ID of the dispute.
-    /// @return cost The appeal cost.
-    function appealCost(uint256 _disputeID) public view returns (uint256 cost) {
+    /// @return The appeal cost.
+    function appealCost(uint256 _disputeID) public view returns (uint256) {
         Dispute storage dispute = disputes[_disputeID];
         Round storage round = dispute.rounds[dispute.rounds.length - 1];
         Court storage court = courts[dispute.courtID];
-
         (uint96 newCourtID, , uint256 nbVotesAfterAppeal) = _getCompatibleNextRoundSettings(
             dispute,
             round,
             court,
             _disputeID
         );
-
-        if (newCourtID != dispute.courtID) {
-            // Jump to parent court.
-            if (dispute.courtID == GENERAL_COURT) {
-                // TODO: Handle the forking when appealed in General court.
-                cost = NON_PAYABLE_AMOUNT; // Get the cost of the parent court.
-            } else {
-                cost = courts[court.parent].feeForJuror * nbVotesAfterAppeal;
-            }
-        } else {
-            // Stay in current court.
-            cost = court.feeForJuror * nbVotesAfterAppeal;
+        if (newCourtID == dispute.courtID) {
+            // No court jump
+            return court.feeForJuror * nbVotesAfterAppeal;
+        }
+        if (dispute.courtID != GENERAL_COURT && newCourtID != FORKING_COURT) {
+            // Court jump but not to the Forking court
+            return courts[newCourtID].feeForJuror * nbVotesAfterAppeal;
         }
+        return NON_PAYABLE_AMOUNT; // Jumping to the Forking Court is not supported yet.
     }
 
     /// @notice Gets the start and the end of a specified dispute's current appeal period.
@@ -1256,12 +1251,22 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable {
             disputeKitID,
             _round.nbVotes
         );
-
+        if (
+            newCourtID == FORKING_COURT ||
+            newCourtID >= courts.length ||
+            newDisputeKitID == NULL_DISPUTE_KIT ||
+            newDisputeKitID >= disputeKits.length
+        ) {
+            // Falling back to the current court and dispute kit with default nbVotes.
+            newCourtID = _dispute.courtID;
+            newDisputeKitID = disputeKitID;
+            newRoundNbVotes = (_round.nbVotes * 2) + 1;
+        }
         // Ensure compatibility between the next round's court and dispute kit.
-        if (!courts[newCourtID].supportedDisputeKits[newDisputeKitID] || newDisputeKitID == NULL_DISPUTE_KIT) {
-            // Fall back to `DisputeKitClassic` which is always supported.
+        if (!courts[newCourtID].supportedDisputeKits[newDisputeKitID]) {
+            // Falling back to `DisputeKitClassic` which is always supported and with default nbVotes.
             newDisputeKitID = DISPUTE_KIT_CLASSIC;
-            newRoundNbVotes = (_round.nbVotes * 2) + 1; // Reset nbVotes to the default logic.
+            newRoundNbVotes = (_round.nbVotes * 2) + 1;
         }
     }
 

contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol

@@ -60,10 +60,10 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
 
     struct NextRoundSettings {
         bool enabled; // True if the settings are enabled, false otherwise.
-        uint96 jumpCourtID; // The ID of the court for the next round. Zero is considered as undefined.
-        uint256 jumpDisputeKitID; // The ID of the dispute kit for the next round. Zero is considered as undefined.
-        uint256 jumpDisputeKitIDOnCourtJump; // The ID of the dispute kit if the court jumps and `jumpDisputeKitID` is undefined. Zero is considered as undefined.
-        uint256 nbVotes; // The number of votes for the next round. Zero is considered as undefined.
+        uint96 jumpCourtID; // A non-zero value makes the next round use this court ID. Zero is considered as undefined.
+        uint256 jumpDisputeKitID; // A non-zero value makes the next round use this dispute kit ID. Zero is considered as undefined.
+        uint256 jumpDisputeKitIDOnCourtJump; // A non-zero value makes the next round use this dispute kit ID ONLY IF the court jumps and `jumpDisputeKitID` is undefined. Zero is considered as undefined.
+        uint256 nbVotes; // A non-zero value makes the next round use this number of votes. Zero is considered as undefined.
     }
 
     // ************************************* //
@@ -130,9 +130,9 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     event ChoiceFunded(uint256 indexed _coreDisputeID, uint256 indexed _coreRoundID, uint256 indexed _choice);
 
     /// @notice To be emitted when the next round settings are changed.
-    /// @param _currentCourtID The ID of the current court.
+    /// @param _courtID The ID of the court that the settings are changed for.
     /// @param _nextRoundSettings The settings for the next round.
-    event NextRoundSettingsChanged(uint96 indexed _currentCourtID, NextRoundSettings _nextRoundSettings);
+    event NextRoundSettingsChanged(uint96 indexed _courtID, NextRoundSettings _nextRoundSettings);
 
     // ************************************* //
     // *              Modifiers            * //
@@ -198,14 +198,14 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     }
 
     /// @notice Changes the settings for the next round.
-    /// @param _currentCourtID The ID of the current court.
+    /// @param _courtID The ID of the court that the settings are changed for.
     /// @param _nextRoundSettings The settings for the next round.
     function changeNextRoundSettings(
-        uint96 _currentCourtID,
+        uint96 _courtID,
         NextRoundSettings memory _nextRoundSettings
     ) external onlyByOwner {
-        courtIDToNextRoundSettings[_currentCourtID] = _nextRoundSettings;
-        emit NextRoundSettingsChanged(_currentCourtID, _nextRoundSettings);
+        courtIDToNextRoundSettings[_courtID] = _nextRoundSettings;
+        emit NextRoundSettingsChanged(_courtID, _nextRoundSettings);
     }
 
     // ************************************* //