fix(DisputeKit): check that dispute belongs to DK

Author: unknownunknown1

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

@@ -64,6 +64,8 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     mapping(uint256 localDisputeID => mapping(uint256 localRoundID => mapping(address drawnAddress => bool)))
         public alreadyDrawn; // 'true' if the address has already been drawn, false by default. To be added to the Round struct when fully redeploying rather than upgrading.
 
+    mapping(uint256 => uint256) public coreDisputeIDToDisputeLength; // Maps core dispute ID with the current length of disputes array to avoid falling back to 0 index and make sure core dispute is indeed connected to this DK.
+
     // ************************************* //
     // *              Events               * //
     // ************************************* //
@@ -204,6 +206,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         round.tied = true;
 
         coreDisputeIDToLocal[_coreDisputeID] = localDisputeID;
+        coreDisputeIDToDisputeLength[_coreDisputeID] = localDisputeID + 1;
         emit DisputeCreation(_coreDisputeID, _numberOfChoices, _extraData);
     }
 
@@ -250,6 +253,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         (, , KlerosCore.Period period, , ) = core.disputes(_coreDisputeID);
         require(period == KlerosCoreBase.Period.commit, "The dispute should be in Commit period.");
         require(_commit != bytes32(0), "Empty commit.");
+        require(coreDisputeIDToDisputeLength[_coreDisputeID] != 0, "No local dispute for core ID");
 
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
         Round storage round = dispute.rounds[dispute.rounds.length - 1];
@@ -279,6 +283,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         (, , KlerosCore.Period period, , ) = core.disputes(_coreDisputeID);
         require(period == KlerosCoreBase.Period.vote, "The dispute should be in Vote period.");
         require(_voteIDs.length > 0, "No voteID provided");
+        require(coreDisputeIDToDisputeLength[_coreDisputeID] != 0, "No local dispute for core ID");
 
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
         require(_choice <= dispute.numberOfChoices, "Choice out of bounds");
