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

Conversation

@kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Aug 22, 2020

Currently, as it seems we eject the prime when we remove a members, or typically alter the members via set_members_sorted. This makes sense because the call site does not really know if the new set contains the previous noted prime or not.

This PR changes this behaviour so that if we know that the removed member is not the current prime, then we set it again (i.e. keep it).

In kusama, the first block after this transaction caused the prime to be ejected for example:

https://polkascan.io/kusama/transaction/0x715eae46d4ec9c458b9d835bce5a1a3bbbb67d0e501c09fafe8fbbaddb292710


Additionally, I've mistakingly changed the calculation of the prime to use u64 (VoteWeight). I reverted it as it was a horrible idea and it is very likely to overflow. Also made the arithmetic saturating anyhow.


  • cc @rrtti
  • Needs a sign off from @gavofyork
  • Probably should be runtime-note-worthy.

@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. labels Aug 22, 2020
@kianenigma kianenigma requested a review from gavofyork August 22, 2020 00:02
@shawntabrizi shawntabrizi self-requested a review August 22, 2020 11:14
@kianenigma
Copy link
Contributor Author

As predicted, Kusama has been recovered and we will have a prime again. I will close this for now and just make a minimal PR to harden the arithmetic a bit.

@kianenigma kianenigma closed this Aug 22, 2020
@bkchr bkchr deleted the kiz-keep-that-prime branch August 23, 2020 21:59
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. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants