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

Keep prime when possible. #6938

Closed
wants to merge 4 commits into from
Closed

Keep prime when possible. #6938

wants to merge 4 commits into from

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
@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.

1 participant