Skip to content

fix(DisputeKit): check that dispute belongs to DK #2039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(DisputeKit): check that dispute belongs to DK #2039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 * //
// ************************************* //
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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]];
Expand Down
97 changes: 96 additions & 1 deletion contracts/test/foundry/KlerosCore.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
}
Loading