Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix wrong outgoing calculation in election #7384

Merged
3 commits merged into from
Oct 24, 2020
Merged

Conversation

kianenigma
Copy link
Contributor

This was a very unfortunate oversight from my end; The outgoing calculation seem to have been wrong because the inputs are simply not sorted. This can then cause accounts to be immaturely deemed loser, and subject to slash_reserved().

This should be merged and deployed asap. Furthermore, I will lead an investigation over polkadot's state to make sure we can obtain a list of all accounts affected and properly reimburse them through governance.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Please add a test that this never happens again.

@shawntabrizi
Copy link
Member

@kianenigma actually it is not exactly clear to me how sorting by AccountIds matters?

Can you elaborate?

@apopiak
Copy link
Contributor

apopiak commented Oct 24, 2020

ChangeMembers::compute_members_diff assumes that the inputs are sorted, I assume that's the reason.

@kianenigma
Copy link
Contributor Author

Please add a test that this never happens again.

added.

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Oct 24, 2020

Waiting for commit status.

@ghost ghost merged commit c60f008 into master Oct 24, 2020
@ghost ghost deleted the kiz-fix-faulty-election branch October 24, 2020 11:53
@xlc
Copy link
Contributor

xlc commented Oct 25, 2020

@eskimor
Copy link
Member

eskimor commented Dec 15, 2020

Hmm, coming from Haskell. Can't we check this at the type level instead of just tests? E.g. encode the sorting requirement in the calling contract of ChangeMembers::compute_members_diff, by making it take a newtype wrapper that can only be constructed by a sorting smart constructor?

@kianenigma
Copy link
Contributor Author

Both points are sensible improvements. I will make a follow up issue.

fwiw both Polkadot and Kusama have passed motions to refund all the immature slashing done over here. https://polkadot.polkassembly.io/motion/43

@eskimor
Copy link
Member

eskimor commented Dec 16, 2020

There is even a crate for this, which looks sane and could be useful.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants