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

Adds target approvals buffering below a threshold #5168

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Jul 28, 2024

This PR adds target stake updates buffering to stake-tracker. It exposes a configuration that defines a threshold below which the target score should not be updated automatically in the target list. The Config::ScoreStrictUpdateThreshold defines said threshold. If a target stake update is below the threshold, the stake update is buffered in the UnsettledTargetScore storage map while the target list is not affected. Multiple approvals updates can be buffered for the same target. Calling Call::settle will setttle the buffered approvals tally for a given target. Setting Config::ScoreStrictUpdateThreshold to None disables the stake approvals buffering.

  • benchmarks
  • try-state checks considering the unsettled score
  • docs

@gpestana gpestana added the R0-silent Changes should not be mentioned in any release notes label Jul 28, 2024
@gpestana gpestana self-assigned this Jul 28, 2024
@gpestana gpestana marked this pull request as draft July 28, 2024 22:15
@gpestana
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 29, 2024

@gpestana https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6849944 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-e85aaf92-c2a9-4c3f-bb30-7a8fcd10d1a1 to cancel this command or bot cancel to cancel all commands in this pull request.

Copy link

We are migrating the command bot to be a GitHub Action

Please, see the documentation on how to use it

@command-bot
Copy link

command-bot bot commented Jul 29, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6849944 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6849944/artifacts/download.

@gpestana gpestana marked this pull request as ready for review July 30, 2024 12:24
@gpestana gpestana requested a review from a team as a code owner July 30, 2024 12:24
@gpestana
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

@gpestana https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6858441 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 3-61d8be0e-1972-4991-84b3-8952a71c90e8 to cancel this command or bot cancel to cancel all commands in this pull request.

Copy link

We are migrating the command bot to be a GitHub Action

Please, see the documentation on how to use it

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6858441 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6858441/artifacts/download.

@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/6858442

@gpestana gpestana changed the title Adds lazy update of target approvals Adds target approvals buffering below a threshold Jul 30, 2024
@gpestana gpestana added T2-pallets This PR/Issue is related to a particular pallet. R0-silent Changes should not be mentioned in any release notes and removed R0-silent Changes should not be mentioned in any release notes T2-pallets This PR/Issue is related to a particular pallet. labels Jul 30, 2024
@gpestana gpestana merged commit 933e159 into gpestana/stake-tracker_integration Aug 1, 2024
44 of 172 checks passed
@gpestana gpestana deleted the gpestana/stake-tracker_integration-lazy branch August 1, 2024 11:15
@kianenigma
Copy link
Contributor

Sorry for being late, should have reviewed this sooner.

I think we should revert and re-do this, as it is different from what I believe was the intended fix.

The original issue is that each ledger update in a nominator can correspond to up 16 approval stake updates. With this PR, we have reduced that to 1, but I think we can do better.

I belive what we actually need to store is not Unsettled but instead LastSettled. This is a mapping from each nominator to their last tracked approval stake, and keeps track of the last time that a nominator's stake was counted towards validators approval stake.

As rewards and slashes come in, we do nothing. So suppose LastSettled(alice) = 100, meaning that upon our last calculation, Alice had 100 stake, and it had contributed 100 to a set of up to 16 validators.

Then, once alice gets reward and slashes, and now she has 200, anyone can call re_balance(alice). This call will update LastSettled(alice) to 200, and also update the approval stake of all validators that alice has backed.

This approach is a lot less computation and weight update, and a lot more storage, which is totally fine in a parachain.

gpestana added a commit that referenced this pull request Aug 5, 2024
@gpestana gpestana restored the gpestana/stake-tracker_integration-lazy branch August 5, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants