srml-module: Phragmen election

[kianenigma]

substrate/srml/elections-phragmen/src/lib.rs

Lines 17 to 63 in 88e8108

	//! # Phragmen Election Module.
	//!
	//! An election module based on sequential phragmen.
	//!
	//! ### Term and Round
	//!
	//! The election happens in _rounds_: every `N` blocks, all previous members are retired and a new
	//! set is elected (which may or may not have an intersection with the previous set). Each round
	//! lasts for some number of blocks defined by `TermDuration` storage item. The works _term_ and
	//! _round_ can be used interchangeably in this context.
	//!
	//! `TermDuration` might change during a round. This can shorten or extend the length of the round.
	//! The next election round's block number is never stored but rather always checked on the fly.
	//! Based on the current block height and `TermDuration`, `BlockNumber % TermDuration == 0` will
	//! always trigger a new election round.
	//!
	//! ### Voting
	//!
	//! Voters can vote for as many of the candidates by providing a list of account ids. Invalid votes
	//! (voting for non-candidates) are ignored during election. Yet, a voter _might_ vote for a
	//! future candidate. Voters reserve a bond as they vote. Each vote defines a `value`. This amount
	//! is locked from the account of the voter and indicated the weight of the vote. Voters can update
	//! their votes at any time by calling `vote()` again. This keeps the bond untouched but can
	//! optionally change the locked `value`. After a round, votes are kept and might still be valid
	//! for further rounds. A voter is responsible to call `remove_voter` upon once they are done to
	//! have their bond back and remove the lock.
	//!
	//! ### Candidacy and Members
	//!
	//! Candidates also reserve a bond as they submit candidacy. A candidate cannot take their candidacy
	//! back. A candidate can end up in one of the below situations:
	//! - **Winner**: A winner is kept as a _member_. They must still have a bond in reserve
	//! and they are automatically counted as a candidate for the next election.
	//! - **Runner-up**: Runner ups are the best candidates immediately after the winners. The number
	//! of runner-ups to keep is configurable. Runner-ups serve one major role: if a member is
	//! forcefully removed, the next best runner-up is chosen as a replacement. If not, a new
	//! election is triggered.
	//! - **Loser**: Any of the candidate who are not a winner are left as losers. A loser might be an
	//! _outgoing member_, meaning that they are an active member who failed to keep their spot. In
	//! this case, the outgoing member will get their bond back. Otherwise, the bond is slashed from
	//! the loser candidate.
	//!
	//! Note that with the members being the default candidates for the next round and votes
	//! persisting in storage, the election system is entirely stable given no further input. This means
	//! that if the system has a particular set of candidates `C` and voters `V` that lead to a
	//! set of members `M` being elected, as long as `V` and `C` don't remove their candidacy and votes,
	//! `M` will keep being re-elected at the end of each round.

Before merge, should consider:

• Add the module to the node/runtime if needed.

Maybe in this PR, maybe later:

• ability to report other voters.
• ability to remove candidacy.

[gavofyork]

runtime version should be bumped.

[kianenigma]

cc @AlfonsoCev

[gavofyork]

Suggested change

	/// `who` cannot be an old voter who has not yet called [`remove_voter`].
	/// `who` cannot be an existing voter

[kianenigma]

actually: it can be an existing voter to update their vote.

It cannot be, exactly as phrased: old voter who has not yet called remove_voter

[gavofyork]

is "old voter" a technical term? it sounds like a voter who is reaching retirement age.

[kianenigma]

The full story is:

The code does not clean the votes at all and relies on two incentivization that voters themselves will do it. As a voter, once your vote has been counted once, then you are an old voter aka. retired if you want to name it like that. You should manually remove yourself via another tx because:

1. you want your reserved bond back.
2. you can't vote and participate anymore in rounds.

If you are fine with the logic I can formulate it better as inactive voter is the module docs. I agree that the term old voter is not clear.

[gavofyork]

defunct_voter

[gavofyork]

does one storage write and it's 10k. the one above does 3 mutations and a binary search and costs 100x as much. wtf?

[kianenigma]

about your 3 comments on the steepness of weights:

1. they are taken by approximately halving the weights that are decided for the other election (maybe a wrong decision, but that's not the main reason of your complaint) via benchmarking, which is not through manual analysis but rather execution time measuring and some manual juice added by me. Some examples:

   1. submitting a candidate (as you recently discussed in some chats) is quite expensive with regards to computing the outcome of the election so I bumped it up a bit.
   2. removing a voter and adding one is more or less the same in terms of cpu/io but I manually reduced the weight of removing a bit for the same reason: it will potentially reduce the block time of the next election.

2. and more importantly this type of weight that you are analyzing, while I agree with it, is the state-weight which we, could, but at the moment have not yet introduced anywhere. Hence, I didn't apply it here either. The current weight numbers are merely a representation of computation time. Based on this, setting the desired seats could be arguably substantially less computationally intensive than submitting candidacy, hence the 100x difference.

[gavofyork]

not sure i understand point (2) could you rephrase?

in general, weight should not be used to encode indirect costs because it is used to determine raw computation complexity to manage block fullness and is only incidentally used for fees. this is a good example whereby mixing the two creates a perverse incentive for validators to do less work by choosing transactions like these that are over-priced because they're accounting for a future cost, to be born by Some Other Guy.

if it's one-off indirect cost, then an additional non-weight fee is reasonable, to be charged in a discrete instance as part of the call. if it's an on-going cost, then an accordingly large deposit should be held (ReservableCurrrency can be used for this).

[kianenigma]

not sure i understand point (2) could you rephrase?

What I mean is that judging these based on how many mutate/read/writes they have is correct but not something that we yet do; the current weight in all modules is merely based on execution time. regardless of what they do internally. Totally black-boxed. analyzing read/write/mutate etc. will be one of the early next steps after Kusama.

... perverse incentive for validators to do less work by choosing transactions like these that are over-priced because they're accounting for a future cost, to be born by Some Other Guy.

Noted and I can conclude what the little bits of my manual tweaks are wrong. Will reverse them + fix the rest of them in further weight updates.

[gavofyork]

should be runners_up. you might want to do a search-replace.