@@ -325,6 +330,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
     function fundAppeal(uint256 _coreDisputeID, uint256 _choice) external payable notJumped(_coreDisputeID) {
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
         require(_choice <= dispute.numberOfChoices, "There is no such ruling to fund.");
+        require(coreDisputeIDToDisputeLength[_coreDisputeID] != 0, "No local dispute for core ID");
 
         (uint256 appealPeriodStart, uint256 appealPeriodEnd) = core.appealPeriod(_coreDisputeID);
         require(block.timestamp >= appealPeriodStart && block.timestamp < appealPeriodEnd, "Appeal period is over.");
@@ -404,6 +410,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi
         (, , , bool isRuled, ) = core.disputes(_coreDisputeID);
         require(isRuled, "Dispute should be resolved.");
         require(!core.paused(), "Core is paused");
+        require(coreDisputeIDToDisputeLength[_coreDisputeID] != 0, "No local dispute for core ID");
 
         Dispute storage dispute = disputes[coreDisputeIDToLocal[_coreDisputeID]];
         Round storage round = dispute.rounds[dispute.coreRoundIDToLocal[_coreRoundID]];

contracts/test/foundry/KlerosCore.t.sol

@@ -1393,6 +1393,7 @@ contract KlerosCoreTest is Test {
         assertEq(jumped, false, "jumped should be false");
         assertEq(extraData, newExtraData, "Wrong extra data");
         assertEq(disputeKit.coreDisputeIDToLocal(0), disputeID, "Wrong local disputeID");
+        assertEq(disputeKit.coreDisputeIDToDisputeLength(0), 1, "Wrong disputes length");
 
         (
             uint256 winningChoice,
@@ -1783,7 +1784,7 @@ contract KlerosCoreTest is Test {
         (address account, bytes32 commit, uint256 choice, bool voted) = disputeKit.getVoteInfo(0, 0, 0); // Dispute - Round - VoteID
         assertEq(account, staker1, "Wrong drawn account");
         assertEq(commit, bytes32(0), "Commit should be empty");
-        assertEq(choice, 2, "Choice should be empty");
+        assertEq(choice, 2, "Choice should be 2");
         assertEq(voted, true, "Voted should be true");
 
         assertEq(disputeKit.isVoteActive(0, 0, 0), true, "Vote should be active"); // Dispute - Round - VoteID
@@ -2827,4 +2828,98 @@ contract KlerosCoreTest is Test {
         assertEq(crowdfunder2.balance, 10 ether, "Wrong balance of the crowdfunder2");
         assertEq(address(disputeKit).balance, 0, "Wrong balance of the DK");
     }
+
+    function test_castVote_differentDK() public {
+        DisputeKitClassic dkLogic = new DisputeKitClassic();
+        // Create a new DK to check castVote.
+        bytes memory initDataDk = abi.encodeWithSignature("initialize(address,address)", governor, address(core));
+
+        UUPSProxy proxyDk = new UUPSProxy(address(dkLogic), initDataDk);
+        DisputeKitClassic newDisputeKit = DisputeKitClassic(address(proxyDk));
+
+        vm.prank(governor);
+        core.addNewDisputeKit(newDisputeKit);
+
+        uint256 newDkID = 2;
+        uint256[] memory supportedDK = new uint256[](1);
+        bytes memory newExtraData = abi.encodePacked(uint256(GENERAL_COURT), DEFAULT_NB_OF_JURORS, newDkID);
+
+        vm.prank(governor);
+        vm.expectEmit(true, true, true, true);
+        emit KlerosCoreBase.DisputeKitEnabled(GENERAL_COURT, newDkID, true);
+        supportedDK[0] = newDkID;
+        core.enableDisputeKits(GENERAL_COURT, supportedDK, true);
+        assertEq(core.isSupported(GENERAL_COURT, newDkID), true, "New DK should be supported by General court");
+
+        vm.prank(staker1);
+        core.setStake(GENERAL_COURT, 20000);
+
+        // Create one dispute for the old DK and two disputes for the new DK.
+        vm.prank(disputer);
+        arbitrable.createDispute{value: feeForJuror * DEFAULT_NB_OF_JURORS}("Action");
+
+        arbitrable.changeArbitratorExtraData(newExtraData);
+
+        vm.prank(disputer);
+        arbitrable.createDispute{value: feeForJuror * DEFAULT_NB_OF_JURORS}("Action");
+
+        vm.prank(disputer);
+        arbitrable.createDispute{value: feeForJuror * DEFAULT_NB_OF_JURORS}("Action");
+
+        uint256 disputeID = 2; // Use the latest dispute for reference. This is the ID in the core contract
+
+        vm.warp(block.timestamp + minStakingTime);
+        sortitionModule.passPhase(); // Generating
+        vm.roll(block.number + rngLookahead + 1);
+        sortitionModule.passPhase(); // Drawing phase
+
+        KlerosCoreBase.Round memory round = core.getRoundInfo(disputeID, 0);
+        assertEq(round.disputeKitID, newDkID, "Wrong DK ID");
+
+        core.draw(disputeID, DEFAULT_NB_OF_JURORS);
+        // Draw jurors for the old DK as well to prepare round.votes array
+        core.draw(0, DEFAULT_NB_OF_JURORS);
+
+        vm.warp(block.timestamp + timesPerPeriod[0]);
+        core.passPeriod(disputeID); // Vote
+
+        // Check that the new DK has the info but not the old one.
+
+        assertEq(disputeKit.coreDisputeIDToDisputeLength(disputeID), 0, "Should be 0 for old DK");
+
+        // This is the DK where dispute was created. Core dispute points to index 1 because new DK has two disputes.
+        assertEq(newDisputeKit.coreDisputeIDToLocal(disputeID), 1, "Wrong local dispute ID for new DK");
+        assertEq(newDisputeKit.coreDisputeIDToDisputeLength(disputeID), 2, "Wrong disputes length for new DK");
+        (uint256 numberOfChoices, , bytes memory extraData) = newDisputeKit.disputes(1);
+        assertEq(numberOfChoices, 2, "Wrong numberOfChoices in new DK");
+        assertEq(extraData, newExtraData, "Wrong extra data");
+
+        uint256[] memory voteIDs = new uint256[](3);
+        voteIDs[0] = 0;
+        voteIDs[1] = 1;
+        voteIDs[2] = 2;
+
+        // Deliberately cast votes using the old DK to see if the exception will be caught.
+        vm.prank(staker1);
+        vm.expectRevert(bytes("No local dispute for core ID"));
+        disputeKit.castVote(disputeID, voteIDs, 2, 0, "XYZ");
+
+        // And check the new DK.
+        vm.prank(staker1);
+        newDisputeKit.castVote(disputeID, voteIDs, 2, 0, "XYZ");
+
+        (
+            uint256 winningChoice,
+            bool tied,
+            uint256 totalVoted,
+            uint256 totalCommited,
+            ,
+            uint256 choiceCount
+        ) = newDisputeKit.getRoundInfo(disputeID, 0, 2);
+        assertEq(winningChoice, 2, "Wrong winning choice");
+        assertEq(tied, false, "tied should be false");
+        assertEq(totalVoted, 3, "totalVoted should be 3");
+        assertEq(totalCommited, 0, "totalCommited should be 0");
+        assertEq(choiceCount, 3, "choiceCount should be 3");
+    }
 }