Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change sorted list types and simplify type conversions in stake-tracker #4361

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented May 2, 2024

Changes the voter and sorted lists score types.

  • Target list score from Balance to ExtendedBalance: for safety.
  • Voter list score from VoteWeight to Balance: to avoid conversions to vote weight. (can we do this though? what if the vote weight is > u64)

This way, the conversion to VoteWeight (which requires the a call to T::Currency::total_issuance) is reduced to a minimum and never required at the stake-tracker level. The target list score is ExtendedBalance for safety, as the approvals stakes with unbounded nominations may overflow u64.

Note that the end of the day, the voter and target lists are used to keep stakers sorted by stake/approval stakes.

Converting the voter score from VoteWeight to Balance requires a migration since vote_weight(account) != active_stake(account)`

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6116029

@gpestana gpestana marked this pull request as draft May 2, 2024 20:36
gpestana added a commit that referenced this pull request May 10, 2024
- Changes the target list score type from balance to `u128`
(`ExtendedBalance`)
  - simplifies `balance/ vote_weight/ extended_balance` conversions
  - relies less on the currency total issuance call
  - ensures approval stake calculations/storage is safe 
- Simplifies stake tracker logic
  - removes a few redundant checks
  - split `on_stake_update` code
- Ensures `VoterUpdateMode` is taken into consideration in try-state
checks
  - use voter mode lazy in benchmarks
  - update weights + check the diff with voter lazy 
- Docs improvements
- All tests passing after merging validator disabling and virtual
stakers

**new**
- ensures duplicate nominations are dedup before calculating the
approvals stake
- chills nominator after total slash to ensure the target can be
reaped/kill without leaving nominations behind.
- ensures that switching from validator to nominator and back is correct
- a nominator may have an entry in the targetlist (if it was a validator
+ has nominations)
  - ensure target node is removed if balance is 0 and it is a nominator.
- addresses
paritytech-secops/srlabs_findings#383

---
**Other experiments:** (not included)
- #4402 (passing the
status of the staker with who in the OnStakingUpdate interface is less
clean, it turns out)
- #4361

---------

Co-authored-by: command-bot <>
@gpestana
Copy link
Contributor Author

closing as this change will not be introduced in stake tracker. Opened a tracking issue to tackle this refactor later on #4725

@gpestana gpestana closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants