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.

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

feat(KC): add foundry test file #1765

Merged
merged 15 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
18 changes: 10 additions & 8 deletions contracts/src/arbitration/KlerosCoreBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This is going to break the subgraph

uint96 indexed _parent,
bool _hiddenVotes,
uint256 _minStake,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand All @@ -1177,4 +1178,5 @@ abstract contract KlerosCoreBase is IArbitratorV2 {
error TransferFailed();
error WhenNotPausedOnly();
error WhenPausedOnly();
error StakingZeroWhenNoStake();
}
12 changes: 8 additions & 4 deletions contracts/src/arbitration/SortitionModuleBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,14 @@ abstract contract SortitionModuleBase is ISortitionModule {
DelayedStake storage delayedStake = delayedStakes[i];
// Delayed stake could've been manually removed already. In this case simply move on to the next item.
if (delayedStake.account != address(0)) {
// Nullify the index so the delayed stake won't get deleted before its own execution.
delete latestDelayedStakeIndex[delayedStake.account][delayedStake.courtID];
core.setStakeBySortitionModule(
delayedStake.account,
delayedStake.courtID,
delayedStake.stake,
delayedStake.alreadyTransferred
);
delete latestDelayedStakeIndex[delayedStake.account][delayedStake.courtID];
delete delayedStakes[i];
}
}
Expand Down Expand Up @@ -265,13 +266,16 @@ 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 (phase != Phase.staking) {
pnkWithdrawal = _deleteDelayedStake(_courtID, _account);
if (currentStake == 0 && _newStake == 0) {
return (0, 0, StakingResult.CannotStakeZeroWhenNoStake); // Forbid staking 0 amount when current stake is 0 to avoid flaky behaviour.
}

pnkWithdrawal = _deleteDelayedStake(_courtID, _account);
if (phase != Phase.staking) {
// Store the stake change as delayed, to be applied when the phase switches back to Staking.
DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex];
delayedStake.account = _account;
Expand Down
5 changes: 4 additions & 1 deletion contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions contracts/src/arbitration/interfaces/IDisputeKit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions contracts/src/libraries/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,5 +33,6 @@ enum StakingResult {
CannotStakeInThisCourt,
CannotStakeLessThanMinStake,
CannotStakeMoreThanMaxStakePerJuror,
CannotStakeMoreThanMaxTotalStaked
CannotStakeMoreThanMaxTotalStaked,
CannotStakeZeroWhenNoStake
}
25 changes: 25 additions & 0 deletions contracts/src/test/KlerosCoreMock.sol
Original file line number Diff line number Diff line change
@@ -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);
}
}
23 changes: 23 additions & 0 deletions contracts/src/test/SortitionModuleMock.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}
2 changes: 1 addition & 1 deletion contracts/test/arbitration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Empty file.
Loading
Loading