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

Store validator approval-stakes in TargetList, use for ranking validators. #11013

Closed

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Mar 11, 2022

This PR re-uses the bags-list pallet to sort validators candidates as well. This bags-list- instance is used to filter the top x validators to be considered for election, i.e. become electable. See here for more info on this.

The Score used in this instance of the bags-list is the approval stake of the validator. The approval stake of each validator is essentially the sum of the stake of any nominator who has nominated them (undivided), plus their self-stake.

Maintaining approval stakes requires an update to the interface of the bags-list pallet. Previously, updates were reported to the bags-list only via:

fn on_insert(id: T::AccountId, score: T::Score) -> Result<(), ListError>;

In this function, the score is the new updated score of id. This won't work for approval stakes, because we don't store the approval stake of validators anywhere in the staking pallet. Instead, the bags-list pallet has been updated to store the last known score. With this, we can use two new functions, which actually get an auto-implementation given the existence of a get_score:

/// Same as `on_update`, but incorporate some increased score.
fn on_increase(id: &AccountId, additional: Self::Score) -> Result<(), Self::Error> {
	let old_score = Self::get_score(id)?;
	let new_score = old_score.saturating_add(additional);
	Self::on_update(id, new_score)
}

/// Same as `on_update`, but incorporate some decreased score.
///
/// If the new score of the item is `Zero`, it is removed.
fn on_decrease(id: &AccountId, decreased: Self::Score) -> Result<(), Self::Error> {
	let old_score = Self::get_score(id)?;
	let new_score = old_score.saturating_sub(decreased);
	if new_score.is_zero() {
		Self::on_remove(id)
	} else {
		Self::on_update(id, new_score)
	}
}

I will probably extract these changes into a separate PR and make it be in an upcoming release soon, because it will require a migration to add the score to the existing bags-list.


All in all, in the new staking system, we have to list providers:

  1. VoterList: something that gives us a sorted list of electing npos voters.
  2. TargetList: something that gives us a sorted list of electable npos targets.

Once we have this, in the staking pallet, we have to keep a detailed bookkeeping: every time a staker updates either their stake amount, or when they update their nomination/validation status, we need to react. Luckily most of this logic has now been placed in a handful of functions:

  • update_ledger: called every time the active ledger of an account changes.
    • If they are a validator
      • update their approval stake in TargetList
      • update their self-vote score in VoterList.
    • If they are a nominator
      • update their vote score in VoterList.
      • update the approval stake of every validator they are nominating in TargetList.
  • do_add_nominator: called every time someone calls nominate
    • compute incoming and outgoing (self-explanatory names) sets of their current and previous nominations, and update the approval stake in TargetList.
  • do_remove_nominator: called every time a nominator is potentially removed (chill, validate, kill_stash).
  • do_add_validator: called every time someone calls validate.
    • IFF they are actually a new validator:
      • insert their stake as self-vote into VoterList.
      • add their self-stake to their TargetList.
  • do_remove_validator:

A nuanced detail here is that now each nominator can potentially create up to 16 more storage maps. The reason for this is that if you nominate vec![a, b, c, ..], we don't (and actually should not) check if a, b, c are actually validators.

Moreover, this means that targets might be pulled out of the target list that are not even validators at the time of election. We need to take care of this by filtering them out and keep trying until we get enough electable targets.

say we keep an approval stake map based on all nominators. In principle, once we try and fetch stuff from the bags-list, what comes out of it could be a non-validators, because in principle nominators can cast nominations to any account. We don't check if they are actually validators, and there's no point in doing so either. This is fine from my PoV. We filter these out, and continue fetching stuff from the bags-list until we have had enough validator candidates, or reached some heuristic upper bound.
example: if we want 1000 candidates, we fetch at most 2000 items from the bags-list, and in almost all cases that should be enough to find that 1000 "actually-validator" items.


PR is almost done, but needs a few more rounds of my self-review and elimination of TODOs.

kianenigma and others added 30 commits February 8, 2022 15:25
Co-authored-by: Zeke Mostov <z.mostov@gmail.com>
Co-authored-by: Zeke Mostov <z.mostov@gmail.com>
@@ -20,8 +20,8 @@
use frame_support::traits::OnRuntimeUpgrade;

/// A struct that does not migration, but only checks that the counter prefix exists and is correct.
pub struct CheckCounterPrefix<T: crate::Config>(sp_std::marker::PhantomData<T>);
impl<T: crate::Config> OnRuntimeUpgrade for CheckCounterPrefix<T> {
pub struct CheckCounterPrefix<I: 'static, T: crate::Config<I>>(sp_std::marker::PhantomData<(I, T)>);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the instance usually the second index?

…ithub.com:paritytech/substrate into kiz-revamp-sorted-list-providers-2-approval-stake
kianenigma and others added 4 commits May 3, 2022 15:10
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
…ithub.com:paritytech/substrate into kiz-revamp-sorted-list-providers-2-approval-stake
@shawntabrizi shawntabrizi changed the base branch from master to shawntabrizi-bags-list-list-error May 3, 2022 17:58
@shawntabrizi shawntabrizi changed the base branch from shawntabrizi-bags-list-list-error to master May 3, 2022 18:01
@kianenigma
Copy link
Contributor Author

kianenigma commented May 10, 2022

@shawntabrizi we should close this once you are done extracting all the pieces.

Once #11357 is done, do you want to also add the staking part?

@stale
Copy link

stale bot commented Jun 9, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jun 9, 2022
@shawntabrizi shawntabrizi self-requested a review June 16, 2022 14:03
@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jun 16, 2022
@kianenigma
Copy link
Contributor Author

@ruseinov might take a stab at bringing this back to life as well. Once done, please close this and open a new PR.

@kianenigma
Copy link
Contributor Author

Note that this is a pretty much the last important piece of the puzzle in making Polkadot staking scalable.

@kianenigma
Copy link
Contributor Author

being worked at in a different PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants