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

Optimize rewards #41

Open
JakeHartnell opened this issue Oct 18, 2022 · 5 comments
Open

Optimize rewards #41

JakeHartnell opened this issue Oct 18, 2022 · 5 comments

Comments

@JakeHartnell
Copy link
Collaborator

PR #40 is adding the withdraw rewards flow (not merged at time of writing), however it is still at proof of concept stage and not optimized at all. It is also not very robust or well tested.

@ethanfrey's comments from the review:

In terms of lazy distribution of tokens, I can recommend the approach we did in wynd dao that does one write on distribution, one on withdrawal, and one extra write each time a staked updates their shares.

I have not looked at the dao-dao distribution logic, but maybe that makes sense as well. However, the wynd dao logic is designed to handle discrete payment events triggered externally, which I think is more fitting here.

The DAO DAO implementation is here: https://github.com/DA0-DA0/dao-contracts/tree/main/contracts/staking/cw20-stake-external-rewards

@Art3miX
Copy link
Collaborator

Art3miX commented Oct 19, 2022

Here is the wynd link: https://github.com/cosmorama/wynddao/blob/main/contracts/wynd-stake/src/distribution.rs

For some reason the other link doesn't work for me.

@orkunkl
Copy link

orkunkl commented Oct 20, 2022

@Art3miX
Copy link
Collaborator

Art3miX commented Oct 21, 2022

Hey guys, im redoing PR #40 to be more performant, its my first try at staking contracts so I would like to run the rewards calculation design by you to see if its correct and make sure I didn't miss anything.

Same design will apply to both withdraw to consumer and withdraw to delegator, the general idea is to calculate the rewards per token per block we need to pay, we take the total rewards divide it by the total staked amount and then divide it by height from last calculation, this gives us rptpb (rewards per token per block).
We can then use this number, to see when and how much consumer/delegator got paid, and only pay the difference.

This will allow us to remove all the bad loops I did, almost similar to DAO DAO Solution, you can see my current code which removes the loops without the calculation here: Art3miX#2 (might be easier to read the code then the PR)

Details, in withdraw from validator (When we receive rewards from validator or from consumer):

  1. last_height = Last rewards update height
  2. old_rptpb = Last rewards_per_token_per_block (how much we need to pay per staked token, per block)
  3. calculate new_rptpb = (new_rewards_amount / total_staked / (curr_height - last_height)) + old_rptpb
  4. new_rptpb is saved per validator, and it basically represents the total amount of rewards this validator got, but factors in the time (height diff) and total staked tokens during withdrawals.
  5. Then on consumer we can save new_rptpb to represent when and how much that consumer got paid.

Then we have update_rewards() function that runs every time consumer stake/unstake or claim rewards, because when the stake changes, we need to also make sure he get rewards based on the new stake in the future:

  1. old_rptpb is saved on the consumer end, to know how much we already paid him before doing new calculation.
  2. blocks_to_pay = curr_height - last_height how much blocks passed since we last paid him.
  3. rptpb_to_pay = new_rptpb - old_rptpb (if 0 we don't have rewards to pay), this is the diff between what we already paid, and what we need to pay since last time paid.
  4. rewards_to_pay = rptpb_to_pay * staked * blocks_to_pay
  5. This function will save the rewards_to_pay in PENDING_REWARDS storage until the consumer withdraws the rewards.
  6. When withdraw_to_consumer is called, we first call update_rewards() to update the amount of rewards, then we take the amount, send it to mesh-consumer and set it to zero.

I think the calculation is correct, and should work, but there is 1 issue, I do not reset rptpb this number increases and never reset, the only way to reset it is to make a loop over all consumers, calculate their rewards, update it and reset this number. N loop with N+1 reads and saves to storage.
Potentially this number can overflow in the future but it should take a while.

Little example:

  1. first withdraw we have rewards 100, staked tokens 10, and last update was 10 blocks away
    new_rptpb = 100 / 10 / 10 + 0 = 1
  2. 2nd withdraw, 100 rewards, 20 staked, 10 height_diff
    new_rptpb = 100 / 20 / 10 + 1 = 1.5
  3. 200 rewards, 40 staked, 10 height_diff
    new_rptpb = 200 / 40 / 10 + 1.5 = 2

@Art3miX
Copy link
Collaborator

Art3miX commented Oct 23, 2022

I added the calculation to the PR, you can find the implementation of the calculation design in meta-staking.

@Art3miX
Copy link
Collaborator

Art3miX commented Oct 26, 2022

The above calculation was wrong, I changed to it be rewards per token, and remove the whole height factor.

Validator calculation: https://github.com/CosmWasm/mesh-security/pull/40/files#diff-525d18d496dfc7f7589e3e5cc521ff82a04622c901787dc68054f9aa9da16352R174

Consumer/delegator calculation: https://github.com/CosmWasm/mesh-security/pull/40/files#diff-525d18d496dfc7f7589e3e5cc521ff82a04622c901787dc68054f9aa9da16352R63

In general we use decimal and only send full tokens, and keep the leftover in pending state.

Also added a very basic unit test for the calculation: https://github.com/CosmWasm/mesh-security/pull/40/files#diff-57f78331e53081f97e605bebd7fde48e220851e35899836db3357047ce129ee5R210

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

No branches or pull requests

3 participants