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

Conversation

@ruseinov
Copy link
Contributor

@ruseinov ruseinov commented Jan 20, 2023

Splitting up #12744
The first part: #13079
polkadot companion: paritytech/polkadot#6639

Terminology

approval stake - a sum of all nominator/nomination pool stakes backing this validator + their self stake.
staking event listener - implementor of OnStakingUpdate, listening to staking events and acting upon them.

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 select look into top x validators in the list to get a set of electable targets.
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 the logic to track validator's approval stake through a staking event listener that is implemented by pallet-stake-tracker in this pr. Note that as long the staker is in the system, whether or not they decide to continue validating - we have to keep track of their approval stake, as there is no easy way to recalculate that. For this reason ApprovalStake storage has been introduced for pallet-stake-tracker, this way we don't need to keep inactive validators in the TargetList, therefore preventing it's bloat.

We are also introducing a migration that calculates the approval stake for each validator. It's quite costly, because we need to iterate all the validators and nominators, updating the storage as we go. For this reason the migration is going to be spanning several blocks, initialized on OnRuntimeUpdate and continued OnInitialize to make sure we have enough weight free for the chain to work without delays. It's quite a tricky change, because we also need to make sure that there are no writes to Validators and Nominators storage in Staking while it's running.

What's missing:

  1. Needs to integrate the changes from the branch it's targeting once that gets approved
  2. Multi-block migration should continue on_initialize

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jan 20, 2023
@ruseinov ruseinov changed the title [Feature] StakeTracker P1 - VotersList [Feature] StakeTracker P1 - TargetList Jan 22, 2023
@ruseinov ruseinov changed the title [Feature] StakeTracker P1 - TargetList [Feature] StakeTracker P2 - TargetList Jan 23, 2023
@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@ruseinov ruseinov marked this pull request as ready for review January 26, 2023 15:45
@ruseinov ruseinov requested a review from kianenigma as a code owner January 26, 2023 15:45
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 26, 2023
}
}

impl<T: Runtime> OnRuntimeUpgrade for InjectValidatorsApprovalStakeIntoTargetList<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this up to date? This is not how this migration should happen.

@ruseinov ruseinov requested a review from a team March 16, 2023 05:10
@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@ruseinov
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-each-crate-macos
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2619454

@kianenigma
Copy link
Contributor

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

@kianenigma kianenigma closed this Apr 21, 2023
@louismerlin louismerlin removed the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label May 4, 2023
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. B1-note_worthy Changes should be noted in the release notes C3-medium PR touches the given topic and has a medium impact on builders. T1-runtime This PR/Issue is related to the topic “runtime”.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants