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

[PM] ensure_can_not_dispute_the_same_outcome only checks for the last outcome, but should for each of them #716

Closed
Chralt98 opened this issue Jul 19, 2022 · 4 comments
Assignees
Labels
p:medium Medium priority, this issue can wait but should be done fairly soon t:enhancement The issue described an enhancement

Comments

@Chralt98
Copy link
Member

While implementing the global disputes mechanism I realised, that each new dispute only compares with the last dispute for a match, but it should compare with all disputes, if there is a match (so that a new dispute gets rejected).

fn ensure_can_not_dispute_the_same_outcome(
disputes: &[MarketDispute<T::AccountId, T::BlockNumber>],
report: &Report<T::AccountId, T::BlockNumber>,
outcome: &OutcomeReport,
) -> DispatchResult {
if let Some(last_dispute) = disputes.last() {
ensure!(&last_dispute.outcome != outcome, Error::<T>::CannotDisputeSameOutcome);
} else {
ensure!(&report.outcome != outcome, Error::<T>::CannotDisputeSameOutcome);
}
Ok(())
}

@Chralt98 Chralt98 added p:high High priority, prioritize the resolution of this issue t:bug The issue describes a bug labels Jul 19, 2022
@Chralt98 Chralt98 self-assigned this Jul 19, 2022
@maltekliemann
Copy link
Contributor

This is by design. Think about how it used to be in simple-disputes: Someone disputes A, then someone disputes B. Should A be off the table forever now? No, of course not.

Now that simple-disputes is removed, we could have a discussion about changing this, though.

@Chralt98
Copy link
Member Author

Chralt98 commented Jul 19, 2022

I just realised it in the test, yeah you are right, it makes sense as it was.

@Chralt98 Chralt98 added p:medium Medium priority, this issue can wait but should be done fairly soon t:maintenance The issue describes necessary maintenance t:enhancement The issue described an enhancement and removed p:high High priority, prioritize the resolution of this issue t:bug The issue describes a bug t:maintenance The issue describes necessary maintenance labels Jul 19, 2022
@Chralt98
Copy link
Member Author

But on the second look for the global dispute mechanism it would make much sense to change that. So that the users don't dispute on the same outcome. I will add that to the Global dispute PR.

@Chralt98
Copy link
Member Author

Fixed by moving the logic to simple disputes #938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p:medium Medium priority, this issue can wait but should be done fairly soon t:enhancement The issue described an enhancement
Projects
None yet
Development

No branches or pull requests

2 participants