feat(KC): add foundry test file

unknownunknown1 · unknownunknown1 · commit f0e4fa4306c0 · 2024-12-02T23:30:43.000+10:00
diff --git a/contracts/lib/forge-std b/contracts/lib/forge-std
@@ -1 +1 @@
-Subproject commit 066ff16c5c03e6f931cd041fd366bc4be1fae82a
+Subproject commit 8f24d6b04c92975e0795b5868aa0d783251cdeaa
diff --git a/contracts/src/arbitration/KlerosCoreBase.sol b/contracts/src/arbitration/KlerosCoreBase.sol
@@ -113,7 +113,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
     event AppealDecision(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable);
     event Draw(address indexed _address, uint256 indexed _disputeID, uint256 _roundID, uint256 _voteID);
     event CourtCreated(
-        uint256 indexed _courtID,
+        uint96 indexed _courtID,
         uint96 indexed _parent,
         bool _hiddenVotes,
         uint256 _minStake,
@@ -237,16 +237,18 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
 
         sortitionModule.createTree(bytes32(uint256(GENERAL_COURT)), _sortitionExtraData);
 
+        uint256[] memory supportedDisputeKits = new uint256[](1);
+        supportedDisputeKits[0] = DISPUTE_KIT_CLASSIC;
         emit CourtCreated(
-            1,
+            GENERAL_COURT,
             court.parent,
             _hiddenVotes,
             _courtParameters[0],
             _courtParameters[1],
             _courtParameters[2],
             _courtParameters[3],
             _timesPerPeriod,
-            new uint256[](0)
+            supportedDisputeKits
         );
         _enableDisputeKit(GENERAL_COURT, DISPUTE_KIT_CLASSIC, true);
     }
@@ -351,7 +353,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
             if (_supportedDisputeKits[i] == 0 || _supportedDisputeKits[i] >= disputeKits.length) {
                 revert WrongDisputeKitIndex();
             }
-            court.supportedDisputeKits[_supportedDisputeKits[i]] = true;
+            _enableDisputeKit(uint96(courtID), _supportedDisputeKits[i], true);
         }
         // Check that Classic DK support was added.
         if (!court.supportedDisputeKits[DISPUTE_KIT_CLASSIC]) revert MustSupportDisputeKitClassic();
@@ -370,7 +372,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
         // Update the parent.
         courts[_parent].children.push(courtID);
         emit CourtCreated(
-            courtID,
+            uint96(courtID),
             _parent,
             _hiddenVotes,
             _minStake,
@@ -1061,7 +1063,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
         bool _alreadyTransferred,
         OnError _onError
     ) internal returns (bool) {
-        if (_courtID == FORKING_COURT || _courtID > courts.length) {
+        if (_courtID == FORKING_COURT || _courtID >= courts.length) {
             _stakingFailed(_onError, StakingResult.CannotStakeInThisCourt); // Staking directly into the forking court is not allowed.
             return false;
         }
@@ -1102,6 +1104,7 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
         if (_result == StakingResult.CannotStakeInMoreCourts) revert StakingInTooManyCourts();
         if (_result == StakingResult.CannotStakeInThisCourt) revert StakingNotPossibeInThisCourt();
         if (_result == StakingResult.CannotStakeLessThanMinStake) revert StakingLessThanCourtMinStake();
+        if (_result == StakingResult.CannotStakeZeroWhenNoStake) revert StakingZeroWhenNoStake();
     }
 
     /// @dev Gets a court ID, the minimum number of jurors and an ID of a dispute kit from a specified extra data bytes array.
@@ -1147,13 +1150,11 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
     error SortitionModuleOnly();
     error UnsuccessfulCall();
     error InvalidDisputKitParent();
-    error DepthLevelMax();
     error MinStakeLowerThanParentCourt();
     error UnsupportedDisputeKit();
     error InvalidForkingCourtAsParent();
     error WrongDisputeKitIndex();
     error CannotDisableClassicDK();
-    error ArraysLengthMismatch();
     error StakingInTooManyCourts();
     error StakingNotPossibeInThisCourt();
     error StakingLessThanCourtMinStake();
@@ -1177,4 +1178,5 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
     error TransferFailed();
     error WhenNotPausedOnly();
     error WhenPausedOnly();
+    error StakingZeroWhenNoStake();
 }
