Drawing bug: test coverage + contract fix

contracts/deploy/fix1148.ts

@@ -0,0 +1,63 @@
+import { HardhatRuntimeEnvironment } from "hardhat/types";
+import { DeployFunction } from "hardhat-deploy/types";
+import { DisputeKitClassic, KlerosCore, SortitionModule } from "../typechain-types";
+import assert from "node:assert";
+
+enum HomeChains {
+  ARBITRUM_ONE = 42161,
+  ARBITRUM_GOERLI = 421613,
+  HARDHAT = 31337,
+}
+
+const deployArbitration: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
+  const { ethers, deployments, getNamedAccounts, getChainId } = hre;
+  const { deploy, execute } = deployments;
+  const { AddressZero } = ethers.constants;
+
+  // fallback to hardhat node signers on local network
+  const deployer = (await getNamedAccounts()).deployer ?? (await ethers.getSigners())[0].address;
+  const chainId = Number(await getChainId());
+  console.log("Deploying to %s with deployer %s", HomeChains[chainId], deployer);
+
+  const klerosCore = (await ethers.getContract("KlerosCore")) as KlerosCore;
+  const oldDisputeKit = (await ethers.getContract("DisputeKitClassic")) as DisputeKitClassic;
+
+  await deploy("DisputeKitClassic", {
+    from: deployer,
+    args: [deployer, AddressZero],
+    log: true,
+  });
+
+  const newDisputeKit = (await ethers.getContract("DisputeKitClassic")) as DisputeKitClassic;
+
+  await execute("DisputeKitClassic", { from: deployer, log: true }, "changeCore", klerosCore.address);
+  await execute("KlerosCore", { from: deployer, log: true }, "addNewDisputeKit", newDisputeKit.address, 0);
+
+  const oldDisputeKitId = 1;
+  const newDisputeKitId = 2;
+
+  assert(
+    await klerosCore.disputeKitNodes(oldDisputeKitId).then((node) => node.disputeKit === oldDisputeKit.address),
+    `wrong dispute kit id ${oldDisputeKitId}`
+  );
+  assert(
+    await klerosCore.disputeKitNodes(newDisputeKitId).then((node) => node.disputeKit === newDisputeKit.address),
+    `wrong dispute kit id ${newDisputeKitId}`
+  );
+
+  await execute("KlerosCore", { from: deployer, log: true }, "enableDisputeKits", 1, [newDisputeKitId], true); // enable the new dispute kit in court 1
+  await execute("KlerosCore", { from: deployer, log: true }, "enableDisputeKits", 2, [newDisputeKitId], true); // enable the new dispute kit in court 2
+  await execute("KlerosCore", { from: deployer, log: true }, "enableDisputeKits", 3, [newDisputeKitId], true); // enable the new dispute kit in court 3
+
+  // Cannot disable the old DK because of https://github.com/kleros/kleros-v2/blob/d9adb8f54e8164eb01880296b4dd62b74cad3a0e/contracts/src/arbitration/KlerosCore.sol#L452
+  // Does not seem correct
+  // await execute("KlerosCore", { from: deployer, log: true }, "enableDisputeKits", 1, [oldDisputeKitId], false); // disable the old dispute kit
+};
+
+deployArbitration.tags = ["Fix1148"];
+deployArbitration.skip = async ({ getChainId }) => {
+  const chainId = Number(await getChainId());
+  return !HomeChains[chainId];
+};
+
+export default deployArbitration;

contracts/src/arbitration/SortitionModule.sol

@@ -289,11 +289,15 @@ contract SortitionModule is ISortitionModule {
         require(phase == Phase.drawing, "Wrong phase.");
         SortitionSumTree storage tree = sortitionSumTrees[_key];
 
-        uint256 treeIndex = 0;
+        if (tree.nodes[0] == 0) {
+            return address(0); // No jurors staked.
+        }
+
         uint256 currentDrawnNumber = uint256(keccak256(abi.encodePacked(randomNumber, _coreDisputeID, _voteID))) %
             tree.nodes[0];
 
         // While it still has children
+        uint256 treeIndex = 0;
         while ((tree.K * treeIndex) + 1 < tree.nodes.length) {
             for (uint256 i = 1; i <= tree.K; i++) {
                 // Loop over children.

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

@@ -557,18 +557,8 @@ contract DisputeKitClassic is BaseDisputeKit, IEvidence {
     // *            Internal               * //
     // ************************************* //
 
-    /// @dev Checks that the chosen address satisfies certain conditions for being drawn.
-    /// @param _coreDisputeID ID of the dispute in the core contract.
-    /// @param _juror Chosen address.
-    /// @return Whether the address can be drawn or not.
-    function _postDrawCheck(uint256 _coreDisputeID, address _juror) internal view override returns (bool) {
-        (uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
-        (, uint256 lockedAmountPerJuror, , , , , , , , ) = core.getRoundInfo(
-            _coreDisputeID,
-            core.getNumberOfRounds(_coreDisputeID) - 1
-        );
-        (uint256 staked, uint256 locked, ) = core.getJurorBalance(_juror, courtID);
-        (, , uint256 minStake, , , , ) = core.courts(courtID);
-        return staked >= locked + lockedAmountPerJuror && staked >= minStake;
+    /// @inheritdoc BaseDisputeKit
+    function _postDrawCheck(uint256 /*_coreDisputeID*/, address /*_juror*/) internal pure override returns (bool) {
+        return true;
     }
 }

contracts/test/arbitration/draw.ts

@@ -1,6 +1,6 @@
 import { anyValue } from "@nomicfoundation/hardhat-chai-matchers/withArgs";
 import { deployments, ethers, getNamedAccounts, network } from "hardhat";
-import { BigNumber } from "ethers";
+import { BigNumber, ContractTransaction, Wallet } from "ethers";
 import {
   PNK,
   KlerosCore,
@@ -52,11 +52,7 @@ describe("Draw Benchmark", async () => {
   let randomizer;
 
   beforeEach("Setup", async () => {
-    deployer = (await getNamedAccounts()).deployer;
-    relayer = (await getNamedAccounts()).relayer;
-
-    console.log("deployer:%s", deployer);
-    console.log("named accounts: %O", await getNamedAccounts());
+    ({ deployer, relayer } = await getNamedAccounts());
 
     await deployments.fixture(["Arbitration", "VeaMock"], {
       fallbackToGlobal: true,
@@ -70,9 +66,28 @@ describe("Draw Benchmark", async () => {
     rng = (await ethers.getContract("RandomizerRNG")) as RandomizerRNG;
     randomizer = (await ethers.getContract("RandomizerMock")) as RandomizerMock;
     sortitionModule = (await ethers.getContract("SortitionModule")) as SortitionModule;
+
+    // CourtId 2
+    const minStake = BigNumber.from(10).pow(20).mul(3); // 300 PNK
+    const alpha = 10000;
+    const feeForJuror = BigNumber.from(10).pow(17);
+    await core.createCourt(
+      1,
+      false,
+      minStake,
+      alpha,
+      feeForJuror,
+      256,
+      [0, 0, 0, 10], // evidencePeriod, commitPeriod, votePeriod, appealPeriod
+      ethers.utils.hexlify(5), // Extra data for sortition module will return the default value of K)
+      [1]
+    );
   });
 
-  it("Draw Benchmark", async () => {
+  type SetStake = (wallet: Wallet) => Promise<void>;
+  type ExpectFromDraw = (drawTx: Promise<ContractTransaction>) => Promise<void>;
+
+  const draw = async (setStake: SetStake, createDisputeCourtId: string, expectFromDraw: ExpectFromDraw) => {
     const arbitrationCost = ONE_TENTH_ETH.mul(3);
     const [bridger] = await ethers.getSigners();
 
@@ -90,7 +105,8 @@ describe("Draw Benchmark", async () => {
       expect(await pnk.balanceOf(wallet.address)).to.equal(ONE_THOUSAND_PNK.mul(10));
 
       await pnk.connect(wallet).approve(core.address, ONE_THOUSAND_PNK.mul(10), { gasLimit: 300000 });
-      await core.connect(wallet).setStake(1, ONE_THOUSAND_PNK.mul(10), { gasLimit: 5000000 });
+
+      await setStake(wallet);
     }
 
     // Create a dispute
@@ -114,7 +130,7 @@ describe("Draw Benchmark", async () => {
           templateId: 0,
           templateUri: "",
           choices: 2,
-          extraData: "0x00",
+          extraData: `0x000000000000000000000000000000000000000000000000000000000000000${createDisputeCourtId}0000000000000000000000000000000000000000000000000000000000000003`,
         },
         { value: arbitrationCost }
       );
@@ -131,12 +147,72 @@ describe("Draw Benchmark", async () => {
     await randomizer.relay(rng.address, 0, ethers.utils.randomBytes(32));
     await sortitionModule.passPhase(); // Generating -> Drawing
 
-    await expect(core.draw(0, 1000, { gasLimit: 1000000 }))
-      .to.emit(core, "Draw")
-      .withArgs(anyValue, 0, 0, 0)
-      .to.emit(core, "Draw")
-      .withArgs(anyValue, 0, 0, 1)
-      .to.emit(core, "Draw")
-      .withArgs(anyValue, 0, 0, 2);
+    await expectFromDraw(core.draw(0, 1000, { gasLimit: 1000000 }));
+  };
+
+  it("Stakes in parent court and should draw jurors in parent court", async () => {
+    const setStake = async (wallet: Wallet) => {
+      await core.connect(wallet).setStake(1, ONE_THOUSAND_PNK.mul(5), { gasLimit: 5000000 });
+    };
+
+    const expectFromDraw = async (drawTx: Promise<ContractTransaction>) => {
+      await expect(drawTx)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 0)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 1)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 2);
+    };
+
+    await draw(setStake, "1", expectFromDraw);
+  });
+
+  it("Stakes in parent court and should draw nobody in subcourt", async () => {
+    const setStake = async (wallet: Wallet) => {
+      await core.connect(wallet).setStake(1, ONE_THOUSAND_PNK.mul(5), { gasLimit: 5000000 });
+    };
+
+    const expectFromDraw = async (drawTx: Promise<ContractTransaction>) => {
+      await expect(drawTx).to.not.emit(core, "Draw");
+    };
+
+    await draw(setStake, "2", expectFromDraw);
+  });
+
+  it("Stakes in subcourt and should draw jurors in parent court", async () => {
+    const setStake = async (wallet: Wallet) => {
+      await core.connect(wallet).setStake(2, ONE_THOUSAND_PNK.mul(5), { gasLimit: 5000000 });
+    };
+
+    const expectFromDraw = async (drawTx: Promise<ContractTransaction>) => {
+      await expect(drawTx)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 0)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 1)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 2);
+    };
+
+    await draw(setStake, "1", expectFromDraw);
+  });
+
+  it("Stakes in subcourt and should draw jurors in subcourt", async () => {
+    const setStake = async (wallet: Wallet) => {
+      await core.connect(wallet).setStake(2, ONE_THOUSAND_PNK.mul(5), { gasLimit: 5000000 });
+    };
+
+    const expectFromDraw = async (drawTx: Promise<ContractTransaction>) => {
+      await expect(drawTx)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 0)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 1)
+        .to.emit(core, "Draw")
+        .withArgs(anyValue, 0, 0, 2);
+    };
+
+    await draw(setStake, "2", expectFromDraw);
   });
 });

contracts/test/arbitration/unstake.ts

@@ -31,10 +31,6 @@ describe("Unstake juror", async () => {
 
   beforeEach("Setup", async () => {
     ({ deployer } = await getNamedAccounts());
-
-    console.log("deployer:%s", deployer);
-    console.log("named accounts: %O", await getNamedAccounts());
-
     await deployments.fixture(["Arbitration"], {
       fallbackToGlobal: true,
       keepExistingDeployments: false,

contracts/test/integration/index.ts

@@ -43,10 +43,6 @@ describe("Integration tests", async () => {
 
   beforeEach("Setup", async () => {
     ({ deployer } = await getNamedAccounts());
-
-    console.log("deployer:%s", deployer);
-    console.log("named accounts: %O", await getNamedAccounts());
-
     await deployments.fixture(["Arbitration", "VeaMock"], {
       fallbackToGlobal: true,
       keepExistingDeployments: false,

cspell.json

@@ -11,13 +11,16 @@
   "words": [
     "arbitrum",
     "autorestart",
+    "codegen",
     "commitlint",
     "COOLDOWN",
     "datetime",
     "devnet",
     "Devnet",
     "dockerhost",
     "Ethfinex",
+    "gluegun",
+    "graphprotocol",
     "hearbeat",
     "IERC",
     "ipfs",

subgraph/package.json

@@ -26,9 +26,12 @@
     "@graphprotocol/graph-ts": "^0.31.0"
   },
   "devDependencies": {
-    "@graphprotocol/graph-cli": "^0.54.0",
+    "@graphprotocol/graph-cli": "0.52.0",
     "@kleros/kleros-v2-eslint-config": "workspace:^",
     "@kleros/kleros-v2-prettier-config": "workspace:^",
     "gluegun": "^5.1.2"
+  },
+  "dependenciesComments": {
+    "@graphprotocol/graph-cli": "pinned because of this issue: https://github.com/graphprotocol/graph-tooling/issues/1399#issuecomment-1676104540"
   }
 }

subgraph/subgraph.yaml

@@ -81,9 +81,9 @@ dataSources:
     name: DisputeKitClassic
     network: arbitrum-goerli
     source:
-      address: "0x9ddf4a70295faaA695b8CF4F22177FA1f3a1B500"
+      address: "0x439f92b61783A752462527f9dA9C6c6180C9253a"
       abi: DisputeKitClassic
-      startBlock: 33436570
+      startBlock: 34139950
     mapping:
       kind: ethereum/events
       apiVersion: 0.0.6

web/.env.testnet.public

@@ -1,4 +1,4 @@
 # Do not enter sensitive information here.
 export REACT_APP_DEPLOYMENT=testnet
-export REACT_APP_KLEROS_CORE_SUBGRAPH_TESTNET=https://api.thegraph.com/subgraphs/name/alcercu/kleroscoretest
+export REACT_APP_KLEROS_CORE_SUBGRAPH_TESTNET=https://api.thegraph.com/subgraphs/name/kleros/kleros-v2-core-arbitrum-goerli
 export REACT_APP_DISPUTE_TEMPLATE_ARBGOERLI_SUBGRAPH_TESTNET=https://api.thegraph.com/subgraphs/name/alcercu/disputetemplateregistryarbgrli

yarn.lock

@@ -3987,9 +3987,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@graphprotocol/graph-cli@npm:^0.54.0":
-  version: 0.54.0
-  resolution: "@graphprotocol/graph-cli@npm:0.54.0"
+"@graphprotocol/graph-cli@npm:0.52.0":
+  version: 0.52.0
+  resolution: "@graphprotocol/graph-cli@npm:0.52.0"
   dependencies:
     "@float-capital/float-subgraph-uncrashable": ^0.0.0-alpha.4
     "@oclif/core": 2.8.6
@@ -4019,7 +4019,7 @@ __metadata:
     yaml: 1.10.2
   bin:
     graph: bin/run
-  checksum: c3bd34b26dc7e09d14e60193b806f1e513ba4fd3a0525506f015573addbb4848de21c9157426b789e99efb1850c0035aaf2d865ef7b985a9bc41884d6e45d012
+  checksum: 7c6e2dbf022f4229b412e1107bb16b8e5914da2c21802af9a072a79a4f463132a192380e9c395a463c285004abedf16461037cebea7387c580c2de46200ddd40
   languageName: node
   linkType: hard
 
@@ -5228,7 +5228,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@kleros/kleros-v2-subgraph@workspace:subgraph"
   dependencies:
-    "@graphprotocol/graph-cli": ^0.54.0
+    "@graphprotocol/graph-cli": 0.52.0
     "@graphprotocol/graph-ts": ^0.31.0
     "@kleros/kleros-v2-eslint-config": "workspace:^"
     "@kleros/kleros-v2-prettier-config": "workspace:^"