From 2ae6bad85d09757d043b0d9baa38d1856a11576f Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Thu, 20 Jan 2022 23:38:44 +0000 Subject: [PATCH] feat(DisputeKit): better abstraction of the dispute kits --- .../DisputeKit.sol => IDisputeKit.sol} | 70 +++---------------- contracts/src/arbitration/KlerosCore.sol | 16 ++--- .../dispute-kits/BaseDisputeKit.sol | 68 ++++++++++++++++++ .../dispute-kits/DisputeKitClassic.sol | 53 +++----------- .../dispute-kits/DisputeKitSybilResistant.sol | 54 +++----------- 5 files changed, 107 insertions(+), 154 deletions(-) rename contracts/src/arbitration/{dispute-kits/DisputeKit.sol => IDisputeKit.sol} (57%) create mode 100644 contracts/src/arbitration/dispute-kits/BaseDisputeKit.sol diff --git a/contracts/src/arbitration/dispute-kits/DisputeKit.sol b/contracts/src/arbitration/IDisputeKit.sol similarity index 57% rename from contracts/src/arbitration/dispute-kits/DisputeKit.sol rename to contracts/src/arbitration/IDisputeKit.sol index 1b5d8e736..9768624dd 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKit.sol +++ b/contracts/src/arbitration/IDisputeKit.sol @@ -10,13 +10,14 @@ pragma solidity ^0.8; -import "../IArbitrator.sol"; +import "./IArbitrator.sol"; /** * @title DisputeKit - * Dispute kit abstraction for Kleros v2. + * An abstraction of the Dispute Kits intended for interfacing with KlerosCore + * This is not intended for use by the front-end clients for voting or appeal funding to allow for implementation-specific differences in parameters. */ -abstract contract DisputeKit { +interface IDisputeKit { // ************************************* // // * State Modifiers * // // ************************************* // @@ -31,63 +32,14 @@ abstract contract DisputeKit { uint256 _disputeID, uint256 _numberOfChoices, bytes calldata _extraData - ) external virtual; + ) external; /** @dev Draws the juror from the sortition tree. The drawn address is picked up by Kleros Core. * Note: Access restricted to Kleros Core only. * @param _disputeID The ID of the dispute in Kleros Core. * @return drawnAddress The drawn address. */ - function draw(uint256 _disputeID) external virtual returns (address drawnAddress); - - /** @dev Sets the caller's commit for the specified votes. - * `O(n)` where - * `n` is the number of votes. - * @param _disputeID The ID of the dispute. - * @param _voteIDs The IDs of the votes. - * @param _commit The commit. - */ - function castCommit( - uint256 _disputeID, - uint256[] calldata _voteIDs, - bytes32 _commit - ) external virtual; - - /** @dev Sets the caller's choices for the specified votes. - * `O(n)` where - * `n` is the number of votes. - * @param _disputeID The ID of the dispute. - * @param _voteIDs The IDs of the votes. - * @param _choice The choice. - * @param _salt The salt for the commit if the votes were hidden. - */ - function castVote( - uint256 _disputeID, - uint256[] calldata _voteIDs, - uint256 _choice, - uint256 _salt - ) external virtual; - - /** @dev Manages contributions, and appeals a dispute if at least two choices are fully funded. - * Note that the surplus deposit will be reimbursed. - * @param _disputeID Index of the dispute in Kleros Core contract. - * @param _choice A choice that receives funding. - */ - function fundAppeal(uint256 _disputeID, uint256 _choice) external payable virtual; - - /** @dev Allows to withdraw any reimbursable fees or rewards after the dispute gets resolved. - * @param _disputeID Index of the dispute in Kleros Core contract. - * @param _beneficiary The address whose rewards to withdraw. - * @param _round The round the caller wants to withdraw from. - * @param _choice The ruling option that the caller wants to withdraw from. - * @return amount The withdrawn amount. - */ - function withdrawFeesAndRewards( - uint256 _disputeID, - address payable _beneficiary, - uint256 _round, - uint256 _choice - ) external virtual returns (uint256 amount); + function draw(uint256 _disputeID) external returns (address drawnAddress); // ************************************* // // * Public Views * // @@ -97,7 +49,7 @@ abstract contract DisputeKit { * @param _disputeID The ID of the dispute in Kleros Core. * @return ruling The current ruling. */ - function currentRuling(uint256 _disputeID) public view virtual returns (uint256 ruling); + function currentRuling(uint256 _disputeID) external view returns (uint256 ruling); /** @dev Gets the degree of coherence of a particular voter. This function is called by Kleros Core in order to determine the amount of the reward. * @param _disputeID The ID of the dispute in Kleros Core. @@ -109,14 +61,14 @@ abstract contract DisputeKit { uint256 _disputeID, uint256 _round, uint256 _voteID - ) external view virtual returns (uint256); + ) external view returns (uint256); /** @dev Gets the number of jurors who are eligible to a reward in this round. * @param _disputeID The ID of the dispute in Kleros Core. * @param _round The ID of the round. * @return The number of coherent jurors. */ - function getCoherentCount(uint256 _disputeID, uint256 _round) external view virtual returns (uint256); + function getCoherentCount(uint256 _disputeID, uint256 _round) external view returns (uint256); /** @dev Returns true if the specified voter was active in this round. * @param _disputeID The ID of the dispute in Kleros Core. @@ -128,7 +80,7 @@ abstract contract DisputeKit { uint256 _disputeID, uint256 _round, uint256 _voteID - ) external view virtual returns (bool); + ) external view returns (bool); function getRoundInfo( uint256 _disputeID, @@ -137,7 +89,6 @@ abstract contract DisputeKit { ) external view - virtual returns ( uint256 winningChoice, bool tied, @@ -154,7 +105,6 @@ abstract contract DisputeKit { ) external view - virtual returns ( address account, bytes32 commit, diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index cad626e1c..5ad830913 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -12,7 +12,7 @@ pragma solidity ^0.8; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "./IArbitrator.sol"; -import "./dispute-kits/DisputeKit.sol"; +import "./IDisputeKit.sol"; import {SortitionSumTreeFactory} from "../data-structures/SortitionSumTreeFactory.sol"; /** @@ -49,7 +49,7 @@ contract KlerosCore is IArbitrator { struct Dispute { uint96 subcourtID; // The ID of the subcourt the dispute is in. IArbitrable arbitrated; // The arbitrable contract. - DisputeKit disputeKit; // ID of the dispute kit that this dispute was assigned to. + IDisputeKit disputeKit; // ID of the dispute kit that this dispute was assigned to. Period period; // The current period of the dispute. bool ruled; // True if the ruling has been executed, false otherwise. uint256 currentRound; // The index of the current appeal round. Note that 0 represents the default dispute round. Former votes.length - 1. @@ -90,7 +90,7 @@ contract KlerosCore is IArbitrator { Court[] public courts; // The subcourts. //TODO: disputeKits forest. - DisputeKit[] public disputeKits; // All supported dispute kits. + IDisputeKit[] public disputeKits; // All supported dispute kits. Dispute[] public disputes; // The disputes. mapping(address => Juror) internal jurors; // The jurors. @@ -138,7 +138,7 @@ contract KlerosCore is IArbitrator { address _governor, IERC20 _pinakion, address _jurorProsecutionModule, - DisputeKit _disputeKit, + IDisputeKit _disputeKit, bool _hiddenVotes, uint256 _minStake, uint256 _alpha, @@ -211,7 +211,7 @@ contract KlerosCore is IArbitrator { /** @dev Add a new supported dispute kit module to the court. * @param _disputeKitAddress The address of the dispute kit contract. */ - function addNewDisputeKit(DisputeKit _disputeKitAddress) external onlyByGovernor { + function addNewDisputeKit(IDisputeKit _disputeKitAddress) external onlyByGovernor { // TODO: the dispute kit data structure. For now keep it a simple array. // Also note that in current state this function doesn't take into account that the added address is actually new. require(disputeKits.length <= 256); // Can't be more than 256 because the IDs are used in a bitfield. @@ -375,7 +375,7 @@ contract KlerosCore is IArbitrator { dispute.subcourtID = subcourtID; dispute.arbitrated = IArbitrable(msg.sender); - DisputeKit disputeKit = disputeKits[disputeKitID]; + IDisputeKit disputeKit = disputeKits[disputeKitID]; dispute.disputeKit = disputeKit; dispute.lastPeriodChange = block.timestamp; @@ -449,7 +449,7 @@ contract KlerosCore is IArbitrator { function draw(uint256 _disputeID, uint256 _iterations) external { Dispute storage dispute = disputes[_disputeID]; require(dispute.period == Period.evidence, "Should be evidence period."); - DisputeKit disputeKit = dispute.disputeKit; + IDisputeKit disputeKit = dispute.disputeKit; uint256 startIndex = dispute.drawnJurors[dispute.currentRound].length; uint256 endIndex = startIndex + _iterations <= dispute.nbVotes ? startIndex + _iterations : dispute.nbVotes; @@ -652,7 +652,7 @@ contract KlerosCore is IArbitrator { * @return ruling The current ruling. */ function currentRuling(uint256 _disputeID) public view returns (uint256 ruling) { - DisputeKit disputeKit = disputes[_disputeID].disputeKit; + IDisputeKit disputeKit = disputes[_disputeID].disputeKit; return disputeKit.currentRuling(_disputeID); } diff --git a/contracts/src/arbitration/dispute-kits/BaseDisputeKit.sol b/contracts/src/arbitration/dispute-kits/BaseDisputeKit.sol new file mode 100644 index 000000000..dae442c3d --- /dev/null +++ b/contracts/src/arbitration/dispute-kits/BaseDisputeKit.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: MIT + +/** + * @authors: [@unknownunknown1, @jaybuidl] + * @reviewers: [] + * @auditors: [] + * @bounties: [] + * @deployments: [] + */ + +pragma solidity ^0.8; + +import "../IDisputeKit.sol"; +import "../KlerosCore.sol"; + +/** + * @title BaseDisputeKit + * Provides common basic behaviours to the Dispute Kit implementations. + */ +abstract contract BaseDisputeKit is IDisputeKit { + // ************************************* // + // * Storage * // + // ************************************* // + + address public governor; // The governor of the contract. + KlerosCore public core; // The Kleros Core arbitrator + + // ************************************* // + // * Function Modifiers * // + // ************************************* // + + modifier onlyByGovernor() { + require(governor == msg.sender, "Access not allowed: Governor only."); + _; + } + + modifier onlyByCore() { + require(address(core) == msg.sender, "Access not allowed: KlerosCore only."); + _; + } + + /** @dev Constructor. + * @param _governor The governor's address. + * @param _core The KlerosCore arbitrator. + */ + constructor(address _governor, KlerosCore _core) { + governor = _governor; + core = _core; + } + + // ************************ // + // * Governance * // + // ************************ // + + /** @dev Allows the governor to call anything on behalf of the contract. + * @param _destination The destination of the call. + * @param _amount The value sent with the call. + * @param _data The data sent with the call. + */ + function executeGovernorProposal( + address _destination, + uint256 _amount, + bytes memory _data + ) external onlyByGovernor { + (bool success, ) = _destination.call{value: _amount}(_data); + require(success, "Unsuccessful call"); + } +} diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol b/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol index 15847e139..6a767cecc 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol @@ -10,8 +10,7 @@ pragma solidity ^0.8; -import "./DisputeKit.sol"; -import "../KlerosCore.sol"; +import "./BaseDisputeKit.sol"; import "../../rng/RNG.sol"; /** @@ -24,7 +23,7 @@ import "../../rng/RNG.sol"; * TODO: * - phase management: Generating->Drawing->Resolving->Generating in coordination with KlerosCore to freeze staking. */ -contract DisputeKitClassic is DisputeKit { +contract DisputeKitClassic is BaseDisputeKit { // ************************************* // // * Structs * // // ************************************* // @@ -64,8 +63,6 @@ contract DisputeKitClassic is DisputeKit { uint256 public constant LOSER_APPEAL_PERIOD_MULTIPLIER = 5000; // Multiplier of the appeal period for the choice that wasn't voted for in the previous round, in basis points. Default is 1/2 of original appeal period. uint256 public constant ONE_BASIS_POINT = 10000; // One basis point, for scaling. - address public governor; // The governor of the contract. - KlerosCore public core; // The Kleros Core arbitrator RNG public rng; // The random number generator Dispute[] public disputes; // Array of the locally created disputes. mapping(uint256 => uint256) public coreDisputeIDToLocal; // Maps the dispute ID in Kleros Core to the local dispute ID. @@ -92,20 +89,6 @@ contract DisputeKitClassic is DisputeKit { event ChoiceFunded(uint256 indexed _disputeID, uint256 indexed _round, uint256 indexed _choice); - // ************************************* // - // * Function Modifiers * // - // ************************************* // - - modifier onlyByCore() { - require(address(core) == msg.sender, "Access not allowed: KlerosCore only."); - _; - } - - modifier onlyByGovernor() { - require(governor == msg.sender, "Access not allowed: Governor only."); - _; - } - /** @dev Constructor. * @param _governor The governor's address. * @param _core The KlerosCore arbitrator. @@ -115,9 +98,7 @@ contract DisputeKitClassic is DisputeKit { address _governor, KlerosCore _core, RNG _rng - ) { - governor = _governor; - core = _core; + ) BaseDisputeKit(_governor, _core) { rng = _rng; } @@ -125,20 +106,6 @@ contract DisputeKitClassic is DisputeKit { // * Governance * // // ************************ // - /** @dev Allows the governor to call anything on behalf of the contract. - * @param _destination The destination of the call. - * @param _amount The value sent with the call. - * @param _data The data sent with the call. - */ - function executeGovernorProposal( - address _destination, - uint256 _amount, - bytes memory _data - ) external onlyByGovernor { - (bool success, ) = _destination.call{value: _amount}(_data); - require(success, "Unsuccessful call"); - } - /** @dev Changes the `governor` storage variable. * @param _governor The new value for the `governor` storage variable. */ @@ -239,7 +206,7 @@ contract DisputeKitClassic is DisputeKit { uint256 _disputeID, uint256[] calldata _voteIDs, bytes32 _commit - ) external override { + ) external { require( core.getCurrentPeriod(_disputeID) == KlerosCore.Period.commit, "The dispute should be in Commit period." @@ -271,7 +238,7 @@ contract DisputeKitClassic is DisputeKit { uint256[] calldata _voteIDs, uint256 _choice, uint256 _salt - ) external override { + ) external { require(core.getCurrentPeriod(_disputeID) == KlerosCore.Period.vote, "The dispute should be in Vote period."); require(_voteIDs.length > 0, "No voteID provided"); @@ -319,7 +286,7 @@ contract DisputeKitClassic is DisputeKit { * @param _disputeID Index of the dispute in Kleros Core contract. * @param _choice A choice that receives funding. */ - function fundAppeal(uint256 _disputeID, uint256 _choice) external payable override { + function fundAppeal(uint256 _disputeID, uint256 _choice) external payable { Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; require(_choice <= dispute.numberOfChoices, "There is no such ruling to fund."); @@ -327,7 +294,7 @@ contract DisputeKitClassic is DisputeKit { require(block.timestamp >= appealPeriodStart && block.timestamp < appealPeriodEnd, "Appeal period is over."); uint256 multiplier; - if (currentRuling(_disputeID) == _choice) { + if (this.currentRuling(_disputeID) == _choice) { multiplier = WINNER_STAKE_MULTIPLIER; } else { require( @@ -385,12 +352,12 @@ contract DisputeKitClassic is DisputeKit { address payable _beneficiary, uint256 _round, uint256 _choice - ) external override returns (uint256 amount) { + ) external returns (uint256 amount) { require(core.isRuled(_disputeID), "Dispute should be resolved."); Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; Round storage round = dispute.rounds[_round]; - uint256 finalRuling = currentRuling(_disputeID); + uint256 finalRuling = this.currentRuling(_disputeID); if (!round.hasPaid[_choice]) { // Allow to reimburse if funding was unsuccessful for this ruling option. @@ -425,7 +392,7 @@ contract DisputeKitClassic is DisputeKit { * @param _disputeID The ID of the dispute in Kleros Core. * @return ruling The current ruling. */ - function currentRuling(uint256 _disputeID) public view override returns (uint256 ruling) { + function currentRuling(uint256 _disputeID) external view override returns (uint256 ruling) { Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; Round storage round = dispute.rounds[dispute.rounds.length - 1]; ruling = round.tied ? 0 : round.winningChoice; diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol b/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol index 7ce63280a..709d73043 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol @@ -10,8 +10,8 @@ pragma solidity ^0.8; -import "./DisputeKit.sol"; -import "../KlerosCore.sol"; +import "./BaseDisputeKit.sol"; + import "../../rng/RNG.sol"; interface IProofOfHumanity { @@ -30,7 +30,7 @@ interface IProofOfHumanity { * - an incentive system: equal split between coherent votes, * - an appeal system: fund 2 choices only, vote on any choice. */ -contract DisputeKitSybilResistant is DisputeKit { +contract DisputeKitSybilResistant is BaseDisputeKit { // ************************************* // // * Structs * // // ************************************* // @@ -70,8 +70,6 @@ contract DisputeKitSybilResistant is DisputeKit { uint256 public constant LOSER_APPEAL_PERIOD_MULTIPLIER = 5000; // Multiplier of the appeal period for the choice that wasn't voted for in the previous round, in basis points. Default is 1/2 of original appeal period. uint256 public constant ONE_BASIS_POINT = 10000; // One basis point, for scaling. - address public governor; // The governor of the contract. - KlerosCore public core; // The Kleros Core arbitrator RNG public rng; // The random number generator IProofOfHumanity public poh; // The Proof of Humanity registry Dispute[] public disputes; // Array of the locally created disputes. @@ -99,20 +97,6 @@ contract DisputeKitSybilResistant is DisputeKit { event ChoiceFunded(uint256 indexed _disputeID, uint256 indexed _round, uint256 indexed _choice); - // ************************************* // - // * Function Modifiers * // - // ************************************* // - - modifier onlyByCore() { - require(address(core) == msg.sender, "Access not allowed: KlerosCore only."); - _; - } - - modifier onlyByGovernor() { - require(governor == msg.sender, "Access not allowed: Governor only."); - _; - } - /** @dev Constructor. * @param _governor The governor's address. * @param _core The KlerosCore arbitrator. @@ -123,9 +107,7 @@ contract DisputeKitSybilResistant is DisputeKit { KlerosCore _core, RNG _rng, IProofOfHumanity _poh - ) { - governor = _governor; - core = _core; + ) BaseDisputeKit(_governor, _core) { rng = _rng; poh = _poh; } @@ -134,20 +116,6 @@ contract DisputeKitSybilResistant is DisputeKit { // * Governance * // // ************************ // - /** @dev Allows the governor to call anything on behalf of the contract. - * @param _destination The destination of the call. - * @param _amount The value sent with the call. - * @param _data The data sent with the call. - */ - function executeGovernorProposal( - address _destination, - uint256 _amount, - bytes memory _data - ) external onlyByGovernor { - (bool success, ) = _destination.call{value: _amount}(_data); - require(success, "Unsuccessful call"); - } - /** @dev Changes the `governor` storage variable. * @param _governor The new value for the `governor` storage variable. */ @@ -250,7 +218,7 @@ contract DisputeKitSybilResistant is DisputeKit { uint256 _disputeID, uint256[] calldata _voteIDs, bytes32 _commit - ) external override { + ) external { require( core.getCurrentPeriod(_disputeID) == KlerosCore.Period.commit, "The dispute should be in Commit period." @@ -282,7 +250,7 @@ contract DisputeKitSybilResistant is DisputeKit { uint256[] calldata _voteIDs, uint256 _choice, uint256 _salt - ) external override { + ) external { require(core.getCurrentPeriod(_disputeID) == KlerosCore.Period.vote, "The dispute should be in Vote period."); require(_voteIDs.length > 0, "No voteID provided"); @@ -330,7 +298,7 @@ contract DisputeKitSybilResistant is DisputeKit { * @param _disputeID Index of the dispute in Kleros Core contract. * @param _choice A choice that receives funding. */ - function fundAppeal(uint256 _disputeID, uint256 _choice) external payable override { + function fundAppeal(uint256 _disputeID, uint256 _choice) external payable { Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; require(_choice <= dispute.numberOfChoices, "There is no such ruling to fund."); @@ -338,7 +306,7 @@ contract DisputeKitSybilResistant is DisputeKit { require(block.timestamp >= appealPeriodStart && block.timestamp < appealPeriodEnd, "Appeal period is over."); uint256 multiplier; - if (currentRuling(_disputeID) == _choice) { + if (this.currentRuling(_disputeID) == _choice) { multiplier = WINNER_STAKE_MULTIPLIER; } else { require( @@ -396,12 +364,12 @@ contract DisputeKitSybilResistant is DisputeKit { address payable _beneficiary, uint256 _round, uint256 _choice - ) external override returns (uint256 amount) { + ) external returns (uint256 amount) { require(core.isRuled(_disputeID), "Dispute should be resolved."); Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; Round storage round = dispute.rounds[_round]; - uint256 finalRuling = currentRuling(_disputeID); + uint256 finalRuling = this.currentRuling(_disputeID); if (!round.hasPaid[_choice]) { // Allow to reimburse if funding was unsuccessful for this ruling option. @@ -436,7 +404,7 @@ contract DisputeKitSybilResistant is DisputeKit { * @param _disputeID The ID of the dispute in Kleros Core. * @return ruling The current ruling. */ - function currentRuling(uint256 _disputeID) public view override returns (uint256 ruling) { + function currentRuling(uint256 _disputeID) external view override returns (uint256 ruling) { Dispute storage dispute = disputes[coreDisputeIDToLocal[_disputeID]]; Round storage round = dispute.rounds[dispute.rounds.length - 1]; ruling = round.tied ? 0 : round.winningChoice;