Go back to block based staking rounds and make inflation dynamic (slot based) #2690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
librelois
added
A3-inprogress
RomarQ
reviewed
Mar 1, 2024
Agusrodri
reviewed
Mar 1, 2024
Coverage Report@@ Coverage Diff @@
## master elois-rework-staking-rounds +/- ##
===============================================================
+ Coverage 80.43% 80.44% +0.01%
+ Files 297 299 +2
+ Lines 91636 91688 +52
===============================================================
+ Hits 73702 73751 +49
+ Misses 17934 17937 +3
|
RomarQ
reviewed
Mar 4, 2024
RomarQ
reviewed
Mar 4, 2024
| let blocks_per_round = <Pallet<T>>::round().length; | ||
| T::SlotsPerYear::get() / blocks_per_round | ||
| let blocks_per_round = <Pallet<T>>::round().length as u64; | ||
| let blocks_per_year = MS_PER_YEAR / T::BlockTime::get(); |
There was a problem hiding this comment.
We could probably have a new helper function named blocks_per_year and replace MS_PER_YEAR / T::BlockTime::get() by it everywhere.
RomarQ
reviewed
Mar 4, 2024
RomarQ
reviewed
Mar 4, 2024
Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com>
RomarQ
reviewed
Mar 4, 2024
Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com>
RomarQ
reviewed
Mar 4, 2024
RomarQ
approved these changes
Mar 4, 2024
Agusrodri
approved these changes
Mar 4, 2024
crystalin
reviewed
Mar 4, 2024
| @@ -84,7 +84,7 @@ describeSuite({ | |||
| title: "should be under the limit of 3_500_000", | |||
| test: async () => { | |||
| // Moves to the next payout block | |||
| await jumpRounds(context, 1); | |||
| await jumpRounds(context, 2); | |||
There was a problem hiding this comment.
Was it an error or did something change ?
There was a problem hiding this comment.
There was a bug before this PR, the collators got rewards for the round 1 twice (at round 2 and at round 3), now it's fixed, so there is no rewards distribution at round 2.
elfedy
approved these changes
Mar 4, 2024
librelois
added a commit
that referenced
this pull request
Mar 4, 2024
…t based) (#2690) * make staking rounds block based again and inflation slot based * remove unused imports * create migration skeleton * implement migration * integrate round migration to common migrations * apply proportion duration to the current round * compile benchs and test * add moonbase migration to multiply round length by 2 * compute round duration before round update * fix some bugs * fix some rust tests * fix migration * apply suggestions * First staking rewards at round 3 * revert moonwall config local changes * fix rounds per year * fix periods calculation in reset_round() * fix rustc warning * fmt * rework compute issuance * Remove Staked storage item * Remove Staked storage item on rust tests * fix two rust tests more * fix test_on_initialize_weights * fix payouts_follow_delegation_changes test * Update pallets/parachain-staking/src/lib.rs Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * Comment moonbase migration * typo * Update runtime/moonbase/src/migrations.rs Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * Update runtime/moonbase/src/migrations.rs * Update pallets/parachain-staking/src/lib.rs --------- Co-authored-by: Agusrodri <agusrodriguez2456@gmail.com> Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does it do?
The initial goal of #2577 was to have a slot based inflation that is not impacted by block production issues. This PR achieve the same goal in a different way to keep to staking round fixed length in blocks.
The storage item
parachainStaking.stakedwas removed.What important points reviewers should know?
This PR contains a migration that should work from both RT2700 and RT2800, and this is challenging becuase we don't use pallet versioning for the pallet parachain staking.
The trick to detect from what version the RoundInfo should be migrated is to measure the raw size of the data, it's 12 bytes on RT2700 and 15 bytes on RT2600.
Is there something left for follow-up PRs?
What alternative implementations were considered?
#2577, but dynamic-length rounds cause bugs and break external tools.
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?
More constant inflation rate.
TODO