diff --git a/contracts/src/arbitration/SortitionModuleBase.sol b/contracts/src/arbitration/SortitionModuleBase.sol
@@ -265,10 +265,14 @@ abstract contract SortitionModuleBase is ISortitionModule {
         uint256 currentStake = stakeOf(_account, _courtID);
 
         uint256 nbCourts = juror.courtIDs.length;
-        if (_newStake == 0 && (nbCourts >= MAX_STAKE_PATHS || currentStake == 0)) {
+        if (currentStake == 0 && nbCourts >= MAX_STAKE_PATHS) {
             return (0, 0, StakingResult.CannotStakeInMoreCourts); // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed.
         }
 
+        if (currentStake == 0 && _newStake == 0) {
+            return (0, 0, StakingResult.CannotStakeZeroWhenNoStake); // Forbid staking 0 amount when current stake is 0 to avoid flaky behaviour.
+        }
+
         if (phase != Phase.staking) {
             pnkWithdrawal = _deleteDelayedStake(_courtID, _account);
 
@@ -296,7 +300,7 @@ abstract contract SortitionModuleBase is ISortitionModule {
                 pnkDeposit = _increaseStake(juror, _courtID, _newStake, currentStake);
             }
         } else {
-            pnkWithdrawal += _decreaseStake(juror, _courtID, _newStake, currentStake);
+            pnkWithdrawal = _decreaseStake(juror, _courtID, _newStake, currentStake);
         }
 
         // Update the sortition sum tree.
diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol b/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol
@@ -238,7 +238,6 @@ contract DisputeKitClassic is IDisputeKit, Initializable, UUPSProxiable {
         (uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
         bytes32 key = bytes32(uint256(courtID)); // Get the ID of the tree.
 
-        // TODO: Handle the situation when no one has staked yet.
         drawnAddress = sortitionModule.draw(key, _coreDisputeID, _nonce);
 
         if (_postDrawCheck(_coreDisputeID, drawnAddress)) {
@@ -603,6 +602,10 @@ contract DisputeKitClassic is IDisputeKit, Initializable, UUPSProxiable {
     /// @param _coreDisputeID ID of the dispute in the core contract.
     /// @param _juror Chosen address.
     /// @return Whether the address can be drawn or not.
+    /// Note that we don't check the minStake requirement here because of the implicit staking in parent courts.
+    /// minStake is checked directly during staking process however it's possible for the juror to get drawn
+    /// while having < minStake if it is later increased by governance.
+    /// This issue is expected and harmless since we check for insolvency anyway.
     function _postDrawCheck(uint256 _coreDisputeID, address _juror) internal view returns (bool) {
         (uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
         uint256 lockedAmountPerJuror = core
diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol b/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol
@@ -255,7 +255,6 @@ contract DisputeKitSybilResistant is IDisputeKit, Initializable, UUPSProxiable {
         (uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
         bytes32 key = bytes32(uint256(courtID)); // Get the ID of the tree.
 
-        // TODO: Handle the situation when no one has staked yet.
         drawnAddress = sortitionModule.draw(key, _coreDisputeID, _nonce);
 
         if (_postDrawCheck(_coreDisputeID, drawnAddress) && !round.alreadyDrawn[drawnAddress]) {
@@ -621,6 +620,10 @@ contract DisputeKitSybilResistant is IDisputeKit, Initializable, UUPSProxiable {
     /// @param _coreDisputeID ID of the dispute in the core contract.
     /// @param _juror Chosen address.
     /// @return Whether the address can be drawn or not.
+    /// Note that we don't check the minStake requirement here because of the implicit staking in parent courts.
+    /// minStake is checked directly during staking process however it's possible for the juror to get drawn
+    /// while having < minStake if it is later increased by governance.
+    /// This issue is expected and harmless since we check for insolvency anyway.
     function _postDrawCheck(uint256 _coreDisputeID, address _juror) internal view returns (bool) {
         (uint96 courtID, , , , ) = core.disputes(_coreDisputeID);
         uint256 lockedAmountPerJuror = core
diff --git a/contracts/src/arbitration/interfaces/IDisputeKit.sol b/contracts/src/arbitration/interfaces/IDisputeKit.sol
@@ -41,6 +41,7 @@ interface IDisputeKit {
     /// @param _coreDisputeID The ID of the dispute in Kleros Core, not in the Dispute Kit.
     /// @param _numberOfChoices Number of choices of the dispute
     /// @param _extraData Additional info about the dispute, for possible use in future dispute kits.
+    /// @param _nbVotes Maximal number of votes this dispute can get. DEPRECATED as we don't need to pass it now. KC handles the count.
     function createDispute(
         uint256 _coreDisputeID,
         uint256 _numberOfChoices,
diff --git a/contracts/src/libraries/Constants.sol b/contracts/src/libraries/Constants.sol
@@ -9,7 +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.
+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.
 
 // Sortition Module
@@ -33,5 +33,6 @@ enum StakingResult {
     CannotStakeInThisCourt,
     CannotStakeLessThanMinStake,
     CannotStakeMoreThanMaxStakePerJuror,
-    CannotStakeMoreThanMaxTotalStaked
+    CannotStakeMoreThanMaxTotalStaked,
+    CannotStakeZeroWhenNoStake
 }
diff --git a/contracts/src/test/KlerosCoreMock.sol b/contracts/src/test/KlerosCoreMock.sol
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: MIT
+
+/// @custom:authors: [@unknownunknown1]
+/// @custom:reviewers: []
+/// @custom:auditors: []
+/// @custom:bounties: []
+/// @custom:deployments: []
+
+pragma solidity 0.8.24;
+
+import "../arbitration/KlerosCore.sol";
+
+/// @title KlerosCoreMock
+/// KlerosCore with view functions to use in Foundry tests.
+contract KlerosCoreMock is KlerosCore {
+    function getCourtChildren(uint256 _courtId) external view returns (uint256[] memory children) {
+        children = courts[_courtId].children;
+    }
+
+    function extraDataToCourtIDMinJurorsDisputeKit(
+        bytes memory _extraData
+    ) external view returns (uint96 courtID, uint256 minJurors, uint256 disputeKitID) {
+        (courtID, minJurors, disputeKitID) = _extraDataToCourtIDMinJurorsDisputeKit(_extraData);
+    }
+}
diff --git a/contracts/src/test/SortitionModuleMock.sol b/contracts/src/test/SortitionModuleMock.sol
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: MIT
+
+/**
+ *  @custom:authors: [unknownunknown1]
+ *  @custom:reviewers: []
+ *  @custom:auditors: []
+ *  @custom:bounties: []
+ *  @custom:deployments: []
+ */
+
+pragma solidity 0.8.24;
+
+import "../arbitration/SortitionModule.sol";
+
+/// @title SortitionModuleMock
+/// @dev Adds getter functions to sortition module for Foundry tests.
+contract SortitionModuleMock is SortitionModule {
+    function getSortitionProperties(bytes32 _key) external view returns (uint256 K, uint256 nodeLength) {
+        SortitionSumTree storage tree = sortitionSumTrees[_key];
+        K = tree.K;
+        nodeLength = tree.nodes.length;
+    }
+}
diff --git a/contracts/test/arbitration/index.ts b/contracts/test/arbitration/index.ts
@@ -30,7 +30,7 @@ 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([]);
+    expect(events2[0].args._supportedDisputeKits).to.deep.equal([1]);
 
     const events3 = await core.queryFilter(core.filters.DisputeKitEnabled());
     expect(events3.length).to.equal(1);
diff --git a/contracts/test/foundry/Contract.t.sol b/contracts/test/foundry/Contract.t.sol
diff --git a/contracts/test/foundry/KlerosCore.t.sol b/contracts/test/foundry/KlerosCore.t.sol
