refactor!: use time-based queued staking and reserve unharvested rewards #305
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
|
@hallazzang Great works!
|
jaybxyz
suggested changes
May 4, 2022
github-actions
bot
added
the
documentation
jaybxyz
approved these changes
May 4, 2022
dongsam
reviewed
May 6, 2022
dongsam
reviewed
May 6, 2022
dongsam
reviewed
May 6, 2022
dongsam
reviewed
May 6, 2022
to eliminate bottlenecks of processing queued coins at end-of-epoch, we decided to use time-based processing of queued stakings this commit does not include full tests refactoring
also check end time when getting queued staking coins
also prune total stakings immediately in Unstake
also panic when HistoricalRewards is not found
- rename previous `Stakings` endpoint to `Position` - add new `Stakings` and `QueuedStakings` endpoints
hallazzang
force-pushed
the
302-manual-claim
branch
from
35c10f4 to
bac9f4f
Compare
- add `rewards` field to `QueryPositionResponse` - make `Rewards` query endpoint to return rewards by staking coin denom
|
@hallazzang looks |
9 tasks
dongsam
pushed a commit
that referenced
this pull request
Aug 31, 2022
refactor!: use time-based queued staking and reserve unharvested rewards #305
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.
Description
This PR changes queued stakings to be handled in time-based(24 hours) manner, rather than epoch-based.
Also, auto-withdrawn rewards due to changes of staked coin amount now accumulated in separate module account.
Farmers have to manually claim their unharvested rewards using
MsgHarvest.Note that unstaking whole staked coin results in withdrawal of all current rewards and unharvested rewards.
Background
Since
ProcessQueuedCoinsis being called at the end of epoch, when there are too many queued stakings it could impact the chain's performance(see #274). By handling queued staking at the moment it should be changed to staked, it mitigates the mentioned performance issue.Also, automatic withdrawal of accrued rewards when there's a change in staking coin amount(by staking more or unstaking) could confuse many users. Our solution is to move rewards to a separate module account(
UnharvestedRewardsReserveAcc) and make users to claim it throughMsgHarvest. In this way, users can know how many rewards are still unharvested and claim rewards as they wish.Note that when unstaking whole amount of coin for a specific denom, all previous unharvested rewards are automatically withdrawn for convenience.
Upgrading
When upgrading a chain which uses the farming module, set
EndTimeof newQueuedStakingto24 * CurrentEpochDayshours after current (block)time. It'll result in smooth transition to new time-based staking logic.Closes #302
Closes #274
Tasks
UnharvestedRewardsstructCurrentEpochpreservation ruleUnharvestedRewardsgenesis(UnharvestedRewardsRecords)clinet/docs/config.jsonReferences