Skip to content
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

fix(KC): remove null dk #1893

Closed
wants to merge 2 commits into from
Closed

fix(KC): remove null dk #1893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de1c7bd
fix(KC): remove null dk
unknownunknown1 Feb 13, 2025
e7f0d52
fix(KlerosCore): small fixes
unknownunknown1 Mar 5, 2025
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
2 changes: 1 addition & 1 deletion contracts/scripts/simulations/tasks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ task("simulate:create-court", "callable by Governor only. Create a new Court")
const hiddenVotes = false as boolean;
const timesPerPeriod = [300, 300, 300, 300] as [BigNumberish, BigNumberish, BigNumberish, BigNumberish];
const sortitionSumTreeK = ethers.toBeHex(3);
const supportedDisputeKits = [1] as BigNumberish[]; // IDs of supported dispute kits
const supportedDisputeKits = [0] as BigNumberish[]; // IDs of supported dispute kits
let courtID;
try {
const tx = await (
Expand Down
12 changes: 4 additions & 8 deletions contracts/src/arbitration/KlerosCoreBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@ abstract contract KlerosCoreBase is IArbitratorV2, Initializable, UUPSProxiable
jurorProsecutionModule = _jurorProsecutionModule;
sortitionModule = _sortitionModuleAddress;

// NULL_DISPUTE_KIT: an empty element at index 0 to indicate when a dispute kit is not supported.
disputeKits.push();

// DISPUTE_KIT_CLASSIC
disputeKits.push(_disputeKit);

Expand Down Expand Up @@ -346,7 +343,7 @@ abstract contract KlerosCoreBase is IArbitratorV2, Initializable, UUPSProxiable
Court storage court = courts.push();

for (uint256 i = 0; i < _supportedDisputeKits.length; i++) {
if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKits.length) {
if (_supportedDisputeKits[i] >= disputeKits.length) {
revert WrongDisputeKitIndex();
}
_enableDisputeKit(uint96(courtID), _supportedDisputeKits[i], true);
Expand Down Expand Up @@ -422,7 +419,7 @@ abstract contract KlerosCoreBase is IArbitratorV2, Initializable, UUPSProxiable
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] >= disputeKits.length) {
if (_disputeKitIDs[i] >= disputeKits.length) {
revert WrongDisputeKitIndex();
}
_enableDisputeKit(_courtID, _disputeKitIDs[i], true);
Expand Down Expand Up @@ -1126,8 +1123,8 @@ abstract contract KlerosCoreBase is IArbitratorV2, Initializable, UUPSProxiable
if (minJurors == 0) {
minJurors = DEFAULT_NB_OF_JURORS;
}
if (disputeKitID == NULL_DISPUTE_KIT || disputeKitID >= disputeKits.length) {
disputeKitID = DISPUTE_KIT_CLASSIC; // 0 index is not used.
if (disputeKitID >= disputeKits.length) {
disputeKitID = DISPUTE_KIT_CLASSIC;
}
} else {
courtID = GENERAL_COURT;
Expand All @@ -1145,7 +1142,6 @@ abstract contract KlerosCoreBase is IArbitratorV2, Initializable, UUPSProxiable
error DisputeKitOnly();
error SortitionModuleOnly();
error UnsuccessfulCall();
error InvalidDisputKitParent();
error MinStakeLowerThanParentCourt();
error UnsupportedDisputeKit();
error InvalidForkingCourtAsParent();
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/arbitration/SortitionModuleBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ abstract contract SortitionModuleBase is ISortitionModule, Initializable, UUPSPr
/// @return The stake of the juror in the court.
function stakeOf(bytes32 _key, bytes32 _ID) public view returns (uint256) {
SortitionSumTree storage tree = sortitionSumTrees[_key];
uint treeIndex = tree.IDsToNodeIndexes[_ID];
uint256 treeIndex = tree.IDsToNodeIndexes[_ID];
if (treeIndex == 0) {
return 0;
}
Expand Down
3 changes: 0 additions & 3 deletions contracts/src/arbitration/devtools/KlerosCoreRuler.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,6 @@ contract KlerosCoreRuler is IArbitratorV2, UUPSProxiable, Initializable {
if (minJurors == 0) {
minJurors = DEFAULT_NB_OF_JURORS;
}
if (disputeKitID == NULL_DISPUTE_KIT) {
disputeKitID = DISPUTE_KIT_CLASSIC; // 0 index is not used.
}
} else {
courtID = GENERAL_COURT;
minJurors = DEFAULT_NB_OF_JURORS;
Expand Down
11 changes: 4 additions & 7 deletions contracts/src/arbitration/university/KlerosCoreUniversity.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,6 @@ contract KlerosCoreUniversity is IArbitratorV2, UUPSProxiable, Initializable {
jurorProsecutionModule = _jurorProsecutionModule;
sortitionModule = _sortitionModuleAddress;

// NULL_DISPUTE_KIT: an empty element at index 0 to indicate when a dispute kit is not supported.
disputeKits.push();

// DISPUTE_KIT_CLASSIC
disputeKits.push(_disputeKit);

Expand Down Expand Up @@ -341,7 +338,7 @@ contract KlerosCoreUniversity is IArbitratorV2, UUPSProxiable, Initializable {
Court storage court = courts.push();

for (uint256 i = 0; i < _supportedDisputeKits.length; i++) {
if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKits.length) {
if (_supportedDisputeKits[i] >= disputeKits.length) {
revert WrongDisputeKitIndex();
}
court.supportedDisputeKits[_supportedDisputeKits[i]] = true;
Expand Down Expand Up @@ -415,7 +412,7 @@ contract KlerosCoreUniversity 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] >= disputeKits.length) {
if (_disputeKitIDs[i] >= disputeKits.length) {
revert WrongDisputeKitIndex();
}
_enableDisputeKit(_courtID, _disputeKitIDs[i], true);
Expand Down Expand Up @@ -1118,8 +1115,8 @@ contract KlerosCoreUniversity is IArbitratorV2, UUPSProxiable, Initializable {
if (minJurors == 0) {
minJurors = DEFAULT_NB_OF_JURORS;
}
if (disputeKitID == NULL_DISPUTE_KIT || disputeKitID >= disputeKits.length) {
disputeKitID = DISPUTE_KIT_CLASSIC; // 0 index is not used.
if (disputeKitID >= disputeKits.length) {
disputeKitID = DISPUTE_KIT_CLASSIC;
}
} else {
courtID = GENERAL_COURT;
Expand Down
3 changes: 1 addition & 2 deletions contracts/src/libraries/Constants.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ uint96 constant FORKING_COURT = 0; // Index of the forking court.
uint96 constant GENERAL_COURT = 1; // Index of the default (general) court.

// Dispute Kits
uint256 constant NULL_DISPUTE_KIT = 0; // Null pattern to indicate a top-level DK which has no parent. DEPRECATED, as its main purpose was to accommodate forest structure which is not used now.
uint256 constant DISPUTE_KIT_CLASSIC = 1; // Index of the default DK. 0 index is skipped.
uint256 constant DISPUTE_KIT_CLASSIC = 0; // Index of the default DK.

// Sortition Module
uint256 constant MAX_STAKE_PATHS = 4; // The maximum number of stake paths a juror can have.
Expand Down
2 changes: 1 addition & 1 deletion contracts/test/arbitration/draw.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe("Draw Benchmark", async () => {
256,
[0, 0, 0, 10], // evidencePeriod, commitPeriod, votePeriod, appealPeriod
ethers.toBeHex(5), // Extra data for sortition module will return the default value of K)
[1]
[0]
)
.then((tx) => tx.wait());
});
Expand Down
6 changes: 3 additions & 3 deletions contracts/test/arbitration/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("DisputeKitClassic", async () => {
it("Kleros Core initialization", async () => {
const events = await core.queryFilter(core.filters.DisputeKitCreated());
expect(events.length).to.equal(1);
expect(events[0].args._disputeKitID).to.equal(1);
expect(events[0].args._disputeKitID).to.equal(0);
expect(events[0].args._disputeKitAddress).to.equal(disputeKit.target);

// Reminder: the Forking court will be added which will break these expectations.
Expand All @@ -30,12 +30,12 @@ describe("DisputeKitClassic", async () => {
expect(events2[0].args._feeForJuror).to.equal(ethers.parseUnits("0.1", 18));
expect(events2[0].args._jurorsForCourtJump).to.equal(256);
expect(events2[0].args._timesPerPeriod).to.deep.equal([0, 0, 0, 10]);
expect(events2[0].args._supportedDisputeKits).to.deep.equal([1]);
expect(events2[0].args._supportedDisputeKits).to.deep.equal([0]);

const events3 = await core.queryFilter(core.filters.DisputeKitEnabled());
expect(events3.length).to.equal(1);
expect(events3[0].args._courtID).to.equal(1);
expect(events3[0].args._disputeKitID).to.equal(1);
expect(events3[0].args._disputeKitID).to.equal(0);
expect(events3[0].args._enable).to.equal(true);
});

Expand Down
4 changes: 2 additions & 2 deletions contracts/test/arbitration/staking-neo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ describe("Staking", async () => {
describe("When outside the Staking phase", async () => {
const createSubcourtStakeAndCreateDispute = async () => {
expect(await sortition.phase()).to.be.equal(0); // Staking
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [1]); // Parent - general court, Classic dispute kit
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [0]); // Parent - general court, Classic dispute kit

await pnk.approve(core.target, PNK(4000));
await core.setStake(1, PNK(2000));
Expand Down Expand Up @@ -734,7 +734,7 @@ describe("Staking", async () => {
});

it("Should unstake from all courts", async () => {
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [1]); // Parent - general court, Classic dispute kit
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [0]); // Parent - general court, Classic dispute kit

await pnk.approve(core.target, PNK(4000));
await core.setStake(1, PNK(2000));
Expand Down
4 changes: 2 additions & 2 deletions contracts/test/arbitration/staking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe("Staking", async () => {
const reachDrawingPhase = async () => {
expect(await sortition.phase()).to.be.equal(0); // Staking
const arbitrationCost = ETH(0.1) * 3n;
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [1]); // Parent - general court, Classic dispute kit
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [0]); // Parent - general court, Classic dispute kit

await pnk.approve(core.target, PNK(4000));
await core.setStake(1, PNK(2000));
Expand Down Expand Up @@ -386,7 +386,7 @@ describe("Staking", async () => {

it("Should unstake from all courts", async () => {
const arbitrationCost = ETH(0.1) * 3n;
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [1]); // Parent - general court, Classic dispute kit
await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], ethers.toBeHex(3), [0]); // Parent - general court, Classic dispute kit

await pnk.approve(core.target, PNK(4000));
await core.setStake(1, PNK(2000));
Expand Down
51 changes: 14 additions & 37 deletions contracts/test/foundry/KlerosCore.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {TestERC20} from "../../src/token/TestERC20.sol";
import {ArbitrableExample, IArbitrableV2} from "../../src/arbitration/arbitrables/ArbitrableExample.sol";
import {DisputeTemplateRegistry} from "../../src/arbitration/DisputeTemplateRegistry.sol";
import "../../src/libraries/Constants.sol";
import {IKlerosCore, KlerosCoreSnapshotProxy} from "../../src/snapshot-proxy/KlerosCoreSnapshotProxy.sol";
import {IKlerosCore, KlerosCoreSnapshotProxy} from "../../src/arbitration/view/KlerosCoreSnapshotProxy.sol";

contract KlerosCoreTest is Test {
event Initialized(uint64 version);
Expand Down Expand Up @@ -156,7 +156,7 @@ contract KlerosCoreTest is Test {
assertEq(address(core.pinakion()), address(pinakion), "Wrong pinakion address");
assertEq(core.jurorProsecutionModule(), jurorProsecutionModule, "Wrong jurorProsecutionModule address");
assertEq(address(core.sortitionModule()), address(sortitionModule), "Wrong sortitionModule address");
assertEq(core.getDisputeKitsLength(), 2, "Wrong DK array length");
assertEq(core.getDisputeKitsLength(), 1, "Wrong DK array length");
(
uint96 courtParent,
bool courtHiddenVotes,
Expand Down Expand Up @@ -197,19 +197,16 @@ contract KlerosCoreTest is Test {
assertEq(courtTimesPerPeriod[i], timesPerPeriod[i], "Wrong times per period");
}

assertEq(address(core.disputeKits(NULL_DISPUTE_KIT)), address(0), "Wrong address NULL_DISPUTE_KIT");
assertEq(
address(core.disputeKits(DISPUTE_KIT_CLASSIC)),
address(disputeKit),
"Wrong address DISPUTE_KIT_CLASSIC"
);
assertEq(core.isSupported(FORKING_COURT, NULL_DISPUTE_KIT), false, "Forking court null dk should be false");
assertEq(
core.isSupported(FORKING_COURT, DISPUTE_KIT_CLASSIC),
false,
"Forking court classic dk should be false"
);
assertEq(core.isSupported(GENERAL_COURT, NULL_DISPUTE_KIT), false, "General court null dk should be false");
assertEq(core.isSupported(GENERAL_COURT, DISPUTE_KIT_CLASSIC), true, "General court classic dk should be true");
assertEq(core.paused(), false, "Wrong paused value");

Expand Down Expand Up @@ -438,18 +435,18 @@ contract KlerosCoreTest is Test {
core.addNewDisputeKit(newDK);
vm.prank(governor);
vm.expectEmit(true, true, true, true);
emit KlerosCoreBase.DisputeKitCreated(2, newDK);
emit KlerosCoreBase.DisputeKitCreated(1, newDK);
core.addNewDisputeKit(newDK);
assertEq(address(core.disputeKits(2)), address(newDK), "Wrong address of new DK");
assertEq(core.getDisputeKitsLength(), 3, "Wrong DK array length");
assertEq(address(core.disputeKits(1)), address(newDK), "Wrong address of new DK");
assertEq(core.getDisputeKitsLength(), 2, "Wrong DK array length");
}

function test_createCourt() public {
vm.expectRevert(KlerosCoreBase.GovernorOnly.selector);
vm.prank(other);
uint256[] memory supportedDK = new uint256[](2);
supportedDK[0] = DISPUTE_KIT_CLASSIC;
supportedDK[1] = 2; // New DK is added below.
supportedDK[1] = 1; // New DK is added below.
core.createCourt(
GENERAL_COURT,
true, // Hidden votes
Expand Down Expand Up @@ -506,21 +503,6 @@ contract KlerosCoreTest is Test {
);

uint256[] memory badSupportedDK = new uint256[](2);
badSupportedDK[0] = NULL_DISPUTE_KIT; // Include NULL_DK to check that it reverts
badSupportedDK[1] = DISPUTE_KIT_CLASSIC;
vm.expectRevert(KlerosCoreBase.WrongDisputeKitIndex.selector);
vm.prank(governor);
core.createCourt(
GENERAL_COURT,
true, // Hidden votes
2000, // min stake
10000, // alpha
0.03 ether, // fee for juror
50, // jurors for jump
[uint256(10), uint256(20), uint256(30), uint256(40)], // Times per period
abi.encode(uint256(4)), // Sortition extra data
badSupportedDK
);

badSupportedDK[0] = DISPUTE_KIT_CLASSIC;
badSupportedDK[1] = 2; // Check out of bounds index
Expand All @@ -543,7 +525,7 @@ contract KlerosCoreTest is Test {
vm.prank(governor);
core.addNewDisputeKit(newDK);
badSupportedDK = new uint256[](1);
badSupportedDK[0] = 2; // Include only sybil resistant dk
badSupportedDK[0] = 1; // Include only sybil resistant dk
vm.expectRevert(KlerosCoreBase.MustSupportDisputeKitClassic.selector);
vm.prank(governor);
core.createCourt(
Expand All @@ -562,7 +544,7 @@ contract KlerosCoreTest is Test {
vm.expectEmit(true, true, true, true);
emit KlerosCoreBase.DisputeKitEnabled(2, DISPUTE_KIT_CLASSIC, true);
vm.expectEmit(true, true, true, true);
emit KlerosCoreBase.DisputeKitEnabled(2, 2, true);
emit KlerosCoreBase.DisputeKitEnabled(2, 1, true);
vm.expectEmit(true, true, true, true);
emit KlerosCoreBase.CourtCreated(
2,
Expand Down Expand Up @@ -721,7 +703,7 @@ contract KlerosCoreTest is Test {

function test_enableDisputeKits() public {
DisputeKitSybilResistant newDK = new DisputeKitSybilResistant();
uint256 newDkID = 2;
uint256 newDkID = 1;
vm.prank(governor);
core.addNewDisputeKit(newDK);

Expand All @@ -733,12 +715,7 @@ contract KlerosCoreTest is Test {

vm.expectRevert(KlerosCoreBase.WrongDisputeKitIndex.selector);
vm.prank(governor);
supportedDK[0] = NULL_DISPUTE_KIT;
core.enableDisputeKits(GENERAL_COURT, supportedDK, true);

vm.expectRevert(KlerosCoreBase.WrongDisputeKitIndex.selector);
vm.prank(governor);
supportedDK[0] = 3; // Out of bounds
supportedDK[0] = 2; // Out of bounds
core.enableDisputeKits(GENERAL_COURT, supportedDK, true);

vm.expectRevert(KlerosCoreBase.CannotDisableClassicDK.selector);
Expand Down Expand Up @@ -821,12 +798,12 @@ contract KlerosCoreTest is Test {
core.addNewDisputeKit(disputeKit);
core.addNewDisputeKit(disputeKit);
core.addNewDisputeKit(disputeKit);
extraData = abi.encodePacked(uint256(50), uint256(41), uint256(6));
extraData = abi.encodePacked(uint256(50), uint256(41), uint256(5));

(courtID, minJurors, disputeKitID) = core.extraDataToCourtIDMinJurorsDisputeKit(extraData);
assertEq(courtID, GENERAL_COURT, "Wrong courtID"); // Value in extra data is out of scope so fall back
assertEq(minJurors, 41, "Wrong minJurors");
assertEq(disputeKitID, 6, "Wrong disputeKitID");
assertEq(disputeKitID, 5, "Wrong disputeKitID");
}

// *************************************** //
Expand Down Expand Up @@ -1316,7 +1293,7 @@ contract KlerosCoreTest is Test {
uint256 newFee = 0.01 ether;
uint96 newCourtID = 2;
uint256 newNbJurors = 4;
uint256 newDkID = 2;
uint256 newDkID = 1;
uint256[] memory supportedDK = new uint256[](1);
supportedDK[0] = DISPUTE_KIT_CLASSIC;
bytes memory newExtraData = abi.encodePacked(uint256(newCourtID), newNbJurors, newDkID);
Expand Down Expand Up @@ -2082,7 +2059,7 @@ contract KlerosCoreTest is Test {
DisputeKitClassic newDisputeKit = DisputeKitClassic(address(proxyDk));

uint96 newCourtID = 2;
uint256 newDkID = 2;
uint256 newDkID = 1;
uint256[] memory supportedDK = new uint256[](1);
supportedDK[0] = DISPUTE_KIT_CLASSIC;
bytes memory newExtraData = abi.encodePacked(uint256(newCourtID), DEFAULT_NB_OF_JURORS, newDkID);
Expand Down
Loading