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

Do not include voters that have zero voter weight in the election snapshot #14245

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

gpestana
Copy link
Contributor

In the current staking system, there can be cases when the ledger.active balance is 0 (i.e. the voter weight is 0) or below the MinNominatorBond and those nominators will still be included in the election snapshot. E.g. when nominator has unlocking chunks queued (i.e. ledger.total balance > 0) and if the nominate has been executed with an older, lower MinNominatorBond.

This PR ensures that:

  • Voters with voter_weight == 0 (i.e. ledger.active == 0) are not considered by fn DataProvider::electing_voters and thus not included in the snapshot.
  • MinActiveStake can never be zero (although it may be lower than MinNominatorBond)
  • The only case where MinActiveStake is zero is if the voters list doesn't have any "valid" nominator (i.e. a voter that is decodable and with active stake > 0).

Changes to EPM mock
In addition, this PR removes the automatic self staking in the EPM mock StakingMock::add_target implementation to be in sync with the add_target in the pallet staking implementation. Not including the voters with stake = 0 in the snapshot surfaced the sync issue between test and prod benchmarking setups.

More detailed info: https://hackmd.io/NCe-ZVJ2TOC6Tt78bU15BQ
Closes #13938

New version of previously opened PR

@gpestana gpestana added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”. labels May 27, 2023
@gpestana gpestana self-assigned this May 27, 2023
@gpestana gpestana requested review from a team May 27, 2023 12:18
@gpestana gpestana requested review from kianenigma and Ank4n May 27, 2023 12:19
let voters = <Staking as ElectionDataProvider>::electing_voters(None).unwrap();
// number of returned voters decreases since ledger entry of stash 101 is now
// corrupt.
assert_eq!(voters.len(), 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Vamos 👍

@stale
Copy link

stale bot commented Jul 9, 2023

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 A3-stale label Jul 9, 2023
@kianenigma
Copy link
Contributor

bot rebase

@stale stale bot removed the A3-stale label Jul 10, 2023
@paritytech-processbot
Copy link

Rebased

@gpestana
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 984f852 into master Jul 24, 2023
5 checks passed
@paritytech-processbot paritytech-processbot bot deleted the gpestana/13938-zero-weight-voters_2 branch July 24, 2023 12:32
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

MinimumActivesStake reported as 0
3 participants