Generalizes the elections-phragmen pallet to use an ElectionProvider#12563
Generalizes the elections-phragmen pallet to use an ElectionProvider#12563
Conversation
…s an ElectionDataProvider
| Self::register_weight(T::WeightInfo::electable_candidates(targets.len() as u32)); | ||
|
|
||
| if let Some(max_candidates) = maybe_max_len { | ||
| // TODO(gpestana):: or should return Err instead? |
There was a problem hiding this comment.
Should return an error here or silently truncate the list of eligible candidates?
There was a problem hiding this comment.
should maintain whatever was the current behavior for now, unless if we can improve it without breaking our backs.
| }, | ||
| } | ||
|
|
||
| Self::register_weight(T::WeightInfo::electing_voters(voters_and_stakes.len() as u32)); |
There was a problem hiding this comment.
I wonder if we should add the self-weighing logic in DataProvider::elect() only and reuse most of the weighting logic of the former do_phragmen(). I think that should be OK since I would expect the DataProvider to be used only by the election provider. OTOH, if the developers call the DataProvider explicitly for some reason, if the weights are implicit under elect() we may get into inconsistent weights.
|
|
||
| let _ = T::ElectionProvider::elect().map(|mut winners| { | ||
| // sort winners by stake | ||
| winners.sort_by(|i, j| j.1.total.cmp(&i.1.total)); |
There was a problem hiding this comment.
Instead of explicitly sorting the winners by stake for every election, should we have a SortedListProvider? If not, should the sorting be done in the ElectionDataProvider?
There was a problem hiding this comment.
Unfortunately nothing in the new interface requires this to be sorted. the old interface was sorted though: https://paritytech.github.io/substrate/master/sp_npos_elections/phragmen/fn.seq_phragmen.html
I think using Solver might help with this as well, other than simplifying a few other things.
| ) -> frame_election_provider_support::data_provider::Result<Vec<Self::AccountId>> { | ||
| let mut candidates_and_deposit = Self::candidates(); | ||
| // add all the previous members and runners-up as candidates as well. | ||
| candidates_and_deposit.append(&mut Self::implicit_candidates_with_deposit()); |
There was a problem hiding this comment.
What is implicit_candidates_with_deposit here? I don't see this function being defined anywhere on Self.
I recall now that all candidates are considered to be voters, and now since they are two distinct functions, we would need to sync this somehow, or re-read them etc.
I am more and more leaning toward type Solver: NPoSSolver was a better approach :(
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Yes, I agree. I will close this PR and open a new one using the |
2 participants
This PR refactors the current
elections-phragmenpallet to use an election provider instead of hardcoded phragmen election logic. The pallet itself implements theDataProviderused by the election provider.WIP, todo:
pallet-electionsCloses #8250