-
Notifications
You must be signed in to change notification settings - Fork 47
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(Arbitration): standard update #1
feat(Arbitration): standard update #1
Conversation
/** | ||
* @title Arbitrator | ||
* Arbitrator interface for CourtV2. | ||
* This interface for the most part follows the ERC-792 standard but also allows the appeal crowdfunding on arbitrator's side. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"follows for the most part the ERC-792 standard"
I suggest that we either:
- Amend the ERC-792 specification so that this interface complies with it - but the specs must deal with v1 compatibility. The amendment can be made outside the ERC-1 process because the status is "pre-draft" and not formally tracked.
- Start a new ERC spec which leaves kleros-v1 compatibility untouched at the expense of having to rebuild legitimacy for this new ERC from scratch.
- Allow v1 to fall out of compliance with ERC-792
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally,I was thinking that it'll be a new ERC
* @param _extraData Can be used to give additional info of the dispute. | ||
* @return cost Required appeal cost. | ||
*/ | ||
function appealCost(uint256 _disputeID, bytes memory _extraData) external view returns (uint256 cost); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appealCost() is now an internal function of the Arbitrator which does not need standardization so it should be possible to remove it just like appeal() has been removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
|
||
/** | ||
* @dev Make a contribution to fund one (or the subset) of possible choices in order to appeal the ruling. | ||
* Note that the desired appeal system will be defined by the selected Dispute Kit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not mention the dispute kit here, it's an implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, makes sense
* @param _choices The choices to contribute to. | ||
*/ | ||
function fundAppeal(uint256 _disputeID, uint256[] calldata _choices) external payable; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A view function showing the funding status, funding goal and number of required funded choices would be useful. It's different from the old appealCost()
which did not account for the funders reward. Maybe something like:
/**
* @dev Return the funded amount and funding goal for this choice, as well as the minimum number of choices which must be funded to obtain an appeal. The status info may not be available if the dispute is not currently appealable.
* @param _disputeID The ID of the dispute to appeal.
* @param _choice The choice corresponding to this status.
* @return funded The amount funded so far for this choice in wei.
* @return goal The amount to fully fund this choice in wei.
* @return minNbOfChoicesToFund The number of choices which must be fully funded to obtain an appeal.
*/
function fundingStatus(uint256 _disputeID, uint256 _choice)
external
view
returns (
uint256 funded,
uint256 goal,
uint256 minNbOfChoicesToFund
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment in the doc.
uint256 minNbOfChoicesToFund - Is it possible to have more than two choices though? Even if fund a certain subset it'll be always funded as a countermeasure to the first funded choices that should always be separated, according to the yellow paper. So even with the subset funded choices are de-facto just 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed that's more consistent: uint256 _choice
--> uint256[] _choices
feat(Arbitration): standard update
The document describing the updated standard https://docs.google.com/document/d/1VIBspauNpw7A8ZeHaSXN2t4RLyGdtognpZMvSk1cJPE/edit