diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 08653e057..7b1013a64 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -424,7 +424,8 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { sortitionModule = _sortitionModule; } - /// @notice Add a new supported dispute kit module to the court. + /// @notice Add a new supported dispute kit, without enabling it. + /// Use `enableDisputeKits()` to enable the dispute kit for a specific court. /// @param _disputeKitAddress The address of the dispute kit contract. function addNewDisputeKit(IDisputeKit _disputeKitAddress) external onlyByOwner { uint256 disputeKitID = disputeKits.length; @@ -461,7 +462,7 @@ contract KlerosCore 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] == NULL_DISPUTE_KIT || _supportedDisputeKits[i] >= disputeKits.length) { revert WrongDisputeKitIndex(); } _enableDisputeKit(uint96(courtID), _supportedDisputeKits[i], true); @@ -483,7 +484,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { // Update the parent. courts[_parent].children.push(courtID); emit CourtCreated( - uint96(courtID), + courtID, _parent, _hiddenVotes, _minStake, @@ -518,7 +519,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { } for (uint256 i = 0; i < court.children.length; i++) { if (courts[court.children[i]].minStake < _minStake) { - revert MinStakeLowerThanParentCourt(); + revert MinStakeHigherThanChildCourt(court.children[i]); } } court.minStake = _minStake; @@ -544,10 +545,10 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { /// @param _enable Whether add or remove the dispute kits from the court. function enableDisputeKits(uint96 _courtID, uint256[] memory _disputeKitIDs, bool _enable) external onlyByOwner { for (uint256 i = 0; i < _disputeKitIDs.length; i++) { + if (_disputeKitIDs[i] == NULL_DISPUTE_KIT || _disputeKitIDs[i] >= disputeKits.length) { + revert WrongDisputeKitIndex(); + } if (_enable) { - if (_disputeKitIDs[i] == 0 || _disputeKitIDs[i] >= disputeKits.length) { - revert WrongDisputeKitIndex(); - } _enableDisputeKit(_courtID, _disputeKitIDs[i], true); } else { // Classic dispute kit must be supported by all courts. @@ -940,7 +941,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { } if (pnkBalance == 0 || !disputeKit.isVoteActive(_params.disputeID, _params.round, _params.repartition)) { - // The juror is inactive or their balance is can't cover penalties anymore, unstake them from all courts. + // The juror is inactive or their balance can't cover penalties anymore, unstake them from all courts. sortitionModule.forcedUnstakeAllCourts(account); } else if (newCourtStake < courts[penalizedInCourtID].minStake) { // The juror's balance fell below the court minStake, unstake them from the court. @@ -1365,7 +1366,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { function _extraDataToCourtIDMinJurorsDisputeKit( bytes memory _extraData ) internal view returns (uint96 courtID, uint256 minJurors, uint256 disputeKitID) { - // Note that if the extradata doesn't contain 32 bytes for the dispute kit ID it'll return the default 0 index. + // Note that if the _extraData doesn't contain 32 bytes, default values are used. if (_extraData.length >= 64) { assembly { // solium-disable-line security/no-inline-assembly @@ -1380,7 +1381,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { minJurors = DEFAULT_NB_OF_JURORS; } if (disputeKitID == NULL_DISPUTE_KIT || disputeKitID >= disputeKits.length) { - disputeKitID = DISPUTE_KIT_CLASSIC; // 0 index is not used. + disputeKitID = DISPUTE_KIT_CLASSIC; } } else { courtID = GENERAL_COURT; @@ -1398,8 +1399,9 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable { error DisputeKitOnly(); error SortitionModuleOnly(); error UnsuccessfulCall(); - error InvalidDisputKitParent(); + error InvalidDisputeKitParent(); error MinStakeLowerThanParentCourt(); + error MinStakeHigherThanChildCourt(uint256 _childCourtID); error UnsupportedDisputeKit(); error InvalidForkingCourtAsParent(); error WrongDisputeKitIndex(); diff --git a/contracts/src/arbitration/SortitionModule.sol b/contracts/src/arbitration/SortitionModule.sol index e46753f0a..125c44c3a 100644 --- a/contracts/src/arbitration/SortitionModule.sol +++ b/contracts/src/arbitration/SortitionModule.sol @@ -53,9 +53,9 @@ contract SortitionModule is ISortitionModule, Initializable, UUPSProxiable { mapping(TreeKey key => SortitionTrees.Tree) sortitionSumTrees; // The mapping of sortition trees by keys. mapping(address account => Juror) public jurors; // The jurors. mapping(uint256 => DelayedStake) public delayedStakes; // Stores the stakes that were changed during Drawing phase, to update them when the phase is switched to Staking. - uint256 public maxStakePerJuror; // The maximum amount of PNK a juror can stake in a court. - uint256 public maxTotalStaked; // The maximum amount of PNK that can be staked in all courts. - uint256 public totalStaked; // The amount that is currently staked in all courts. + uint256 public maxStakePerJuror; // The maximum amount of PNK that a juror can stake across the courts. + uint256 public maxTotalStaked; // The maximum amount of PNK that all the jurors can stake across the courts. + uint256 public totalStaked; // The amount of PNK that is currently staked across the courts. // ************************************* // // * Events * // @@ -105,8 +105,8 @@ contract SortitionModule is ISortitionModule, Initializable, UUPSProxiable { /// @param _minStakingTime Minimal time to stake /// @param _maxDrawingTime Time after which the drawing phase can be switched /// @param _rng The random number generator. - /// @param _maxStakePerJuror The maximum amount of PNK a juror can stake in a court. - /// @param _maxTotalStaked The maximum amount of PNK that can be staked in all courts. + /// @param _maxStakePerJuror The maximum amount of PNK a juror can stake across the courts. + /// @param _maxTotalStaked The maximum amount of PNK that all the jurors can stake across the courts. function initialize( address _owner, KlerosCore _core, @@ -343,14 +343,14 @@ contract SortitionModule is ISortitionModule, Initializable, UUPSProxiable { if (availablePenalty == 0) return (juror.stakedPnk, newCourtStake, 0); // No penalty to apply. - uint256 currentStake = _stakeOf(_account, _courtID); + uint256 currentStake = newCourtStake; uint256 newStake = 0; if (currentStake >= availablePenalty) { newStake = currentStake - availablePenalty; } _setStake(_account, _courtID, 0, availablePenalty, newStake); pnkBalance = juror.stakedPnk; // updated by _setStake() - newCourtStake = _stakeOf(_account, _courtID); // updated by _setStake() + newCourtStake = newStake; } /// @inheritdoc ISortitionModule diff --git a/contracts/src/arbitration/arbitrables/DisputeResolver.sol b/contracts/src/arbitration/arbitrables/DisputeResolver.sol index 64fef2ee6..691178cd7 100644 --- a/contracts/src/arbitration/arbitrables/DisputeResolver.sol +++ b/contracts/src/arbitration/arbitrables/DisputeResolver.sol @@ -30,12 +30,22 @@ contract DisputeResolver is IArbitrableV2 { DisputeStruct[] public disputes; // Local disputes. mapping(uint256 => uint256) public arbitratorDisputeIDToLocalID; // Maps arbitrator-side dispute IDs to local dispute IDs. + // ************************************* // + // * Function Modifiers * // + // ************************************* // + + modifier onlyByOwner() { + if (owner != msg.sender) revert OwnerOnly(); + _; + } + // ************************************* // // * Constructor * // // ************************************* // /// @notice Constructor /// @param _arbitrator Target global arbitrator for any disputes. + /// @param _templateRegistry The dispute template registry. constructor(IArbitratorV2 _arbitrator, IDisputeTemplateRegistry _templateRegistry) { owner = msg.sender; arbitrator = _arbitrator; @@ -48,18 +58,15 @@ contract DisputeResolver is IArbitrableV2 { /// @notice Changes the owner. /// @param _owner The address of the new owner. - function changeOwner(address _owner) external { - if (owner != msg.sender) revert OwnerOnly(); + function changeOwner(address _owner) external onlyByOwner { owner = _owner; } - function changeArbitrator(IArbitratorV2 _arbitrator) external { - if (owner != msg.sender) revert OwnerOnly(); + function changeArbitrator(IArbitratorV2 _arbitrator) external onlyByOwner { arbitrator = _arbitrator; } - function changeTemplateRegistry(IDisputeTemplateRegistry _templateRegistry) external { - if (owner != msg.sender) revert OwnerOnly(); + function changeTemplateRegistry(IDisputeTemplateRegistry _templateRegistry) external onlyByOwner { templateRegistry = _templateRegistry; } diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol b/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol index 464e848d6..698a9a547 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol @@ -370,7 +370,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi if (!coreDisputeIDToActive[_coreDisputeID]) revert NotActiveForCoreDisputeID(); (uint256 appealPeriodStart, uint256 appealPeriodEnd) = core.appealPeriod(_coreDisputeID); - if (block.timestamp < appealPeriodStart || block.timestamp >= appealPeriodEnd) revert AppealPeriodIsOver(); + if (block.timestamp < appealPeriodStart || block.timestamp >= appealPeriodEnd) revert NotAppealPeriod(); uint256 multiplier; (uint256 ruling, , ) = this.currentRuling(_coreDisputeID); @@ -381,7 +381,7 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi block.timestamp - appealPeriodStart >= ((appealPeriodEnd - appealPeriodStart) * LOSER_APPEAL_PERIOD_MULTIPLIER) / ONE_BASIS_POINT ) { - revert AppealPeriodIsOverForLoser(); + revert NotAppealPeriodForLoser(); } multiplier = LOSER_STAKE_MULTIPLIER; } @@ -759,8 +759,8 @@ abstract contract DisputeKitClassicBase is IDisputeKit, Initializable, UUPSProxi error ChoiceOutOfBounds(); error HashDoesNotMatchHiddenVoteCommitment(); error VoteAlreadyCast(); - error AppealPeriodIsOver(); - error AppealPeriodIsOverForLoser(); + error NotAppealPeriod(); + error NotAppealPeriodForLoser(); error AppealFeeIsAlreadyPaid(); error DisputeNotResolved(); error CoreIsPaused(); diff --git a/contracts/src/arbitration/interfaces/IArbitratorV2.sol b/contracts/src/arbitration/interfaces/IArbitratorV2.sol index 125baab92..41c7e48c5 100644 --- a/contracts/src/arbitration/interfaces/IArbitratorV2.sol +++ b/contracts/src/arbitration/interfaces/IArbitratorV2.sol @@ -50,7 +50,7 @@ interface IArbitratorV2 { ) external payable returns (uint256 disputeID); /// @notice Create a dispute and pay for the fees in a supported ERC20 token. - /// @dev Must be called by the arbitrable contract and pay at least `arbitrationCost(_extraData)` in the supported ERC20 token. + /// @dev Must be called by the arbitrable contract and pay at least `arbitrationCost(_extraData, _feeToken)` in the supported ERC20 token. /// @param _numberOfChoices The number of choices the arbitrator can choose from in this dispute. /// @param _extraData Additional info about the dispute. We use it to pass the ID of the dispute's court (first 32 bytes), the minimum number of jurors required (next 32 bytes) and the ID of the specific dispute kit (last 32 bytes). /// @param _feeToken The ERC20 token used to pay fees. diff --git a/contracts/src/arbitration/interfaces/IEvidence.sol b/contracts/src/arbitration/interfaces/IEvidence.sol index 4b6477ff2..f27ce8447 100644 --- a/contracts/src/arbitration/interfaces/IEvidence.sol +++ b/contracts/src/arbitration/interfaces/IEvidence.sol @@ -6,7 +6,7 @@ pragma solidity >=0.8.0 <0.9.0; interface IEvidence { /// @notice To be raised when evidence is submitted. Should point to the resource (evidences are not to be stored on chain due to gas considerations). /// @param _externalDisputeID Unique identifier for this dispute outside Kleros. It's the submitter responsibility to submit the right external dispute ID. - /// @param _party The address of the party submiting the evidence. Note that 0x0 refers to evidence not submitted by any party. + /// @param _party The address of the party submitting the evidence. /// @param _evidence Stringified evidence object, example: '{"name" : "Justification", "description" : "Description", "fileURI" : "/ipfs/QmWQV5ZFFhEJiW8Lm7ay2zLxC2XS4wx1b2W7FfdrLMyQQc"}'. event Evidence(uint256 indexed _externalDisputeID, address indexed _party, string _evidence); } diff --git a/contracts/src/arbitration/interfaces/ISortitionModule.sol b/contracts/src/arbitration/interfaces/ISortitionModule.sol index 0803666ff..ccc66a2f5 100644 --- a/contracts/src/arbitration/interfaces/ISortitionModule.sol +++ b/contracts/src/arbitration/interfaces/ISortitionModule.sol @@ -78,7 +78,7 @@ interface ISortitionModule { uint256 _newStake ) external; - /// @notice Update the state of the stakes with a PNK reward deposit, called by KC during rewards execution. + /// @notice Update the state of the stakes with a PNK penalty, called by KC during rewards execution. /// /// @dev `O(n + p * log_k(j))` where /// `n` is the number of courts the juror has staked in, diff --git a/contracts/src/arbitration/university/KlerosCoreUniversity.sol b/contracts/src/arbitration/university/KlerosCoreUniversity.sol index bf27dfbb2..37098d7e1 100644 --- a/contracts/src/arbitration/university/KlerosCoreUniversity.sol +++ b/contracts/src/arbitration/university/KlerosCoreUniversity.sol @@ -1173,7 +1173,7 @@ contract KlerosCoreUniversity is IArbitratorV2, UUPSProxiable, Initializable { error DisputeKitOnly(); error SortitionModuleOnly(); error UnsuccessfulCall(); - error InvalidDisputKitParent(); + error InvalidDisputeKitParent(); error MinStakeLowerThanParentCourt(); error UnsupportedDisputeKit(); error InvalidForkingCourtAsParent(); diff --git a/contracts/test/foundry/KlerosCore_Appeals.t.sol b/contracts/test/foundry/KlerosCore_Appeals.t.sol index 79e575e81..9a88bb1b2 100644 --- a/contracts/test/foundry/KlerosCore_Appeals.t.sol +++ b/contracts/test/foundry/KlerosCore_Appeals.t.sol @@ -127,7 +127,7 @@ contract KlerosCore_AppealsTest is KlerosCore_TestBase { disputeKit.castVote(disputeID, voteIDs, 2, 0, "XYZ"); vm.prank(crowdfunder1); - vm.expectRevert(DisputeKitClassicBase.AppealPeriodIsOver.selector); + vm.expectRevert(DisputeKitClassicBase.NotAppealPeriod.selector); disputeKit.fundAppeal{value: 0.1 ether}(disputeID, 1); core.passPeriod(disputeID); @@ -135,14 +135,14 @@ contract KlerosCore_AppealsTest is KlerosCore_TestBase { vm.prank(crowdfunder1); vm.warp(block.timestamp + ((end - start) / 2 + 1)); - vm.expectRevert(DisputeKitClassicBase.AppealPeriodIsOverForLoser.selector); + vm.expectRevert(DisputeKitClassicBase.NotAppealPeriodForLoser.selector); disputeKit.fundAppeal{value: 0.1 ether}(disputeID, 1); // Losing choice disputeKit.fundAppeal(disputeID, 2); // Winning choice funding should not revert yet vm.prank(crowdfunder1); vm.warp(block.timestamp + (end - start) / 2); // Warp one more to cover the whole period - vm.expectRevert(DisputeKitClassicBase.AppealPeriodIsOver.selector); + vm.expectRevert(DisputeKitClassicBase.NotAppealPeriod.selector); disputeKit.fundAppeal{value: 0.1 ether}(disputeID, 2); } diff --git a/contracts/test/foundry/KlerosCore_Governance.t.sol b/contracts/test/foundry/KlerosCore_Governance.t.sol index c95ba123f..7c803c058 100644 --- a/contracts/test/foundry/KlerosCore_Governance.t.sol +++ b/contracts/test/foundry/KlerosCore_Governance.t.sol @@ -310,7 +310,7 @@ contract KlerosCore_GovernanceTest is KlerosCore_TestBase { 50, // jurors for jump [uint256(10), uint256(20), uint256(30), uint256(40)] // Times per period ); - vm.expectRevert(KlerosCore.MinStakeLowerThanParentCourt.selector); + vm.expectRevert(abi.encodeWithSelector(KlerosCore.MinStakeHigherThanChildCourt.selector, newCourtID)); vm.prank(owner); // Min stake of a parent became higher than of a child core.changeCourtParameters(