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.

Sign up for GitHub

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

Jump to bottom

Stake vesting accounts #124

Merged
merged 22 commits into from
Mar 25, 2022
Merged

Stake vesting accounts #124

merged 22 commits into from
Mar 25, 2022

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Mar 3, 2022 •
edited
Loading

This is the contract side of confio/tgrade#220.

TODO:

  • Query vesting stake. Make current queries return the illiquid (vesting) staked amounts too.
  • Vesting account tests. Confirm the illiquid (vesting) vs. liquid staking logic works.
  • Delegate / Undelegate multi-tests. Some simple tests to check delegate / undelegate messages routing. (Unnecessary. Contract is already tested / integrated in the blockchain)
  • Slashing support for vesting account. Add support for slashing illiquid (vesting) tokens along with liquid ones.

@maurolacy maurolacy requested review from alpe and ethanfrey March 3, 2022 07:23
@maurolacy maurolacy marked this pull request as draft March 3, 2022 07:24
@maurolacy maurolacy marked this pull request as ready for review March 8, 2022 16:21
@maurolacy maurolacy mentioned this pull request Mar 9, 2022
Closed
@alpe alpe mentioned this pull request Mar 17, 2022
Merged
@alpe
Copy link

alpe commented Mar 17, 2022

Thanks for the quick fix. I can confirm that it works with vesting amounts only, now. 💯

This was referenced Mar 23, 2022
Closed
Closed
@maurolacy maurolacy requested a review from ueco-jb March 25, 2022 06:56
ueco-jb
ueco-jb reviewed Mar 25, 2022
View reviewed changes
Copy link

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

Mostly reiterating same nitpicky argument about possibly unnecessary DB operations.

contracts/tg4-stake/src/contract.rs Show resolved Hide resolved
contracts/tg4-stake/src/contract.rs Outdated Show resolved Hide resolved
contracts/tg4-stake/src/contract.rs Show resolved Hide resolved
contracts/tg4-stake/src/contract.rs Outdated Show resolved Hide resolved
contracts/tg4-stake/src/contract.rs Show resolved Hide resolved
contracts/tg4-stake/src/msg.rs Outdated Show resolved Hide resolved
packages/bindings-test/src/multitest.rs Show resolved Hide resolved
ueco-jb
ueco-jb approved these changes Mar 25, 2022
View reviewed changes
Copy link

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

Seems lgtm

Those FIXME in multitest delegate calls - I think (only?) Ethan can address those. But it can be done in #126 anyway.

@maurolacy maurolacy merged commit 9f76c76 into main Mar 25, 2022
@maurolacy maurolacy deleted the stake-vesting-accounts branch March 25, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alpe alpe alpe left review comments

@ueco-jb ueco-jb ueco-jb approved these changes

@ethanfrey ethanfrey Awaiting requested review from ethanfrey

@uint uint Awaiting requested review from uint

@hashedone hashedone Awaiting requested review from hashedone

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maurolacy @alpe @ueco-jb