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

Introduce default-setting prime for collective #5137

Merged
merged 8 commits into from
Mar 5, 2020

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Mar 5, 2020

Allows the existence of a "prime member" of the collective whose vote acts as the default for any other members.

CC @jacogr

@gavofyork gavofyork added A0-please_review Pull request needs code review. B1-runtimenoteworthy labels Mar 5, 2020
@shawntabrizi shawntabrizi self-requested a review March 5, 2020 12:38
Comment on lines +192 to +194
let old = Members::<T, I>::get();
<Self as ChangeMembers<T::AccountId>>::set_members_sorted(&new_members[..], &old);
Prime::<T, I>::set(prime);
Copy link
Member

Choose a reason for hiding this comment

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

Updating Members storage with new_members has been lost with this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah. that was superfluous logic as it happens anyway in <Self as ChangeMembers<T::AccountId>>::set_members_sorted(&new_members[..], &old).

Copy link
Member

Choose a reason for hiding this comment

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

I think this is an issue since other parts of logic rely on Self::members() which reads from the local members list.

We need some trait to tell the module where to read the correct list of members from, and use it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean. Members is always kept up to date regardless of how the set is changed.

Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

It is a little strange to me that reset_members keeps prime if they happen to be in the old and new set, but sure, just an implementation detail.

Please review my question above (RE: Members storage item) before you merge.

@gavofyork gavofyork merged commit 2c1ce06 into master Mar 5, 2020
@gavofyork gavofyork deleted the gav-collective-prime branch March 5, 2020 14:57
General-Beck pushed a commit to General-Beck/substrate that referenced this pull request Mar 6, 2020
* Introduce default-setting prime for collective

* Docs.

* Elections phragmen supports prime

* Fix

* Membership supports prime

* Fix

* Update frame/collective/src/lib.rs

Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
General-Beck pushed a commit to General-Beck/substrate that referenced this pull request Mar 17, 2020
* Introduce default-setting prime for collective

* Docs.

* Elections phragmen supports prime

* Fix

* Membership supports prime

* Fix

* Update frame/collective/src/lib.rs

Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
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.

3 participants