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

[multi-test] Add staking and distribution module #782

Merged
merged 25 commits into from
Oct 13, 2022

Conversation

ueco-jb
Copy link
Contributor

@ueco-jb ueco-jb commented Aug 18, 2022

Closes #753

Implementation of Staking and Distribution modules in multitest package, which allows now to test staking and delegation.

@ueco-jb ueco-jb self-assigned this Aug 18, 2022
@ueco-jb ueco-jb force-pushed the multitest-add-staking-module branch from 22c89db to 524c632 Compare August 18, 2022 13:46
Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice start.
The two big TODO are obvious missing points you mentioned to me.
I designed this to be possible. Probably need 30 minutes in the morning to push such a commit that will compile and do that

packages/multi-test/src/staking.rs Outdated Show resolved Hide resolved
packages/multi-test/src/staking.rs Outdated Show resolved Hide resolved
packages/multi-test/src/staking.rs Outdated Show resolved Hide resolved
packages/multi-test/src/staking.rs Outdated Show resolved Hide resolved
@ethanfrey ethanfrey added this to the 0.16.0 milestone Sep 16, 2022
@chipshort chipshort force-pushed the multitest-add-staking-module branch 5 times, most recently from 24bf15b to e6255be Compare October 7, 2022 14:21
@chipshort
Copy link
Contributor

chipshort commented Oct 7, 2022

This now allows adding multiple validators and the rewards are calculated based on a configurable APR, the validator's commission and the elapsed time, so you can just change block.time and send the withdrawal message. I also implemented the unbonding period (tough only when undelegating, not when redelegating). To trigger the actual payout when unbonding, a sudo message has to be sent to process the unbonding queue.
Not sure if this is a good solution, but I couldn't come up with a better way.

Looks like tarpaulin flags a lot of lines that are covered.

@chipshort chipshort marked this pull request as ready for review October 7, 2022 14:23
@JakeHartnell
Copy link
Collaborator

Can't wait for this!

@chipshort chipshort requested a review from ethanfrey October 10, 2022 07:00
@ueco-jb ueco-jb assigned chipshort and unassigned ueco-jb Oct 10, 2022
Copy link
Contributor

@uint uint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine! I can't easily verify this is precisely how staking/distribution works, so I'm taking that on faith.

packages/multi-test/src/staking.rs Show resolved Hide resolved
packages/multi-test/src/staking.rs Outdated Show resolved Hide resolved
@ethanfrey
Copy link
Member

@ueco-jb can you add "Closes #753 " to the description, and maybe update it (I think it isn't WIP anymore)

@ueco-jb
Copy link
Contributor Author

ueco-jb commented Oct 12, 2022

@chipshort In reward formula remember to subtract the validator commision
reward * (1 - commision)

@chipshort
Copy link
Contributor

@chipshort In reward formula remember to subtract the validator commision reward * (1 - commision)

That should already be the case in calculate_rewards, albeit I calculated it in a slightly different way:

let commission = reward * validator_commission;

reward - commission

@chipshort chipshort force-pushed the multitest-add-staking-module branch from 42b927d to f02f345 Compare October 13, 2022 10:11
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.

Some multitest bindings for staking are missing such as BondedDenom
5 participants