[Feature] StakeTracker P1 - VotersList

[ruseinov]

Original issue: paritytech/polkadot-sdk#443
Splitting up #12744

polkadot companion: paritytech/polkadot#6527

Terminology

Approval stake - a sum of all the nominator/nomination pools stakes that are backing this validator + self-stake.

Goals

The original goal of this series of PR's is to introduce a list of validators sorted by their approval stake, therefore eliminating the need for Max Validator Intentions parameter. This way we'll be able to look into top x validators in the list to get a set of electable ones.
While doing that we also introduce a concept of EventListeners into Staking, that would allow us to hook into various staking methods acting upon emitted events. In this particular case this allows us to delegate the sorted list maintenance to a 3rd party EventListener.

Technical details

This PR introduces a new Staking-related abstraction called OnStakingUpdate that basically provides the ability to subscribe to certain events that Staking fires and act upon them.

/// A generic staking event listener.
/// Note that the interface is designed in a way that the events are fired post-action, so any
/// pre-action data that is needed needs to be passed to interface methods.
/// The rest of the data can be retrieved by using `StakingInterface`.
pub trait OnStakingUpdate<AccountId: Clone, Balance: Copy> {
	/// Fired when the stake amount of someone updates.
	///
	/// Also called when someone stakes for the first time. (TODO: is it? this is why we need unit
	/// tests for this pallet alone).
	///
	/// This is effectively any changes to the bond amount, such as bonding more funds, and
	/// unbonding.
	fn on_stake_update(who: &AccountId, prev_stake: Option<Stake<AccountId, Balance>>);
	/// Fired when someone sets their intention to nominate, either new, or existing one.
	fn on_nominator_update(who: &AccountId, prev_nominations: Vec<AccountId>);
	/// Fired when someone sets their intention to validate, either new, or existing one.
	fn on_validator_update(who: &AccountId);
	/// Fired when someone removes their intention to validate, either due to chill or nominating.
	fn on_validator_remove(who: &AccountId); // only fire this event when this is an actual Validator
	/// Fired when someone removes their intention to nominate, either due to chill or validating.
	fn on_nominator_remove(who: &AccountId, nominations: Vec<AccountId>); // only fire this if this is an actual Nominator
	/// fired when someone is fully unstaked.
	fn on_unstake(who: &AccountId); // -> basically `kill_stash`
}

The first event listener implementing this interface, also introduced in this PR, is called pallet-stake-tracker which currently takes care of maintaining VoterList (a sorted list of nominators and validator's self-stake), effectively moving this logic out of Staking. It's also supplying Staking with a read-only version of this list, which is currently implemented by adding some defensive checks into insert/updated/remove methods.

Some other things that were introduced in this PR include:

1. Staking tests that verify the correct OnStakingUpdate events are being fired with the right arguments.
2. A read-only wrapper for SortedListProvider, that stubs out update/insert/remove methods and only leaves unsafe methods to allow for easier benchmarking/testing/migrations.
3. CurrencyToVote moved from frame_support to primitives/staking.

[ruseinov]

@kianenigma This does not have dedicated tests for StakeTracker yet, but it runs Staking tests against StakeTracker and it all works. Separate unit-tests for tracker are going to be introduced, however I think we shall retain the Staking part of those tests too as Staking relies on the outcome of the VotersList.

There is one test that is failing: https://github.com/paritytech/substrate/pull/13079/files#diff-64c049b4e94a55bdeaf757c725cf7d1df623b754557af4ced157024a943c4703R5145 that I think has to be removed.
As discussed in Frame channel before - we don't want to cater for un-decodables due to MaxNominations decrease, this should be accompanied by a migration. And it simplifies the code by quite a bit.

[ruseinov]

bot rebase

[paritytech-processbot]

Rebased

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2548672

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2566054

[ruseinov]

bot rebase

[paritytech-processbot]

Rebased

[ruseinov]

bot rebase

[paritytech-processbot]

Rebased

[kianenigma]

is who controller or stash? We decided to try and move all things in the StakingApi and OnStakingUpdate to use stash.

This also makes me wonder: T::Staking::stake is queried by the stash, and return the stash to you as well? that does not seem reasonable.

[ruseinov]

Yes, who is a stash. Indeed, it does not make a whole lot of sense. stash can indeed be removed from Stake struct as it's redundant.

[kianenigma]

Gave this a good review again, the logic seems good, but I am still not content with some of the testing details, and given that this is among the "critical" work in staking, I don't want to sacrifice it.

• The testing setup is both partially wrong and unnecessarily complex. We are keeping track of balances, which is irrelevant entirely.
• We are registering some accounts as validators and nominators notionally, but we are actually not making them go through the real code path.
• The way that the accounts is setup could be made better.

I've demonstrated this in https://github.com/paritytech/substrate/compare/ru/feature/stake-tracker-voter...kiz-voter-list-stuff?expand=1

Other than the bot, another testing idea is to use #12896 in the companion, and make sure that we can produce enough blocks to create the snapshot for the next election. This is a pretty good use case of the fast-forward subcommand.

[gpestana]

Suggested change

	/// A wrapper for a given `SortedListProvider` that introduces defensive checks for insert, update
	/// A wrapper for a given `SortedListProvider` that introduces defensive checks for insert, update

[gpestana]

Maybe you could elaborate more what are unsafe operations and why we need them here? It sounds important

[gpestana]

Suggested change

[ruseinov]

bot rebase

[paritytech-processbot]

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/reference/orgs#check-organization-membership-for-a-user","message":"User does not exist or is not a member of the organization"}

[kianenigma]

I will re-open this from a new PR, based on your commits. Thank you Roman for your contribution 💯

[ruseinov]

@kianenigma I'm planning to finish this up in a new PR coming from my fork sometime soon.