fix(DK): withdraw local round bug

Author: unknownunknown1

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

@@ -25,7 +25,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         Round[] rounds; // Rounds of the dispute. 0 is the default round, and [1, ..n] are the appeal rounds.
         uint256 numberOfChoices; // The number of choices jurors have when voting. This does not include choice `0` which is reserved for "refuse to arbitrate".
         bool jumped; // True if dispute jumped to a parent dispute kit and won't be handled by this DK anymore.
-        mapping(uint256 => uint256) coreRoundIDToLocal; // Maps id of the round in the core contract to the index of the round of related local dispute.
+        mapping(uint256 => uint256) coreRoundIDToLocal; // Maps id of the round in the core contract to the length of the rounds array after the creation of a corresponding local dispute. Note we rely on length to skip the default 0 index.
         bytes extraData; // Extradata for the dispute.
         uint256[10] __gap; // Reserved slots for future upgrades.
     }
@@ -222,12 +222,13 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
 
         dispute.numberOfChoices = _numberOfChoices;
         dispute.extraData = _extraData;
-        // New round in the Core should be created before the dispute creation in DK.
-        dispute.coreRoundIDToLocal[core.getNumberOfRounds(_coreDisputeID) - 1] = dispute.rounds.length;
 
         Round storage round = dispute.rounds.push();
         round.tied = true;
 
+        // New round in the Core should be created before the dispute creation in DK.
+        dispute.coreRoundIDToLocal[core.getNumberOfRounds(_coreDisputeID) - 1] = dispute.rounds.length;
+
         emit DisputeCreation(_coreDisputeID, _numberOfChoices, _extraData);
     }
 
@@ -428,7 +429,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
                 dispute.jumped = true;
             } else {
                 // Don't subtract 1 from length since both round arrays haven't been updated yet.
-                dispute.coreRoundIDToLocal[coreRoundID + 1] = dispute.rounds.length;
+                dispute.coreRoundIDToLocal[coreRoundID + 1] = dispute.rounds.length + 1;
 
                 Round storage newRound = dispute.rounds.push();
                 newRound.tied = true;
@@ -458,7 +459,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         if (!coreDisputeIDToActive[_coreDisputeID]) revert NotActiveForCoreDisputeID();
 
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Round storage round = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]];
+        Round storage round = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1];
         (uint256 finalRuling, , ) = core.currentRuling(_coreDisputeID);
 
         if (!round.hasPaid[_choice]) {
@@ -562,7 +563,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     ) internal view returns (uint256 coherence) {
         // In this contract this degree can be either 0 or 1, but in other dispute kits this value can be something in between.
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]].votes[_voteID];
+        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1].votes[_voteID];
         (uint256 winningChoice, bool tied, ) = core.currentRuling(_coreDisputeID);
 
         if (vote.voted && (vote.choice == winningChoice || tied)) {
@@ -575,7 +576,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     /// @inheritdoc IDisputeKit
     function getCoherentCount(uint256 _coreDisputeID, uint256 _coreRoundID) external view override returns (uint256) {
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Round storage currentRound = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]];
+        Round storage currentRound = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1];
         (uint256 winningChoice, bool tied, ) = core.currentRuling(_coreDisputeID);
 
         if (currentRound.totalVoted == 0 || (!tied && currentRound.counts[winningChoice] == 0)) {
@@ -643,7 +644,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         uint256 _voteID
     ) external view override returns (bool) {
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]].votes[_voteID];
+        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1].votes[_voteID];
         return vote.voted;
     }
 
@@ -666,7 +667,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         )
     {
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Round storage round = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]];
+        Round storage round = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1];
         return (
             round.winningChoice,
             round.tied,
@@ -694,7 +695,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         uint256 _coreRoundID
     ) external view returns (uint256 localDisputeID, uint256 localRoundID) {
         localDisputeID = coreDisputeIDToLocal[_coreDisputeID];
-        localRoundID = disputes[localDisputeID].coreRoundIDToLocal[_coreRoundID];
+        localRoundID = disputes[localDisputeID].coreRoundIDToLocal[_coreRoundID] - 1;
     }
 
     /// @inheritdoc IDisputeKit
@@ -704,7 +705,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         uint256 _voteID
     ) external view override returns (address account, bytes32 commit, uint256 choice, bool voted) {
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
-        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]].votes[_voteID];
+        Vote storage vote = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID] - 1].votes[_voteID];
         return (vote.account, vote.commit, vote.choice, vote.voted);
     }
 

contracts/test/foundry/KlerosCore_Appeals.t.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.24;
 
 import {KlerosCore_TestBase} from "./KlerosCore_TestBase.sol";
+import {stdError} from "forge-std/Test.sol";
 import {KlerosCore} from "../../src/arbitration/KlerosCore.sol";
 import {DisputeKitClassic, DisputeKitClassicBase} from "../../src/arbitration/dispute-kits/DisputeKitClassic.sol";
 import {UUPSProxy} from "../../src/proxy/UUPSProxy.sol";
@@ -762,15 +763,19 @@ contract KlerosCore_AppealsTest is KlerosCore_TestBase {
         vm.expectEmit();
         emit DisputeKitClassicBase.Withdrawal(disputeID, 0, 2, payable(crowdfunder2), 0.84 ether);
         disputeKit3.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 0, 2); // REWARDS
+        vm.expectRevert(stdError.arithmeticError); // underflow
         disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 0, 1); // wrong DK, no reward
+        vm.expectRevert(stdError.arithmeticError); // underflow
         disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 0, 2); // wrong DK, no reward
         vm.expectRevert(DisputeKitClassicBase.NotActiveForCoreDisputeID.selector);
         disputeKit.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 0, 1); // wrong DK, no reward
         vm.expectRevert(DisputeKitClassicBase.NotActiveForCoreDisputeID.selector);
         disputeKit.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 0, 2); // wrong DK, no reward
 
         // Appeal Rewards for Round 2 //
+        vm.expectRevert(stdError.arithmeticError); // underflow
         disputeKit3.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 1, 1); // wrong DK, no reward
+        vm.expectRevert(stdError.arithmeticError); // underflow
         disputeKit3.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 1, 2); // wrong DK, no reward
         disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 1, 1); // wrong side, no reward
         vm.expectEmit();
@@ -784,8 +789,10 @@ contract KlerosCore_AppealsTest is KlerosCore_TestBase {
         // Appeal Rewards for Round 3 //
         disputeKit3.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 2, 1); // no appeal, no reward
         disputeKit3.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 2, 2); // no appeal, no reward
-        disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 2, 1); // no appeal, no reward
-        disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 2, 2); // no appeal, no reward
+        vm.expectRevert(stdError.arithmeticError); // underflow
+        disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 2, 1); // wrong DK, no reward
+        vm.expectRevert(stdError.arithmeticError); // underflow
+        disputeKit2.withdrawFeesAndRewards(disputeID, payable(crowdfunder2), 2, 2); // wrong DK, no reward
         vm.expectRevert(DisputeKitClassicBase.NotActiveForCoreDisputeID.selector);
         disputeKit.withdrawFeesAndRewards(disputeID, payable(crowdfunder1), 2, 1); // wrong DK, no reward
         vm.expectRevert(DisputeKitClassicBase.NotActiveForCoreDisputeID.selector);